Refine "can be handled by parent" condition when generting bridges #13092
Conversation
Fixes scala#13087 Reverses the outcome in i1240.scala.
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 1 running. |
|
Performance test finished successfully: Visit https://dotty-bench.epfl.ch/13092/ to see the changes. Benchmarks is based on merging with master (20e2b95) |
| if sym1.owner.is(JavaDefined, butNot = Trait) | ||
| && sym2.owner.is(JavaDefined, butNot = Trait) | ||
| then false // javac already handles these checks and inserts bridges | ||
| else if sym1.isType then true |
There was a problem hiding this comment.
I guess this isn't new code, but it's weird that this overload of isOverridingPair simply allows all types to match whereas the other overload will check that the kinds match, could this be harmonized?
There was a problem hiding this comment.
I am not sure. Maybe. But not in this PR. Somebody would have to take the time to experiment and figure this out. It's best to be conservative here since I am sure there are lots of code bases out there that we have not seen yet and that test this logic in interesting ways.
| sd1.matchesLoosely(sd2) | ||
| && (sym1.hasTargetName(sym2.targetName) | ||
| || compatibleTypes(sym1, sd1.info, sym2, sd2.info)) | ||
|
|
||
| val opc = new OverridingPairs.Cursor(clazz): |
There was a problem hiding this comment.
PrepJSInterop also performs some overriding checks using Cursor, so I wonder if it should use a subclass of Cursor with the same logic we're now using here in RefChecks, or perhaps the checking logic in PrepJSInterop should be moved to RefChecks? /cc @sjrd
There was a problem hiding this comment.
It should probably be the same logic as in RefChecks, yes.
There was a problem hiding this comment.
OK, I let you experiment with it. But I am going to merge this as is, since I do not think I have the time to look at all code that considers overriding pairs.
Fixes #13087
Reverses the outcome in i1240.scala.