Skip to content

Improve owner handling in Reflection #10352

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 19, 2020
Merged

Improve owner handling in Reflection #10352

merged 1 commit into from
Nov 19, 2020

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Nov 17, 2020 •
edited
Loading

@nicolasstucki nicolasstucki force-pushed the improve-owner-handling branch 9 times, most recently from 7c6e94b to cd34a2a Compare November 17, 2020 14:26
@nicolasstucki
Copy link
Contributor Author

test performance with #quotes please

@dottybot
Copy link
Member

performance test scheduled: 7 job(s) in queue, 1 running.

@nicolasstucki nicolasstucki self-assigned this Nov 17, 2020
@dottybot
Copy link
Member

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/10352/ to see the changes.

Benchmarks is based on merging with master (ac806b8)

@nicolasstucki nicolasstucki added this to the 3.0.0-RC1 milestone Nov 19, 2020
@nicolasstucki nicolasstucki marked this pull request as ready for review November 19, 2020 12:13
liufengyun
liufengyun approved these changes Nov 19, 2020
View reviewed changes
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.

LGTM

Make owner-handling explicit in the programming model with debugging utilities will avoid frustrations of meta-programers.

The following tests are empty:

  • tests/pos-macros/i10151
  • tests/pos-macros/i10211

compiler/src/scala/quoted/internal/impl/QuoteContextImpl.scala
|
|""".stripMargin)
case _ => traverseChildren(t)
}.traverse(tree)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is very helpful, greatly improves debuggability 👍

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 would not have been able to fix the tests without this. It proved to be quite useful.

@nicolasstucki
Improve handling of owners in reflection API
49dd953 
* Add `changeOwner`
* Add owner and symbol to `Lambda.apply`
* Add owner to etaExpand for the creation of the closure
* Ycheck the owners durring macro expansion
* Fix scala#10151
* Fix scala#10211
@nicolasstucki
Copy link
Contributor Author

I added the missing tests

@nicolasstucki nicolasstucki merged commit 01c9a8d into scala:master Nov 19, 2020
@nicolasstucki nicolasstucki deleted the improve-owner-handling branch November 19, 2020 15:41
@nicolasstucki nicolasstucki mentioned this pull request Nov 19, 2020
Closed
@Kordyjan Kordyjan modified the milestones: 3.0.0-M3, 3.0.0 Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liufengyun liufengyun liufengyun approved these changes

Assignees

@liufengyun liufengyun

Labels
None yet
Projects
None yet
Milestone
3.0.0
4 participants
@nicolasstucki @dottybot @liufengyun @Kordyjan