<< Back to previous view

[CLJ-1243] Cannot resolve public generic method from package-private base class Created: 01/Aug/13  Updated: 27/Sep/16

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.3, Release 1.4, Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: interop

Attachments: GZip Archive clj-1243-demo1.tar.gz     Text File invocation_target_selection.patch    
Patch: Code and Test
Approval: Triaged


The Clojure compiler cannot resolve a public generic method inherited from a package-private base class.

Instructions to reproduce:

  • In package P1
    • Define a package-private class A with generic type parameters
    • Define a public method M in A using generic types in either its arguments or return value
    • Define a public class B which extends A
  • In package P2
    • Construct an instance of B
    • Invoke B.M()

This is valid in Java. In Clojure, invoking B.M produces a reflection warning, followed by the error "java.lang.IllegalArgumentException: Can't call public method of non-public class." No amount of type-hinting prevents the warning or the error.

Attachment clj-1243-demo1.tar.gz contains sample code and script to demonstrate the problem.

Examples of Java projects which use public methods in package-private classes:

Comment by Stuart Sierra [ 01/Aug/13 5:11 PM ]

It is also not possible to call the method reflectively from Java.

This may be a bug in Java reflection: JDK-4283544

But why does it only happen on generic methods?

Comment by Stuart Sierra [ 08/Aug/13 11:59 AM ]

According to Rich Hickey, the presence of bridge methods is unspecified and inconsistent across JDK versions.

A possible solution is to use ASM to examine the bytecode of third-party Java classes, instead of the reflection API. That way the Clojure compiler would have access to the same information as the Java compiler.

Comment by Andy Fingerhut [ 17/Nov/13 11:01 PM ]

CLJ-1183 was closed as a duplicate of this one. Mentioning it here in case anyone working on this ticket wants to follow the link to it and read discussion or test cases described there.

Comment by Noam Ben Ari [ 21/Feb/15 4:55 AM ]

The current work around I use is to define a new Java class, add a static method that does what I need, and call that from Clojure.

Comment by Noam Ben Ari [ 21/Feb/15 9:28 AM ]

Also, I'm seeing this issue in 1.6 and 1.7(alpha5) but the issue mentions only up to 1.5 .

Comment by Adam Tait [ 03/Apr/16 5:32 PM ]

Just ran into this issue trying to use Google's Cloud APIs.
To use Google's Cloud Datastore, you need to access the .kind method on a protected generic subclass (BaseKey), to which KeyFactory extends.

Tested on both Clojure 1.7 & 1.8 at runtime, the following exception persists;

IllegalArgumentException Can't call public method of non-public class: public com.google.gcloud.datastore.BaseKey$Builder com.google.gcloud.datastore.BaseKey$Builder.kind(java.lang.String) clojure.lang.Reflector.invokeMatchingMethod (Reflector.java:88)

Comment by Kai Strempel [ 18/Jun/16 1:19 PM ]

I ran into the exact same issue with Google's Cloud API's.

Tested it with 1.8 and with 1.9.0-alpha7. Same Problem.

Comment by Kai Strempel [ 18/Jun/16 1:19 PM ]

I ran into the exact same issue with Google's Cloud API's.

Tested it with 1.8 and with 1.9.0-alpha7. Same Problem.

Comment by Michal Růžička [ 23/Sep/16 1:08 PM ]

I ran into the same issue. The attached patch fixes the problem for me.
All tests in the project still pass, but this desperately needs a review of someone knowledgeable.

Comment by Alex Miller [ 23/Sep/16 3:00 PM ]

Hey Michal,

Thanks for looking at it.

1. Please follow the instructions on how to create a patch in the proper format here: http://dev.clojure.org/display/community/Developing+Patches
2. If you can provide some explanation of the changes to aid in review that would be most helpful. Otherwise screeners have to re-engineer your thought processes from scratch.
3. Before getting screened, this change will also need some tests (admittedly not particularly fun to write, but I think it's necessary here)

Comment by Michal Růžička [ 27/Sep/16 8:56 AM ]

I've added tests and updated the patch according to the instructions.

Here is some reasoning behind it. Below is an excerpt from the src/jvm/clojure/lang/Compiler.java file:

1462:	if(target.hasJavaClass() && target.getJavaClass() != null)
1463:		{
1464:		List methods = Reflector.getMethods(target.getJavaClass(), args.count(), methodName, false);
1465:		if(methods.isEmpty())
1466:			{
1467:			method = null;
1468:			if(RT.booleanCast(RT.WARN_ON_REFLECTION.deref()))
1469:				{
1470:				RT.errPrintWriter()
1471:					.format("Reflection warning, %s:%d:%d - call to method %s on %s can't be resolved (no such method).\n",
1472:						SOURCE_PATH.deref(), line, column, methodName, target.getJavaClass().getName());
1473:				}
1474:			}
1475:		else
1476:			{
1477:			int methodidx = 0;
1478:			if(methods.size() > 1)
1479:				{
1480:				ArrayList<Class[]> params = new ArrayList();
1481:				ArrayList<Class> rets = new ArrayList();
1482:				for(int i = 0; i < methods.size(); i++)
1483:					{
1484:					java.lang.reflect.Method m = (java.lang.reflect.Method) methods.get(i);
1485:					params.add(m.getParameterTypes());
1486:					rets.add(m.getReturnType());
1487:					}
1488:				methodidx = getMatchingParams(methodName, params, args, rets);
1489:				}
1490:			java.lang.reflect.Method m =
1491:					(java.lang.reflect.Method) (methodidx >= 0 ? methods.get(methodidx) : null);
1492:			if(m != null && !Modifier.isPublic(m.getDeclaringClass().getModifiers()))
1493:				{
1494:				//public method of non-public class, try to find a public descendant
1495:				if((type=Reflector.getDeepestPublicDescendant(m.getDeclaringClass(), target.getJavaClass())) == null)
1496:					//if descendant not found, try to find an ancestor
1497:					m = Reflector.getAsMethodOfPublicBase(m.getDeclaringClass(), m);
1498:				}
1499:			method = m;
1500:			if(method == null && RT.booleanCast(RT.WARN_ON_REFLECTION.deref()))
1501:				{
1502:				RT.errPrintWriter()
1503:					.format("Reflection warning, %s:%d:%d - call to method %s on %s can't be resolved (argument types: %s).\n",
1504:						SOURCE_PATH.deref(), line, column, methodName, target.getJavaClass().getName(), getTypeStringForArgs(args));
1505:				}
1506:			}
1507:		}
  • the condition on line 1462 ensures that the type/class of the target is known
  • the clojure.lang.Reflector.getMethods() method called on line 1464 returns a list of all public methods of the given name defined for the target type
  • then the best method to call is selected on lines 1477-1491
  • if the declaring class of the selected method is not public then an attempt is made to find a public class which is both superclass of the target type and a subclass of the class declaring the selected method - this is implemented in the clojure.lang.Reflector.getDeepestPublicDescendant() method
  • if such a class is found than it is used instead of the method's declaring class when emitting the byte code for the method call
  • if no such class is found then an attempt is made to find a compatible method in the public ancestors of the class declaring the selected method

Note that the change may result in a different method being called than prior to the change as demonstrated by the selecting-method-on-nonpublic-interface test. This is IMO an acceptable change as it:

  • results in better matching (with respect to the argument types) method to be called
  • makes the method selection in clojure behave in a more similar way to that in java
Generated at Tue Oct 25 21:28:29 CDT 2016 using JIRA 4.4#649-r158309.