groovy
  1. groovy
  2. GROOVY-2409

Problems calling private static methods

    Details

    • Testcase included:
      yes
    • Number of attachments :
      1

      Description

      Like Java, Groovy allows for static methods to be called in different ways. Surprisingly, not all of them work for private static methods. Specifically:

      class Test { 
        // all errors go away if method is declared non-private 
        private static foo() {} 
        
        static callFooFromStaticMethod() { 
          Test.foo()        // ok 
          foo()             // ok 
          this.foo()        // ok 
          new Test().foo()  // java.lang.IncompatibleClassChangeError 
        } 
        
        def callFooFromInstanceMethod() { 
          Test.foo()       // ok 
          foo()            // java.lang.IncompatibleClassChangeError 
          this.foo()       // java.lang.IncompatibleClassChangeError 
          new Test().foo() // java.lang.IncompatibleClassChangeError 
        } 
      } 
      
      Test.callFooFromStaticMethod() 
      new Test().callFooFromInstanceMethod() 
      

      As indicated, several invocations throw an IncompatibleClassChangeError. I would expect all of them to succeed, just as they do for non-private static methods.

      1. StaticCalls.groovy
        0.6 kB
        Peter Niederwieser

        Issue Links

          Activity

          Hide
          Peter Niederwieser added a comment -

          Because formatting of the code was lost, here it is again.

          Show
          Peter Niederwieser added a comment - Because formatting of the code was lost, here it is again.
          Hide
          Paul King added a comment -

          Add code tags

          Show
          Paul King added a comment - Add code tags
          Hide
          Paul King added a comment -

          Java behavior for comparison:

          class Temp {
            private static void foo(int arg) { System.out.println("arg=" + arg);}
          
            static void callFooFromStaticMethod() {
              Temp.foo(0);          // ok: arg=0
              foo(1);               // ok: arg=1
          //    this.foo(2);        // non-static variable this cannot be referenced from a static context
              new Temp().foo(3);    // ok: arg=3
            }
          
            void callFooFromInstanceMethod() {
              Temp.foo(4);         // ok: arg=4
              foo(5);              // ok: arg=5
              this.foo(6);         // ok: arg=6
              new Temp().foo(7);   // ok: arg=7
            }
          
            public static void main(String[] args){
              Temp.callFooFromStaticMethod();
              new Temp().callFooFromInstanceMethod();
            }
          }
          
          Show
          Paul King added a comment - Java behavior for comparison: class Temp { private static void foo( int arg) { System .out.println( "arg=" + arg);} static void callFooFromStaticMethod() { Temp.foo(0); // ok: arg=0 foo(1); // ok: arg=1 // this .foo(2); // non- static variable this cannot be referenced from a static context new Temp().foo(3); // ok: arg=3 } void callFooFromInstanceMethod() { Temp.foo(4); // ok: arg=4 foo(5); // ok: arg=5 this .foo(6); // ok: arg=6 new Temp().foo(7); // ok: arg=7 } public static void main( String [] args){ Temp.callFooFromStaticMethod(); new Temp().callFooFromInstanceMethod(); } }
          Hide
          Paul King added a comment -

          I made some improvements to StaticImportResolver to try to detect some of these edge cases. The current behaviour is:

          class Test {
            // all errors go away if method is declared non-private
            private static foo(arg) { println "arg=" + arg }
          
            static callFooFromStaticMethod() {
              Test.foo(0)        // ok: arg=0
              foo(1)             // ok: arg=1
              this.foo(2)        // Non-static variable 'this' cannot be referenced from the static method callFooFromStaticMethod.
              new Test().foo(3)  // java.lang.IncompatibleClassChangeError
            }
          
            def callFooFromInstanceMethod() {
              Test.foo(4)        // ok: arg=4
              foo(5)             // ok: arg=5
              this.foo(6)        // ok: arg=6
              new Test().foo(7)  // java.lang.IncompatibleClassChangeError
            }
          }
          
          Test.callFooFromStaticMethod()
          new Test().callFooFromInstanceMethod()
          
          Show
          Paul King added a comment - I made some improvements to StaticImportResolver to try to detect some of these edge cases. The current behaviour is: class Test { // all errors go away if method is declared non- private private static foo(arg) { println "arg=" + arg } static callFooFromStaticMethod() { Test.foo(0) // ok: arg=0 foo(1) // ok: arg=1 this .foo(2) // Non- static variable ' this ' cannot be referenced from the static method callFooFromStaticMethod. new Test().foo(3) // java.lang.IncompatibleClassChangeError } def callFooFromInstanceMethod() { Test.foo(4) // ok: arg=4 foo(5) // ok: arg=5 this .foo(6) // ok: arg=6 new Test().foo(7) // java.lang.IncompatibleClassChangeError } } Test.callFooFromStaticMethod() new Test().callFooFromInstanceMethod()
          Hide
          Paul King added a comment - - edited

          Also extended the 'Non-static variable 'this' cannot be referenced from the static method callFooFromStaticMethod' error to apply to super and properties.

          Show
          Paul King added a comment - - edited Also extended the ' Non-static variable 'this' cannot be referenced from the static method callFooFromStaticMethod ' error to apply to super and properties.
          Hide
          Paul King added a comment -

          I recommend fixing this last case (the 4th case in the methods above) be pushed to 1.6. It is the most contraversial and doesn't have a simple solution in my opinion.

          The other cases are handled by detecting "an obvious" degenerative case where we can "bypass" normal processing and assume we have a static context. In this last case I think this is too dangerous and we would need to through normal channels to do proper MOP processing. So I don't think we can apply the same cheat. The issue is for that case becomes one that we don't get a good error message at this point. My suspicions are that if we made further improvements in the general around of handling private fields, then it might also catch this case and give a better error message (or at least highlight the path better as to how we should resolve this remaining case).

          Show
          Paul King added a comment - I recommend fixing this last case (the 4th case in the methods above) be pushed to 1.6. It is the most contraversial and doesn't have a simple solution in my opinion. The other cases are handled by detecting "an obvious" degenerative case where we can "bypass" normal processing and assume we have a static context. In this last case I think this is too dangerous and we would need to through normal channels to do proper MOP processing. So I don't think we can apply the same cheat. The issue is for that case becomes one that we don't get a good error message at this point. My suspicions are that if we made further improvements in the general around of handling private fields, then it might also catch this case and give a better error message (or at least highlight the path better as to how we should resolve this remaining case).
          Hide
          blackdrag blackdrag added a comment -

          "this" is intended to be allowed in a static context and referring to the class

          Show
          blackdrag blackdrag added a comment - "this" is intended to be allowed in a static context and referring to the class
          Hide
          blackdrag blackdrag added a comment -

          fixed

          Show
          blackdrag blackdrag added a comment - fixed

            People

            • Assignee:
              blackdrag blackdrag
              Reporter:
              Peter Niederwieser
            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: