Uploaded image for project: 'Scala Programming Language'
  1. Scala Programming Language
  2. SI-5610

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

    Details

    • Type: Bug
    • Status: CLOSED
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Scala 2.9.1
    • Fix Version/s: Scala 2.10.0-M3
    • Component/s: Compiler (Misc)
    • 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.

        Attachments

          Issue Links

            Activity

            Hide
            rytz Lukas Rytz added a comment -

            right... will look at the spec

            Show
            rytz Lukas Rytz added a comment - right... will look at the spec
            Show
            apm 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
            jrudolph 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
            jrudolph 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
            sethtisue 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
            sethtisue 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
            rytz Lukas Rytz added a comment -

            pending spec change logged in SI-6069

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

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: