From 23cabeecc4cd8224b800963a852e1340d1a6546b Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Thu, 11 Apr 2024 23:09:47 +0200 Subject: [PATCH] null analysis should merge null contracts from equivalent super methods + add safety net in case considering null contracts could have messed up --- .../jdt/internal/compiler/lookup/Scope.java | 212 +++++++++--------- 1 file changed, 110 insertions(+), 102 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java index 8793c9d3de4..aab21ce3d5a 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java @@ -4845,125 +4845,133 @@ public void acceptPotentiallyCompatibleMethods(MethodBinding[] methods) {/* igno // see if they are equal after substitution of type variables (do the type variables have to be equal to be considered an override???) if (receiverType != null) receiverType = receiverType instanceof CaptureBinding ? receiverType : (ReferenceBinding) receiverType.erasure(); - nextSpecific : for (int i = 0; i < visibleSize; i++) { - MethodBinding current = moreSpecific[i]; - if (current != null) { - ReferenceBinding[] mostSpecificExceptions = null; - MethodBinding original = current.original(); - boolean shouldIntersectExceptions = original.declaringClass.isAbstract() && original.thrownExceptions != Binding.NO_EXCEPTIONS; // only needed when selecting from interface methods - for (int j = 0; j < visibleSize; j++) { - MethodBinding next = moreSpecific[j]; - if (next == null || i == j) continue; - MethodBinding original2 = next.original(); - if (TypeBinding.equalsEquals(original.declaringClass, original2.declaringClass)) - break nextSpecific; // duplicates thru substitution - - if (!original.isAbstract()) { - if (original2.isAbstract() || original2.isDefaultMethod()) - continue; // only compare current against other concrete methods - - original2 = original.findOriginalInheritedMethod(original2); - if (original2 == null) - continue nextSpecific; // current's declaringClass is not a subtype of next's declaringClass - if (current.hasSubstitutedParameters() || original.typeVariables != Binding.NO_TYPE_VARIABLES) { - if (!environment().methodVerifier().isParameterSubsignature(original, original2)) - continue nextSpecific; // current does not override next - } - } else if (receiverType != null) { // should not be null if original isAbstract, but be safe - TypeBinding superType = receiverType.findSuperTypeOriginatingFrom(original.declaringClass.erasure()); - if (TypeBinding.equalsEquals(original.declaringClass, superType) || !(superType instanceof ReferenceBinding)) { - // keep original - } else { - // must find inherited method with the same substituted variables - MethodBinding[] superMethods = ((ReferenceBinding) superType).getMethods(original.selector, argumentTypes.length); - for (MethodBinding superMethod : superMethods) { - if (superMethod.original() == original) { - original = superMethod; - break; + + boolean hasConsideredNullContract = false; + // perform 1 or 2 attempts, the second being the safety net, in case considering null contracts may have prevented finding a solution. + for (int attempt = 0; attempt < 2; attempt++) { + nextSpecific : for (int i = 0; i < visibleSize; i++) { + MethodBinding current = moreSpecific[i]; + if (current != null) { + ReferenceBinding[] mostSpecificExceptions = null; + MethodBinding original = current.original(); + boolean shouldIntersectExceptions = original.declaringClass.isAbstract() && original.thrownExceptions != Binding.NO_EXCEPTIONS; // only needed when selecting from interface methods + for (int j = 0; j < visibleSize; j++) { + MethodBinding next = moreSpecific[j]; + if (next == null || i == j) continue; + MethodBinding original2 = next.original(); + if (TypeBinding.equalsEquals(original.declaringClass, original2.declaringClass)) + break nextSpecific; // duplicates thru substitution + + if (!original.isAbstract()) { + if (original2.isAbstract() || original2.isDefaultMethod()) + continue; // only compare current against other concrete methods + + original2 = original.findOriginalInheritedMethod(original2); + if (original2 == null) + continue nextSpecific; // current's declaringClass is not a subtype of next's declaringClass + if (current.hasSubstitutedParameters() || original.typeVariables != Binding.NO_TYPE_VARIABLES) { + if (!environment().methodVerifier().isParameterSubsignature(original, original2)) + continue nextSpecific; // current does not override next + } + } else if (receiverType != null) { // should not be null if original isAbstract, but be safe + TypeBinding superType = receiverType.findSuperTypeOriginatingFrom(original.declaringClass.erasure()); + if (TypeBinding.equalsEquals(original.declaringClass, superType) || !(superType instanceof ReferenceBinding)) { + // keep original + } else { + // must find inherited method with the same substituted variables + MethodBinding[] superMethods = ((ReferenceBinding) superType).getMethods(original.selector, argumentTypes.length); + for (MethodBinding superMethod : superMethods) { + if (superMethod.original() == original) { + original = superMethod; + break; + } } } - } - superType = receiverType.findSuperTypeOriginatingFrom(original2.declaringClass.erasure()); - if (TypeBinding.equalsEquals(original2.declaringClass, superType) || !(superType instanceof ReferenceBinding)) { - // keep original2 - } else { - // must find inherited method with the same substituted variables - MethodBinding[] superMethods = ((ReferenceBinding) superType).getMethods(original2.selector, argumentTypes.length); - for (MethodBinding superMethod : superMethods) { - if (superMethod.original() == original2) { - original2 = superMethod; - break; + superType = receiverType.findSuperTypeOriginatingFrom(original2.declaringClass.erasure()); + if (TypeBinding.equalsEquals(original2.declaringClass, superType) || !(superType instanceof ReferenceBinding)) { + // keep original2 + } else { + // must find inherited method with the same substituted variables + MethodBinding[] superMethods = ((ReferenceBinding) superType).getMethods(original2.selector, argumentTypes.length); + for (MethodBinding superMethod : superMethods) { + if (superMethod.original() == original2) { + original2 = superMethod; + break; + } } } - } - if (original.typeVariables != Binding.NO_TYPE_VARIABLES) - original2 = original.computeSubstitutedMethod(original2, environment()); - if (original2 == null || !original.areParameterErasuresEqual(original2)) - continue nextSpecific; // current does not override next - if (TypeBinding.notEquals(original.returnType, original2.returnType)) { - if (next.original().typeVariables != Binding.NO_TYPE_VARIABLES) { - if (original.returnType.erasure().findSuperTypeOriginatingFrom(original2.returnType.erasure()) == null) + if (original.typeVariables != Binding.NO_TYPE_VARIABLES) + original2 = original.computeSubstitutedMethod(original2, environment()); + if (original2 == null || !original.areParameterErasuresEqual(original2)) + continue nextSpecific; // current does not override next + if (TypeBinding.notEquals(original.returnType, original2.returnType)) { + if (next.original().typeVariables != Binding.NO_TYPE_VARIABLES) { + if (original.returnType.erasure().findSuperTypeOriginatingFrom(original2.returnType.erasure()) == null) + continue nextSpecific; + } else if (!current.returnType.isCompatibleWith(next.returnType)) { continue nextSpecific; - } else if (!current.returnType.isCompatibleWith(next.returnType)) { + } + // continue with original 15.12.2.5 + } + if (compilerOptions().isAnnotationBasedNullAnalysisEnabled + && j > i // don't go backwards + && NullAnnotationMatching.hasMoreSpecificNullness(next, current)) + { + // In this case we want to prefer 'next' among equivalent methods. + // (the case where JLS 15.12.2.5 says "...is chosen arbitrarily...") + // To try if 'next' matches all criteria, skip outer loop to j (after increment): + i = j -1 ; + hasConsideredNullContract = true; continue nextSpecific; } - // continue with original 15.12.2.5 - } - if (compilerOptions().isAnnotationBasedNullAnalysisEnabled - && j > i // don't go backwards - && NullAnnotationMatching.hasMoreSpecificNullness(next, current)) - { - // In this case we want to prefer next among equivalent methods. - // (the case where JLS 15.12.2.5 says "...is chosen arbitrarily...") - // To try if 'next' matches all criteria, skip outer loop to j (after increment): - i = j -1 ; - continue nextSpecific; - } - if (shouldIntersectExceptions && original2.declaringClass.isInterface()) { - if (current.thrownExceptions != next.thrownExceptions) { - if (next.thrownExceptions == Binding.NO_EXCEPTIONS) { - mostSpecificExceptions = Binding.NO_EXCEPTIONS; - } else { - if (mostSpecificExceptions == null) { - mostSpecificExceptions = current.thrownExceptions; - } - int mostSpecificLength = mostSpecificExceptions.length; - ReferenceBinding[] nextExceptions = getFilteredExceptions(next); - int nextLength = nextExceptions.length; - SimpleSet temp = new SimpleSet(mostSpecificLength); - boolean changed = false; - nextException : for (int t = 0; t < mostSpecificLength; t++) { - ReferenceBinding exception = mostSpecificExceptions[t]; - for (int s = 0; s < nextLength; s++) { - ReferenceBinding nextException = nextExceptions[s]; - if (exception.isCompatibleWith(nextException)) { - temp.add(exception); - continue nextException; - } else if (nextException.isCompatibleWith(exception)) { - temp.add(nextException); - changed = true; - continue nextException; - } else { - changed = true; + if (shouldIntersectExceptions && original2.declaringClass.isInterface()) { + if (current.thrownExceptions != next.thrownExceptions) { + if (next.thrownExceptions == Binding.NO_EXCEPTIONS) { + mostSpecificExceptions = Binding.NO_EXCEPTIONS; + } else { + if (mostSpecificExceptions == null) { + mostSpecificExceptions = current.thrownExceptions; + } + int mostSpecificLength = mostSpecificExceptions.length; + ReferenceBinding[] nextExceptions = getFilteredExceptions(next); + int nextLength = nextExceptions.length; + SimpleSet temp = new SimpleSet(mostSpecificLength); + boolean changed = false; + nextException : for (int t = 0; t < mostSpecificLength; t++) { + ReferenceBinding exception = mostSpecificExceptions[t]; + for (int s = 0; s < nextLength; s++) { + ReferenceBinding nextException = nextExceptions[s]; + if (exception.isCompatibleWith(nextException)) { + temp.add(exception); + continue nextException; + } else if (nextException.isCompatibleWith(exception)) { + temp.add(nextException); + changed = true; + continue nextException; + } else { + changed = true; + } } } - } - if (changed) { - mostSpecificExceptions = temp.elementSize == 0 ? Binding.NO_EXCEPTIONS : new ReferenceBinding[temp.elementSize]; - temp.asArray(mostSpecificExceptions); + if (changed) { + mostSpecificExceptions = temp.elementSize == 0 ? Binding.NO_EXCEPTIONS : new ReferenceBinding[temp.elementSize]; + temp.asArray(mostSpecificExceptions); + } } } } } } + if (mostSpecificExceptions != null && mostSpecificExceptions != current.thrownExceptions) { + return new MostSpecificExceptionMethodBinding(current, mostSpecificExceptions); + } + return current; } - if (mostSpecificExceptions != null && mostSpecificExceptions != current.thrownExceptions) { - return new MostSpecificExceptionMethodBinding(current, mostSpecificExceptions); - } - return current; } + if (!hasConsideredNullContract) + break; + // otherwise retry without considering null contracts } - return new ProblemMethodBinding(visible[0], visible[0].selector, visible[0].parameters, ProblemReasons.Ambiguous); }