typedSelect refactorings #11037
typedSelect refactorings #11037
Conversation
odersky
force-pushed
the
fix-#8182
branch
5 times, most recently
from
30ee52c to
876e6e6
Compare
tests/neg/i8032.check
Outdated
| -- [E008] Not Found Error: tests/neg/i8032.scala:3:12 ------------------------------------------------------------------ | ||
| 3 |val y = ???.map // error | ||
| | ^^^^^^^ | ||
| | value map is not a member of Nothing |
There was a problem hiding this comment.
Is this removed by accident? Or it's due to the TODO in the typer about the improvement on error message.
There was a problem hiding this comment.
No it was removed since the error message now contained an additional clause
Note that implicit extension methods cannot be applied because they are ambiguous;
|both method charArrayOps in object Predef and method doubleArrayOps in object Predef provide an extension method `map` on (??? : => Nothing)
2
but that clause was not deterministic (different conversion candidates showed up). I have now changed the reporting
so that the eplanation is dropped for arguments of type Nothing and brought the check file back.
| if isExtension then return found | ||
| else | ||
| checkImplicitConversionUseOK(found) | ||
| val qual1 = withoutMode(Mode.ImplicitsEnabled)(adapt(found, selProto, locked)) |
There was a problem hiding this comment.
In Implicits.scala, the method tryConversionForSelection seems to already ensure compatibility w.r.t the selProto. Is the adaptation here still necessary?
There was a problem hiding this comment.
Good catch. It looks like the adapt can be dropped here.
| tryEither { | ||
| val tree1 = tryExtensionOrConversion(tree, pt, pt, qual, locked, NoViewsAllowed, privateOK = false) | ||
| if tree1.isEmpty then None | ||
| else Some(adapt(tree1, pt, locked)) |
There was a problem hiding this comment.
Isn't the result tree1 already adapted w.r.t. to pt in the call tryExtensionOrConversion?
There was a problem hiding this comment.
No it isn't. tryExtensionOrConversion only does a typedSelect, not a full typed. typedSelect does not adapt.
The aim of the refactoring is to be clearer and to be able to drop ExtMethodApply.
Two main strategies:
1. Move some code out of adapt and assignType into typedSelect.
2. Concentrate extension method expansion and conversions for selections in a
method `tryExtensionOrConversion. The method is called from `typedSelect`
and `tryInsertImplicitOnQualifier`.
The aim of the refactoring is to move all select-dependent adaptations and re-tries into
typedSelect. Right now some are in adapt and some are in assignType. This is awkward
since it means that
- we do too much in TypeAssigner. Sine it aims to be minimal TypeAssigner should have no business doing adaptations
and retries.
- we do some adaptations too early in adpt. This means we need the kludge of wrapping
trees in ExtMethodApply which causes scala#8182, among others.
Fixes scala#8182
|
test performance please |
|
performance test scheduled: 29 job(s) in queue, 1 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/11037/ to see the changes. Benchmarks is based on merging with master (f5e84f4) |
The aim of the refactoring is to be clearer and to be able to drop ExtMethodApply.
Two main strategies:
method
tryExtensionOrConversion. The method is called fromtypedSelectandtryInsertImplicitOnQualifier`.The aim of the refactoring is to move all select-dependent adaptations and re-tries into
typedSelect. Right now some are in adapt and some are in assignType. This is awkward
since it means that
and retries.
trees in ExtMethodApply which causes byname implicit depending on recursive given crash #8182, among others.
Fixes #8182