Get reference to enclosing closure

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Get reference to enclosing closure

Leonard Brünings

Hi,

I'm Leonard from the Spock framework team. Guillaume suggested that I write to the dev-list with this problem.

Some context:

Spock has a method `with(Object, Closure)` in its Specification class that sets the object as the delegate of the closure and transforms,
every call inside the closure to an implicit assertion.

given:
      def person = new Person(name: "Peter", age: 28)
expect:
      with(person) {
          name == 'Peter'
          age == 28
      }

This worked fine for properties, however for single methods like `contains` it didn't work.

The initial problem is described here https://github.com/spockframework/spock/pull/606

Here is the gist:

This snippet

def list = [1, 2]
with(list) {
  contains(1)
}

transforms in AST (simplified) to

def list = [1, 2]
with(list) {
  SpockRuntime.verifyMethodCondition(this, "contains", [1])
}

then when the AST is written to bytecode it gets transformed again

def list = [1, 2]
with(list) {
  SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains", [1])
}

The problem is that the `contains` is now invoked on the containing `Specification` instead of the `List`.

With the aforementioned PR it was changed to this

def list = [1, 2]
with(list) {
  SpockRuntime.verifyMethodCondition(this.each(groovy.lang.Closure.IDENTITY), "contains", [1])
}

This "fix" now broke the nesting of `with` blocks as described here: https://github.com/spockframework/spock/pull/782

Do you have any ideas on how to fix this elegantly?

- Cheers
Leonard

Reply | Threaded
Open this post in threaded view
|

RE: Get reference to enclosing closure

eric.milles

I think "this" or "getThisObject()" should really be "delegate" or "getDelegate()".

 

From: Leonard Brünings [mailto:[hidden email]]
Sent: Monday, November 20, 2017 2:27 PM
To: [hidden email]
Subject: Get reference to enclosing closure

 

Hi,

I'm Leonard from the Spock framework team. Guillaume suggested that I write to the dev-list with this problem.

Some context:

Spock has a method `with(Object, Closure)` in its Specification class that sets the object as the delegate of the closure and transforms,
every call inside the closure to an implicit assertion.

given:
      def person = new Person(name: "Peter", age: 28)
expect:
      with(person) {
          name == 'Peter'
          age == 28
      }

This worked fine for properties, however for single methods like `contains` it didn't work.

The initial problem is described here https://github.com/spockframework/spock/pull/606

Here is the gist:

This snippet

 
def list = [1, 2]
with(list) {
  contains(1)
}
 

transforms in AST (simplified) to

def list = [1, 2]
with(list) {
  SpockRuntime.verifyMethodCondition(this, "contains", [1])
}
 

then when the AST is written to bytecode it gets transformed again

def list = [1, 2]
with(list) {
  SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains", [1])
}

The problem is that the `contains` is now invoked on the containing `Specification` instead of the `List`.

With the aforementioned PR it was changed to this

def list = [1, 2]
with(list) {
  SpockRuntime.verifyMethodCondition(this.each(groovy.lang.Closure.IDENTITY), "contains", [1])
}
 

This "fix" now broke the nesting of `with` blocks as described here: https://github.com/spockframework/spock/pull/782

Do you have any ideas on how to fix this elegantly?

- Cheers
Leonard

Reply | Threaded
Open this post in threaded view
|

Re: Get reference to enclosing closure

Leonard Brünings

Hi Eric,

transforming every implicit method call this way would also break the nested `with` blocks.
It would basically be a DELEGATE_ONLY strategy, which would prevent helper methods defined
in the class to be used.

-Leo


Am 20.11.2017 um 21:29 schrieb [hidden email]:

I think "this" or "getThisObject()" should really be "delegate" or "getDelegate()".

 

From: Leonard Brünings [[hidden email]]
Sent: Monday, November 20, 2017 2:27 PM
To: [hidden email]
Subject: Get reference to enclosing closure

 

Hi,

