Skip to content

Commit

Permalink
null analysis should merge null contracts from equivalent super methods
Browse files Browse the repository at this point in the history 
+ add safety net in case considering null contracts could have messed up
  • Loading branch information
@stephan-herrmann
stephan-herrmann committed Apr 11, 2024
1 parent f9f5117 commit 23cabee
Showing 1 changed file with 110 additions and 102 deletions.
212 changes: 110 additions & 102 deletions org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down

0 comments on commit 23cabee

Please sign in to comment.