Skip to content

Don't allow inherited names to shadow names defined in the outer scope #8617

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

Closed
odersky opened this issue Mar 26, 2020 · 3 comments
Closed

Comments

@odersky
Copy link
Contributor

odersky commented Mar 26, 2020

See scala/bug#11921 (comment)

I think it's worth experimenting with a rule which says that an inherited member is not allowed to shadow a definition of the same name in an outer scope. Imports in outer scopes would be unaffected.

This could be a warning in 3.0 and an error in 3.1

@odersky
Copy link
Contributor Author

odersky commented Mar 26, 2020

Example:

class C:
  val x = 2
object a:
  val x = 1
  class D extends C:
    println(x)

Here we currently print 2. Instead, I would like to see an error or warning like this:

Reference to `x` is ambiguous
It is both defined in object a
and inherited subsequently by class D

Importantly, I would flag it as an error only if x is referenced as a simple name. Writing this.x is OK. And using x not at all in D is also OK.

odersky added a commit to dotty-staging/dotty that referenced this issue Mar 27, 2020
This implements the additional rule for name resolution:

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.
@dwijnand
Copy link
Member

What if they're split files?

// C.scala
class C:
  val x = 2
// a.scala
object a:
  val x = 1
  class D extends C:
    println(x)

Should x shadow C's x cleanly there, no warnings nor errors?

Just for the record, I, personally, dislike differences in behaviour based on same-file vs split files (like for imports/scope, and the empty package iirc), but I wanted to mention it to avoid the differences being inconsistent.

@odersky odersky closed this as completed in 9d28345 Apr 2, 2020
odersky added a commit that referenced this issue Apr 2, 2020
Fix #8617: Check that there is no inheritance/definition shadowing
@dwijnand
Copy link
Member

dwijnand commented Apr 3, 2020

To answer my own question, no it doesn't matter (which I think is inconsistent?):

$ dotc $(f "class C { val x = 2 }; object a { val x = 1; class D extends C { println(x) } }")
Wrote /var/folders/yh/qsn4cxqj47gbhh9_prlxqy3r0000gn/T/scala-test.My0o23jD.scala
-- [E049] Reference Error: /var/folders/yh/qsn4cxqj47gbhh9_prlxqy3r0000gn/T/scala-test.My0o23jD.scala:1:73
1 |class C { val x = 2 }; object a { val x = 1; class D extends C { println(x) } }
  |                                                                         ^
  |                                     Reference to x is ambiguous,
  |                                     it is both defined in object a
  |                                     and inherited subsequently in class D

longer explanation available when compiling with `-explain`
1 error found
$ dotc $(f "package p; class C { val x = 2 }; object a { val x = 1; class D extends C { println(x) } }")
Wrote /var/folders/yh/qsn4cxqj47gbhh9_prlxqy3r0000gn/T/scala-test.ig9YONiu.scala
-- [E049] Reference Error: /var/folders/yh/qsn4cxqj47gbhh9_prlxqy3r0000gn/T/scala-test.ig9YONiu.scala:1:84
1 |package p; class C { val x = 2 }; object a { val x = 1; class D extends C { println(x) } }
  |                                                                                    ^
  |                                     Reference to x is ambiguous,
  |                                     it is both defined in object a
  |                                     and inherited subsequently in class D

longer explanation available when compiling with `-explain`
1 error found
$ dotc $(f "class C { val x = 2 }") $(f "object a { val x = 1; class D extends C { println(x) } }")
Wrote /var/folders/yh/qsn4cxqj47gbhh9_prlxqy3r0000gn/T/scala-test.69wrO4Fc.scala
Wrote /var/folders/yh/qsn4cxqj47gbhh9_prlxqy3r0000gn/T/scala-test.LFHSk5nu.scala
-- [E049] Reference Error: /var/folders/yh/qsn4cxqj47gbhh9_prlxqy3r0000gn/T/scala-test.LFHSk5nu.scala:1:50
1 |object a { val x = 1; class D extends C { println(x) } }
  |                                                  ^
  |                                     Reference to x is ambiguous,
  |                                     it is both defined in object a
  |                                     and inherited subsequently in class D

longer explanation available when compiling with `-explain`
1 error found
$ dotc $(f "package p; class C { val x = 2 }") $(f "package p; object a { val x = 1; class D extends C { println(x) } }")
Wrote /var/folders/yh/qsn4cxqj47gbhh9_prlxqy3r0000gn/T/scala-test.YA2xd2pD.scala
Wrote /var/folders/yh/qsn4cxqj47gbhh9_prlxqy3r0000gn/T/scala-test.2DE5jCMH.scala
-- [E049] Reference Error: /var/folders/yh/qsn4cxqj47gbhh9_prlxqy3r0000gn/T/scala-test.2DE5jCMH.scala:1:61
1 |package p; object a { val x = 1; class D extends C { println(x) } }
  |                                                             ^
  |                                     Reference to x is ambiguous,
  |                                     it is both defined in object a
  |                                     and inherited subsequently in class D

longer explanation available when compiling with `-explain`
1 error found
$ dotc -version
Dotty compiler version 0.24.0-bin-SNAPSHOT-git-3e888f2 -- Copyright 2002-2020, LAMP/EPFL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants