-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Replace TypeArgRefs by normal TypeRefs #3602
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
Conversation
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.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Commit Messages
We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).
Please stick to these guidelines for commit messages:
- Separate subject from body with a blank line
- When fixing an issue, start your commit message with
Fix #<ISSUE-NBR>:
- Limit the subject line to 72 characters
- Capitalize the subject line
- Do not end the subject line with a period
- Use the imperative mood in the subject line ("Added" instead of "Add")
- Wrap the body at 80 characters
- Use the body to explain what and why vs. how
adapted from https://chris.beams.io/posts/git-commit
Have an awesome day! ☀️
* `<this> . <symd>` is an actual argument reference, i.e. `this` is different | ||
* from the ThisType of `symd`'s owner. | ||
*/ | ||
def isArgPrefix(symd: SymDenotation)(implicit ctx: Context) = |
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.
Suggestion: call the method isArgPrefixOf
.
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.
Good point. In fact, I still want to try something else: Make internal class parameter references take a NoPrefix
. Then any class parameter reference with some other prefix would be considered an external (type-arg) reference.
e150b0b
to
a4a2d04
Compare
13e2170
to
bacf9a6
Compare
compiler/test-resources/repl/i2554
Outdated
@@ -1,7 +0,0 @@ | |||
scala> object foo { trait ShapeLevel; trait FlatShapeLevel extends ShapeLevel; trait ColumnsShapeLevel extends FlatShapeLevel; abstract class Shape[Level <: ShapeLevel, -Mixed, Unpacked, Packed]; object Shape extends TupleShapeImplicits { }; trait TupleShapeImplicits { implicit final def tuple2Shape[Level <: ShapeLevel, M1,M2, U1,U2, P1,P2](implicit u1: Shape[_ <: Level, M1, U1, P1], u2: Shape[_ <: Level, M2, U2, P2]): Shape[Level, (M1,M2), (U1,U2), (P1,P2)] = ???; }; } |
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.
This commit can be removed now that #3603 is fixed.
final val EXPANDPREFIX = 4 // An expansion prefix `<prefix>$<suffix>`, | ||
// used by Scala-2 for private names. | ||
|
||
final val UNIQUE = 10 // A unique name `<name>$<num>` where `<num>` |
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.
Since we're breaking compat anyway, we could renumber the tags so that all the tasty tags are continuous (and non-tasty nametags could have numbers starting with 1000 to distinguish them)
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.
I tried this but got weird bootstrap errors and don't have the time to chase them down. Maybe somebody else wants to give it a try? Since so much else is sitting on this commit, I am getting it merged now.
This commit avoids replaces a TypeArgRef by a normal reference to a type parameter.
The i2554 repl script test had issues, as will be outlined in a separate issue. pos/i2554.scala contains a version of the fixed test.
pos/escapingRefs.scala fails to compile under -optimise. This seems to be an existing problem that was not detected before, but is detected now with the change from TypeArgRefs to TypeRefs. I will file a separate issue detailing the problem.
- unexpandedName in names that cannot be expanded - meregTypeParamsAndAliases in TreeUnpickler (there are no such aliases anymore). - ModuleVarName
- drop tags that cannot be part of Tasty trees from TastyFormat - move them to a new object, NameTags. - add comments explaining what each name tag represents.
bacf9a6
to
d652e0d
Compare
Replace TypeArgRefs by normal TypeRefs. This streamlines the treatment of wildcard arguments and brings it in line with path dependent types. We can also eliminate that way both a fundamental type constructor and a Tasty node class.
Based on #3494.
The change unveiled two errors, which are tracked in #3603 and #3604.