I'm Leonard from the Spock framework team. Guillaume suggested that I write to the dev-list with this problem.

Some context:

Spock has a method `with(Object, Closure)` in its Specification class that sets the object as the delegate of the closure and transforms,
every call inside the closure to an implicit assertion.

given:
      def person = new Person(name: "Peter", age: 28)
expect:
      with(person) {
          name == 'Peter'
          age == 28
      }

This worked fine for properties, however for single methods like `contains` it didn't work.

The initial problem is described here https://github.com/spockframework/spock/pull/606

Here is the gist:

This snippet

 
def list = [1, 2]
with(list) {
  contains(1)
}
 

transforms in AST (simplified) to

def list = [1, 2]
with(list) {
  SpockRuntime.verifyMethodCondition(this, "contains", [1])
}
 

then when the AST is written to bytecode it gets transformed again

def list = [1, 2]
with(list) {
  SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains", [1])
}

The problem is that the `contains` is now invoked on the containing `Specification` instead of the `List`.

With the aforementioned PR it was changed to this

def list = [1, 2]
with(list) {
  SpockRuntime.verifyMethodCondition(this.each(groovy.lang.Closure.IDENTITY), "contains", [1])
}
 

This "fix" now broke the nesting of `with` blocks as described here: https://github.com/spockframework/spock/pull/782

Do you have any ideas on how to fix this elegantly?

- Cheers
Leonard


Reply | Threaded
Open this post in threaded view
|

RE: Get reference to enclosing closure

eric.milles

When you rewrite to this form:

 

with(list) {
  SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains", [1])

}

 

You've esentially frozen your choice of delegate, owner or this.  The dynamic resolution of "implicit this" to one of those is no longer going to happen.  I guess you'd need to re-implement the resolution strategy.

Reply | Threaded
Open this post in threaded view
|

Re: Get reference to enclosing closure

Leonard Brünings

Spock doesn't rewrite it to `this.getThisObject()`.

This is what Spock does

with(list) {
  SpockRuntime.verifyMethodCondition(this, "contains", [1])
}

The transformation from `this` to `this.getThisObject()` is done by groovy at a later stage.
My guess somewhere in the AST-to-Bytecode ASM code as I can see the upper version in the
last stage of the AST-Transforms.


with(list) {
  SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains", [1])
}
Am 20.11.2017 um 22:09 schrieb [hidden email]:

When you rewrite to this form:

 

with(list) {
  SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains", [1])

}

 

You've esentially frozen your choice of delegate, owner or this.  The dynamic resolution of "implicit this" to one of those is no longer going to happen.  I guess you'd need to re-implement the resolution strategy.


Reply | Threaded
Open this post in threaded view
|

RE: Get reference to enclosing closure

eric.milles

If this snippet, "this" will always refer to the enclosing class, never the delegate "list" or the owner (which may be the enclosing closure or "this" if no enclosing closure).  That is, the choice of "this" has already frozen where "contains" can come from.

 

with(list) {
  SpockRuntime.verifyMethodCondition(this, "contains", [1])

}

 

 

From: Leonard Brünings [mailto:[hidden email]]
Sent: Monday, November 20, 2017 5:52 PM
To: [hidden email]
Subject: Re: Get reference to enclosing closure

 

Spock doesn't rewrite it to `this.getThisObject()`.

This is what Spock does

with(list) {
  SpockRuntime.verifyMethodCondition(this, "contains", [1])

}

The transformation from `this` to `this.getThisObject()` is done by groovy at a later stage.
My guess somewhere in the AST-to-Bytecode ASM code as I can see the upper version in the
last stage of the AST-Transforms.

 

with(list) {
  SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains", [1])

}

Am 20.11.2017 um 22:09 schrieb [hidden email]:

When you rewrite to this form:

 

with(list) {
  SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains", [1])

}

 

You've esentially frozen your choice of delegate, owner or this.  The dynamic resolution of "implicit this" to one of those is no longer going to happen.  I guess you'd need to re-implement the resolution strategy.

 

