groovy
  1. groovy
  2. GROOVY-704

Primitive function arguments cause VerifyError

    Details

    • Type: Bug Bug
    • Status: Closed 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
    • Number of attachments :
      1

      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.

        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: