Bypass eligible caches for implicit search under GADT constraints #14072
Conversation
isEmpty is pointless since it is always false if we are inside a polymorphic method. isNarrowing computes whether the constraint actually has tigher bounds for a type variable than the original bounds.
Under a non-empty GADT constraint, bypass caches when computing eligible implicits. The GADT constraint might influence what implicits are available locally, so cached results would be unreliable. Fixes scala#13974
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit https://dotty-bench.epfl.ch/14072/ to see the changes. Benchmarks is based on merging with master (d6fe4b4) |
| @@ -304,7 +312,7 @@ final class ProperGadtConstraint private( | |||
|
|
|||
| override def fresh = new ProperGadtConstraint | |||
| override def restore(other: GadtConstraint): Unit = | |||
| if (!other.isEmpty) sys.error("cannot restore a non-empty GADTMap") | |||
| assert(!other.isNarrowing, "cannot restore a non-empty GADTMap") | |||
There was a problem hiding this comment.
Was this changed intentionally? I don't think we should allow restoring a non-empty GadtConstraint into an empty one. GadtConstraint is also a scope, and EmptyGadtConstraint doesn't allow constraining anything, so such a call to restore would lead to errors down the road.
With that being said, this is more of a nitpick than anything else, I think we should merge this PR and then I'll tweak GadtConstraint to define EmptyGadtConstraint a bit more gracefully.
|
@odersky so what's the policy right now on merging PRs into master? I think this PR fits into a patch release, so I guess this means we can just merge it, right? |
Under a non-empty GADT constraint, bypass caches when computing eligible implicits.
The GADT constraint might influence what implicits are available locally, so cached
results would be unreliable.
What this means for performance depends on how many implicit searches are performed
under non-empty GADT constraints. I measured for dotty itself. There, about 2% of givens
were checked in uncached mode because they were under a nonempty GADT constraint.
Rechecking those 2% might have been avoided if they had been cached already otherwise.
Fixes #13974