-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Include top-level symbols from same file in outer ambiguity error #17033
Conversation
f60ed45
to
3fcb75c
Compare
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 am a it worried about the added complexity (both runtime and conceptual) this introduces. In the original PR there was this spec:
It is an error if an identifier x is available as an inherited member
in an inner scope and the same name x is defined in an outer scope, unless
- the inherited member (has an overloaded alternative that) coincides with
(an overloaded alternative of) the definition x- x is global, i.e. a member of a package.
I assume the last clause would have to be changed to this:
- x is global, i.e. a member of a package, and it is defined not in the same source as the inner scope.
I am not sure we need this additional complication however. Have we observed cases in the wild where this is a problem that should be diagnosed?
I did come across such a case when deploying the 2.12 backport of this change at our customer, that's how I noticed. The following is not ambiguous: File 1
File 2
Changing |
OK, I think we can change the spec as follows (and it would be good to include it as a doc comment):
Then we can argue that it's a simplification, spec-wise. |
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.
Otherwise LGTM
When checking if an inherited definition is ambiguous with an outer definition, include top-level outer definitions defined in the same compilation unit.
It seems this change caused a lot of regressions in the open community build. See #17433. We should probably revert it. |
If this is the error that regressed in 3.3.1 and succeeds in nightly it should be backported. |
This PR was reverted in main (#17438), but it seems that revert is not in the 3.3.1 branch (https://github.com/lampepfl/dotty/blob/release-3.3.1/compiler/src/dotty/tools/dotc/typer/Typer.scala#L437). Follow-up PR is here: #17439 (comment) |
When checking if an inherited definition is ambiguous with an outer definition, include top-level outer definitions defined in the same compilation unit.
Follow-up for #8622.