Skip to content

Fix Scope.exists and some missing Inlined trees #6233

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

Merged
merged 4 commits into from
Apr 8, 2019

Conversation

nicolasstucki
Copy link
Contributor

No description provided.

@nicolasstucki nicolasstucki self-assigned this Apr 4, 2019
@nicolasstucki nicolasstucki force-pushed the fix-scope-exists branch 3 times, most recently from 948f071 to 841f815 Compare April 4, 2019 15:48
@nicolasstucki nicolasstucki changed the title Fix Scope.exists Fix Scope.exists and some missing Inlined trees Apr 4, 2019
@nicolasstucki nicolasstucki marked this pull request as ready for review April 4, 2019 18:48
@nicolasstucki nicolasstucki removed their assignment Apr 4, 2019
@nicolasstucki nicolasstucki requested a review from liufengyun April 4, 2019 18:48
@nicolasstucki
Copy link
Contributor Author

Rebased

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

Otherwise, LGTM

else (call.symbol.unforcedDecls.exists(_.is(Macro)) || call.symbol.unforcedDecls.toList.exists(_.is(Macro)))
if (ctx.phase <= ctx.postTyperPhase) call.symbol.is(Macro)
else call.isInstanceOf[Select] // The call of a macro after typer is encoded as a Select while other inlines are Ident
// TODO remove this distinction once Inline nodes of expanded macros can be trusted (also in Inliner.inlineCallTrace)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the method alone, call.isInstanceOf[Select] seems to be a too weak test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a heuristic to know if we should YCheck the positions inside this Inlined tree or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At some point we will test the contents of these Inlined trees. Then this test would be removed.

@nicolasstucki nicolasstucki merged commit eae5a38 into scala:master Apr 8, 2019
@ghost ghost removed the stat:needs review label Apr 8, 2019
@nicolasstucki nicolasstucki deleted the fix-scope-exists branch April 8, 2019 11:47
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