groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject [2/2] groovy git commit: GROOVY-8384 - Regression in 2.4.13 snapshot with STC and intdiv plus some cleanup (closes #639)
Date Sun, 19 Nov 2017 05:04:32 GMT
GROOVY-8384 - Regression in 2.4.13 snapshot with STC and intdiv plus some cleanup (closes #639)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/8294b627
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/8294b627
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/8294b627

Branch: refs/heads/master
Commit: 8294b6272eeb81293605827b4fb34b179bf25770
Parents: 0bdbe55
Author: paulk <paulk@asert.com.au>
Authored: Sun Nov 19 00:29:08 2017 +1000
Committer: paulk <paulk@asert.com.au>
Committed: Sun Nov 19 15:03:29 2017 +1000

----------------------------------------------------------------------
 .../groovy/classgen/asm/CallSiteWriter.java     |  7 --
 .../NumberMathModificationInfo.java             | 10 ++-
 .../stc/StaticTypeCheckingSupport.java          | 23 +++++-
 .../stc/StaticTypeCheckingVisitor.java          | 83 ++++++++++++--------
 .../groovy/transform/stc/MiscSTCTest.groovy     | 19 +++++
 5 files changed, 97 insertions(+), 45 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/8294b627/src/main/org/codehaus/groovy/classgen/asm/CallSiteWriter.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/asm/CallSiteWriter.java b/src/main/org/codehaus/groovy/classgen/asm/CallSiteWriter.java
index 0309138..4983b4b 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/CallSiteWriter.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/CallSiteWriter.java
@@ -34,16 +34,9 @@ import static org.objectweb.asm.Opcodes.*;
 /**
  * This class represents non public API used by AsmClassGenerator. Don't
  * use this class in your code
- * @author Jochen Theodorou
  */
 public class CallSiteWriter {
     
-    private static final Set<String> NAMES = new HashSet<String>();
-    private static final Set<String> BASIC = new HashSet<String>();
-    static {
-        Collections.addAll(NAMES, "plus", "minus", "multiply", "div", "compareTo", "or",
"and", "xor", "intdiv", "mod", "leftShift", "rightShift", "rightShiftUnsigned");
-        Collections.addAll(BASIC, "plus", "minus", "multiply", "div");
-    }
     private static String [] sig = new String [255];
     private static String getCreateArraySignature(int numberOfArguments) {
         if (sig[numberOfArguments] == null) {

http://git-wip-us.apache.org/repos/asf/groovy/blob/8294b627/src/main/org/codehaus/groovy/runtime/typehandling/NumberMathModificationInfo.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/runtime/typehandling/NumberMathModificationInfo.java
b/src/main/org/codehaus/groovy/runtime/typehandling/NumberMathModificationInfo.java
index 4525c06..65dbf41 100644
--- a/src/main/org/codehaus/groovy/runtime/typehandling/NumberMathModificationInfo.java
+++ b/src/main/org/codehaus/groovy/runtime/typehandling/NumberMathModificationInfo.java
@@ -29,12 +29,14 @@ public class NumberMathModificationInfo {
 
     public static final NumberMathModificationInfo instance = new NumberMathModificationInfo();
 
-    private final HashSet<String> names = new HashSet<String>();
+    private static final HashSet<String> NAMES = new HashSet<String>();
 
-    private NumberMathModificationInfo() {
-        Collections.addAll(names, "plus", "minus", "multiply", "div", "compareTo", "or",
"and", "xor", "intdiv", "mod", "leftShift", "rightShift", "rightShiftUnsigned");
+    static {
+        Collections.addAll(NAMES, "plus", "minus", "multiply", "div", "compareTo", "or",
"and", "xor", "intdiv", "mod", "leftShift", "rightShift", "rightShiftUnsigned");
     }
 
+    private NumberMathModificationInfo() { }
+
     public void checkIfStdMethod(MetaMethod method) {
         if (method.getClass() != NewInstanceMetaMethod.class) {
             String name = method.getName();
@@ -45,7 +47,7 @@ public class NumberMathModificationInfo {
             if (!method.getParameterTypes()[0].isNumber && method.getParameterTypes()[0].getTheClass()
!= Object.class)
                 return;
 
-            if (!names.contains(name))
+            if (!NAMES.contains(name))
                 return;
 
             checkNumberOps(name, method.getDeclaringClass().getTheClass());

http://git-wip-us.apache.org/repos/asf/groovy/blob/8294b627/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index 30584a9..9f9fc5f 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -76,6 +76,22 @@ public abstract class StaticTypeCheckingSupport {
                 put(Double_TYPE, 5);
             }});
 
+    protected static final Map<String, Integer> NUMBER_OPS = Collections.unmodifiableMap(
+            new HashMap<String, Integer>() {{
+                put("plus", PLUS);
+                put("minus", MINUS);
+                put("multiply", MULTIPLY);
+                put("div", DIVIDE);
+                put("or", BITWISE_OR);
+                put("and", BITWISE_AND);
+                put("xor", BITWISE_XOR);
+                put("mod", MOD);
+                put("intdiv", INTDIV);
+                put("leftShift", LEFT_SHIFT);
+                put("rightShift", RIGHT_SHIFT);
+                put("rightShiftUnsigned", RIGHT_SHIFT_UNSIGNED);
+            }});
+
     protected static final ClassNode GSTRING_STRING_CLASSNODE = WideningCategories.lowestUpperBound(
             STRING_TYPE,
             GSTRING_TYPE
@@ -373,7 +389,7 @@ public abstract class StaticTypeCheckingSupport {
     }
 
     static boolean isBoolIntrinsicOp(int op) {
-        return op == LOGICAL_AND || op == LOGICAL_OR ||
+        return op == LOGICAL_AND || op == LOGICAL_OR || op == COMPARE_NOT_IDENTICAL || op
== COMPARE_IDENTICAL ||
                 op == MATCH_REGEX || op == KEYWORD_INSTANCEOF || op == COMPARE_NOT_INSTANCEOF;
     }
 
@@ -660,7 +676,12 @@ public abstract class StaticTypeCheckingSupport {
         return node.getCompileUnit() != null;
     }
 
+    @Deprecated
     static boolean checkPossibleLooseOfPrecision(ClassNode left, ClassNode right, Expression
rightExpr) {
+        return checkPossibleLossOfPrecision(left, right, rightExpr);
+    }
+
+    static boolean checkPossibleLossOfPrecision(ClassNode left, ClassNode right, Expression
rightExpr) {
         if (left == right || left.equals(right)) return false; // identical types
         int leftIndex = NUMBER_TYPES.get(left);
         int rightIndex = NUMBER_TYPES.get(right);

http://git-wip-us.apache.org/repos/asf/groovy/blob/8294b627/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 18c04de..80a0d06 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -110,9 +110,7 @@ import static org.codehaus.groovy.ast.tools.WideningCategories.lowestUpperBound;
 import static org.codehaus.groovy.syntax.Types.ASSIGN;
 import static org.codehaus.groovy.syntax.Types.ASSIGNMENT_OPERATOR;
 import static org.codehaus.groovy.syntax.Types.COMPARE_EQUAL;
-import static org.codehaus.groovy.syntax.Types.COMPARE_IDENTICAL;
 import static org.codehaus.groovy.syntax.Types.COMPARE_NOT_EQUAL;
-import static org.codehaus.groovy.syntax.Types.COMPARE_NOT_IDENTICAL;
 import static org.codehaus.groovy.syntax.Types.COMPARE_NOT_IN;
 //import static org.codehaus.groovy.syntax.Types.COMPARE_NOT_INSTANCEOF;
 import static org.codehaus.groovy.syntax.Types.COMPARE_TO;
@@ -121,6 +119,8 @@ import static org.codehaus.groovy.syntax.Types.DIVIDE_EQUAL;
 import static org.codehaus.groovy.syntax.Types.ELVIS_EQUAL;
 import static org.codehaus.groovy.syntax.Types.EQUAL;
 import static org.codehaus.groovy.syntax.Types.FIND_REGEX;
+import static org.codehaus.groovy.syntax.Types.INTDIV;
+import static org.codehaus.groovy.syntax.Types.INTDIV_EQUAL;
 import static org.codehaus.groovy.syntax.Types.KEYWORD_IN;
 import static org.codehaus.groovy.syntax.Types.KEYWORD_INSTANCEOF;
 import static org.codehaus.groovy.syntax.Types.LEFT_SQUARE_BRACKET;
@@ -930,7 +930,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
 
     private void addPrecisionErrors(ClassNode leftRedirect, ClassNode lhsType, ClassNode
inferredrhsType, Expression rightExpression) {
         if (isNumberType(leftRedirect) && isNumberType(inferredrhsType)) {
-            if (checkPossibleLooseOfPrecision(leftRedirect, inferredrhsType, rightExpression))
{
+            if (checkPossibleLossOfPrecision(leftRedirect, inferredrhsType, rightExpression))
{
                 addStaticTypeError("Possible loss of precision from " + inferredrhsType +
" to " + leftRedirect, rightExpression);
                 return;
             }
@@ -3098,6 +3098,16 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
                     }
                 }
             }
+            // adjust typing for explicit math methods which have special handling - operator
variants handled elsewhere
+            if (NUMBER_OPS.containsKey(name) && isNumberType(receiver) &&
argumentList.getExpressions().size() == 1
+                    && isNumberType(getType(argumentList.getExpression(0)))) {
+                ClassNode right = getType(argumentList.getExpression(0));
+                ClassNode resultType = getMathResultType(NUMBER_OPS.get(name), receiver,
right, name);
+                if (resultType != null) {
+                    storeType(call, resultType);
+                }
+            }
+
             // now that a method has been chosen, we are allowed to visit the closures
             if (!callArgsVisited) {
                 MethodNode mn = (MethodNode) call.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
@@ -3132,8 +3142,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
      *
      * @param directMethodCallCandidate a method selected by the type checker
      * @param receiver the receiver of the method call
-     *@param args the arguments of the method call
-     * @param returnType the original return type, as inferred by the type checker   @return
fixed return type if the selected method is {@link org.codehaus.groovy.runtime.DefaultGroovyMethods#withTraits(Object,
Class[]) withTraits}
+     * @param args the arguments of the method call
+     * @param returnType the original return type, as inferred by the type checker
+     * @return fixed return type if the selected method is {@link org.codehaus.groovy.runtime.DefaultGroovyMethods#withTraits(Object,
Class[]) withTraits}
      */
     private static ClassNode adjustWithTraits(final MethodNode directMethodCallCandidate,
final ClassNode receiver, final ClassNode[] args, final ClassNode returnType) {
         if (directMethodCallCandidate instanceof ExtensionMethodNode) {
@@ -3593,10 +3604,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
         ClassNode leftRedirect = left.redirect();
         ClassNode rightRedirect = right.redirect();
 
-        if (op == COMPARE_NOT_IDENTICAL || op == COMPARE_IDENTICAL) {
-            return boolean_TYPE;
-        }
-
         Expression leftExpression = expr.getLeftExpression();
         Expression rightExpression = expr.getRightExpression();
         if (op == ASSIGN || op == ASSIGNMENT_OPERATOR || op == ELVIS_EQUAL) {
@@ -3638,9 +3645,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
                 }
             }
             return right;
-        } else if (isBoolIntrinsicOp(op)) {
+        }
+        if (isBoolIntrinsicOp(op)) {
             return boolean_TYPE;
-        } else if (isArrayOp(op)) {
+        }
+        if (isArrayOp(op)) {
             // using getPNR() to ignore generics at this point
             // and a different binary expression not to pollute the AST
             BinaryExpression newExpr = binX(expr.getLeftExpression(), expr.getOperation(),
rightExpression);
@@ -3650,13 +3659,40 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
                 return inferReturnTypeGenerics(left, method, rightExpression);
             }
             return method!=null?inferComponentType(left, right):null;
-        } else if (op == FIND_REGEX) {
+        }
+        if (op == FIND_REGEX) {
             // this case always succeeds the result is a Matcher
             return Matcher_TYPE;
         }
         // the left operand is determining the result of the operation
         // for primitives and their wrapper we use a fixed table here
-        else if (isNumberType(leftRedirect) && isNumberType(rightRedirect)) {
+        String operationName = getOperationName(op);
+        ClassNode mathResultType = getMathResultType(op, leftRedirect, rightRedirect, operationName);
+        if (mathResultType != null) {
+            return mathResultType;
+        }
+
+        // GROOVY-5890
+        // do not mix Class<Foo> with Foo
+        if (leftExpression instanceof ClassExpression) {
+            left = CLASS_Type.getPlainNodeReference();
+        }
+
+        MethodNode method = findMethodOrFail(expr, left, operationName, right);
+        if (method != null) {
+            storeTargetMethod(expr, method);
+            typeCheckMethodsWithGenericsOrFail(left, new ClassNode[]{right}, method, expr);
+            if (isAssignment(op)) return left;
+            if (isCompareToBoolean(op)) return boolean_TYPE;
+            if (op == COMPARE_TO) return int_TYPE;
+            return inferReturnTypeGenerics(left, method, args(rightExpression));
+        }
+        //TODO: other cases
+        return null;
+    }
+
+    private ClassNode getMathResultType(int op, ClassNode leftRedirect, ClassNode rightRedirect,
String operationName) {
+        if (isNumberType(leftRedirect) && isNumberType(rightRedirect)) {
             if (isOperationInGroup(op)) {
                 if (isIntCategory(leftRedirect) && isIntCategory(rightRedirect))
return int_TYPE;
                 if (isLongCategory(leftRedirect) && isLongCategory(rightRedirect))
return long_TYPE;
@@ -3664,7 +3700,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
                 if (isDouble(leftRedirect) && isDouble(rightRedirect)) return double_TYPE;
             } else if (isPowerOperator(op)) {
                 return Number_TYPE;
-            } else if (isBitOperator(op)) {
+            } else if (isBitOperator(op) || op == INTDIV || op == INTDIV_EQUAL) {
                 if (isIntCategory(getUnwrapper(leftRedirect)) && isIntCategory(getUnwrapper(rightRedirect)))
return int_TYPE;
                 if (isLongCategory(getUnwrapper(leftRedirect)) && isLongCategory(getUnwrapper(rightRedirect)))
return long_TYPE;
                 if (isBigIntCategory(getUnwrapper(leftRedirect)) && isBigIntCategory(getUnwrapper(rightRedirect)))
return BigInteger_TYPE;
@@ -3677,9 +3713,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
             }
         }
 
-
         // try to find a method for the operation
-        String operationName = getOperationName(op);
         if (isShiftOperation(operationName) && isNumberCategory(leftRedirect) &&
(isIntCategory(rightRedirect) || isLongCategory(rightRedirect))) {
             return leftRedirect;
         }
@@ -3704,23 +3738,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
         if (isNumberCategory(getWrapper(rightRedirect)) && isNumberCategory(getWrapper(leftRedirect))
&& (MOD == op || MOD_EQUAL == op)) {
             return leftRedirect;
         }
-
-        // GROOVY-5890
-        // do not mix Class<Foo> with Foo
-        if (leftExpression instanceof ClassExpression) {
-            left = CLASS_Type.getPlainNodeReference();
-        }
-
-        MethodNode method = findMethodOrFail(expr, left, operationName, right);
-        if (method != null) {
-            storeTargetMethod(expr, method);
-            typeCheckMethodsWithGenericsOrFail(left, new ClassNode[]{right}, method, expr);
-            if (isAssignment(op)) return left;
-            if (isCompareToBoolean(op)) return boolean_TYPE;
-            if (op == COMPARE_TO) return int_TYPE;
-            return inferReturnTypeGenerics(left, method, args(rightExpression));
-        }
-        //TODO: other cases
         return null;
     }
 

http://git-wip-us.apache.org/repos/asf/groovy/blob/8294b627/src/test/groovy/transform/stc/MiscSTCTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/transform/stc/MiscSTCTest.groovy b/src/test/groovy/transform/stc/MiscSTCTest.groovy
index 0b786d8..94be0de 100644
--- a/src/test/groovy/transform/stc/MiscSTCTest.groovy
+++ b/src/test/groovy/transform/stc/MiscSTCTest.groovy
@@ -409,4 +409,23 @@ class MiscSTCTest extends StaticTypeCheckingTestCase {
             method()
         '''
     }
+
+    // GROOVY-8384
+    void testIntdiv() {
+        assertScript '''
+            def method() {
+                assert new Long(7L.multiply(3)) == 21
+                assert new Long(7L.plus(3)) == 10
+                assert new Long(7L.leftShift(3)) == 56
+                assert new Long(7L.rightShift(1)) == 3
+                assert new Long(7L.mod(3)) == 1
+                assert new Long(7L.intdiv(3)) == 2
+                assert new Integer((-8).intdiv(-4)) == 2
+                Integer x = 9
+                Integer y = 5
+                assert new Integer(x.intdiv(y)) == 1
+            }
+            method()
+        '''
+    }
 }


Mime
View raw message