Skip to content

Commit

Permalink
Check upper bound compatibility during incorporation
Browse files Browse the repository at this point in the history
  • Loading branch information
oowekyala committed Apr 29, 2024
1 parent 42cf1b5 commit 6fea861
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import java.util.ArrayList;
import java.util.Set;

import net.sourceforge.pmd.lang.java.symbols.JClassSymbol;
import net.sourceforge.pmd.lang.java.symbols.JTypeDeclSymbol;
import net.sourceforge.pmd.lang.java.types.InternalApiBridge;
import net.sourceforge.pmd.lang.java.types.JTypeMirror;
import net.sourceforge.pmd.lang.java.types.TypeOps.Convertibility;
Expand Down Expand Up @@ -36,6 +38,66 @@ abstract class IncorporationAction {

abstract void apply(InferenceContext ctx);

/**
* Check that an upper bound with a class (not interface) or array
* is compatible with other upper bounds of class or array type.
* This is necessary to guarantee the existence of a glb for these,
* for {@link ReductionStep#UPPER}.
*
* <p>If the bound is {@code alpha <: T}, then we must check
* that {@code S <: T} or {@code T <: S} holds for all bounds
* {@code alpha <: S}, where S is a class or array type. Otherwise,
* the GLB does not exist.
*/
static class CheckClassUpperBound extends IncorporationAction {

private final JTypeMirror myBound;

CheckClassUpperBound(InferenceVar ivar, JTypeMirror bound) {
super(ivar);
this.myBound = bound;
}

public static boolean needsCheck(BoundKind kind, JTypeMirror bound) {
if (kind == BoundKind.UPPER) {
JTypeDeclSymbol symbol = bound.getSymbol();
return symbol instanceof JClassSymbol && !symbol.isInterface();
}
return false;
}


@Override
public void apply(InferenceContext ctx) {
for (BoundKind k : BoundKind.EQ_UPPER) {
for (JTypeMirror b : ivar.getBounds(k)) {
if (!checkBound(b, ctx)) {
throw ResolutionFailedException.incompatibleBound(ctx.logger, ivar, BoundKind.UPPER, myBound, k, b);
}
}
}
}

private boolean checkBound(JTypeMirror otherBound, InferenceContext ctx) {

JTypeDeclSymbol sym = otherBound.getSymbol();
// either the bound is not a concrete class or array type
return !(sym instanceof JClassSymbol) || sym.isInterface()
// or both bounds are related in some way
|| CheckBound.checkBound(false, otherBound, myBound, ctx)
|| CheckBound.checkBound(false, myBound, otherBound, ctx);

}


@Override
public String toString() {
return "Check class bound " + BoundKind.UPPER.format(ivar, myBound);
}


}

/**
* Check that a bound is compatible with the other current bounds
* of an ivar.
Expand Down Expand Up @@ -97,13 +159,13 @@ private boolean checkBound(JTypeMirror otherBound, BoundKind otherKind, Inferenc
/**
* If 'eq', checks that {@code T = S}, else checks that {@code T <: S}.
*/
boolean checkBound(boolean eq, JTypeMirror t, JTypeMirror s, InferenceContext ctx) {
static boolean checkBound(boolean eq, JTypeMirror t, JTypeMirror s, InferenceContext ctx) {
// eq bounds are so rare we shouldn't care if they're cached
return eq ? InternalApiBridge.isSameTypeInInference(t, s)
: checkSubtype(t, s, ctx);
}

private boolean checkSubtype(JTypeMirror t, JTypeMirror s, InferenceContext ctx) {
private static boolean checkSubtype(JTypeMirror t, JTypeMirror s, InferenceContext ctx) {
if (ctx.getSupertypeCheckCache().isCertainlyASubtype(t, s)) {
return true; // supertype was already cached
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,12 +577,7 @@ private JMethodSig instantiateImpl(JMethodSig m, MethodCallSite site, MethodReso
// we only test it can reduce, we don't commit inferred types at this stage
InferenceContext ctxCopy = infCtx.copy();
LOG.applicabilityTest(ctxCopy, m);
try {
ctxCopy.solve(/*onlyBoundedVars:*/isPreJava8());
} catch (Exception e) {
// applicability test failed for this candidate, but continue with others
throw ResolutionFailedException.fromException(LOG, e);
}
ctxCopy.solve(/*onlyBoundedVars:*/isPreJava8());

// if unchecked conversion was needed, update the site for invocation pass
if (ctxCopy.needsUncheckedConversion()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import net.sourceforge.pmd.lang.java.types.TypeSystem;
import net.sourceforge.pmd.lang.java.types.internal.infer.ExprMirror.InvocationMirror.MethodCtDecl;
import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.CheckBound;
import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.CheckClassUpperBound;
import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.PropagateAllBounds;
import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.PropagateBounds;
import net.sourceforge.pmd.lang.java.types.internal.infer.IncorporationAction.SubstituteInst;
Expand Down Expand Up @@ -388,6 +389,9 @@ void onBoundAdded(InferenceVar ivar, BoundKind kind, JTypeMirror bound, boolean

incorporationActions.add(new CheckBound(ivar, kind, bound));
incorporationActions.add(new PropagateBounds(ivar, kind, bound));
if (CheckClassUpperBound.needsCheck(kind, bound)) {
incorporationActions.add(new CheckClassUpperBound(ivar, bound));
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ public Set<BoundKind> complementSet(boolean eqIsAll) {
// These sets are shared because otherwise *literal millions* of enumsets are created, with the same constants
static final Set<BoundKind> ALL = EnumSet.allOf(BoundKind.class);
static final Set<BoundKind> EQ_LOWER = EnumSet.of(EQ, LOWER);
private static final Set<BoundKind> EQ_UPPER = EnumSet.of(EQ, UPPER);
static final Set<BoundKind> EQ_UPPER = EnumSet.of(EQ, UPPER);
private static final Set<BoundKind> JUST_EQ = Collections.singleton(EQ);

private final String sym;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ import java.io.OutputStream

/**
* Expensive test cases for the overload resolution phase.
*
* Edit: So those used to be very expensive (think minutes of execution),
* but optimisations made them very fast.
*/
class PolyResolutionTest : ProcessorTestSpec({

Expand Down

0 comments on commit 6fea861

Please sign in to comment.