Cosmetic changes to the completions framework #7451
Closed
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
Late compilation counts as loading
So that we can modify their flags.
…types Also fix a neg test for i3067. After this modification to the completion algorithm, implicit definitions without explicit types stay visible in the implicit scope and hence, the error of the missing implicit is not reporter. This is fine, however, since the real error to be fixed is the lack of an explicit type of the implicit value, not the lack of the implicit value itself.
It was needed to make Dotty cross-compile to Scala 2 before full bootstrap. Its direct semantics is about adding the `inline` flag to a symbol annotated by it. `inline` flag, however, is listed as one of the flags that are not supposed to be modified by completion of symbols. Hence, having `forceInline` annotation is in violation of our own assumptions about completion. Before full bootstrap, we were not able to drop this annotation, however, now that we have the full bootstrap, we can do it.
For the purposes of the capability of flag mutation on completion.
This flag is set on symbol completion based on other semantic info.
FTTB it is a cyclic error. It requests the user to fully specify the type of the method. Once they do so, an intended "implicit not found" message appears. Displaying the correct message directly requires adding `Macro` flag to `FromStartFlags`, which is not correct. Until we find a workaround, this solution is a reasonable trade-off because it offers an unambiguous action (to fully specify the type of the method) for the user to take to see the correct error message. The reason the cyclic error appears in first place is as follows. Given `inline def g = qc`, we typecheck `qc`. However, `qc` requires an implicit QuoteContext which can only be synthetically generated for the bodies of macros. Hence we need to see if `g` is a macro, and to do so, we need to complete it. To complete it, we need to typecheck its body, which yields the cyclic error.
Count a denotation as completing if it has the `Touched` flag set. This assumption follows from the definition of the flag `Touched` provided in its javadoc comment in Flags.scala. Also, this commit unconditionally unsets this flag for all denotations once they are completed. The rationale is, again, to comply with the definition of `Touched`.
Some flags do not fall under the definition of the flags that are not mutated during completion. However, in practice, they are almost never mutated on completion and such an event is easy to predict. An example of such a flag is `deferred` which is set only when compiling opaque types – this is part of what makes opaque types work. It is reasonable, hence, to make sure denotations are counted as current even though they have some of such flags if when querying these flags we can predict that they will not change during completion.
Part of the mechanics of how opaque types work is by setting the `opaque` flag at the objects that own them.
Now that Deferred is conditionally immutable, if we query it along with other properly immutable flags, the query may trigger completion. This will happen if the symbol in question is also opaque and is a type. The completion will be rightfully triggered since the opaque mechanism does indeed mutate the deferred flag on opaque types. However, if e.g. we query `is(Method, butNot = Deferred)` on `opaque type B`, we may be able to return `false` on the query as soon as we see that the symbol in question is not a method, without triggering the completion. Too early completion is undesirable since completion of opaque types modifies the self-type of its owner. If the completion of an opaque type happens during unpickling of the owner itself, the self-type of the owner may be rewritten later in its completion.
"Touched" does not carry any obvious semantic meaning. "Completing" reflects the intent of when the flag should be set.
isCompleted is now isCompletedOrStubbed and isCompleting – isCompletingAndNotStubbed. This reflects the fact that sometimes we use a hack where we set `info` on denotations to a non-lazy type during completion to flip these predicates. This hack is possible because both of these predicates depend on checking whether the `info` is non-lazy. However, if we make these predicates with these specific names depend on the `info`'s type, their names became incorrect wrt their semantics. `isCompleting` must be true iff the `Completing` (formerly `Touched`) flag is set to true by definition of that flag. `isCompleted` must be true if we both have a concrete `info` and are not in process of being completed (since further modifications may be done by the completion process, we cannot say a denotation is completed unless this process has finished). In principle we should not use the hack in question in first place and have something more well-defined to avoid cyclic references. The bare minimum we can do right now is to rename the predicates to reflect what they do more precisely so that to avoid confusion when reading the code.
|
Does this PR change behavior, or is it just naming? What is the reason for introducing a new flag. Note that we are almost running out of flags, so adding a new one should be done only if absolately necessary. |
Just the naming.
See #7427 (comment). |
|
Will be solved in the context of #7427 |
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.
On top of #7427.
Rename the
Touchedflag toCompleting.Touchedflag renamed toCompleting. "Touched" does not carry any obvious semantic meaning. "Completing" reflects the intent of when the flag should be set and helps avoid confusion.Rename
isCompletedandisCompletingmethods of SymDenotation. These names carry obvious meanings and induce expectations – which turn out to be not true. They are not true due to the hacks we are using to avoid cyclic references on completion. The mechanics of the hack in question is to set theinfoof a denotation being completed to a temporary non-lazy type so that to flip these predicates. We shouldn't use such hacks in the first place, however, FTTB, we can rename the predicates to reflect what they do.