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 <[email protected]> Authored: Sun Nov 19 00:29:08 2017 +1000 Committer: paulk <[email protected]> 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() + ''' + } }
