-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix bootstrap #1270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix bootstrap #1270
Conversation
@@ -398,10 +398,10 @@ object Symbols { | |||
|
|||
/** Subclass tests and casts */ | |||
final def isTerm(implicit ctx: Context): Boolean = | |||
(if(isDefinedInCurrentRun) lastDenot else denot).isTerm | |||
(if (defRunId == ctx.runId) lastDenot else denot).isTerm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not change the definition of isDefinedInCurrentRun
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it's used elsewhere to mean: In a compilation unit that's currently being compiled.
We now can selectively compile individual dotc files and directories using Tasty to access the information of the other parts of dotc. #1274 demonstrates what I did to verify this. |
When reading Tasty we need to pre-set the info of a class to some ClassInfoType with (as yet) unknown parents and self type. But for module classes, we need to know the source module at all time, and this gets determined by the self type. So we now produce a TermRef for the assumed self type of a module class.
It caused an assertion error when separately compiling parts of dotty against TASTY information. Not sure the test achieves anything or whether it produces a false negative.
Instrument Denotations#current to find CyclicReference errors arising during transforms.
Forcing it led to CyclicReferences involving RefChecks.OptLevelInfo when compiling dotc/*.scala against Tasty files. The problem was that when transforming OptLevelInfo the backend forced a transformInfo of RefChecks in TypeErasure which filtered RefCheck's scope to eliminate non-class type definitions. Without the tweak in this commit this tried to make all symbols current, and so came back to OptLevelInfo.
We had a problem where unpickling an annotation containing a class constant had the wrong type. Unpickling was done after erasure. The type given to the constant was an alias but aliases got eliminated during erasure, so the constant was malformed. Unpickling annotation contents at the same phase as unpickling the annotation carrier solves the problem. It seems similar problems can arise when data is unpickled using a LocalUnpickler. So we now make sure local unpickling runs at the latest at phase Pickler.
Do this in the inferred (result-)type of ValDefs and DefDefs. Without this fix, TreeTraverser#traverseChildren in Trees.scala gets a result type of BoxedUnit (but only when co-compiled from source, not when unpickled).
It's possible that the given phase argument does not exist, in which case we do not want to set the current phase to NoPhase.
Compute initialization flags of possibly enclosing traits elsewhere (in indexStats). Cleans up the logic and makes the module more understandable.
First step for a more robust scheme to access symbols in Tasty. This entailed a swap of two fields in RefiendType, to make tree format more uniform in what concerns where references are found.
Instead of stubbing with potentially wrong owners and hping for the best, we now compute owners on demand, using the lazy data structure of an OwnerTree.
Include member defs inside templates in the enclosing class, otherwise they would get localDummy as onwer.
Rebased to master |
Otherwise we might get a false owner if completing from somewhere else. We do not have a failing test to demonstrate the problem, but it looks like the right thing to do.
As explained in the comment, a scalacLinkedClass must also be a true companion unless the original symbol is a root. This avoids us to drop the (potentially large) unpickled map and save some memory.
@@ -174,32 +174,22 @@ object TreeTransforms { | |||
} | |||
|
|||
/** A helper trait to transform annotations on MemberDefs */ | |||
trait AnnotationTransformer extends MiniPhaseTransform with InfoTransformer { | |||
trait AnnotationTransformer extends MiniPhaseTransform with DenotTransformer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change needed?
As annotation transformers also change types, isn't every annotation transformer an info-transformer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently that's not true, but I see your argument. I.e. if we left out the self type transform from FirstTransform it would not be an InfoTransformer. And that transform is in FirstTransform only by accident, it could well be elsewhere.
However one could argue that every annotation transform should affect annotated types, and annotated types can be infos of symbols so in that sense every annotation transform is an info transform.
But that would require a complete typemap of every type in every info transform, and we know this leads to cyclic references. So we can't do this at present.
Bottom line: When in doubt decouple the functionalities.
ready to merge? |
LGTM |
Fixes needed so that dotc can selectively compile some parts of itself, using TASTY as the pickled symbol format. Based on #1243.