Groovy
  1. Groovy
  2. GROOVY-704

Primitive function arguments cause VerifyError

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0-beta-9
    • Fix Version/s: 1.0-beta-10
    • Component/s: None
    • Labels:
      None
    • Environment:
      Windows XP, Java 1.4.2_06

      Description

      The following code:

      def foo(double x, y) {
      println "x: "+x
      println "y: "+y
      }

      foo(10.0d, 0)

      causes Groovy to complain that the value passed to y is the wrong type. Making foo a member function of a class (in another file), and giving it three arguments, the function runs but y is null and the value that should go to y, goes to the third arg. At one point I had it triggering an internal error in the JVM.

      Changing double to Double fixes the problem.

      1. AsmClassGenerator2.diff
        0.8 kB
        Martin C. Martin

        Issue Links

          Activity

          Hide
          Martin C. Martin added a comment -

          I spent a while on it this evening, and couldn't find the problem. But I did discover that you don't need the function call or the first println, although you do need to access y. So the shortest code that produces this bug is:

          def foo(double x, y)

          { y }

          Everything looks fine, and the methodType in AsmClassGenerator2.visitMethod() is right:

          (DLjava/lang/Object;)Ljava/lang/Object;

          Maybe it's a bug in the objectweb ClassWriter?

          Show
          Martin C. Martin added a comment - I spent a while on it this evening, and couldn't find the problem. But I did discover that you don't need the function call or the first println, although you do need to access y. So the shortest code that produces this bug is: def foo(double x, y) { y } Everything looks fine, and the methodType in AsmClassGenerator2.visitMethod() is right: (DLjava/lang/Object;)Ljava/lang/Object; Maybe it's a bug in the objectweb ClassWriter?
          Hide
          Martin C. Martin added a comment -

          After a little more searching, I discovered this:

          http://java.sun.com/docs/books/vmspec/2nd-edition/html/Overview.doc.html#15722

          "A single local variable can hold a value of type boolean, byte, char, short, int, float, reference, or returnAddress. A pair of local variables can hold a value of type long or double."

          So when an argument is a long or a double, we need to increase the index of all subsequent arguments.

          Show
          Martin C. Martin added a comment - After a little more searching, I discovered this: http://java.sun.com/docs/books/vmspec/2nd-edition/html/Overview.doc.html#15722 "A single local variable can hold a value of type boolean, byte, char, short, int, float, reference, or returnAddress. A pair of local variables can hold a value of type long or double." So when an argument is a long or a double, we need to increase the index of all subsequent arguments.
          Hide
          james strachan added a comment -

          I think this is resolved now - I've added a test case groovy.bugs.DoubleSizeParametersBug to try reproduce it and I think its fixed now?

          Show
          james strachan added a comment - I think this is resolved now - I've added a test case groovy.bugs.DoubleSizeParametersBug to try reproduce it and I think its fixed now?
          Hide
          james strachan added a comment -

          fixing resolve comment

          Show
          james strachan added a comment - fixing resolve comment
          Hide
          james strachan added a comment -

          Just to update my previous comment - it was Pilho Kim who provided me with the bug fix on IRC - many thanks Pilho!

          Show
          james strachan added a comment - Just to update my previous comment - it was Pilho Kim who provided me with the bug fix on IRC - many thanks Pilho!
          Hide
          Martin C. Martin added a comment -

          Adding patch for a slightly cleaner solution

          Show
          Martin C. Martin added a comment - Adding patch for a slightly cleaner solution
          Hide
          Martin C. Martin added a comment -

          Changes the fix; doesn't use dummy variables, instead getNextVariableID() takes size of each var into account.

          Show
          Martin C. Martin added a comment - Changes the fix; doesn't use dummy variables, instead getNextVariableID() takes size of each var into account.

            People

            • Assignee:
              Unassigned
              Reporter:
              Martin C. Martin
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development