Java: Update stats and make some performance tweaks #8241
Conversation
| --- | ||
| * Added `hasDescendant(RefType anc, Type sub)` | ||
| * Added `RefType.getADescendant()` | ||
| * Added `RefType.getAStrictAncestor()` |
There was a problem hiding this comment.
| * Added `RefType.getAStrictAncestor()` | |
| * Added `RefType.getAnAncestor()` | |
| * Added `RefType.getAStrictAncestor()` |
There was a problem hiding this comment.
RefType.getAnAncestor() already exists; this just changes its definition.
| predicate hasDescendant(RefType anc, Type sub) { | ||
| anc = sub | ||
| or | ||
| exists(RefType mid | hasSubtype(anc, mid) and hasDescendant(mid, sub)) |
There was a problem hiding this comment.
Usage of hasSubtype has its issues with generic and parameterized types, most notably:
getAnAcestor()never gets for a parameterized type (e.g.java.util.List<String>) the generic source declaration (e.g.java.util.List)hasSubtypeseems to be unable to return subtypes for generic types, e.g. forjava.util.Listit does not getjava.util.ArrayList(example query); maybe that is actually a bug? (can report that separately if you want)
By adding more predicates in this pull request which suffer from these issues, it will become more likely that users will use these predicates by accident for generic or parameterized types and run into false negatives or not working queries. Though maybe not considering generic and parameterized types here is acceptable / desired for performance?
I just wanted to point that out, in case you were not aware of it.
Related to this might be #5521 and #5595.
(And just to clarify and to avoid any confusion; I am not a member of this project, so feel free to ignore my comment)
There was a problem hiding this comment.
I take your point about people being more likely to use these predicates if we make it easier for them, but I don't think the new predicates are much easier to use than e.g. hasSubtype*. So as this PR doesn't make the behaviour worse, I think I'll go ahead and merge it.
The List/ArrayList example does look suspicious at first glance, though. Could you go ahead and file a ticket for that please, so we can take a proper look into it.
There was a problem hiding this comment.
Have created #8339 for this.
|
The caching of the transitive closure of |
No description provided.