Reply | Threaded
Open this post in threaded view
|

Re: Get reference to enclosing closure

Jochen Theodorou
In reply to this post by Leonard Brünings
I am not saying I am getting the question fully...

but what you want is

def c = { return XY }
assert c() == c

something like that? Extend BytecodeExpersion and do a "ALOAD 0". No
guarantees that this will work forever though. In the future there might
be no closure class anymore.

Oh and if I got you wrong, and you want a reference to the enclosing
context, that would be the owner:

def x = this
def c = { return {owner}}
assert c()() == c
assert c.owner == x

bye Jochen

On 20.11.2017 21:27, Leonard Brünings wrote:

> Hi,
>
> I'm Leonard from the Spock framework team. Guillaume suggested that I
> write to the dev-list with this problem.
>
> Some context:
>
> Spock has a method `with(Object, Closure)` in its Specification class
> that sets the object as the delegate of the closure and transforms,
> every call inside the closure to an implicit assertion.
>
> given:
>        def person = new Person(name: "Peter", age: 28)
>
> expect:
>        with(person) {
>            name == 'Peter'
>            age == 28
>        }
>
> This worked fine for properties, however for single methods like
> `contains` it didn't work.
>
> The initial problem is described here
> https://github.com/spockframework/spock/pull/606
>
> Here is the gist:
>
> This snippet
>
> d|ef list = [1, 2] with(list) { contains(1) } |
>
> transforms in AST (simplified) to
>
> |def list = [1, 2] with(list) { SpockRuntime.verifyMethodCondition(this,
> "contains", [1]) } |
>
> then when the AST is written to bytecode it gets transformed again
>
> |def list = [1, 2] with(list) {
> SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains", [1]) }|
>
> The problem is that the `contains` is now invoked on the containing
> `Specification` instead of the `List`.
>
> With the aforementioned PR it was changed to this
>
> |def list = [1, 2] with(list) {
> SpockRuntime.verifyMethodCondition(this.each(groovy.lang.Closure.IDENTITY),
> "contains", [1]) } |
>
> This "fix" now broke the nesting of `with` blocks as described here:
> https://github.com/spockframework/spock/pull/782
>
> Do you have any ideas on how to fix this elegantly?
>
> - Cheers
> Leonard
>
> ||||||
>
> ||
>

Reply | Threaded
Open this post in threaded view
|

Re: Get reference to enclosing closure

Leonard Brünings
Hi Jochen,

your first suggestion might do the trick and if closures disappear then
there will be much more that doesn't work.

We currently do something like this:


   public void replaceImplicitThis(Expression invocation) {
     if (invocation instanceof MethodCallExpression) {
       MethodCallExpression methodCallExpression =
(MethodCallExpression)invocation;
       if (methodCallExpression.isImplicitThis()) {

         Expression target = referenceToCurrentClosure();
         methodCallExpression.setObjectExpression(target);
       }
     }
   }

   private MethodCallExpression referenceToCurrentClosure() {
     return new MethodCallExpression(
       new VariableExpression("this"),
       new ConstantExpression("each"),
       new ArgumentListExpression(
         new PropertyExpression(
           new
ClassExpression(ClassHelper.makeWithoutCaching(Closure.class)),
           new ConstantExpression("IDENTITY")
         )
       )
     );
   }

This looks a similar to your second suggestion.

Could you give me a hint on how to write a BytecodeExpersion for "ALOAD
0"? Could I just use this in place of the other MethodCallExpression
from referenceToCurrentClosure?

-Leo

Am 21.11.2017 um 19:32 schrieb Jochen Theodorou:

> I am not saying I am getting the question fully...
>
> but what you want is
>
> def c = { return XY }
> assert c() == c
>
> something like that? Extend BytecodeExpersion and do a "ALOAD 0". No
> guarantees that this will work forever though. In the future there
> might be no closure class anymore.
>
> Oh and if I got you wrong, and you want a reference to the enclosing
> context, that would be the owner:
>
> def x = this
> def c = { return {owner}}
> assert c()() == c
> assert c.owner == x
>
> bye Jochen
>
> On 20.11.2017 21:27, Leonard Brünings wrote:
>> Hi,
>>
>> I'm Leonard from the Spock framework team. Guillaume suggested that I
>> write to the dev-list with this problem.
>>
>> Some context:
>>
>> Spock has a method `with(Object, Closure)` in its Specification class
>> that sets the object as the delegate of the closure and transforms,
>> every call inside the closure to an implicit assertion.
>>
>> given:
>>        def person = new Person(name: "Peter", age: 28)
>>
>> expect:
>>        with(person) {
>>            name == 'Peter'
>>            age == 28
>>        }
>>
>> This worked fine for properties, however for single methods like
>> `contains` it didn't work.
>>
>> The initial problem is described here
>> https://github.com/spockframework/spock/pull/606
>>
>> Here is the gist:
>>
>> This snippet
>>
>> d|ef list = [1, 2] with(list) { contains(1) } |
>>
>> transforms in AST (simplified) to
>>
>> |def list = [1, 2] with(list) {
>> SpockRuntime.verifyMethodCondition(this, "contains", [1]) } |
>>
>> then when the AST is written to bytecode it gets transformed again
>>
>> |def list = [1, 2] with(list) {
>> SpockRuntime.verifyMethodCondition(this.getThisObject(), "contains",
>> [1]) }|
>>
>> The problem is that the `contains` is now invoked on the containing
>> `Specification` instead of the `List`.
>>
>> With the aforementioned PR it was changed to this
>>
>> |def list = [1, 2] with(list) {
>> SpockRuntime.verifyMethodCondition(this.each(groovy.lang.Closure.IDENTITY),
>> "contains", [1]) } |
>>
>> This "fix" now broke the nesting of `with` blocks as described here:
>> https://github.com/spockframework/spock/pull/782
>>
>> Do you have any ideas on how to fix this elegantly?
>>
>> - Cheers
>> Leonard
>>
>> ||||||
>>
>> ||
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: Get reference to enclosing closure

Jochen Theodorou
On 21.11.2017 21:43, Leonard Brünings wrote:

> Hi Jochen,
>
> your first suggestion might do the trick and if closures disappear then
> there will be much more that doesn't work.
>
> We currently do something like this:
>
>
>    public void replaceImplicitThis(Expression invocation) {
>      if (invocation instanceof MethodCallExpression) {
>        MethodCallExpression methodCallExpression =
> (MethodCallExpression)invocation;
>        if (methodCallExpression.isImplicitThis()) {
>
>          Expression target = referenceToCurrentClosure();
>          methodCallExpression.setObjectExpression(target);
>        }
>      }
>    }
>
>    private MethodCallExpression referenceToCurrentClosure() {
>      return new MethodCallExpression(
>        new VariableExpression("this"),
>        new ConstantExpression("each"),
>        new ArgumentListExpression(
>          new PropertyExpression(
>            new
> ClassExpression(ClassHelper.makeWithoutCaching(Closure.class)),
>            new ConstantExpression("IDENTITY")
>          )
>        )
>      );
>    }
>
> This looks a similar to your second suggestion.

similar in that both get the current reference, yes.

> Could you give me a hint on how to write a BytecodeExpersion for "ALOAD
> 0"? Could I just use this in place of the other MethodCallExpression
> from referenceToCurrentClosure?

You use it in place of referenceToCurrentClosure,yes. Something like

>                         new BytecodeExpression(ClassHelper.CLOSURE) {
>                             public void visit(MethodVisitor mv) {
>                                 mv.visitVarInsn(ALOAD, 0);
>                             }
>                         });

