Skip to content

Add new EXPLICITtpt to TASTy format #17298

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 8, 2023
Merged

Add new EXPLICITtpt to TASTy format #17298

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9897592
Add new EXPLICITtpt to TASTy format
nicolasstucki Apr 18, 2023
46a8d13
Only use EXPLICITtpt for TypeTree containing TermRefs
nicolasstucki Aug 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,9 @@ class TreePickler(pickler: TastyPickler) {
writeNat(idx)
pickleType(tree.tpe, richTypes = true)
args.foreach { arg =>
if arg.isType then writeByte(EXPLICITtpt)
arg.tpe match
case _: TermRef if arg.isType => writeByte(EXPLICITtpt)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For 3.3 we can "backport" this change as

arg.tpe match
  case _: TermRef if arg.isType => report.warning(em"Term reference used in type of nested quote. This quote may fail to unpickle due to a bug that cannot be fixed in 3.3.x. This issue is fixed in 3.4.0.", arg)
  case _ =>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smarter I added this new commit containing the check to identify the problematic cases and optimize the pickled quote size by eliding the EXPLICITtpt when it is unnecessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also believe this should be an error not a warning, and if possible give a workaround that can be used in 3.3?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have yet to find an example that fails in 3.3. We used the wrong pickling/unpickling for those term refs.

The issues only arose when trying to fix code that would not compile with some specific term-refs and this-types. When fixing those issues we start encountering the ambiguity in pickling/unpickling of those term-refs.

case _ =>
pickleTree(arg)
}
}
Expand Down