Skip to content
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

Fix #8617: Check that there is no inheritance/definition shadowing #8622

Merged
merged 4 commits into from
Apr 2, 2020

Conversation

odersky
Copy link
Contributor

@odersky odersky commented 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.

@odersky
Copy link
Contributor Author

odersky commented Mar 27, 2020

An example of the first exception is this:

class C(x: T):
  class D extends C(2):
    println(x)

Here x is defined and inherited, but that's not an error since it's the same x.

@odersky odersky force-pushed the fix-#8617 branch 4 times, most recently from feabc40 to b58ed5c Compare March 27, 2020 15:19
@odersky
Copy link
Contributor Author

odersky commented Mar 27, 2020

This was actually less cumbersome than I anticipated. Only a handful of places had to be updated, and the updates were straightforward (just insert a this.). That's now also automated with a -rewrite rule.

So in the end we produce errors for 3.0 and migration warnings under -laguage:Scala2Compat.

@odersky
Copy link
Contributor Author

odersky commented Mar 31, 2020

@biboudis Unfortunately we got out of sync with this on scalatest now. There were two branches that are now in conflict. Can you please rebase and make sure it passes?

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.
And offer an automatic rewrite rule.
@odersky odersky merged commit ca0ef84 into scala:master Apr 2, 2020
@odersky odersky deleted the fix-#8617 branch April 2, 2020 17:49
smarter pushed a commit to dotty-staging/scala that referenced this pull request May 25, 2020
In that, avoid infix & varargs expansion in Console.printf:

    [error] 280 |  def printf(text: String, args: Any*): Unit = { out.print(text format (args : _*)) }
    [error]     |                                                                               ^
    [error]     |        `_*` can be used only for last argument of method application.
    [error]     |        It is no longer allowed in operands of infix operations.

Also, avoid ambiguity errors after scala/scala3#8622
griggt added a commit to griggt/dotty that referenced this pull request Sep 22, 2020
@smarter smarter mentioned this pull request Nov 23, 2020
@lrytz
Copy link
Member

lrytz commented Nov 15, 2022

there is a backport of this to 2.13 in progress by @som-snytt. I'm testing it on a large codebase, and it would be helpful to skip the warning when the two symbols are aliases. Here are two minimized real-world examples that don't compile with Scala 3.2.1:

trait Context
class A[C <: Context](val c: C)
class B(val c: Context) { b =>
  val a = new A[c.type](c) {
    def m = c  // ambiguous, `this.c` vs `B.this.c`
  }
}

class C[T] { type TT = T }
object O {
  type TT = String
  class D extends C[TT] {
    def n(x: TT) = x  // ambiguous, `this.TT` vs `O.TT`
  }
}

Proposal to fix that: #16401

odersky added a commit that referenced this pull request Nov 30, 2022
Follow-up for #8622, which
introduced a new lookup ambiguity if a name is inherited from a parent
and also found in an outer scope.

The error is avoided if the two symbols are aliases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants