Java: Improve algorithm for subtyping of parameterized types. #7088
Conversation
| typePrefixContains(pragma[only_bind_into](pps0), pragma[only_bind_into](ppt0)) and | ||
| pps = TTypeParam(pragma[only_bind_into](pps0), t) and | ||
| ppt = TTypeParam(ppt0, t) |
There was a problem hiding this comment.
Worth commenting what the load of pragmas intend to achieve? It looks like you want:
Take all TTypeParam(ppt0, t)
Join with all TTypeParam(pps0, t)
Restrict by typePrefixContains(pps0, ppt0)
?
There was a problem hiding this comment.
No, that join order would be horrible (and isn't what the pragmas describe). There are two instances of canonical pragma use. The only_bind_into on pps0 means "don't join on this" and the set of only_bind_into on all the columns of typePrefixContains is the canonical use saying "put this predicate first in the pipeline". So this should be entirely standard pragma use.
There was a problem hiding this comment.
Right, I was reading the pragma as if it was only_bind_out. Oops.
| ParameterizedPrefix ppt, ParameterizedPrefix ppt0, RefType s | ||
| ) { | ||
| exists(GenericType g, int n, RefType t | | ||
| ppt.split(g, ppt0, t, n) and |
There was a problem hiding this comment.
| ppt.split(g, ppt0, t, n) and | |
| // Implies ppt = TTypeParam(ppt0, t) | |
| ppt.split(g, ppt0, t, n) and |
There was a problem hiding this comment.
It's in the qldoc of split. Do you think it should be repeated here?
There was a problem hiding this comment.
Yes, because it enables the reader to understand the whole of this predicate in one go without referring elsewhere
| exists(ParameterizedPrefix pps0 | | ||
| typePrefixContains(pps0, ppt0) and | ||
| pps = TTypeParam(pps0, s) and | ||
| s instanceof Wildcard |
There was a problem hiding this comment.
Surprising not see the wildcard condition in the algorithm outline at line 150
There was a problem hiding this comment.
That's because it's semantically irrelevant. It's just a piece of manual magic implied by typeArgumentContains(_, s, t, _).
There was a problem hiding this comment.
In that case suggest adding // manual magic to make clear that's all this is doing
| private predicate subtypeStarMagic1(RefType t) { t = any(Wildcard w).getUpperBound().getType() } | ||
|
|
||
| private predicate subtypeStarMagic2(RefType t) { t = any(Wildcard w).getLowerBound().getType() } |
There was a problem hiding this comment.
Call them getAWildcardUpper/LowerBound?
There was a problem hiding this comment.
We could... But they really are pieces manual magic, so I thought these names were clearer.
There was a problem hiding this comment.
Similar to s instanceof Wildcard above suggest naming them for what they are and using // manual magic to signal intent
aschackmull
force-pushed
the
java/parameterized-subtyping
branch
from
c22813b to
76606b5
Compare
|
Added a commit to fix |
This drastically improves performance when there are many instantiations of generic types with 3 or more type variables. One observed case improved from 85 minutes to less than one minute.