-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix Tasty errors #1211
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 Tasty errors #1211
Conversation
For explicit arguments of this(...) constrictor calls we have a special context that hides members of the current class. But for implicit arguments we did not. This led to implicit shadowing errors for scala.collection.immutable.PagedSeq when secondary constructor type parameters were fixed (as done in subsequent commits).
Previously, those were inferred from arguments, but this is wrong because we implicitly assume that the type parameters of the constructor and the type parameters of the class are the same. I could not find a test that fails for this. But if you look at the -Xprint:front output of pos/i941.scala, you notice that the inferred argument to the this(...) call was `Nothing` where it should have been `A`.
... of class parameters using the gadt mechanism. Previously they were encoded as aliases by hardcoding alias bounds in the type parameter declaration, but that then leads to weird behavior and failures in unpickling. To make this work, we also need to propagate gadt bounds into the this-call context. Test case in pickling/i1202a.scala.
`isTyper` is used to enable some error checking and handling, which need not be done when in FromTasty.
The previous path name always had a "Simple(...)" wrapped around it.
If a file was loaded from TASTY, it can not still have a non-null source file, since the source file is unpickled into the annotation of a top-level class. Also, fix typo in previous commit.
ParamAccessor is not a pickled flag. This is not a problem for normal parameter accessors which are pickled as PARAM fields. But setters of parameter accessors also need to have the flag set (and Deferred reset).
SeqLiteral have an elemTpt, which was missing in format.
Two problems were fixed: - isJava needs to look at function symbol, not its type (references to Java methods get normal MethodTypes not JavMethodTypes) - we also need to handle the case where the repeated argument is wrspped in a type ascription.
Now the other errors reported in #1202 should also be fixed. |
@odersky Thank you! LGTM |
@@ -64,6 +64,9 @@ object FromTasty extends Driver { | |||
} | |||
|
|||
class ReadTastyTreesFromClasses extends FrontEnd { | |||
|
|||
override def isTyper = false |
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.
Unfortunately, this is not enough.
A lot of code checks for Phases.isAfterTyper
that is entirely independent from this method
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.
True. But it does not matter for anyway. The change did not affect the fixes. I'll add a comment to explain better what isTyper is.
This is the same as what Java does for its ClassFile attribute.
Instead of separate source file sections, pickle SourceFile as an annotation of all toplevel classes. We represent it like this anyway when reading back Tasty-defined classes.
This leads to an infinite cycle when trying to unpickling, because the modifiers and annotations of a symbol are read before the symbol itself is created. See scala#1212 for the general case.
1) Move passing test to pickling 2) Add test case for scala#1212 in pending 3) Disable annotations/internal in pickling tests. They lead to a stable symbol error which is explainable (modifiers are read before symbol is created).
I'm not sure if change proposed in 5aa59ac is good for tooling. It makes it very complicated for an external tool to match TASTY files with source files. As the tool would be required to be able to read the TASTY section. Though it also has bright sides, as it would simplify storing TASTY for classes originating from different source files. |
If necessary we could provide a method that returns the top level classes of a tasty file. Then we can read the source files from them. I think it is an advantage that A tasty file is not tied to one source. Sent from my iPhone
|
I added commit with updates for TASTY tests (for dotty/src) |
Fixes one of the errors reported in #1202. If I can fix more errors, I'll try to push those fixes on this PR later. Review by @VladimirNik .