Scala Programming Language
  1. Scala Programming Language
  2. SI-5610

Overly-Eager Evaluation of By-Name Parameter in Inferred Partial Application

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: Scala 2.9.1
    • Fix Version/s: Scala 2.10.0-M3
    • Component/s: Misc Compiler
    • Environment:

      Scala version 2.9.1.final (Java HotSpot(TM) 64-Bit Server VM, Java 1.7.0_04-ea)

      Also attempted with the latest from master, but it isn't building right now...

      Description

      object Bug extends App {
        class Result(_str: => String) {
          lazy val str = _str
        }
        
        def foo(str: => String)(i: Int) = new Result(str)
        
        def bar(f: Int => Result) = f(42)
        
        var test: String = null
        val result = bar(foo(test))
        test = "bar"
        
        if (result.str == null) {
          println("Destroy ALL THE THINGS!!!")
        } else {
          println("Stroke a kitten")
        }
      }
      

      We would expect the output of this code would be "Stroke a kitten", since the value of `test` is given a non-null value before the `str` lazy val is realized. However, the actual printout is "Destroy ALL THE THINGS!!!", which isn't a polite thing to do.

      The problem lies in the bytecode generated for `bar(foo(test))`. More specifically, the issue is in the lifting of the `test` actual for the `foo` invocation. Bytecode (from Bug$delayedInit$body.apply)Ljava/lang/Object:

            18: aload_0       
            19: getfield      #12                 // Field $outer:LBug$;
            22: invokevirtual #22                 // Method Bug$.test:()Ljava/lang/String;
            25: astore_1      
            26: new           #24                 // class Bug$$anonfun$1
            29: dup           
            30: aload_1       
            31: invokespecial #27                 // Method Bug$$anonfun$1."<init>":(Ljava/lang/String;)V
      

      Notice instruction 22. We're getting a raw (un-thunked) value for `test` and then lifting it into the requisite closure for the foo(test) invocation.

      Using an explicit thunk (of type () => String) rather than a by-name parameter for `str` avoids this issue.

        Issue Links

          Activity

          Hide
          Lukas Rytz added a comment -

          right... will look at the spec

          Show
          Lukas Rytz added a comment - right... will look at the spec
          Show
          A. P. Marki added a comment - Johannes Rudolph proposed spec wording on the application side in comment to SI-302 https://issues.scala-lang.org/browse/SI-302?focusedCommentId=57412&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-57412
          Hide
          Johannes Rudolph added a comment -

          Funny, I missed this one completely. I had a similar patch in work for SI-308 but since the block was on how to change the spec I first wanted to make my homework to propose a reasonable addition for the 'Eta-Expansion' section of the spec. Good to see it fixed.

          Show
          Johannes Rudolph added a comment - Funny, I missed this one completely. I had a similar patch in work for SI-308 but since the block was on how to change the spec I first wanted to make my homework to propose a reasonable addition for the 'Eta-Expansion' section of the spec. Good to see it fixed.
          Hide
          Seth Tisue added a comment -

          shouldn't this ticket and SI-302 both be closed now? the new behavior is already in M4.

          (is it because the spec change is still pending?)

          Show
          Seth Tisue added a comment - shouldn't this ticket and SI-302 both be closed now? the new behavior is already in M4. (is it because the spec change is still pending?)
          Hide
          Lukas Rytz added a comment -

          pending spec change logged in SI-6069

          Show
          Lukas Rytz added a comment - pending spec change logged in SI-6069

            People

            • Assignee:
              Lukas Rytz
              Reporter:
              Daniel Spiewak
            • Votes:
              1 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development