frankly this is using a feature I do not like so much and that is that
Groovy handles a call on a closure instance the same way as a call from
within that closure instance (well, not 100% the same, there are
differences).But in this case you profit from it. Otherwise you would
need a way to express the implicit this as parameter... Maybe this is
actually something we should do..

bye Jochen
Reply | Threaded
Open this post in threaded view
|

Re: Get reference to enclosing closure

Leonard Brünings
Great now I run into another problem, spock uses groovy-all, which
repackages asm.
The BytecodeExpression has a MethodVisitor from asm, which is now in
groovyjarjarasm.asm.MethodVisitor
This compiles fine, but when the AST transforms are executed it breaks
down with this exception:

Caused by: java.lang.LinkageError: loader constraint violation: loader
(instance of groovy/lang/GroovyClassLoader) previously initiated loading
for a different type with name "groovyjarjarasm/asm/MethodVisitor"
         at
org.spockframework.compiler.DeepBlockRewriter$1.visit(DeepBlockRewriter.java:190)
         at
org.gradle.api.internal.tasks.compile.ApiGroovyCompiler.execute(ApiGroovyCompiler.java:174)
         at
org.gradle.api.internal.tasks.compile.ApiGroovyCompiler.execute(ApiGroovyCompiler.java:56)
         at
org.gradle.api.internal.tasks.compile.daemon.AbstractDaemonCompiler$CompilerRunnable.run(AbstractDaemonCompiler.java:87)
         at
