-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Replace Reflect Context with Owner #9712
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
Replace Reflect Context with Owner #9712
Conversation
d6388cd
to
05d2929
Compare
test performance with #quotes please |
performance test scheduled: 15 job(s) in queue, 1 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9712/ to see the changes. Benchmarks is based on merging with master (5ea8694) |
ae76553
to
5d0950f
Compare
test performance with #quotes please |
performance test scheduled: 1 job(s) in queue, 1 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9712/ to see the changes. Benchmarks is based on merging with master (d11e137) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about Owner
as a type on its own. Conceptually, ownership is a relation, not an entity. There is a worry that we create entities unnecessarily (Occam's razor).
What name would you suggest? |
Can it just be |
The issue is that this is not the context, the |
5d0950f
to
b99adbc
Compare
Replace
Reflect.Context
withReflec.Owner
and remove it from where it is not needed. The name is changed to be explicit on what this parameter is tracking and to disambiguate withQuoteContext
.Migration tips
Update signatures in
TreeMap
,TreeAcumulator
andTreeTraverser
Get the current owner
Update the current owner explicitly in a
TreeMap
,TreeAcumulator
andTreeTraverser