Skip to content

Only track inlined positions #9966

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

Closed

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Oct 7, 2020

This PR changes inlineContext and makes it track all source changes. Then, the rest are simplifications that can be done due to this change.

@nicolasstucki nicolasstucki self-assigned this Oct 7, 2020
@nicolasstucki nicolasstucki force-pushed the only-keep-inlined-positions branch from 78b1cfc to 767e24e Compare October 8, 2020 08:28
@nicolasstucki nicolasstucki marked this pull request as ready for review October 8, 2020 10:37
@nicolasstucki nicolasstucki requested a review from odersky October 8, 2020 10:37
This makes the workaround for the missing JSR-45 support slightly less precise.
@nicolasstucki
Copy link
Contributor Author

The changes in i4947b.check will be fixed by implementing JSR-45 support or can be patched using the solution in #9954

@@ -37,7 +37,6 @@ class Compiler {
/** Phases dealing with the frontend up to trees ready for TASTY pickling */
protected def frontendPhases: List[List[Phase]] =
List(new FrontEnd) :: // Compiler frontend: scanner, parser, namer, typer
List(new YCheckPositions) :: // YCheck positions
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we have to remove the phase? Does that mean positions are not checked anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

YCheckPositions used to check that the sources we have on the trees align with the source that is in the call of the Inlined trees. Initially, this was used when we added sources to the trees to make sure those where correct and catch Inlined trees that had an empty/non-nonempty call when they should not. This was important to make sure the positions of the trees where properly unpickled and that the transformation phases did not lose information about the sources on the Inlined.calls before erasure.
This invariant does not hold after PostTyper anymore as all calls are empty and all the inlined source logic relies on the sources of the trees instead of the calls. Therefore we can't check what we were originally checking and we don't need to as we use exclusively the sources of the trees.

@odersky odersky assigned nicolasstucki and unassigned odersky Oct 11, 2020
Co-authored-by: odersky <odersky@gmail.com>
@nicolasstucki nicolasstucki marked this pull request as draft October 22, 2020 08:23
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