From 2248a1cab87657dfcd26312e71b67ab54205e71f Mon Sep 17 00:00:00 2001 From: lnsun <57457122+lnsun@users.noreply.github.com> Date: Tue, 23 Jun 2020 21:51:55 -0400 Subject: [PATCH 1/5] Merge branch 'pull-pico-changes' into ppc-temp-master # Conflicts: # framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java --- .../framework/type/AnnotatedTypeFactory.java | 2 +- .../poly/AbstractQualifierPolymorphism.java | 126 +++++- .../util/QualifierPolymorphismUtil.java | 361 ++++++++++++++++++ .../DefaultTypeArgumentInference.java | 2 +- .../typeinference/TypeArgInferenceUtil.java | 15 +- .../all-systems/GetAssignmentContext.java | 26 ++ .../checkerframework/javacutil/TreeUtils.java | 53 ++- 7 files changed, 570 insertions(+), 15 deletions(-) create mode 100644 framework/src/main/java/org/checkerframework/framework/util/QualifierPolymorphismUtil.java create mode 100644 framework/tests/all-systems/GetAssignmentContext.java diff --git a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java index fd0d2430d5ac..1a63dd63543c 100644 --- a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java +++ b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java @@ -1466,7 +1466,7 @@ public void postAsMemberOf( addAnnotationFromFieldInvariant(type, owner, (VariableElement) element); } addComputedTypeAnnotations(element, type); - if (viewpointAdapter != null) { + if (viewpointAdapter != null && type.getKind() != TypeKind.EXECUTABLE) { viewpointAdapter.viewpointAdaptMember(owner, element, type); } } diff --git a/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java b/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java index 3317af2accfc..e02c00e7ff23 100644 --- a/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java +++ b/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java @@ -2,6 +2,8 @@ import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.NewClassTree; +import com.sun.source.util.TreePath; +import com.sun.tools.javac.code.Type; import java.util.ArrayList; import java.util.Collections; import java.util.IdentityHashMap; @@ -12,6 +14,7 @@ import javax.annotation.processing.ProcessingEnvironment; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.type.TypeKind; +import javax.lang.model.type.TypeMirror; import org.checkerframework.framework.type.AnnotatedTypeFactory; import org.checkerframework.framework.type.AnnotatedTypeMirror; import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedArrayType; @@ -29,6 +32,7 @@ import org.checkerframework.framework.util.AnnotatedTypes; import org.checkerframework.framework.util.AnnotationMirrorMap; import org.checkerframework.framework.util.AnnotationMirrorSet; +import org.checkerframework.framework.util.QualifierPolymorphismUtil; import org.checkerframework.javacutil.BugInCF; import org.checkerframework.javacutil.TreeUtils; import org.checkerframework.javacutil.TypesUtils; @@ -178,6 +182,41 @@ public void resolve(MethodInvocationTree tree, AnnotatedExecutableType type) { atypeFactory.getReceiverType(tree), type.getReceiverType())); } + // receiver-sensitive poly resolution: resolve return value with assignment context l-value + TreePath path = atypeFactory.getPath(tree); + if (path != null) { + AnnotatedTypeMirror assignmentContext = + QualifierPolymorphismUtil.assignedTo(atypeFactory, path); + + if (assignmentContext != null) { + // get erased subtype relation + TypeMirror javaLhstype = + atypeFactory.types.erasure(assignmentContext.getUnderlyingType()); + TypeMirror javaReturntype = + atypeFactory.types.erasure(type.getReturnType().getUnderlyingType()); + // primitive types need special care + boolean polyIsSub = + javaReturntype instanceof Type.JCPrimitiveType + || atypeFactory.types.isSubtype(javaReturntype, javaLhstype); + + boolean polyIsSuper = + !(javaReturntype instanceof Type.ArrayType) + && // isSubtype cannot handle this + atypeFactory.types.isSubtype(javaLhstype, javaReturntype); + + // only perform receiver-sensitive poly resolution when return type is erased + // subtype of lhs + if (polyIsSub || polyIsSuper) { // lhs and return type are comparable + instantiationMapping = + collector.reduceWithUpperBounds( + instantiationMapping, + collector.visit( + // Actual assignment lhs type + assignmentContext, type.getReturnType(), polyIsSub)); + } + } + } + if (instantiationMapping != null && !instantiationMapping.isEmpty()) { replacer.visit(type, instantiationMapping); } else { @@ -332,7 +371,7 @@ private class PolyCollector * {@code Pair} may be equal, but they both should be visited. */ private final Set visitedTypes = - Collections.newSetFromMap(new IdentityHashMap()); + Collections.newSetFromMap(new IdentityHashMap<>()); /** * Returns true if the {@link AnnotatedTypeMirror} has been visited. If it has not, then it @@ -381,6 +420,53 @@ public AnnotationMirrorMap reduce( return res; } + /** + * Reduces lower bounds r1 with upper bounds r2. + * + * @param r1 a map from {@link AnnotationMirror} to {@link AnnotationMirror} + * @param r2 a map from {@link AnnotationMirror} to {@link AnnotationMirror} + * @return the reduced {@link AnnotationMirrorMap} + */ + private AnnotationMirrorMap reduceWithUpperBounds( + AnnotationMirrorMap r1, + AnnotationMirrorMap r2) { + + if (r1 == null || r1.isEmpty()) { + return r2; + } + if (r2 == null || r2.isEmpty()) { + return r1; + } + + AnnotationMirrorMap res = new AnnotationMirrorMap<>(); + // Ensure that all qualifiers from r1 and r2 are visited. + AnnotationMirrorSet r2remain = new AnnotationMirrorSet(); + r2remain.addAll(r2.keySet()); + for (Map.Entry kv1 : r1.entrySet()) { + AnnotationMirror key1 = kv1.getKey(); + AnnotationMirror a1Anno = kv1.getValue(); + AnnotationMirror a2Anno = r2.get(key1); + if (a2Anno != null) { + r2remain.remove(key1); + AnnotationMirror subres = null; + for (AnnotationMirror top : topQuals) { + if (qualHierarchy.isSubtype(a1Anno, top)) { + subres = a1Anno; + } else if (qualHierarchy.isSubtype(a2Anno, top)) { + subres = a2Anno; + } + } + res.put(key1, subres); + } else { + res.put(key1, a1Anno); + } + } + for (AnnotationMirror key2 : r2remain) { + res.put(key2, r2.get(key2)); + } + return res; + } + /** * Calls {@link #visit(AnnotatedTypeMirror, AnnotatedTypeMirror)} for each type in {@code * types}. @@ -421,10 +507,12 @@ private AnnotationMirrorMap visit( * * @param type AnnotateTypeMirror used to find instantiations * @param polyType AnnotatedTypeMirror that may have polymorphic qualifiers + * @param polyIsSub boolean indicates whether {@code polyType} is the subtype of {@code + * type} * @return a mapping of polymorphic qualifiers to their instantiations */ private AnnotationMirrorMap visit( - AnnotatedTypeMirror type, AnnotatedTypeMirror polyType) { + AnnotatedTypeMirror type, AnnotatedTypeMirror polyType, boolean polyIsSub) { if (type.getKind() == TypeKind.NULL) { return mapQualifierToPoly(type, polyType); } @@ -440,19 +528,39 @@ private AnnotationMirrorMap visit( switch (polyType.getKind()) { case WILDCARD: - AnnotatedTypeMirror asSuper = - AnnotatedTypes.asSuper(atypeFactory, wildcardType, polyType); - return visit(asSuper, polyType, null); - case TYPEVAR: - return mapQualifierToPoly(wildcardType.getExtendsBound(), polyType); + if (polyIsSub) { + return visit( + wildcardType, + AnnotatedTypes.asSuper(atypeFactory, polyType, wildcardType), + null); + } else { + return visit( + AnnotatedTypes.asSuper(atypeFactory, wildcardType, polyType), + polyType, + null); + } default: return mapQualifierToPoly(wildcardType.getExtendsBound(), polyType); } } - AnnotatedTypeMirror asSuper = AnnotatedTypes.asSuper(atypeFactory, type, polyType); + if (polyIsSub) { + return visit(type, AnnotatedTypes.asSuper(atypeFactory, polyType, type), null); + } else { + return visit(AnnotatedTypes.asSuper(atypeFactory, type, polyType), polyType, null); + } + } - return visit(asSuper, polyType, null); + /** + * A helper method of {@link #visit(AnnotatedTypeMirror, AnnotatedTypeMirror, boolean)}. + * + * @param type AnnotateTypeMirror used to find instantiations + * @param polyType AnnotatedTypeMirror that may have polymorphic qualifiers + * @return {@code visit(type, polyType, false)} + */ + private AnnotationMirrorMap visit( + AnnotatedTypeMirror type, AnnotatedTypeMirror polyType) { + return visit(type, polyType, false); } @Override diff --git a/framework/src/main/java/org/checkerframework/framework/util/QualifierPolymorphismUtil.java b/framework/src/main/java/org/checkerframework/framework/util/QualifierPolymorphismUtil.java new file mode 100644 index 000000000000..6570346a9df2 --- /dev/null +++ b/framework/src/main/java/org/checkerframework/framework/util/QualifierPolymorphismUtil.java @@ -0,0 +1,361 @@ +package org.checkerframework.framework.util; + +import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.CompoundAssignmentTree; +import com.sun.source.tree.ConditionalExpressionTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.LambdaExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.NewArrayTree; +import com.sun.source.tree.NewClassTree; +import com.sun.source.tree.ReturnTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.VariableTree; +import com.sun.source.util.TreePath; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.type.TypeVariable; +import org.checkerframework.framework.type.AnnotatedTypeFactory; +import org.checkerframework.framework.type.AnnotatedTypeMirror; +import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedArrayType; +import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedExecutableType; +import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedTypeVariable; +import org.checkerframework.framework.type.GenericAnnotatedTypeFactory; +import org.checkerframework.framework.type.visitor.AnnotatedTypeScanner; +import org.checkerframework.javacutil.BugInCF; +import org.checkerframework.javacutil.Pair; +import org.checkerframework.javacutil.TreeUtils; +import org.checkerframework.javacutil.TypeAnnotationUtils; + +/** + * A utility class made for {@link org.checkerframework.framework.type.poly.QualifierPolymorphism}. + */ +public class QualifierPolymorphismUtil { + + /** A set of {@code TreePath} which contains the visited {@code TreePath}s. */ + private static final Set visitedPaths = new HashSet<>(); + + /** + * Returns the annotated type that the leaf of path is assigned to, if it is within an + * assignment context. Returns the annotated type that the method invocation at the leaf is + * assigned to. If the result is a primitive, return the boxed version. + * + * @param atypeFactory the type factory + * @param path the tree path + * @return type that path leaf is assigned to + */ + public static AnnotatedTypeMirror assignedTo(AnnotatedTypeFactory atypeFactory, TreePath path) { + if (visitedPaths.contains(path)) { + // inform the caller to skip assignment context resolution + return null; + } + + visitedPaths.add(path); + + Tree assignmentContext = TreeUtils.getAssignmentContext(path); + AnnotatedTypeMirror res; + if (assignmentContext == null) { + res = null; + } else if (assignmentContext instanceof AssignmentTree) { + ExpressionTree variable = ((AssignmentTree) assignmentContext).getVariable(); + res = atypeFactory.getAnnotatedType(variable); + } else if (assignmentContext instanceof CompoundAssignmentTree) { + ExpressionTree variable = ((CompoundAssignmentTree) assignmentContext).getVariable(); + res = atypeFactory.getAnnotatedType(variable); + } else if (assignmentContext instanceof MethodInvocationTree) { + MethodInvocationTree methodInvocation = (MethodInvocationTree) assignmentContext; + ExecutableElement methodElt = TreeUtils.elementFromUse(methodInvocation); + // TODO move to getAssignmentContext + // if (methodInvocation.getMethodSelect() instanceof MemberSelectTree + // && ((MemberSelectTree) methodInvocation.getMethodSelect()).getExpression() + // == path.getLeaf()) { + // // treepath's leaf is assigned to the method declared receiver type + // AnnotatedExecutableType declMethodType = + // atypeFactory.getAnnotatedType(methodElt); + // return declMethodType.getReceiverType(); + // } // Removing above if block causes StackOverflowException via + // getReceiverType -> + // assignedTo -> getReceiverType loop! + // AnnotatedTypeMirror receiver = null; + // try { + // receiver = atypeFactory.getReceiverType(methodInvocation); + // } catch (Throwable e) { + // new Object(); + // } + AnnotatedTypeMirror receiver = atypeFactory.getReceiverType(methodInvocation); + res = + assignedToExecutable( + atypeFactory, + path, + methodElt, + receiver, + methodInvocation.getArguments()); + } else if (assignmentContext instanceof NewArrayTree) { + // TODO: I left the previous implementation below, it definitely caused infinite loops + // TODO: if you called it from places like the TreeAnnotator. + res = null; + + // TODO: This may cause infinite loop + // AnnotatedTypeMirror type = + // atypeFactory.getAnnotatedType((NewArrayTree)assignmentContext); + // type = AnnotatedTypes.innerMostType(type); + // return type; + + } else if (assignmentContext instanceof NewClassTree) { + // This need to be basically like MethodTree + NewClassTree newClassTree = (NewClassTree) assignmentContext; + ExecutableElement constructorElt = TreeUtils.constructor(newClassTree); + AnnotatedTypeMirror receiver = atypeFactory.fromNewClass(newClassTree); + res = + assignedToExecutable( + atypeFactory, + path, + constructorElt, + receiver, + newClassTree.getArguments()); + } else if (assignmentContext instanceof ReturnTree) { + HashSet kinds = new HashSet<>(Arrays.asList(Kind.LAMBDA_EXPRESSION, Kind.METHOD)); + Tree enclosing = TreeUtils.enclosingOfKind(path, kinds); + + if (enclosing.getKind() == Kind.METHOD) { + res = atypeFactory.getAnnotatedType((MethodTree) enclosing).getReturnType(); + } else { + Pair fninf = + atypeFactory.getFnInterfaceFromTree((LambdaExpressionTree) enclosing); + res = fninf.second.getReturnType(); + } + + } else if (assignmentContext instanceof VariableTree) { + res = assignedToVariable(atypeFactory, assignmentContext); + } else { + throw new BugInCF("AnnotatedTypes.assignedTo: shouldn't be here"); + } + visitedPaths.remove(path); + return res; + } + + /** + * Return the annotated type that is assigned to executable. + * + * @param atypeFactory the type factory + * @param path the tree path + * @param methodElt the method element + * @param receiver the receiver + * @param arguments the passed arguments + * @return the annotated type + */ + private static AnnotatedTypeMirror assignedToExecutable( + AnnotatedTypeFactory atypeFactory, + TreePath path, + ExecutableElement methodElt, + AnnotatedTypeMirror receiver, + List arguments) { + AnnotatedExecutableType method = + AnnotatedTypes.asMemberOf( + atypeFactory.getContext().getTypeUtils(), + atypeFactory, + receiver, + methodElt); + int treeIndex = -1; + for (int i = 0; i < arguments.size(); ++i) { + ExpressionTree argumentTree = arguments.get(i); + if (isArgument(path, argumentTree)) { + treeIndex = i; + break; + } + } + final AnnotatedTypeMirror paramType; + if (treeIndex == -1) { + // The tree wasn't found as an argument, so it has to be the receiver. + // This can happen for inner class constructors that take an outer class argument. + paramType = method.getReceiverType(); + } else if (treeIndex + 1 >= method.getParameterTypes().size() && methodElt.isVarArgs()) { + AnnotatedTypeMirror varArgType = + method.getParameterTypes().get(method.getParameterTypes().size() - 1); + List params = + AnnotatedTypes.expandVarArgs(atypeFactory, method, arguments); + if (params.get(params.size() - 1).equals(varArgType)) { + return varArgType; + } + paramType = ((AnnotatedArrayType) varArgType).getComponentType(); + } else { + paramType = method.getParameterTypes().get(treeIndex); + } + + // Examples like this: + // T outMethod() + // void inMethod(U u); + // inMethod(outMethod()) + // would require solving the constraints for both type argument inferences simultaneously + if (paramType == null || containsUninferredTypeParameter(paramType, method)) { + return null; + } + + return paramType; + } + + /** + * Returns whether argumentTree is the tree at the leaf of path. if tree is a conditional + * expression, isArgument is called recursively on the true and false expressions. + * + * @param path the tree path + * @param argumentTree the argument tree + * @return true if argumentTree is the tree at the leaf of path, else false + */ + private static boolean isArgument(TreePath path, ExpressionTree argumentTree) { + argumentTree = TreeUtils.withoutParens(argumentTree); + if (argumentTree == path.getLeaf()) { + return true; + } else if (argumentTree.getKind() == Kind.CONDITIONAL_EXPRESSION) { + ConditionalExpressionTree conditionalExpressionTree = + (ConditionalExpressionTree) argumentTree; + return isArgument(path, conditionalExpressionTree.getTrueExpression()) + || isArgument(path, conditionalExpressionTree.getFalseExpression()); + } + return false; + } + + /** + * If the variable's type is a type variable, return getAnnotatedTypeLhsNoTypeVarDefault(tree). + * Rational: + * + *