org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:36)
         at
org.gradle.workers.internal.WorkerDaemonServer.execute(WorkerDaemonServer.java:46)
         at
org.gradle.workers.internal.WorkerDaemonServer.execute(WorkerDaemonServer.java:30)
         at
org.gradle.process.internal.worker.request.WorkerAction.run(WorkerAction.java:100)
         at
org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
         at
org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
         at
org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:146)
         at
org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:128)
         at
org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:404)

I tried using the non-repackaged classes, but then it doesn't even
compile since BytecodeExpression wants groovyjarjarasm.

Thanks

-Leo


Am 22.11.2017 um 00:49 schrieb Jochen Theodorou:

> On 21.11.2017 21:43, Leonard Brünings wrote:
>> Hi Jochen,
>>
>> your first suggestion might do the trick and if closures disappear
>> then there will be much more that doesn't work.
>>
>> We currently do something like this:
>>
>>
>>    public void replaceImplicitThis(Expression invocation) {
>>      if (invocation instanceof MethodCallExpression) {
>>        MethodCallExpression methodCallExpression =
>> (MethodCallExpression)invocation;
>>        if (methodCallExpression.isImplicitThis()) {
>>
>>          Expression target = referenceToCurrentClosure();
>>          methodCallExpression.setObjectExpression(target);
>>        }
>>      }
>>    }
>>
>>    private MethodCallExpression referenceToCurrentClosure() {
>>      return new MethodCallExpression(
>>        new VariableExpression("this"),
>>        new ConstantExpression("each"),
>>        new ArgumentListExpression(
>>          new PropertyExpression(
>>            new
>> ClassExpression(ClassHelper.makeWithoutCaching(Closure.class)),
>>            new ConstantExpression("IDENTITY")
>>          )
>>        )
>>      );
>>    }
>>
>> This looks a similar to your second suggestion.
>
> similar in that both get the current reference, yes.
>
>> Could you give me a hint on how to write a BytecodeExpersion for
>> "ALOAD 0"? Could I just use this in place of the other
>> MethodCallExpression from referenceToCurrentClosure?
>
> You use it in place of referenceToCurrentClosure,yes. Something like
>
>>                         new BytecodeExpression(ClassHelper.CLOSURE) {
>>                             public void visit(MethodVisitor mv) {
>>                                 mv.visitVarInsn(ALOAD, 0);
>>                             }
>>                         });
>
> frankly this is using a feature I do not like so much and that is that
> Groovy handles a call on a closure instance the same way as a call
> from within that closure instance (well, not 100% the same, there are
> differences).But in this case you profit from it. Otherwise you would
> need a way to express the implicit this as parameter... Maybe this is
> actually something we should do..
>
> bye Jochen

12