Skip to content
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

Add new HOLETYPES to TASTy format #17225

Closed
wants to merge 12 commits into from

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Apr 11, 2023

This is a new encoding of HOLE that differentiates between type and
term arguments of the hole.

Term      = ...
            HOLE Length idx_Nat tpe_Type HoleTypes? arg_Tree*
                -- Splice hole with index `idx`, the type of the hole `tpe`,
                   type arguments of the hole `targ`s and term arguments
                   of the hole `arg`s
HoleTypes = HOLETYPES Length targ_Type*

We will only have hole targs 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.

Based on #17144

@nicolasstucki nicolasstucki marked this pull request as ready for review April 11, 2023 11:03
@nicolasstucki nicolasstucki requested a review from smarter April 11, 2023 11:04
@nicolasstucki nicolasstucki added the needs-minor-release This PR cannot be merged until the next minor release label Apr 11, 2023
@Kordyjan Kordyjan added this to the 3.4.0 milestone Apr 11, 2023
if targs.nonEmpty then
writeByte(HOLETYPES)
withLength {
targs.foreach(targ => pickleType(targ.tpe))
Copy link
Member

Choose a reason for hiding this comment

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

Is there no position information in the targ that we're loosing by storing only the type?

until(typesEnd)(readType()).map(TypeTree(_))
else Nil
val args = until(end)(readTerm())
TastyQuoteHole(true, idx, targs ::: args, TypeTree(tpe)).withType(tpe)
Copy link
Member

Choose a reason for hiding this comment

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

Is it useful to have the TypeTree parameter here if it's the same as the type of the tree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe not. It is useful to have it in the TASTy HOLE. But we might be able to remove it from here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we remove it, we don't have a way to retype the tree. I do not have another way to infer the type of hole. This is an issue when Ychecking the tree.

Copy link
Member

@smarter smarter Apr 12, 2023

Choose a reason for hiding this comment

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

The TreeChecker uses promote to retype trees with their existing type.


HOLE Length idx_Nat tpe_Type (HOLETYPES Length targ_Type*)? arg_Tree*
-- Splice hole with index `idx`, the type of the hole `tpe`, type arguments of the hole `targ`s and term arguments of the hole `arg`s
-- Note: From 3.0 to 3.3 `args` could contain type arguments as well. This is no longer the case.
Copy link
Member

Choose a reason for hiding this comment

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

To be clear, args when you say "type arguments" here you mean TypeTrees, as opposed to the use of "type arguments" in the previous sentence where they are Types?

tasty/src/dotty/tools/tasty/TastyFormat.scala Outdated Show resolved Hide resolved
@smarter smarter assigned nicolasstucki and unassigned smarter Apr 11, 2023
@nicolasstucki nicolasstucki force-pushed the add-tasty-hole-types branch 2 times, most recently from 84d5293 to e14f569 Compare April 12, 2023 08:56
This is a new encoding of HOLE that differentiates between type and
term arguments of the hole.

```
Term      = ...
            HOLE Length idx_Nat tpe_Type HoleTypes? arg_Tree*
                -- Splice hole with index `idx`, the type of the hole `tpe`,
                   type arguments of the hole `targ`s and term arguments
                   of the hole `arg`s
HoleTypes = HOLETYPES Length targ_Type*
```
nicolasstucki added a commit that referenced this pull request Aug 8, 2023
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 pickle type trees in `arg_Tree` using the `EXPLICITtpt` tag.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-minor-release This PR cannot be merged until the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants