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

Linking errors on Scala 2.11 if an `object` is named `class` #3888

Closed
Atry opened this issue Nov 28, 2019 · 6 comments
Closed

Linking errors on Scala 2.11 if an `object` is named `class` #3888

Atry opened this issue Nov 28, 2019 · 6 comments
Assignees
Labels
bug
Milestone

Comments

@Atry
Copy link

@Atry Atry commented Nov 28, 2019

The following code works fine on Scala 2.12, but does not link on Scala 2.11:

object `class` {
  def foo() = {
    println("Hello, World!")
  }
}

`class`.foo()

https://scalafiddle.io/sf/LRfrjg0/0

Atry added a commit to Atry/html.scala that referenced this issue Nov 28, 2019
@Atry Atry changed the title Linking errors on Scala 2.11 Linking errors on Scala 2.11 if an `object` is named `class` Nov 28, 2019
Atry added a commit to Atry/html.scala that referenced this issue Nov 28, 2019
@sjrd

This comment has been minimized.

Copy link
Member

@sjrd sjrd commented Nov 28, 2019

Some questions:

  • Does it work if the object is in a package, not in an enclosing object? (ScalaFiddle always wraps its script in an object.)
  • Does it work in the JVM? With/without wrapping in an enclosing object.
  • Does it work in 1.x?
@Atry

This comment has been minimized.

Copy link
Author

@Atry Atry commented Nov 28, 2019

I guess a top level object in a package should link but I have not tested yet.

@sjrd

This comment has been minimized.

Copy link
Member

@sjrd sjrd commented Nov 28, 2019

OK I can also reproduce this in Scala.js 1.x + Scala 2.11. And it does work on the JVM, somehow. However, I'm very tempted to mark this as won't fix, because there are other problems with the same root cause on the JVM with nested classes/objects named class. For example:

scala> trait Foo { def foo: Int = 5 }; object Foo { class `class` { def bar: Int = 6 } }
<console>:11: warning: Class Foo$class differs only in case from Foo$class. Such classes will overwrite one another on case-insensitive filesystems.
       trait Foo { def foo: Int = 5 }; object Foo { class `class` { def bar: Int = 6 } }
             ^
defined trait Foo
defined object Foo

scala> new Foo {}.foo
java.lang.NoSuchMethodError: Foo$class.$init$(LFoo;)V
  ... 33 elided

The implementation class of trait Foo is called Foo$class, and the class class is also called Foo$class, so they clash at the JVM level and cause LinkageErrors like that NoSuchMethodError.

I think we should just consider that a nested class/trait/object called class is invalid as implementation restriction for Scala 2.11 as a whole (JVM or JS) and that we don't want to go great lengths to kind-of support the bits that Scala/JVM tolerates.

@sjrd sjrd added the bug label Nov 28, 2019
@Atry

This comment has been minimized.

Copy link
Author

@Atry Atry commented Nov 28, 2019

It's OK to not fix it as Scala 2.11 is a legacy version. Just treat this issue as a backlog.

sjrd added a commit to sjrd/scala-js that referenced this issue Nov 28, 2019
Before, we sometimes used name-based identification of impl
classes. This was introduced in
ac54f18, before Scala.js 0.1! At
the time, we were still using a custom `Global` subtrait instead of
a compiler plugin, and a custom `.jstype` file reader instead of
the normal class-file reader of scalac. The flag was not present in
some cases of separate compilation. Now that we use a normal
compiler plugin, this issue does not seem to manifest itself
anymore. It seems this code was left there for 6 years for nothing.
sjrd added a commit to sjrd/scala-js that referenced this issue Nov 28, 2019
Before, we sometimes used name-based identification of impl
classes. This was introduced in
ac54f18, before Scala.js 0.1! At
the time, we were still using a custom `Global` subtrait instead of
a compiler plugin, and a custom `.jstype` file reader instead of
the normal class-file reader of scalac. The flag was not present in
some cases of separate compilation. Now that we use a normal
compiler plugin, this issue does not seem to manifest itself
anymore. It seems this code was left there for 6 years for nothing.
gzm0 added a commit that referenced this issue Nov 29, 2019
Fix #3888: Always identify impl classes by their IMPLCLASS flag.
@gzm0 gzm0 added this to the v0.6.32 milestone Nov 29, 2019
@gzm0

This comment has been minimized.

Copy link
Contributor

@gzm0 gzm0 commented Nov 29, 2019

Fixed in 36a92e3

@gzm0 gzm0 closed this Nov 29, 2019
@sjrd sjrd self-assigned this Nov 29, 2019
@sjrd

This comment has been minimized.

Copy link
Member

@sjrd sjrd commented Nov 29, 2019

Just treat this issue as a backlog.

We don't really do backlog for compiler bugs. ;) Either there is a very good reason not to fix something, and we close it as won't fix, or we fix it ASAP. And for the former case, there are exceptionally few issues. Overall there are 20 won't-fix bugs that are not internal nor 0.6.x-only, and not all of those are compiler bugs. And with this issue fixed, we are back to 0 open compiler bugs.

sjrd added a commit to sjrd/scala-js that referenced this issue Dec 9, 2019
This is a forward port of the test in
36a92e3.

The issue was fixed in 281f83f.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.