blackdrag commented on code in PR #2714:
URL: https://github.com/apache/groovy/pull/2714#discussion_r4509185907


##########
src/main/java/org/codehaus/groovy/runtime/PackedClosure.java:
##########
@@ -243,12 +246,19 @@ public PackedClosure(final Object owner, final 
GeneratedDispatcher.Bundle dispat
         this.parameterTypes = visibleTypes;
     }
 
+    /** Attaches the open body's resolver, so delegate/strategy changes 
reconfigure name resolution. */
+    void openOver(final OpenClosure.ClassicResolver resolver) {
+        this.classic = resolver;
+        resolver.bind(this);
+    }
+
     // GEP-27 runtime guard: a hoisted body binds free names to the owner at 
compile time, so it
     // cannot honour a caller-set delegate. Rather than silently mis-resolve 
(returning the owner's
     // member where a normal closure would reach the delegate), fail fast at 
the point of misuse.
     // Setting the delegate to the owner is the harmless default and stays a 
no-op.
     @Override
     public void setDelegate(final Object delegate) {
+        if (classic != null) classic.setDelegate(delegate);
         if (strict && delegate != null && delegate != getOwner()) {
             throw new UnsupportedOperationException(

Review Comment:
   I assume classic and strict exclude each other. Just looks a bit strange to 
first set a delegate and then to potentially throw an exception that it is not 
supported. It would read better to move the classic check after the strict 
check. This is minor, as long as they exclude each other. If they do not 
exclude each other, then this needs at least more explanation. That goes for 
setResolveStrategy as well of course.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to