scala / bug

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

Compiler should not use value of unitialized val #4152

Closed scabug closed 12 years ago

scabug commented 13 years ago

Note: I am not sure if I should mark this a defect or an enhancement, to me it feels more like a compiler-defect.

= problem =

It's been said about Scala that you can program in an (almost) pure functional style if you don't use mutable state, implying no vars, only vals.

However, it can be negatively experienced that a val is also mutable in Scala: In an template ('initializer') statement, a reference to a val, which is defined later in the same template, equals the default value for the type of the val, NOT the value that it is given in its definition.

e.g


object ValRefInInitializer extends Application {

  // static initializer 
  println(a) // compiles but prints 0! (expected: 1!)
  val a = 1 

  new { 
    //object initializer
    println(a) // same here (compiles but prints 0, expected: 1)
    val a = 1
  }

}

If one is not aware of this mutability of vals, this can lead to unexpected misbehaviour, leading to a significant debugging effort.

= analysis =

I think this is very misleading, one should be able to see [the expansion of] a reference to a val as a constant, to reflect the one and only value the val can have.

In comparison, the compiler will disallow the same construct in another type of block statement:


object CompileError extends Application {

  def b() {
    println(a) // compile error:
           // "forward reference extends over
               // definition of value o".
    val a = 2
  }

}

I am aware that this issue also exists in Java with final variables in the same position, but this is another place where Scala can improve over the old Java.

(Java example)

public class FinalVarTestJava {

  static class A {
    {
      useB();
    }

    final Object b = new Object();

    void useB() {
      System.out.println("b=" + b);
    }
  }

  public static void main(String[] args) {
    new A();
  }

  /* 
   * in comparison, in normal blocks a compile-error occurs for 
   * the same construct.
   * (Uncomment and compile to see.)

   static void c() {
     System.out.println(o); // compile-error:
                            // "o can not be resolved".
     Object o = new Object();
   }
   */
}

= enhancement recommendation =

The compiler should either not allow the reference, with a message similar to what happens in other types of statements: "forward reference extends over definition of value [x]"

or it should use the correct value of the val without complaining, maybe by doing the assignment to the val on first reference, in that case like a lazy val.

A lazy val is a work-around for the issue, apart from reordering the definitions. But then at least give an error to the user.

Or maybe I am not aware now of proper use-cases for an un-initialized val and it should remain like it is? (No cynical intention here!)

scabug commented 13 years ago

Imported From: https://issues.scala-lang.org/browse/SI-4152?orig=1 Reporter: me (scalll)

scabug commented 13 years ago

@paulp said: I believe the reasoning given in the past is that there are situations which cannot be recognized so it's not clear how far to chase the penguin. I absolutely agree that we can and really must improve on the present state of affairs, even if there is a goodness ceiling.

scabug commented 13 years ago

me (scalll) said: Ok, thanks. Maybe worth considering is to make all vals lazy (implicitly) in such blocks. But I do not know if that would impact performance in any way significantly.

scabug commented 13 years ago

@paulp said: I can't say for sure, but I quite doubt that any kind of auto-promotion to lazy vals is going to be considered. My own ambition is limited for now to issuing more warnings.

scabug commented 13 years ago

@lindydonna said: I'm marking this as an enhancement. Compiling things to the JVM really impacts language semantics wrt to initialization.

paul, is it ok if I assign to you? feel free to reassign.

scabug commented 13 years ago

@lindydonna said: scalll, you may also be interested in [http://scala-programming-language.1934581.n4.nabble.com/Order-of-field-initialization-cyclic-field-dependency-detection-tp1935158p1935158.html this mailing list thread] if you haven't already seen it.

scabug commented 13 years ago

@paulp said: Replying to [comment:4 malayeri]:

paul, is it ok if I assign to you? feel free to reassign.

That's me, case _ => !

scabug commented 13 years ago

me (scalll) said: Replying to [comment:5 malayeri]:

Hadn't seen the newsgroup thread yet, thanks, it's more complicated than I'd quickly thought of indeed. Especially the side-effects like Piotr Kołaczkowski said make me agree that a solution is probably best limited to error raising. As Iulian and Donna pointed out, there's a difference between statically easy to detect 'violations' and ones that are harder to track. To keep work low, maybe issue a compiler warning for the easy cases, and for the harder cases, build in a runtime check in the generated bytecode, which throws a special runtime-error. (e.g. named ReferenceToUnitializedValError or something). To prevent escalation leading to misbehaving and sometimes hard-to-debug programs. The 'read-detection' required to implement this could perhaps similar to that for lazy vals?

scabug commented 13 years ago

me (scalll) said: Well, this runtime checking should not severally impact performance I guess, which it prob. will?

scabug commented 13 years ago

@paulp said: Replying to [comment:8 scalll]:

Well, this runtime checking should not severally impact performance I guess, which it prob. will?

I'm going back in time to 2008 to implement this feature, but my time traveling self is calling it -Xcheckinit for some reason. He says it definitely impacts performance too much to be left on all the time, but that he'll leave -Xcheckinit around for you to try it out.

scabug commented 13 years ago

me (scalll) said: Replying to [comment:9 extempore]:

Replying to [comment:8 scalll]:

Well, this runtime checking should not severally impact performance I guess, which it prob. will?

I'm going back in time to 2008 to implement this feature, but my time traveling self is calling it -Xcheckinit for some reason. He says it definitely impacts performance too much to be left on all the time, but that he'll leave -Xcheckinit around for you to try it out.

Haha, thanks. While you're still there, can you walk up to me and say I should start buying Apple shares? :-)

I more or less overlooked it in the thread. Well that's very handy indeed. Should get more attention (in the book?) Btw, it's not in the scalac doc anymore.

scabug commented 13 years ago

@paulp said: Replying to [comment:10 scalll]:

Haha, thanks. While you're still there, can you walk up to me and say I should start buying Apple shares? :-)

Sure.

If you weren't reading my blog, send yourself back in time and fix that.

scabug commented 12 years ago

@adriaanm said: ... but "fixed" by -Xcheckinit since, like, forever

SethTisue commented 5 years ago

ticket of record is #399