You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/06/13 13:34:20 UTC

[groovy] branch master updated (0aafd97 -> 9c11061)

This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git.


    from 0aafd97  GROOVY-8840: use inferred type for subscript if operand is placeholder
     new 3bf8f9f  GROOVY-7701: do not cast RHS before LHS of assignment is visited
     new 9c11061  GROOVY-7701: SC: replace field/property reference with dynamic variable

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../classgen/asm/BinaryExpressionHelper.java       |  8 ++----
 .../transform/stc/StaticTypeCheckingVisitor.java   |  6 +++++
 src/test/groovy/lang/ClosureResolvingTest.groovy   | 31 +++++++++++++++++++---
 .../groovy/transform/stc/ClosuresSTCTest.groovy    | 30 +++++++++++++++++++++
 4 files changed, 65 insertions(+), 10 deletions(-)


[groovy] 01/02: GROOVY-7701: do not cast RHS before LHS of assignment is visited

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 3bf8f9fe71aaa471a72029d410cedbffa83d52fb
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Jun 4 13:59:12 2020 -0500

    GROOVY-7701: do not cast RHS before LHS of assignment is visited
---
 .../classgen/asm/BinaryExpressionHelper.java       |  8 ++----
 src/test/groovy/lang/ClosureResolvingTest.groovy   | 31 +++++++++++++++++++---
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
index 02f02e9..0e3f623 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
@@ -382,8 +382,8 @@ public class BinaryExpressionHelper {
             operandStack.loadOrStoreVariable(var, false);
             return;
         }
-
-        // let's evaluate the RHS and store the result
+        // evaluate the RHS and store the result
+        // TODO: LHS has not been visited, it could be a variable in a closure and type chooser is not aware.
         ClassNode lhsType = controller.getTypeChooser().resolveType(leftExpression, controller.getClassNode());
         if (rightExpression instanceof ListExpression && lhsType.isArray()) {
             ListExpression list = (ListExpression) rightExpression;
@@ -467,11 +467,7 @@ public class BinaryExpressionHelper {
         } else {
             // normal assignment
             int mark = operandStack.getStackLength();
-            // to leave a copy of the rightExpression value on the stack after the assignment.
             rhsValueLoader.visit(acg);
-            TypeChooser typeChooser = controller.getTypeChooser();
-            ClassNode targetType = typeChooser.resolveType(leftExpression, controller.getClassNode());
-            operandStack.doGroovyCast(targetType);
             leftExpression.visit(acg);
             operandStack.remove(operandStack.getStackLength() - mark);
         }
diff --git a/src/test/groovy/lang/ClosureResolvingTest.groovy b/src/test/groovy/lang/ClosureResolvingTest.groovy
index 14f27a2..69d2932 100644
--- a/src/test/groovy/lang/ClosureResolvingTest.groovy
+++ b/src/test/groovy/lang/ClosureResolvingTest.groovy
@@ -84,7 +84,33 @@ class ClosureResolvingTest extends GroovyTestCase {
         assertEquals "stuff", c.call()
         c.delegate = new TestResolve1()
         assertEquals "foo", c.call()
+    }
+
+    // GROOVY-7701
+    void testResolveDelegateFirst2() {
+        assertScript '''
+            class Foo {
+                List type
+            }
+            class Bar {
+                int type = 10
+                List<Foo> something = { ->
+                    List<Foo> tmp = []
+                    def foo = new Foo()
+                    foo.with {
+                        type = ['String']
+                    //  ^^^^ should be Foo.type, not Bar.type
+                    }
+                    tmp.add(foo)
+                    return tmp
+                }()
+            }
 
+            def bar = new Bar()
+            assert bar.type == 10
+            assert bar.something*.type == [['String']]
+            assert bar.type == 10
+        '''
     }
 
     void testResolveOwnerFirst() {
@@ -125,7 +151,6 @@ class ClosureResolvingTest extends GroovyTestCase {
         c.resolveStrategy = Closure.DELEGATE_ONLY
         c.delegate = new TestResolve1()
         assertEquals "foo", c.call()
-
     }
 
     void testResolveOwnerOnly() {
@@ -142,7 +167,6 @@ class ClosureResolvingTest extends GroovyTestCase {
         c.resolveStrategy = Closure.OWNER_ONLY
         c.delegate = new TestResolve1()
         assertEquals "stuff", c.call()
-
     }
 
     void testOwnerDelegateChain() {
@@ -196,10 +220,9 @@ class TestResolve2 {
 class TestResolve3 {
     def del;
 
-    String toString() {del}
+    String toString() { del }
 
     def whoisThis() { return this }
 
     def met() { return "I'm the method inside '" + del + "'" }
 }
-


[groovy] 02/02: GROOVY-7701: SC: replace field/property reference with dynamic variable

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 9c110618ad1fef0ddbec49c4d0592b872a3e5ebd
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Jun 5 10:00:18 2020 -0500

    GROOVY-7701: SC: replace field/property reference with dynamic variable
    
    - VariableScopeVisitor lacks closure delegation information
---
 .../transform/stc/StaticTypeCheckingVisitor.java   |  6 +++++
 .../groovy/transform/stc/ClosuresSTCTest.groovy    | 30 ++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index da55cbf..be84a4c 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -664,6 +664,12 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
             vexp.removeNodeMetaData(INFERRED_TYPE);
             ClassNode type = pexp.getNodeMetaData(INFERRED_TYPE);
             storeType(vexp, Optional.ofNullable(type).orElseGet(pexp::getType));
+
+            String receiver = vexp.getNodeMetaData(IMPLICIT_RECEIVER);
+            // GROOVY-7701: correct false assumption made by VariableScopeVisitor
+            if (receiver != null && !receiver.endsWith("owner") && !(vexp.getAccessedVariable() instanceof DynamicVariable)) {
+                vexp.setAccessedVariable(new DynamicVariable(dynName, false));
+            }
             return true;
         }
         return false;
diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
index 80bd2bc..17b7a33 100644
--- a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
@@ -127,6 +127,36 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
+    // GROOVY-7701
+    void testWithDelegateVsOwnerField() {
+        assertScript '''
+            class Foo {
+                List type
+            }
+
+            class Bar {
+                int type = 10
+
+                @Lazy
+                List<Foo> something = { ->
+                    List<Foo> tmp = []
+                    def foo = new Foo()
+                    foo.with {
+                        type = ['String']
+                    //  ^^^^ should be Foo.type, not Bar.type
+                    }
+                    tmp.add(foo)
+                    tmp
+                }()
+            }
+
+            def bar = new Bar()
+            assert bar.type == 10
+            assert bar.something*.type == [['String']]
+            assert bar.type == 10
+        '''
+    }
+
     void testClosureShouldNotChangeInferredType() {
         assertScript '''
             def x = '123';