groovy

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.

Issue Links

This issue depends upon:
GROOVY-3100 allow this in static context again Major Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
Paul King added a comment -

Add code tags

Show
Paul King added a comment - Add code tags
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
blackdrag blackdrag added a comment -

fixed

Show
blackdrag blackdrag added a comment - fixed

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.