Fix #9103: Add additional config to NamedParts accumulator #9106
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The NamedParts accumulator always searched the underlying types of TermRef, but this is the right thing to do only for avoidance. In other situations, notably implicit scope construction we should only count the TermRef itself, but not its underlying type.
widen only if `widenSingletons` is true.
The previous changes failed the community build for squants. What happened was that an implicit in the package object of squants was not found in motion/AngularAcceleration. Normally, the context object would be found in the normal implicit context of the use site. But AngularAcceleration was defined as ```scala package squants.motion ``` which meant that `squants` by itself was not in the implicit scope. If it had been defined like this: ```scala package squants package motion ``` it would have worked fine. So the implicit hit it relied on the fact that under -source:3.0-migration we also search package prefixes of types. Only, I believe there were no such prefixes normally since the types in question started with `this[motion]` so no mention of `squants`. But if we widen the term refs on the paths then we mention something that mentions in turn `scala` and things are OK. I did not track down what this was, but that's what must have happened. So, to keep compatibility with old code, we should still enable this under -source:3.0-migration.
Do consider underlying types of singleton types that are not path prefixes. This is necessary since arguments in apply prototypes are often singeletons, and in this case we want to include the implicit scope of their underlying type.
The idea of LazyRefs is that they are always completed in the current context. But that did not work if the LazyRef is mapped in a type map. Here, the TypeMap's context was used to do the remap. We now use the context that forced the LazyVal instead. The problem manifested itself with failing replTests when the new implicitScope scheme was used.
In that way, we include all of the implicit scope of Scala 2, except for package objects. The idea is that everything that is reasonable should be included in the implicit scope. Package objects were excluded since they had a dual purpose: 1. as a way to define implicits that are in scope for a whole project 2. as a prefix for implicits of all types coming out of that project It's often that one wants only (1) but then is surprised to also get (2).
|
Dropped in favor of #9119 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The NamedParts accumulator always searched the underlying types of TermRef, but this
is the right thing to do only for avoidance. In other situations, notably implicit
scope construction, we should only count the TermRef itself, but not its underlying
type.