Fix issues to make new collections compile again #4162
Conversation
odersky
commented
Mar 22, 2018
odersky
commented
- Revert Do not allow named self in objects #4118
- Better disambiguation to avoid MergeErrors
This reverts commit ecdadc9.
| @@ -523,7 +529,9 @@ object Denotations { | |||
| try infoMeet(info1, info2) | |||
| catch { | |||
| case ex: MergeError => | |||
| if (pre.widen.classSymbol.is(Scala2x) || ctx.scala2Mode) | |||
| if (preferSym(sym2, sym1)) info2 | |||
There was a problem hiding this comment.
Would it be difficult to construct a test case to exercise this? Also, are we confident this is sound?
There was a problem hiding this comment.
I tried for a while but did not manage. The code that breaks is in class Id in MapView of the collection strawman.
There was a problem hiding this comment.
Regarding soundness: What happens is that we get two denotations that should be overloaded and we pick one and forget about the other. I see that as a possible problem for completeness, not sure how it would affect soundness.
There was a problem hiding this comment.
Maybe add a comment to note that this may be worth revisiting at some point?
vscode-dotty/test/extension.test.ts
Outdated
| @@ -12,10 +12,10 @@ import * as vscode from 'vscode'; | |||
| import * as dotty from '../src/extension'; | |||
|
|
|||
| // Defines a Mocha test suite to group tests of similar kind together | |||
| suite("vscode-dotty tests", function () { | |||
| //suite("vscode-dotty tests", function () { | |||
There was a problem hiding this comment.
Shouldn't be necessary anymore.
There was a problem hiding this comment.
That was added by accident. I removed it now.
If there's a MergeError we now disambiguate following symbol preference. This is needed to make new collections compile. Also, better error messages for those cases where a MergeError is still emitted.
