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

Name binding is not as ambiguous as specified #8059

Open
som-snytt opened this issue Jan 22, 2020 · 5 comments
Open

Name binding is not as ambiguous as specified #8059

som-snytt opened this issue Jan 22, 2020 · 5 comments

Comments

@som-snytt
Copy link
Contributor

minimized code

Given a compilation unit:

package p.q

class X {
  override def toString() = "p.q.X"
}

Either of these references to X should be ambiguous:

import reflect.ClassTag

package p {
  
  class X {
    override def toString() = "p.X"
  }

  package q {
    object Test {
      def f = implicitly[ClassTag[X]]   // ambiguous
    }
  }

  object Test {
    def main(args: Array[String]) = println {
      p.q.Test.f
    }
  }
}

or

import reflect.ClassTag

package p {
  
  class X {
    override def toString() = "p.X"
  }

  package q {
    import r.X
    object Test {
      def f = implicitly[ClassTag[X]]   // ambiguous
    }
  }

  package r {
    class X {
      override def toString() = "p.r.X"
    }
  }

  object Test {
    def main(args: Array[String]) = println {
      p.q.Test.f
    }
  }
}
Compilation output
  scala git:(2.13.x) skalac test/files/neg/t10662/*.scala            
test/files/neg/t10662/px_2.scala:19: error: reference to X is ambiguous;
it is both defined in package p and available as class X in package q
      implicitly[T[X]]   // ambiguous
                   ^
1 error
➜  scala git:(2.13.x) skalac test/files/neg/t10662b/*.scala
test/files/neg/t10662b/px_2.scala:16: error: reference to X is ambiguous;
it is both defined in package p and imported subsequently by
import r.X
      implicitly[T[X]]   // ambiguous
                   ^
1 error

It compiles on both Scala 2.13.0 and 3, but Scala 2 picks p.q.X and p.r.X respectively, and Dotty picks p.X in both cases.

expectation

p.q.X has lowest precedence: it is made available by the packaging package q but cannot shadow the high priority definition p.X, and is therefore ambiguous.

Similarly for the import r.X, as imports never shadow definitions in the current file.

@smarter
Copy link
Member

smarter commented Jan 22, 2020

p.q.X has lowest precedence: it is made available by the packaging package q but cannot shadow the high priority definition p.X, and is therefore ambiguous.

Is this the behavior of scalac or something written down in the spec somewhere ?

@som-snytt
Copy link
Contributor Author

som-snytt commented Jan 22, 2020

Spec is at https://scala-lang.org/files/archive/spec/2.13/02-identifiers-names-and-scopes.html

It's fixed in 2.13.1, but as someone noted, no one writes code like that.

@som-snytt
Copy link
Contributor Author

The linked Scala 2 PR has the two tests that should have ambiguous references.

From the OP, it looks like Dotty behavior changed, because the second test picks p.r.X, not p.X as claimed by OP:

-- Error: test/files/neg/t10662b/px_2.scala:16:22 ----------------------------------------------------------------------
16 |      implicitly[T[X]]   // ambiguous
   |                      ^
   |            no given instance of type p.T[p.r.X] was found for parameter e of method implicitly in object Predef
1 error found

The first test still compiles in Dotty and still picks p.X:

implicitly[p.T[p.X]](p.X.tx)

@som-snytt
Copy link
Contributor Author

What?

In the absence of p.X, the first example prints p.q.X. The presence of p.X should make X ambiguous because p.q.X cannot shadow p.X.

Contrary to whatever that guy tried last summer, test 2 is the same result. I hope he doesn't think I'm trying to "show him up" or make him look stupid.

Oh, did you try 2.13.10?

Let me try that. Yes, 2.13 correctly reports the ambiguity.

But Scala 3 has a very simple and pleasant mechanism for ranking the bindings. Maybe it's simple enough for you to understand and maybe diagnose the issue.

OK, I'll give a shot in the new year.

@som-snytt
Copy link
Contributor Author

Periodic ticket validation, as dotty typer still has code from 2016 claiming that scala 2 doesn't follow the spec.

Also #17439 is unresolved.

➜  i8059 scalac -d /tmp/sandbox x.scala t1.scala
t1.scala:13: error: reference to X is ambiguous;
it is both defined in package p and available as class X in package q
      def f = implicitly[ClassTag[X]]   // ambiguous
                                  ^
1 error
➜  i8059 scalac -d /tmp/sandbox x.scala t2.scala
t2.scala:12: error: reference to X is ambiguous;
it is both defined in package p and imported subsequently by
import r.X
      def f = implicitly[ClassTag[X]]   // ambiguous
                                  ^
1 error
➜  i8059 sdk use scala 3.4.2

Using scala version 3.4.2 in this shell.
➜  i8059 scalac -d /tmp/sandbox x.scala t1.scala
➜  i8059 scalac -d /tmp/sandbox x.scala t2.scala
➜  i8059

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