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 quote ASTs to TASTy #20165

Merged
merged 3 commits into from
Apr 25, 2024
Merged

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Apr 11, 2024

Add AST nodes for Quote, Splice, QuotePattern, and QuoteSplice to TASTy.

Term          = 
                  ...
                  QUOTE          Length body_Term bodyTpe_Type                     -- Quoted expression `'{ body }` of a body typed as `bodyTpe`
                  SPLICE         Length expr_Term tpe_Type                         -- Spliced expression `${ expr }` typed as `tpe`
                  SPLICEPATTEN   Length pat_Term tpe_Type targs_Type* args_Term*   -- Pattern splice `${pat}` or `$pat[targs*](args*)` in a quoted pattern of type `tpe`.
-- patterns:
                  ...
                  QUOTEPATTERN   Length body_Term quotes_Term pat_Type bindings_Term* -- Quote pattern node `'{ bindings*; body }(using quotes)`

@nicolasstucki nicolasstucki added the needs-minor-release This PR cannot be merged until the next minor release label Apr 11, 2024
@nicolasstucki nicolasstucki self-assigned this Apr 12, 2024
@nicolasstucki nicolasstucki force-pushed the add-quote-ast-to-tasty branch 3 times, most recently from e12cb19 to f7063f0 Compare April 16, 2024 07:07
@nicolasstucki nicolasstucki marked this pull request as ready for review April 16, 2024 10:04
@nicolasstucki nicolasstucki assigned sjrd and jchyb and unassigned nicolasstucki Apr 16, 2024
@nicolasstucki
Copy link
Contributor Author

@WojciechMazur could you test this PR on the open community build?

@WojciechMazur
Copy link
Contributor

The OpenCB was started, I'll post the result tomorrow when it's done

@WojciechMazur
Copy link
Contributor

We have 4 projects that started to fail after this change. In most cases it seems like the typer issue, eg.

Error:     |           Found:    Int => String
Error:     |           Required: T => Any
Project Version Build URL Notes
dwickern/scala-nameof 4.0.0 Open CB logs Type Mismatch Error for macro inputs
yakivy/jam 0.4.5 Open CB logs Type Mismatch Error for macro inputs
zio/zio-direct 1.0.0-RC7 Open CB logs Type Mismatch Error for macro inputs
zio/zio-prelude 1.0.0-RC23 Open CB logs Type Mismatch Error for macro inputs

@nicolasstucki nicolasstucki self-assigned this Apr 19, 2024
Comment on lines 727 to 729
// TODO: Handle `targs` for #18271. We should be able to pickle the
// `targs ::: args` if we add an `EXPLICITtpt` to the targs.
// See comment in SPLICEPATTERN unpickling.
Copy link
Member

Choose a reason for hiding this comment

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

Does this TODO need to be addressed now? If not, how can we do it backward-compatibly in the future?

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 previously thought that we could do it in a backward-compatible way, but now I see that my strategy will not work. I will propose another solution that will be future-proof for #18271.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually my original idea will work. I replaced the comments with a partial implementation.

@nicolasstucki
Copy link
Contributor Author

We have 4 projects that started to fail after this change. In most cases it seems like the typer issue, eg.
...

I fixed these with the change in 3f2db24. This also removed the changes in the checkfiles (these where suspicious).

Add AST nodes for Quote, Splice, QuotePattern, and QuoteSplice to TASTy.
These types where encoded explicitly before in the type parameter of the
`runtime.Expr.{expr,splice,nestedSplice}` methods. We still need them.
@Gedochao
Copy link
Contributor

This has been decided to be included in the 3.5.0 release.

@nicolasstucki nicolasstucki merged commit 5261ee0 into scala:main Apr 25, 2024
19 checks passed
@nicolasstucki nicolasstucki deleted the add-quote-ast-to-tasty branch April 25, 2024 16:47
@Kordyjan Kordyjan added this to the 3.5.0 milestone May 10, 2024
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.

6 participants