Add new EXPLICITtpt to TASTy format #17298
Conversation
nicolasstucki
force-pushed
the
explicit-type-tree-tasty-hole-encoding
branch
from
5e95030 to
fe1268b
Compare
smarter
added
the
needs-minor-release
nicolasstucki
force-pushed
the
explicit-type-tree-tasty-hole-encoding
branch
2 times, most recently
from
0076a28 to
a181327
Compare
nicolasstucki
force-pushed
the
explicit-type-tree-tasty-hole-encoding
branch
from
a181327 to
bb17e27
Compare
This is a new encoding of HOLE that differentiates between type and
term arguments of the hole.
```
-- pickled quote trees: These trees can only appear in pickled quotes. They will never be in a TASTy file.
EXPLICITtpt tpt_Term
-- Tag for a type tree that in a context where it is not explicitly known that this tree is a type.
HOLE Length idx_Nat tpe_Type arg_Tree*
-- Splice hole with index `idx`, the type of the hole `tpe`, type and term arguments of the hole `arg`s
```
We will only have hole captured types if there is a type defined in a quote and used in a nested quote.
Most of the time we do not have those types and therefore no overhead in the encoding compared to before this change.
nicolasstucki
force-pushed
the
explicit-type-tree-tasty-hole-encoding
branch
from
bb17e27 to
9897592
Compare
|
There is nothing we can or must do in the 3.3 branch. #17137 will not be fixed in 3.3. The fix will be available in 3.4 and is backward compatible. |
|
Does #17137 cause crashes in 3.3? Could we detect in the pickler the pattern and abort compilation? |
|
It will cause crashes. It might be possible to detect them in the quote pickler. I will try to do this on 3.3. We would not need that chack in 3.4+ (i.e. for this PR). |
| @@ -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) | |||
There was a problem hiding this comment.
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 _ =>
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
I also believe this should be an error not a warning, and if possible give a workaround that can be used in 3.3?
There was a problem hiding this comment.
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.
This is a new encoding of HOLE that differentiates between type and term arguments of the hole.
We will pickle type trees in
arg_Treeusing theEXPLICITtpttag.We will only have hole captured types if there is a type defined in a quote and used in a nested quote. Most of the time we do not have those types and therefore no overhead in the encoding compared to before this change.
Alternative to #17225
Fixes #17137