Skip to content

Remove bindings from Inlined tree #6087

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

Conversation

nicolasstucki
Copy link
Contributor

No description provided.

@nicolasstucki nicolasstucki self-assigned this Mar 13, 2019
@nicolasstucki nicolasstucki changed the title Add bindings inside the expansion Remove bindings from inlined tree Mar 13, 2019
@nicolasstucki nicolasstucki force-pushed the remove-bindings-from-inlined-tree branch 4 times, most recently from 2e4383f to ae555be Compare March 14, 2019 11:42
@nicolasstucki nicolasstucki changed the title Remove bindings from inlined tree Remove bindings from Inlined tree Mar 14, 2019
@nicolasstucki nicolasstucki force-pushed the remove-bindings-from-inlined-tree branch 3 times, most recently from da82756 to e6f4452 Compare March 14, 2019 15:40
@nicolasstucki nicolasstucki requested a review from odersky March 14, 2019 16:17
// If the return type of an inline function is a singleton type of a parameter (like in `DottyPredef.the`)
// the expansion will be `{ val x = y; (x: x.type) }`. We ascribe the type to itself widening the
// local TermRef of the binding `{ val x = y; (x: y.type) }`.
tpd.Typed(finalExpansion, TypeTree(ctx.typer.avoidingType(finalExpansion, finalBindings))).withSpan(finalExpansion.span)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I had to add some extra logic to avoid type refs of the binding.

@nicolasstucki nicolasstucki marked this pull request as ready for review March 14, 2019 16:20
@nicolasstucki
Copy link
Contributor Author

Will need rebase once #6066 is merged.

@nicolasstucki nicolasstucki force-pushed the remove-bindings-from-inlined-tree branch from e6f4452 to 48c6a0e Compare March 15, 2019 16:54
@nicolasstucki
Copy link
Contributor Author

Rebased

@smarter
Copy link
Member

smarter commented Mar 17, 2019

What's the reason for this change ?

@nicolasstucki
Copy link
Contributor Author

Mostly remove unessesary complexity and some code duplication.

Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

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

Previously, bindings got the position of the calling context, instead of a position in the inlined code that was called. How is this handled now? I did not see any code to account for the change.

@odersky odersky assigned nicolasstucki and unassigned odersky Mar 17, 2019
@nicolasstucki
Copy link
Contributor Author

Indeed, we are missing some Inlined(EmptyTree, ...) nodes around the RHS of the bindings. I will add these Inlined and tests to check those positions.

@nicolasstucki
Copy link
Contributor Author

The positions do have the correct source/span, we already set them correctly. I will need to add some extra assertions on the Inlined nodes to check the invariant.

@nicolasstucki nicolasstucki force-pushed the remove-bindings-from-inlined-tree branch from da6a5aa to 82e917d Compare March 27, 2019 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants