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

Do not crash when reading a Scala 2 macro definition. #420

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

sjrd
Copy link
Contributor

@sjrd sjrd commented Dec 12, 2023

It contains an annotation with a subtree that is an IDENTtree without prefix nor symbol, with the name macro. That is then Applyed and TypeApplyed in ways that make no semantic sense.

We define a special symbol in Definitions, which we attach to such fake identifiers.

@sjrd sjrd requested a review from adpi2 December 12, 2023 09:56
Copy link
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

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

Don't you have some tests for Scala 2?

@sjrd
Copy link
Contributor Author

sjrd commented Dec 12, 2023

Only for things that we can find in the standard library. We don't have a setup for other weird stuff that we only find elsewhere, unfortunately.

@adpi2
Copy link
Member

adpi2 commented Dec 13, 2023

@sjrd sjrd force-pushed the no-crash-on-scala2-macro-info-annot branch from 253434a to 12152c4 Compare December 14, 2023 09:42
It contains an annotation with a subtree that is an `IDENTtree`
without prefix nor symbol, with the name `macro`. That is then
`Apply`ed and `TypeApply`ed in ways that make no semantic sense.

We define a special symbol in `Definitions`, which we attach to
such fake identifiers.
@sjrd sjrd force-pushed the no-crash-on-scala2-macro-info-annot branch from 12152c4 to 1a83796 Compare December 14, 2023 15:01
@sjrd sjrd requested a review from adpi2 December 14, 2023 16:24
@sjrd
Copy link
Contributor Author

sjrd commented Dec 14, 2023

All right, I added a scala2TestSources project to be able to test targeted Scala 2 stuff, and I added a specific test for the fix in this PR.

@sjrd sjrd merged commit d44af7b into scalacenter:main Dec 18, 2023
4 checks passed
@sjrd sjrd deleted the no-crash-on-scala2-macro-info-annot branch December 18, 2023 09:06
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants