-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix implicit scope implementation #6832
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
3d5bf8d
to
aa4aee6
Compare
8c77860
to
93b4832
Compare
test performance please |
performance test scheduled: 1 job(s) in queue, 1 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/6832/ to see the changes. Benchmarks is based on merging with master (15b9cd8) |
93b4832
to
60a9c96
Compare
val companion = sym.companionModule | ||
if (companion.exists) addRef(companion.termRef) | ||
def addCompanion(pre: Type, companion: Symbol) = | ||
if (companion.exists && !companion.isAbsent) comps += TermRef(pre, companion) |
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.
Use of isAbsent
here breaks after #6842.
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.
What do you mean by break ? The signature of isAbsent
changed, so at the very least you need to add parens to it: isAbsent()
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.
Well, yes, precisely :-)
Maybe my terminology is off here, but no git conflict, CI passes, yet will fail if merged.
The previous implicit scope was hard to understand, and did not conform to spec. The new implementation is simpler to understand. It also does not conform to spec as written but there's a chance that we might change the spec to conform to it.
60a9c96
to
fd3edbd
Compare
LGTM. Also verified that the technique proposed to fix shapeless after this change will work. |
Change implicit scope implementation so that it can be understood and adapted to different strategies.
Based on #6824. First new commit is aa4aee6.