[WIP] Separate the namer out from the typer. #13762
Conversation
More unused.
| @@ -62,7 +63,7 @@ class CompilationUnit protected (val source: SourceFile) { | |||
| /** Can this compilation unit be suspended */ | |||
| def isSuspendable: Boolean = true | |||
|
|
|||
| /** Suspends the compilation unit by thowing a SuspendException | |||
| /** Suspends the compilation unit by throwing a SuspendException | |||
| @@ -85,6 +86,9 @@ class CompilationUnit protected (val source: SourceFile) { | |||
| def assignmentSpans(using Context): Map[Int, List[Span]] = | |||
| if myAssignmentSpans == null then myAssignmentSpans = Nullables.assignmentSpans | |||
| myAssignmentSpans | |||
|
|
|||
| private[dotc] var inlineAccessors: InlineAccessors = null | |||
There was a problem hiding this comment.
This is the new unpleasant state.
| @@ -57,7 +57,11 @@ class ProtectedAccessors extends MiniPhase { | |||
| ctx.property(AccessorsKey).get | |||
|
|
|||
| override def prepareForUnit(tree: Tree)(using Context): Context = | |||
| ctx.fresh.setProperty(AccessorsKey, new Accessors) | |||
| var acc = ctx.compilationUnit.protectedAccessors.asInstanceOf[Accessors] | |||
There was a problem hiding this comment.
I could make this a little cleaner but I wanted to wait for feedback on the right place to put the state.
| @@ -29,7 +29,11 @@ object PrepareInlineable { | |||
| private val InlineAccessorsKey = new Property.Key[InlineAccessors] | |||
|
|
|||
| def initContext(ctx: Context): Context = | |||
| ctx.fresh.setProperty(InlineAccessorsKey, new InlineAccessors) | |||
| var acc = ctx.compilationUnit.inlineAccessors | |||
There was a problem hiding this comment.
Same comment as above.
adampauls
force-pushed
the
wip_separate_parser_phase
branch
from
b06867d to
1dae662
Compare
| * following `fullName`. This is necessary so that we avoid reading an annotation from | ||
| * the classpath that is also compiled from source. | ||
| */ | ||
| def makeAnnotated(fullName: String, tree: Tree)(using Context): Annotated = { |
adampauls
force-pushed
the
wip_separate_parser_phase
branch
from
1dae662 to
dbd9675
Compare
|
I would like to see some motivation first. Why separate Namer and Typer? How can this even work since Namer does its job repeatedly as Typer progresses into trees. If Namer is its own phase that means we have less context in name resolving. Without a very solid reason I see no interest in doing this. I also note that separating Parser into its own phase was ultimately the right thing, but not without its cost. I remember it causing a serious regression in meta programming that required us to backport fixes to a previous broken version. So let's be very careful before doing further sweeping refactoring steps like this. |
|
I tried to see if we could find good reasons to split this out in the git history, but it looks like it's been split out in scala/scala since 2005? scala/scala@b058c90#diff-a64ffd2347b88aa879a67c2871d73e5b229063d95904d308c73fe18099fa8ae5R240. I think that's Scala 1 btw, not even Scala 2 :) |
|
There were two motivations: the first was a simple observation that there already was a separate phase in TyperPhase -- the call to I do not understand enough about the compiler internals to know whether it would be a good idea to separate these two phases. The fact that they currently are separated within TyperPhase made me think that it is. Maybe it would make more sense to call the phase in this PR |
But that's not a phase. It is a call that is done per scope that is visited in Typer.
How would you look up the symbol of |
What I mean is that there was call that looped over all compilation units and performed an operation. That operation (
You are of course right that you can't fully resolve symbols without also doing type checking. In the expression Of course I'll close the PR if this does not seem like the right thing. |
Yes, but that's only for top-level definitions. They will get completers and then Typer will start. If a symbol is completed it can trigger further index operations for its members, and so on recursively. So I think making Namer a separate phase would be misleading since it would suggest that the whole parse tree is transformed (or indexed), which is not the case. I think we can safely close this. But thank you for the suggestion and thinking about it! |
Following up on #13173, separates out the namer from the typer. This was almost totally clean, but unfortunately there is some state in Accessors that needs to be passed between the namer and typer. I put the state right onto CompilationUnit but I'm open to other suggestions.
There are two commits. The first is what I think is an uncontroversial removal of dead code from the TyperPhase. The second does the heavy lifting. Tests all pass locally for me.
cc @dwijnand @smarter. I hope I'm not overstepping here!