Skip to content

Commit

Permalink
GROOVY-11290: STC: merge instanceof guard(s)
Browse files Browse the repository at this point in the history 
3_0_X backport
  • Loading branch information
@eric-milles
eric-milles committed Jan 23, 2024
1 parent 557a196 commit c181ef7
Show file tree
Hide file tree
Showing 5 changed files with 233 additions and 126 deletions.
9 changes: 6 additions & 3 deletions src/main/java/org/codehaus/groovy/ast/ClassNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -904,12 +904,15 @@ public boolean isDerivedFrom(ClassNode type) {
if (type.equals(ClassHelper.OBJECT_TYPE)) {
return true;
}
ClassNode node = this;
while (node != null) {
if (this.isArray() && type.isArray()
&& type.getComponentType().equals(ClassHelper.OBJECT_TYPE)
&& !ClassHelper.isPrimitiveType(this.getComponentType())){
return true;
}
for (ClassNode node = this; node != null; node = node.getSuperClass()) {
if (type.equals(node)) {
return true;
}
node = node.getSuperClass();
}
return false;
}
Expand Down
10 changes: 8 additions & 2 deletions src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1310,12 +1310,18 @@ public static <T> List<T> unique(List<T> self) {
* @since 1.8.1
*/
public static <T> Collection<T> unique(Collection<T> self, boolean mutate) {
List<T> answer = uniqueItems(self);
Collection<T> answer = null;
if (mutate || (self != null && self.size() > 1)) {
answer = uniqueItems(self);
}
if (mutate) {
self.clear();
self.addAll(answer);

return self;
} else {
return answer;
}
return mutate ? self : answer ;
}

private static <T> List<T> uniqueItems(Iterable<T> self) {
Expand Down
217 changes: 121 additions & 96 deletions src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@
import static org.codehaus.groovy.ast.tools.GeneralUtils.binX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.elvisX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getGetterName;
Expand Down Expand Up @@ -1131,18 +1132,6 @@ private void adjustGenerics(final ClassNode source, final ClassNode target) {
target.setGenericsTypes(genericsTypes);
}

/**
* Stores information about types when [objectOfInstanceof instanceof typeExpression] is visited.
*
* @param objectOfInstanceOf the expression which must be checked against instanceof
* @param typeExpression the expression which represents the target type
*/
protected void pushInstanceOfTypeInfo(final Expression objectOfInstanceOf, final Expression typeExpression) {
List<ClassNode> potentialTypes = typeCheckingContext.temporaryIfBranchTypeInformation.peek()
.computeIfAbsent(extractTemporaryTypeInfoKey(objectOfInstanceOf), key -> new LinkedList<>());
potentialTypes.add(typeExpression.getType());
}

private boolean typeCheckMultipleAssignmentAndContinue(final Expression leftExpression, Expression rightExpression) {
// multiple assignment check
if (!(leftExpression instanceof TupleExpression)) return true;
Expand Down Expand Up @@ -1441,37 +1430,6 @@ protected MethodNode checkGroovyStyleConstructor(final ClassNode node, final Cla
return constructors.get(0);
}

/**
* When instanceof checks are found in the code, we store temporary type
* information data in the {@link TypeCheckingContext#temporaryIfBranchTypeInformation}
* table. This method computes the key which must be used to store this type
* info.
*
* @param expression the expression for which to compute the key
* @return a key to be used for {@link TypeCheckingContext#temporaryIfBranchTypeInformation}
*/
protected Object extractTemporaryTypeInfoKey(final Expression expression) {
return expression instanceof VariableExpression ? findTargetVariable((VariableExpression) expression) : expression.getText();
}

/**
* A helper method which determines which receiver class should be used in error messages when a field or attribute
* is not found. The returned type class depends on whether we have temporary type information available (due to
* instanceof checks) and whether there is a single candidate in that case.
*
* @param expr the expression for which an unknown field has been found
* @param type the type of the expression (used as fallback type)
* @return if temporary information is available and there's only one type, returns the temporary type class
* otherwise falls back to the provided type class.
*/
protected ClassNode findCurrentInstanceOfClass(final Expression expr, final ClassNode type) {
if (!typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
List<ClassNode> nodes = getTemporaryTypesForExpression(expr);
if (nodes != null && nodes.size() == 1) return nodes.get(0);
}
return type;
}

protected boolean existsProperty(final PropertyExpression pexp, final boolean checkForReadOnly) {
return existsProperty(pexp, checkForReadOnly, null);
}
Expand Down Expand Up @@ -2249,7 +2207,14 @@ public void visitMethodCallExpression(final MethodCallExpression mce) {
public void visitExpressionStatement(final ExpressionStatement statement) {
typeCheckingContext.pushTemporaryTypeInfo();
super.visitExpressionStatement(statement);
typeCheckingContext.popTemporaryTypeInfo();
Map<?,List<ClassNode>> tti = typeCheckingContext.temporaryIfBranchTypeInformation.pop();
if (!tti.isEmpty() && !typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
tti.forEach((k, tempTypes) -> {
if (tempTypes.contains(VOID_TYPE))
typeCheckingContext.temporaryIfBranchTypeInformation.peek()
.computeIfAbsent(k, x -> new LinkedList<>()).add(VOID_TYPE);
});
}
}

@Override
Expand Down Expand Up @@ -2405,29 +2370,6 @@ protected ClassNode[] getArgumentTypes(final ArgumentListExpression args) {
).toArray(ClassNode[]::new);
}

private ClassNode getInferredTypeFromTempInfo(final Expression expression, final ClassNode expressionType) {
if (expression instanceof VariableExpression && !typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
List<ClassNode> tempTypes = getTemporaryTypesForExpression(expression);
if (tempTypes != null && !tempTypes.isEmpty()) {
List<ClassNode> types = new ArrayList<>(tempTypes.size() + 1);
if (expressionType != null && !expressionType.equals(OBJECT_TYPE) // GROOVY-7333
&& tempTypes.stream().noneMatch(t -> implementsInterfaceOrIsSubclassOf(t, expressionType))) { // GROOVY-9769
types.add(expressionType);
}
types.addAll(tempTypes);

if (types.isEmpty()) {
return OBJECT_TYPE;
} else if (types.size() == 1) {
return types.get(0);
} else {
return new UnionTypeClassNode(types.toArray(ClassNode.EMPTY_ARRAY));
}
}
}
return expressionType;
}

@Override
public void visitClosureExpression(final ClosureExpression expression) {
// collect every variable expression used in the closure body
Expand Down Expand Up @@ -3925,19 +3867,6 @@ protected void checkForbiddenSpreadArgument(final ArgumentListExpression argumen
}
}

protected List<ClassNode> getTemporaryTypesForExpression(final Expression objectExpression) {
List<ClassNode> classNodes = null;
int depth = typeCheckingContext.temporaryIfBranchTypeInformation.size();
while (classNodes == null && depth > 0) {
Map<Object, List<ClassNode>> tempo = typeCheckingContext.temporaryIfBranchTypeInformation.get(--depth);
Object key = objectExpression instanceof ParameterVariableExpression
? ((ParameterVariableExpression) objectExpression).parameter
: extractTemporaryTypeInfoKey(objectExpression);
classNodes = tempo.get(key);
}
return classNodes;
}

protected void storeTargetMethod(final Expression call, final MethodNode directMethodCallCandidate) {
call.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, directMethodCallCandidate);

Expand Down Expand Up @@ -4495,13 +4424,9 @@ protected void storeType(final Expression exp, ClassNode cn) {
List<ClassNode> assignedTypes = typeCheckingContext.closureSharedVariablesAssignmentTypes.computeIfAbsent(var, k -> new LinkedList<>());
assignedTypes.add(cn);
}
if (!typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
List<ClassNode> temporaryTypesForExpression = getTemporaryTypesForExpression(var);
if (temporaryTypesForExpression != null && !temporaryTypesForExpression.isEmpty()) {
// a type inference has been made on a variable whose type was defined in an instanceof block
// erase available information with the new type
temporaryTypesForExpression.clear();
}
if (!var.isThisExpression() && !var.isSuperExpression() && !typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
// GROOVY-5226, GROOVY-11290: assignment voids instanceof
pushInstanceOfTypeInfo(var, classX(VOID_TYPE));
}
}
}
Expand Down Expand Up @@ -5909,8 +5834,8 @@ protected void addCategoryMethodCallError(final Expression call) {
addStaticTypeError("Due to their dynamic nature, usage of categories is not possible with static type checking active", call);
}

protected void addAssignmentError(final ClassNode leftType, final ClassNode rightType, final Expression assignmentExpression) {
addStaticTypeError("Cannot assign value of type " + rightType.toString(false) + " to variable of type " + leftType.toString(false), assignmentExpression);
protected void addAssignmentError(final ClassNode leftType, final ClassNode rightType, final Expression expression) {
addStaticTypeError("Cannot assign value of type " + prettyPrintType(rightType) + " to variable of type " + prettyPrintType(leftType), expression);
}

protected void addUnsupportedPreOrPostfixExpressionError(final Expression expression) {
Expand Down Expand Up @@ -5946,7 +5871,7 @@ public void performSecondPass() {
addStaticTypeError("A closure shared variable [" + target.getName() + "] has been assigned with various types and the method" +
" [" + toMethodParametersString(message, getType(((BinaryExpression) expression).getRightExpression())) + "]" +
" does not exist in the lowest upper bound of those types: [" +
lub.toString(false) + "]. In general, this is a bad practice (variable reuse) because the compiler cannot" +
prettyPrintType(lub) + "]. In general, this is a bad practice (variable reuse) because the compiler cannot" +
" determine safely what is the type of the variable at the moment of the call in a multithreaded context.", expression);
}
}
Expand Down Expand Up @@ -5974,7 +5899,7 @@ public void performSecondPass() {
addStaticTypeError("A closure shared variable [" + target.getName() + "] has been assigned with various types and the method" +
" [" + toMethodParametersString(methodNode.getName(), params) + "]" +
" does not exist in the lowest upper bound of those types: [" +
lub.toString(false) + "]. In general, this is a bad practice (variable reuse) because the compiler cannot" +
prettyPrintType(lub) + "]. In general, this is a bad practice (variable reuse) because the compiler cannot" +
" determine safely what is the type of the variable at the moment of the call in a multithreaded context.", call);
}
}
Expand Down Expand Up @@ -6025,15 +5950,115 @@ private static BinaryExpression assignX(final Expression lhs, final Expression r
return exp;
}

//--------------------------------------------------------------------------
// temporaryIfBranchTypeInformation support; migrate to TypeCheckingContext?

/**
* Stores information about types when [objectOfInstanceof instanceof typeExpression] is visited.
*
* @param objectOfInstanceOf the expression to be checked against instanceof
* @param typeExpression the expression which represents the target type
*/
protected void pushInstanceOfTypeInfo(final Expression objectOfInstanceOf, final Expression typeExpression) {
Object ttiKey = extractTemporaryTypeInfoKey(objectOfInstanceOf); ClassNode type = typeExpression.getType();
typeCheckingContext.temporaryIfBranchTypeInformation.peek().computeIfAbsent(ttiKey, x -> new LinkedList<>()).add(type);
}

/**
* Computes the key to use for {@link TypeCheckingContext#temporaryIfBranchTypeInformation}.
*/
protected Object extractTemporaryTypeInfoKey(final Expression expression) {
return expression instanceof VariableExpression ? findTargetVariable((VariableExpression) expression) : expression.getText();
}

/**
* A helper method which determines which receiver class should be used in error messages when a field or attribute
* is not found. The returned type class depends on whether we have temporary type information available (due to
* instanceof checks) and whether there is a single candidate in that case.
*
* @param expression the expression for which an unknown field has been found
* @param type the type of the expression (used as fallback type)
* @return if temporary information is available and there's only one type, returns the temporary type class
* otherwise falls back to the provided type class.
*/
protected ClassNode findCurrentInstanceOfClass(final Expression expression, final ClassNode type) {
List<ClassNode> tempTypes = getTemporaryTypesForExpression(expression);
if (tempTypes.size() == 1) return tempTypes.get(0);
return type;
}

protected List<ClassNode> getTemporaryTypesForExpression(final Expression expression) {
Object key = extractTemporaryTypeInfoKey(expression);
List<ClassNode> tempTypes = typeCheckingContext.temporaryIfBranchTypeInformation.stream().flatMap(tti ->
tti.getOrDefault(key, Collections.emptyList()).stream()
).collect(Collectors.toList());
int i = tempTypes.lastIndexOf(VOID_TYPE);
if (i != -1) { // assignment overwrites instanceof
tempTypes = tempTypes.subList(i + 1, tempTypes.size());
}
return DefaultGroovyMethods.unique(tempTypes); // GROOVY-6429
}

private ClassNode getInferredTypeFromTempInfo(final Expression expression, final ClassNode expressionType) {
if (expression instanceof VariableExpression) {
List<ClassNode> tempTypes = getTemporaryTypesForExpression(expression);
if (!tempTypes.isEmpty()) {
ClassNode superclass;
ClassNode[] interfaces;
if (expressionType instanceof WideningCategories.LowestUpperBoundClassNode) {
superclass = expressionType.getSuperClass();
interfaces = expressionType.getInterfaces();
} else if (expressionType != null && expressionType.isInterface()) {
superclass = OBJECT_TYPE;
interfaces = new ClassNode[]{expressionType};
} else {
superclass = expressionType;
interfaces = ClassNode.EMPTY_ARRAY;
}

List<ClassNode> types = new ArrayList<>();
if (superclass != null && !superclass.equals(OBJECT_TYPE) // GROOVY-7333
&& tempTypes.stream().noneMatch(t -> !t.equals(superclass) && t.isDerivedFrom(superclass))) { // GROOVY-9769
types.add(superclass);
}
for (ClassNode anInterface : interfaces) {
if (tempTypes.stream().noneMatch(t -> t.implementsInterface(anInterface))) { // GROOVY-9769
types.add(anInterface);
}
}
int tempTypesCount = tempTypes.size();
if (tempTypesCount == 1 && types.isEmpty()) {
types.add(tempTypes.get(0));
} else for (ClassNode tempType : tempTypes) {
if (!tempType.isInterface() // GROOVY-11290: keep most-specific types
? (superclass == null || !superclass.isDerivedFrom(tempType))
&& (tempTypesCount == 1 || tempTypes.stream().noneMatch(t -> !t.equals(tempType) && t.isDerivedFrom(tempType)))
: (expressionType == null || !isOrImplements(expressionType, tempType))
&& (tempTypesCount == 1 || tempTypes.stream().noneMatch(t -> t != tempType && t.implementsInterface(tempType)))) {
types.add(tempType);
}
}

int typesCount = types.size();
if (typesCount == 1) {
return types.get(0);
} else if (typesCount > 1) {
return new UnionTypeClassNode(types.toArray(ClassNode.EMPTY_ARRAY));
}
}
}
return expressionType;
}

//--------------------------------------------------------------------------

public static class SignatureCodecFactory {
public static SignatureCodec getCodec(final int version, final ClassLoader classLoader) {
switch (version) {
case 1:
return new SignatureCodecVersion1(classLoader);
default:
return null;
case 1:
return new SignatureCodecVersion1(classLoader);
default:
return null;
}
}
}
Expand Down Expand Up @@ -6107,7 +6132,7 @@ private class ParameterVariableExpression extends VariableExpression {
if (inferredType == null) {
inferredType = getTypeFromClosureArguments(parameter); // @ClosureParams or SAM-type coercion
}
setNodeMetaData(INFERRED_TYPE, inferredType != null ? inferredType : parameter.getType()); // to parameter
setNodeMetaData(INFERRED_TYPE, inferredType != null ? inferredType : parameter.getType()); // GROOVY-10651
}
}

Expand Down
Loading

0 comments on commit c181ef7

Please sign in to comment.