Skip to content

Commit b4ccc85

Browse files
paulk-asertpaulk-asert
committed
GROOVY-8384 - Regression in 2.4.13 snapshot with STC and intdiv plus some cleanup (port to 2_5_X)
1 parent e4a31f2 commit b4ccc85

File tree

5 files changed

+97
-39
lines changed

5 files changed

+97
-39
lines changed

‎src/main/org/codehaus/groovy/classgen/asm/CallSiteWriter.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,9 @@
4343
/**
4444
* This class represents non public API used by AsmClassGenerator. Don't
4545
* use this class in your code
46-
* @author Jochen Theodorou
4746
*/
4847
public class CallSiteWriter {
4948

50-
private static final Set<String> NAMES = new HashSet<String>();
51-
private static final Set<String> BASIC = new HashSet<String>();
52-
static {
53-
Collections.addAll(NAMES, "plus", "minus", "multiply", "div", "compareTo", "or", "and", "xor", "intdiv", "mod", "leftShift", "rightShift", "rightShiftUnsigned");
54-
Collections.addAll(BASIC, "plus", "minus", "multiply", "div");
55-
}
5649
private static String [] sig = new String [255];
5750
private static String getCreateArraySignature(int numberOfArguments) {
5851
if (sig[numberOfArguments] == null) {

‎src/main/org/codehaus/groovy/runtime/typehandling/NumberMathModificationInfo.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,14 @@ public class NumberMathModificationInfo {
2929

3030
public static final NumberMathModificationInfo instance = new NumberMathModificationInfo();
3131

32-
private final HashSet<String> names = new HashSet<String>();
32+
private static final HashSet<String> NAMES = new HashSet<String>();
3333

34-
private NumberMathModificationInfo() {
35-
Collections.addAll(names, "plus", "minus", "multiply", "div", "compareTo", "or", "and", "xor", "intdiv", "mod", "leftShift", "rightShift", "rightShiftUnsigned");
34+
static {
35+
Collections.addAll(NAMES, "plus", "minus", "multiply", "div", "compareTo", "or", "and", "xor", "intdiv", "mod", "leftShift", "rightShift", "rightShiftUnsigned");
3636
}
3737

38+
private NumberMathModificationInfo() { }
39+
3840
public void checkIfStdMethod(MetaMethod method) {
3941
if (method.getClass() != NewInstanceMetaMethod.class) {
4042
String name = method.getName();
@@ -45,7 +47,7 @@ public void checkIfStdMethod(MetaMethod method) {
4547
if (!method.getParameterTypes()[0].isNumber && method.getParameterTypes()[0].getTheClass() != Object.class)
4648
return;
4749

48-
if (!names.contains(name))
50+
if (!NAMES.contains(name))
4951
return;
5052

5153
checkNumberOps(name, method.getDeclaringClass().getTheClass());

‎src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,22 @@ public abstract class StaticTypeCheckingSupport {
9898
put(Double_TYPE, 5);
9999
}});
100100

101+
protected static final Map<String, Integer> NUMBER_OPS = Collections.unmodifiableMap(
102+
new HashMap<String, Integer>() {{
103+
put("plus", PLUS);
104+
put("minus", MINUS);
105+
put("multiply", MULTIPLY);
106+
put("div", DIVIDE);
107+
put("or", BITWISE_OR);
108+
put("and", BITWISE_AND);
109+
put("xor", BITWISE_XOR);
110+
put("mod", MOD);
111+
put("intdiv", INTDIV);
112+
put("leftShift", LEFT_SHIFT);
113+
put("rightShift", RIGHT_SHIFT);
114+
put("rightShiftUnsigned", RIGHT_SHIFT_UNSIGNED);
115+
}});
116+
101117
protected static final ClassNode GSTRING_STRING_CLASSNODE = WideningCategories.lowestUpperBound(
102118
STRING_TYPE,
103119
GSTRING_TYPE
@@ -395,7 +411,7 @@ static boolean isArrayOp(int op) {
395411
}
396412

397413
static boolean isBoolIntrinsicOp(int op) {
398-
return op == LOGICAL_AND || op == LOGICAL_OR ||
414+
return op == LOGICAL_AND || op == LOGICAL_OR || op == COMPARE_NOT_IDENTICAL || op == COMPARE_IDENTICAL ||
399415
op == MATCH_REGEX || op == KEYWORD_INSTANCEOF;
400416
}
401417

@@ -679,7 +695,12 @@ public static boolean isBeingCompiled(ClassNode node) {
679695
return node.getCompileUnit() != null;
680696
}
681697

698+
@Deprecated
682699
static boolean checkPossibleLooseOfPrecision(ClassNode left, ClassNode right, Expression rightExpr) {
700+
return checkPossibleLossOfPrecision(left, right, rightExpr);
701+
}
702+
703+
static boolean checkPossibleLossOfPrecision(ClassNode left, ClassNode right, Expression rightExpr) {
683704
if (left == right || left.equals(right)) return false; // identical types
684705
int leftIndex = NUMBER_TYPES.get(left);
685706
int rightIndex = NUMBER_TYPES.get(right);

‎src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@
117117
import static org.codehaus.groovy.syntax.Types.DIVIDE_EQUAL;
118118
import static org.codehaus.groovy.syntax.Types.EQUAL;
119119
import static org.codehaus.groovy.syntax.Types.FIND_REGEX;
120+
import static org.codehaus.groovy.syntax.Types.INTDIV;
121+
import static org.codehaus.groovy.syntax.Types.INTDIV_EQUAL;
120122
import static org.codehaus.groovy.syntax.Types.KEYWORD_IN;
121123
import static org.codehaus.groovy.syntax.Types.KEYWORD_INSTANCEOF;
122124
import static org.codehaus.groovy.syntax.Types.LEFT_SQUARE_BRACKET;
@@ -930,7 +932,7 @@ private boolean addedReadOnlyPropertyError(Expression expr) {
930932

931933
private void addPrecisionErrors(ClassNode leftRedirect, ClassNode lhsType, ClassNode inferredrhsType, Expression rightExpression) {
932934
if (isNumberType(leftRedirect) && isNumberType(inferredrhsType)) {
933-
if (checkPossibleLooseOfPrecision(leftRedirect, inferredrhsType, rightExpression)) {
935+
if (checkPossibleLossOfPrecision(leftRedirect, inferredrhsType, rightExpression)) {
934936
addStaticTypeError("Possible loss of precision from " + inferredrhsType + " to " + leftRedirect, rightExpression);
935937
return;
936938
}
@@ -3077,6 +3079,16 @@ public void visitMethodCallExpression(MethodCallExpression call) {
30773079
}
30783080
}
30793081
}
3082+
// adjust typing for explicit math methods which have special handling - operator variants handled elsewhere
3083+
if (NUMBER_OPS.containsKey(name) && isNumberType(receiver) && argumentList.getExpressions().size() == 1
3084+
&& isNumberType(getType(argumentList.getExpression(0)))) {
3085+
ClassNode right = getType(argumentList.getExpression(0));
3086+
ClassNode resultType = getMathResultType(NUMBER_OPS.get(name), receiver, right, name);
3087+
if (resultType != null) {
3088+
storeType(call, resultType);
3089+
}
3090+
}
3091+
30803092
// now that a method has been chosen, we are allowed to visit the closures
30813093
if (!callArgsVisited) {
30823094
MethodNode mn = (MethodNode) call.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
@@ -3111,8 +3123,9 @@ public void visitMethodCallExpression(MethodCallExpression call) {
31113123
*
31123124
* @param directMethodCallCandidate a method selected by the type checker
31133125
* @param receiver the receiver of the method call
3114-
*@param args the arguments of the method call
3115-
* @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}
3126+
* @param args the arguments of the method call
3127+
* @param returnType the original return type, as inferred by the type checker
3128+
* @return fixed return type if the selected method is {@link org.codehaus.groovy.runtime.DefaultGroovyMethods#withTraits(Object, Class[]) withTraits}
31163129
*/
31173130
private static ClassNode adjustWithTraits(final MethodNode directMethodCallCandidate, final ClassNode receiver, final ClassNode[] args, final ClassNode returnType) {
31183131
if (directMethodCallCandidate instanceof ExtensionMethodNode) {
@@ -3613,9 +3626,11 @@ protected ClassNode getResultType(ClassNode left, int op, ClassNode right, Binar
36133626
}
36143627
}
36153628
return right;
3616-
} else if (isBoolIntrinsicOp(op)) {
3629+
}
3630+
if (isBoolIntrinsicOp(op)) {
36173631
return boolean_TYPE;
3618-
} else if (isArrayOp(op)) {
3632+
}
3633+
if (isArrayOp(op)) {
36193634
// using getPNR() to ignore generics at this point
36203635
// and a different binary expression not to pollute the AST
36213636
BinaryExpression newExpr = binX(expr.getLeftExpression(), expr.getOperation(), rightExpression);
@@ -3625,21 +3640,48 @@ protected ClassNode getResultType(ClassNode left, int op, ClassNode right, Binar
36253640
return inferReturnTypeGenerics(left, method, rightExpression);
36263641
}
36273642
return method!=null?inferComponentType(left, right):null;
3628-
} else if (op == FIND_REGEX) {
3643+
}
3644+
if (op == FIND_REGEX) {
36293645
// this case always succeeds the result is a Matcher
36303646
return Matcher_TYPE;
36313647
}
36323648
// the left operand is determining the result of the operation
36333649
// for primitives and their wrapper we use a fixed table here
3634-
else if (isNumberType(leftRedirect) && isNumberType(rightRedirect)) {
3650+
String operationName = getOperationName(op);
3651+
ClassNode mathResultType = getMathResultType(op, leftRedirect, rightRedirect, operationName);
3652+
if (mathResultType != null) {
3653+
return mathResultType;
3654+
}
3655+
3656+
// GROOVY-5890
3657+
// do not mix Class<Foo> with Foo
3658+
if (leftExpression instanceof ClassExpression) {
3659+
left = CLASS_Type.getPlainNodeReference();
3660+
}
3661+
3662+
MethodNode method = findMethodOrFail(expr, left, operationName, right);
3663+
if (method != null) {
3664+
storeTargetMethod(expr, method);
3665+
typeCheckMethodsWithGenericsOrFail(left, new ClassNode[]{right}, method, expr);
3666+
if (isAssignment(op)) return left;
3667+
if (isCompareToBoolean(op)) return boolean_TYPE;
3668+
if (op == COMPARE_TO) return int_TYPE;
3669+
return inferReturnTypeGenerics(left, method, args(rightExpression));
3670+
}
3671+
//TODO: other cases
3672+
return null;
3673+
}
3674+
3675+
private ClassNode getMathResultType(int op, ClassNode leftRedirect, ClassNode rightRedirect, String operationName) {
3676+
if (isNumberType(leftRedirect) && isNumberType(rightRedirect)) {
36353677
if (isOperationInGroup(op)) {
36363678
if (isIntCategory(leftRedirect) && isIntCategory(rightRedirect)) return int_TYPE;
36373679
if (isLongCategory(leftRedirect) && isLongCategory(rightRedirect)) return long_TYPE;
36383680
if (isFloat(leftRedirect) && isFloat(rightRedirect)) return float_TYPE;
36393681
if (isDouble(leftRedirect) && isDouble(rightRedirect)) return double_TYPE;
36403682
} else if (isPowerOperator(op)) {
36413683
return Number_TYPE;
3642-
} else if (isBitOperator(op)) {
3684+
} else if (isBitOperator(op) || op == INTDIV || op == INTDIV_EQUAL) {
36433685
if (isIntCategory(getUnwrapper(leftRedirect)) && isIntCategory(getUnwrapper(rightRedirect))) return int_TYPE;
36443686
if (isLongCategory(getUnwrapper(leftRedirect)) && isLongCategory(getUnwrapper(rightRedirect))) return long_TYPE;
36453687
if (isBigIntCategory(getUnwrapper(leftRedirect)) && isBigIntCategory(getUnwrapper(rightRedirect))) return BigInteger_TYPE;
@@ -3652,9 +3694,7 @@ else if (isNumberType(leftRedirect) && isNumberType(rightRedirect)) {
36523694
}
36533695
}
36543696

3655-
36563697
// try to find a method for the operation
3657-
String operationName = getOperationName(op);
36583698
if (isShiftOperation(operationName) && isNumberCategory(leftRedirect) && (isIntCategory(rightRedirect) || isLongCategory(rightRedirect))) {
36593699
return leftRedirect;
36603700
}
@@ -3679,23 +3719,6 @@ else if (isNumberType(leftRedirect) && isNumberType(rightRedirect)) {
36793719
if (isNumberCategory(getWrapper(rightRedirect)) && isNumberCategory(getWrapper(leftRedirect)) && (MOD == op || MOD_EQUAL == op)) {
36803720
return leftRedirect;
36813721
}
3682-
3683-
// GROOVY-5890
3684-
// do not mix Class<Foo> with Foo
3685-
if (leftExpression instanceof ClassExpression) {
3686-
left = CLASS_Type.getPlainNodeReference();
3687-
}
3688-
3689-
MethodNode method = findMethodOrFail(expr, left, operationName, right);
3690-
if (method != null) {
3691-
storeTargetMethod(expr, method);
3692-
typeCheckMethodsWithGenericsOrFail(left, new ClassNode[]{right}, method, expr);
3693-
if (isAssignment(op)) return left;
3694-
if (isCompareToBoolean(op)) return boolean_TYPE;
3695-
if (op == COMPARE_TO) return int_TYPE;
3696-
return inferReturnTypeGenerics(left, method, args(rightExpression));
3697-
}
3698-
//TODO: other cases
36993722
return null;
37003723
}
37013724

‎src/test/groovy/transform/stc/MiscSTCTest.groovy

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,4 +409,23 @@ class MiscSTCTest extends StaticTypeCheckingTestCase {
409409
method()
410410
'''
411411
}
412+
413+
// GROOVY-8384
414+
void testIntdiv() {
415+
assertScript '''
416+
def method() {
417+
assert new Long(7L.multiply(3)) == 21
418+
assert new Long(7L.plus(3)) == 10
419+
assert new Long(7L.leftShift(3)) == 56
420+
assert new Long(7L.rightShift(1)) == 3
421+
assert new Long(7L.mod(3)) == 1
422+
assert new Long(7L.intdiv(3)) == 2
423+
assert new Integer((-8).intdiv(-4)) == 2
424+
Integer x = 9
425+
Integer y = 5
426+
assert new Integer(x.intdiv(y)) == 1
427+
}
428+
method()
429+
'''
430+
}
412431
}

0 commit comments

Comments
 (0)