For example: + * + * 

{@code
+     *  S bar () {...}
+     *
+     *  T foo(T p) {
+     *     T local = bar();
+     *     return local;
+     *   }
+     * }
+ * + * During type argument inference of {@code bar}, the assignment context is {@code local}. If + * the local variable default is used, then the type of assignment context type is + * {@code @Nullable T} and the type argument inferred for {@code bar()} is {@code @Nullable T}. + * And an incompatible types in return error is issued. + * + *

If instead, the local variable default is not applied, then the assignment context type is + * {@code T} (with lower bound {@code @NonNull Void} and upper bound {@code @Nullable Object}) + * and the type argument inferred for {@code bar()} is {@code T}. During dataflow, the type of + * {@code local} is refined to {@code T} and the return is legal. + * + *

If the assignment context type was a declared type, for example: + * + * 

{@code
+     *  S bar () {...}
+     * Object foo() {
+     *     Object local = bar();
+     *     return local;
+     * }
+     * }
+ * + * The local variable default must be used or else the assignment context type is missing an + * annotation. So, an incompatible types in return error is issued in the above code. We could + * improve type argument inference in this case and by using the lower bound of {@code S} + * instead of the local variable default. + * + * @param atypeFactory AnnotatedTypeFactory + * @param assignmentContext VariableTree + * @return AnnotatedTypeMirror of Assignment context + */ + public static AnnotatedTypeMirror assignedToVariable( + AnnotatedTypeFactory atypeFactory, Tree assignmentContext) { + if (atypeFactory instanceof GenericAnnotatedTypeFactory) { + final GenericAnnotatedTypeFactory gatf = + ((GenericAnnotatedTypeFactory) atypeFactory); + return gatf.getAnnotatedTypeLhsNoTypeVarDefault(assignmentContext); + } else { + return atypeFactory.getAnnotatedType(assignmentContext); + } + } + + /** + * Check if the {@code type} contains a use of a type variable from {@code methodType}. + * + * @param type the annotated type + * @param methodType the type of an executable + * @return true if the {@code type} contains a use of a type variable from {@code methodType}. + */ + private static boolean containsUninferredTypeParameter( + AnnotatedTypeMirror type, AnnotatedExecutableType methodType) { + final List annotatedTypeVars = methodType.getTypeVariables(); + final List typeVars = new ArrayList<>(annotatedTypeVars.size()); + + for (AnnotatedTypeVariable annotatedTypeVar : annotatedTypeVars) { + typeVars.add( + (TypeVariable) + TypeAnnotationUtils.unannotatedType( + annotatedTypeVar.getUnderlyingType())); + } + + return containsTypeParameter(type, typeVars); + } + + /** + * Returns true if {@code type} contains a use of a type variable in {@code typeVariables}. + * + * @param type type to search + * @param typeVariables collection of type varibles + * @return true if {@code type} contains a use of a type variable in {@code typeVariables} + */ + public static boolean containsTypeParameter( + AnnotatedTypeMirror type, Collection typeVariables) { + // note NULL values creep in because the underlying visitor uses them in various places + final Boolean result = type.accept(new TypeVariableFinder(), typeVariables); + return result != null && result; + } + + /** + * Used to detect if the visited type contains one of the type variables in the typeVars + * parameter. + */ + private static class TypeVariableFinder + extends AnnotatedTypeScanner> { + + @Override + protected Boolean scan( + Iterable types, Collection typeVars) { + if (types == null) { + return false; + } + Boolean result = false; + Boolean first = true; + for (AnnotatedTypeMirror type : types) { + result = (first ? scan(type, typeVars) : scanAndReduce(type, typeVars, result)); + first = false; + } + return result; + } + + @Override + protected Boolean reduce(Boolean r1, Boolean r2) { + if (r1 == null) { + return r2 != null && r2; + + } else if (r2 == null) { + return r1; + } + + return r1 || r2; + } + + @Override + public Boolean visitTypeVariable( + AnnotatedTypeVariable type, Collection typeVars) { + if (typeVars.contains( + (TypeVariable) TypeAnnotationUtils.unannotatedType(type.getUnderlyingType()))) { + return true; + } else { + return super.visitTypeVariable(type, typeVars); + } + } + } +} diff --git a/framework/src/main/java/org/checkerframework/framework/util/typeinference/DefaultTypeArgumentInference.java b/framework/src/main/java/org/checkerframework/framework/util/typeinference/DefaultTypeArgumentInference.java index 35688f17eae2..3950bba76bdb 100644 --- a/framework/src/main/java/org/checkerframework/framework/util/typeinference/DefaultTypeArgumentInference.java +++ b/framework/src/main/java/org/checkerframework/framework/util/typeinference/DefaultTypeArgumentInference.java @@ -139,7 +139,7 @@ public Map inferTypeArgs( if (TreeUtils.enclosingNonParen(pathToExpression).first.getKind() == Tree.Kind.LAMBDA_EXPRESSION || (assignedTo == null - && TreeUtils.getAssignmentContext(pathToExpression) != null)) { + && TreeUtils.getAssignmentContext(pathToExpression, true) != null)) { // If the type of the assignment context isn't found, but the expression is assigned, // then don't attempt to infer type arguments, because the Java type inferred will be // incorrect. The assignment type is null when it includes uninferred type arguments. diff --git a/framework/src/main/java/org/checkerframework/framework/util/typeinference/TypeArgInferenceUtil.java b/framework/src/main/java/org/checkerframework/framework/util/typeinference/TypeArgInferenceUtil.java index 8287a330dc21..8718ceecdd44 100644 --- a/framework/src/main/java/org/checkerframework/framework/util/typeinference/TypeArgInferenceUtil.java +++ b/framework/src/main/java/org/checkerframework/framework/util/typeinference/TypeArgInferenceUtil.java @@ -151,13 +151,24 @@ public static AnnotatedTypeMirror assignedTo(AnnotatedTypeFactory atypeFactory, res = atypeFactory.getAnnotatedType(variable); } else if (assignmentContext instanceof MethodInvocationTree) { MethodInvocationTree methodInvocation = (MethodInvocationTree) assignmentContext; + ExecutableElement methodElt = TreeUtils.elementFromUse(methodInvocation); // TODO move to getAssignmentContext if (methodInvocation.getMethodSelect() instanceof MemberSelectTree && ((MemberSelectTree) methodInvocation.getMethodSelect()).getExpression() == path.getLeaf()) { + // treepath's leaf is assigned to the method declared receiver type + // AnnotatedExecutableType declMethodType = + // atypeFactory.getAnnotatedType(methodElt); + // return declMethodType.getReceiverType(); return null; - } - ExecutableElement methodElt = TreeUtils.elementFromUse(methodInvocation); + } // Removing above if block causes StackOverflowException via getReceiverType -> + // assignedTo -> getReceiverType loop! + // AnnotatedTypeMirror receiver = null; + // try { + // receiver = atypeFactory.getReceiverType(methodInvocation); + // } catch (Throwable e) { + // new Object(); + // } AnnotatedTypeMirror receiver = atypeFactory.getReceiverType(methodInvocation); res = assignedToExecutable( diff --git a/framework/tests/all-systems/GetAssignmentContext.java b/framework/tests/all-systems/GetAssignmentContext.java new file mode 100644 index 000000000000..eedae3a293cb --- /dev/null +++ b/framework/tests/all-systems/GetAssignmentContext.java @@ -0,0 +1,26 @@ +public class GetAssignmentContext { + + public A get() { + return new A(); + } + + public A get1(String a) { + return new A(); + } + + public void test() { + GetAssignmentContext t = new GetAssignmentContext(); + // get().a should be skipped + t.get1(get().a); + t.get1(get().bar()); + } + + static class A { + + String a = "1"; + + String bar(A this) { + return ""; + } + } +} diff --git a/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java b/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java index 622e5cf93824..979db8ab3fa6 100644 --- a/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java +++ b/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java @@ -367,7 +367,7 @@ public static Pair enclosingNonParen(final TreePath path) { } /** - * Returns the tree with the assignment context for the treePath leaf node. (Does not handle + * Returns the tree with the assignment context for the treePath leaf node. (Handles * pseudo-assignment of an argument to a parameter or a receiver expression to a receiver.) * *

The assignment context for the {@code treePath} is the leaf of its parent, if the parent @@ -392,9 +392,13 @@ public static Pair enclosingNonParen(final TreePath path) { * *

Otherwise, null is returned. * + * @param treePath the tree path + * @param conservative if conservative result is needed. Conservative result does not include + * pseudo assignment of method invocation receiver * @return the assignment context as described, {@code null} otherwise */ - public static @Nullable Tree getAssignmentContext(final TreePath treePath) { + public static @Nullable Tree getAssignmentContext( + final TreePath treePath, boolean conservative) { TreePath parentPath = treePath.getParentPath(); if (parentPath == null) { @@ -421,6 +425,41 @@ public static Pair enclosingNonParen(final TreePath path) { case RETURN: case VARIABLE: return parent; + case MEMBER_SELECT: + // since :javacutil:checkWorkingNullness seems not support assertion and living + // variable inference, assign to a local variable to pass the test + Element parentEle = TreeUtils.elementFromTree(parent); + + // Don't process method().field + if (parentEle != null && parentEle.getKind().isField()) { + return null; + } + // Also check case when treepath's leaf tree is used as method + // invocation's actual receiver + // If so, return that method invocation tree too as the assignment + // context tree rather than null as we did before + TreePath grandParentPath = parentPath.getParentPath(); + if (grandParentPath != null + && grandParentPath.getLeaf() instanceof MethodInvocationTree) { + + // do not use foo().bar() scheme as assignment context when conservative, since + // assignedTo for that is not implemented yet + if (conservative) { + MethodInvocationTree grandTree = + (MethodInvocationTree) grandParentPath.getLeaf(); + + // adapted from TypeArgInferenceUtil#assignedTo to be consistent + if (grandTree.getMethodSelect() instanceof MemberSelectTree + && ((MemberSelectTree) grandTree.getMethodSelect()).getExpression() + == treePath.getLeaf()) { + return null; + } + } + return grandParentPath.getLeaf(); + } else { + return null; + } + default: // 11 Tree.Kinds are CompoundAssignmentTrees, // so use instanceof rather than listing all 11. @@ -431,6 +470,16 @@ public static Pair enclosingNonParen(final TreePath path) { } } + /** + * See {@code getAssignmentContext}. Not using extended result. + * + * @param treePath the tree path + * @return the assignment context as described, {@code null} otherwise + */ + public static @Nullable Tree getAssignmentContext(final TreePath treePath) { + return getAssignmentContext(treePath, false); + } + /** * Gets the {@link Element} for the given Tree API node. For an object instantiation returns the * value of the {@link JCNewClass#constructor} field. Note that this result might differ from From 696154c79dabac574df0df69c5abc3ef7751de9e Mon Sep 17 00:00:00 2001 From: lnsun <57457122+lnsun@users.noreply.github.com> Date: Tue, 23 Jun 2020 22:00:26 -0400 Subject: [PATCH 2/5] mute unrelated change --- .../framework/type/poly/AbstractQualifierPolymorphism.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java b/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java index e02c00e7ff23..ec51541f03b4 100644 --- a/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java +++ b/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java @@ -371,7 +371,7 @@ private class PolyCollector * {@code Pair} may be equal, but they both should be visited. */ private final Set visitedTypes = - Collections.newSetFromMap(new IdentityHashMap<>()); + Collections.newSetFromMap(new IdentityHashMap()); /** * Returns true if the {@link AnnotatedTypeMirror} has been visited. If it has not, then it From b66cb307652db9f00e090a305b8f073996b6e41d Mon Sep 17 00:00:00 2001 From: lnsun <57457122+lnsun@users.noreply.github.com> Date: Wed, 1 Jul 2020 09:48:55 -0400 Subject: [PATCH 3/5] fix javadoc --- .../src/main/java/org/checkerframework/javacutil/TreeUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java b/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java index 979db8ab3fa6..1e9e7e9635cf 100644 --- a/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java +++ b/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java @@ -471,7 +471,7 @@ public static Pair enclosingNonParen(final TreePath path) { } /** - * See {@code getAssignmentContext}. Not using extended result. + * See {@code getAssignmentContext}. Not using conservative result. * * @param treePath the tree path * @return the assignment context as described, {@code null} otherwise From 813f680e7e0f80bd2b271065017423bcef44c42c Mon Sep 17 00:00:00 2001 From: lnsun <57457122+lnsun@users.noreply.github.com> Date: Thu, 9 Jul 2020 14:39:45 -0400 Subject: [PATCH 4/5] remove unrelated change --- .../checkerframework/framework/type/AnnotatedTypeFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java index e90d30b0d647..86e7ff79f824 100644 --- a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java +++ b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java @@ -1478,7 +1478,7 @@ public void postAsMemberOf( addAnnotationFromFieldInvariant(type, owner, (VariableElement) element); } addComputedTypeAnnotations(element, type); - if (viewpointAdapter != null && type.getKind() != TypeKind.EXECUTABLE) { + if (viewpointAdapter != null) { viewpointAdapter.viewpointAdaptMember(owner, element, type); } } From 331fec13b6cb47bd3cf03a700d579b25f91f3efb Mon Sep 17 00:00:00 2001 From: lnsun <57457122+lnsun@users.noreply.github.com> Date: Thu, 30 Jul 2020 17:33:51 -0400 Subject: [PATCH 5/5] rm dup reduceWithUpperBounds --- .../poly/AbstractQualifierPolymorphism.java | 68 +++++++++---------- 1 file changed, 31 insertions(+), 37 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java b/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java index 37681715a5c4..7116877210a4 100644 --- a/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java +++ b/framework/src/main/java/org/checkerframework/framework/type/poly/AbstractQualifierPolymorphism.java @@ -392,6 +392,10 @@ protected abstract AnnotationMirror combine( protected abstract void replace( AnnotatedTypeMirror type, AnnotationMirrorMap replacements); + private interface PolyCollectorAnnotationCombiner { + AnnotationMirror combine(AnnotationMirror poly, AnnotationMirror a1, AnnotationMirror a2); + } + /** * A helper class that resolves the polymorphic qualifiers with the most restrictive qualifier. * It returns a mapping from the polymorphic qualifier to the substitution for that qualifier. @@ -424,10 +428,10 @@ protected AnnotationMirrorMap scanWithNull( return new AnnotationMirrorMap<>(); } - @Override - public AnnotationMirrorMap reduce( + private AnnotationMirrorMap genericReduce( AnnotationMirrorMap r1, - AnnotationMirrorMap r2) { + AnnotationMirrorMap r2, + PolyCollectorAnnotationCombiner combiner) { if (r1 == null || r1.isEmpty()) { return r2; @@ -447,7 +451,7 @@ public AnnotationMirrorMap reduce( if (a2Annos == null) { res.put(polyQual, a1Annos); } else { - res.put(polyQual, combine(polyQual, a1Annos, a2Annos)); + res.put(polyQual, combiner.combine(polyQual, a1Annos, a2Annos)); } r2remain.remove(polyQual); } @@ -457,6 +461,13 @@ public AnnotationMirrorMap reduce( return res; } + @Override + protected AnnotationMirrorMap reduce( + AnnotationMirrorMap r1, + AnnotationMirrorMap r2) { + return genericReduce(r1, r2, AbstractQualifierPolymorphism.this::combine); + } + /** * Reduces lower bounds r1 with upper bounds r2. * @@ -468,40 +479,23 @@ private AnnotationMirrorMap reduceWithUpperBounds( AnnotationMirrorMap r1, AnnotationMirrorMap r2) { - if (r1 == null || r1.isEmpty()) { - return r2; - } - if (r2 == null || r2.isEmpty()) { - return r1; - } - - AnnotationMirrorMap res = new AnnotationMirrorMap<>(); - // Ensure that all qualifiers from r1 and r2 are visited. - AnnotationMirrorSet r2remain = new AnnotationMirrorSet(); - r2remain.addAll(r2.keySet()); - for (Map.Entry kv1 : r1.entrySet()) { - AnnotationMirror key1 = kv1.getKey(); - AnnotationMirror a1Anno = kv1.getValue(); - AnnotationMirror a2Anno = r2.get(key1); - if (a2Anno != null) { - r2remain.remove(key1); - AnnotationMirror subres = null; - for (AnnotationMirror top : topQuals) { - if (qualHierarchy.isSubtype(a1Anno, top)) { - subres = a1Anno; - } else if (qualHierarchy.isSubtype(a2Anno, top)) { - subres = a2Anno; + // Note: a different logic is needed to reduce with assignment context. + // For arguments, @Poly is on l-value of pseudo assignment. + // But when resolving with assignment, @Poly is on r-value of assignment. + return genericReduce( + r1, + r2, + (poly, a1, a2) -> { + AnnotationMirror subres = null; + for (AnnotationMirror top : topQuals) { + if (qualHierarchy.isSubtype(a1, top)) { + subres = a1; + } else if (qualHierarchy.isSubtype(a2, top)) { + subres = a2; + } } - } - res.put(key1, subres); - } else { - res.put(key1, a1Anno); - } - } - for (AnnotationMirror key2 : r2remain) { - res.put(key2, r2.get(key2)); - } - return res; + return subres; + }); } /**