scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

Nested case class construction fails with "no legal prefix" #4859

Closed scabug closed 11 years ago

scabug commented 13 years ago

This is a bug, affected versions (at least) 2.9.0.1, 2.9.1.RC1 and 2.10 (a recent nightly)

Try to compile the following:

object O {
  val o = C().CC(0)
}
case class C(s: String = "") {
  case class CC(ii: Int) {
    val ss = s
  }
}

Will result in

error: java.lang.NullPointerException
    at scala.tools.nsc.typechecker.Typers$Typer.typedTypeConstructor(Typers.scala:4329)
    at scala.tools.nsc.typechecker.Typers$Typer.typedTypeConstructor(Typers.scala:4353)
    at scala.tools.nsc.typechecker.Typers$Typer.typedNew$1(Typers.scala:3185) ...
scabug commented 13 years ago

Imported From: https://issues.scala-lang.org/browse/SI-4859?orig=1 Reporter: @hseeberger Affected Versions: 2.9.0, 2.9.1

scabug commented 13 years ago

@paulp said: Minimized.

object O {
  new C().CC()
}

class C() {
  case class CC()
}
scabug commented 13 years ago

Commit Message Bot (anonymous) said: (extempore in r25453) Don't want to chase NPEs around for the rest of my life. Created "NoCompilationUnit" and "NoSourceFile" objects to represent not-present versions of these items. Seems a lot better than null. References

4859, got past NPE only to uncover the actual problem. No review.

scabug commented 13 years ago

@paulp said: NPE fixed, actual problem is shown here (test/pending/pos/bug4859.scala)

object O {
  // error: C is not a legal prefix for a constructor
  C().CC()
  // but this works.
  D().DD()
}

case class C() {
  case class CC()
}

case class D() {
  class DD()
  object DD {
    def apply() = new DD()
  }
}
scabug commented 13 years ago

@adriaanm said: both should work

scabug commented 13 years ago

Matthew Farwell (mfarwell) said: The following is also a manifestation of this bug:

object Test {
case class B() { case class C() }
var b = B()
var c = b.C()
}

This bug is now fixed in version scala-2.10.0.r25798-b20111007023456.

scabug commented 12 years ago

Stefan Wehr (stefanwehr) said: Is this bug fixed or not? The status is "open" and the resolution "unresolved", but Matthew Farwell's comment indicates that the bug is fixed in scala-2.10.0.r25798-b20111007023456.

scabug commented 12 years ago

Matthew Farwell (mfarwell) said: Sorry, the above was my opinion, I'm not an official person, I can't declare a bug fixed. I believe it to be fixed though.

scabug commented 12 years ago

David Leuschner (dleuschner) said: Thank you for the information, Matthew! It's good to know that it's fixed.

I just realized that this bug was already present before 2.9.1 was released on 2011-08-31 and the NPE was even fixed before it was released but it is still present in the current stable release. We as a company (factis research) are considering adopting Scala as the main language. We have a Haskell server in production and one of the reasons of choosing Scala over Haskell is that we're hoping that there's a focus on "industrial strength" quality, tools and commercial support. We got the impression that there's a big community using Scala not only for fun but for commercial services.

With this bug the compiler dies without helpful notice and you can't compile or deploy your product and have to revert to bisecting your code to detect what might be giving Scala hiccups and guess how to "rephrase" the code to make it work. This can cost hours of work and is very frustrating.

The bug has already been fixed three months ago. I'm wondering when the next stable Scala release is planned which will include this fix? Is there a roadmap with planned releases?

I think it would be good to catch all exceptions and give as much information as possible to the user about what happened (like the source file that was currently being processed). With that information you have a chance to guess what might be the problem. GHC outputs a nice message ("the impossible happened") and asks you to report the bug. That's better than just a NPE because now you at least know it's a bug in the compiler (and not sbt or other tools).

scabug commented 12 years ago

@paulp said: As I said in the commit message above, the NPE is fixed but the actual issue is not.

scabug commented 12 years ago

David Leuschner (dleuschner) said: That depends on the point of view. For the compiler developer the "actual issue" might be something different than for the user. My issue is not that I have to rewrite my code but that I have to spend 2 hours to find out what might be going wrong with the build and which change caused the problem (of course I first think that it's MY fault) and that I have to explain to my team that Scala is a good choice although the compiler crashes without giving a message. The "actual issue" is not a problem at all because there's an easy workaround. (Replace "C().CC()" with "val c = C(); c.CC()")

My suggestion is to "fix" the problem by giving a helpful message to the user (such as: known bug in the compiler, see issue #4859 for suggested workaround"). That shouldn't be much work in the compiler but can save users time and gives a much better impression. Of course a "real fix" would be better but I'd prefer an earlier release with a message that improves the user experience.

scabug commented 12 years ago

@adriaanm said: this is as specified: "A constructor invocation is a function application x.ctargs...(argsn), where x is a stable identifier (§3.1)" we could in principle improve on this, but i'm going to say "fixed"

scabug commented 12 years ago

@paulp said: There is no constructor invocation in this code. The compiler is the only one throwing around constructor invocations. As far as the author of the code is concerned, it's two method calls. At the point where the compiler should be inserting a constructor call, it does have a stable prefix: "this".

object O {
  // error: C is not a legal prefix for a constructor
  C().CC()
}

(From the example in my most recent comment containing code.)

scabug commented 12 years ago

@paulp said: ...and thusly do I reopen. The ball is in your court, good sir.

scabug commented 12 years ago

@adriaanm said: right you are

scabug commented 11 years ago

@retronym said: https://github.com/scala/scala/pull/1710

scabug commented 11 years ago

@retronym said: https://github.com/scala/scala/pull/1864