[CLJ-1315] Don't initialize classes when importing them Created: 28/Dec/13 Updated: 07/Oct/14 Resolved: 07/Oct/14
|Affects Version/s:||Release 1.1, Release 1.2, Release 1.3, Release 1.4, Release 1.5|
|Fix Version/s:||Release 1.7|
|Labels:||aot, compiler, interop|
Problem: When classes are imported in Clojure, the class is loaded using Class.forName(), which causes its static initialisers to be executed. This differs from Java where compilation does not cause classes to be loaded.
Motivation: In many cases when those classes are normally loaded by Java code during execution of a framework of some kind (IntelliJ in my case, and RoboVM is another culprit mentioned in that thread) the initialiser expects some infrastructure to be in place and will fail when it's not. This means that it's impossible to AOT compile namespaces importing these classes, which is a fairly serious limitation.
Approach: Modify ImportExpr to call RT.classForNameNonLoading() instead of Class.forName(), which will load the class but not initialise it. This change causes the Clojure test suite to fail, since clojure.test-clojure.genclass imports a gen-class'ed class which no longer loads its namespace on initialisation. I'm not sure if this is considered an incorrect use of such a class (IIRC with records it's required to import the class and require its namespace), but given that it's in the Clojure test case it's reasonable to assume that this fix would be a breaking change for more code out there. This test failure is also corrected in the attached patch.
Screened by: Alex Miller - I have tested many open source Clojure projects with this change (particularly seeking out large, complicated, or known users of genclass/deftype/etc) and have found no projects adversely impacted. I know that Cursive has been running with this modification for a long time with no known issues. I am ok with unconditionally enabling this change (re the comment below). The impact is described in more detail in the suggested changelog diff in the comments below.
Alternative: This patch enables the change unconditionally, but depending on the extent of breakage it causes, it might need to be enabled with a configuration flag. I propose we make it unconditional in an early 1.7 beta and monitor the fall-out.
Background: This issue has been discussed in the following threads
|Comment by Alex Miller [ 29/Dec/13 12:23 PM ]|
From original post:
This issue was originally reported by Zach Oakes and Colin Fleming and this patch was also tested by Colin.
I'm duplicating here my suggested release notes for this issue, which includes my current thoughts on potential breakage (it's also in the commit message of the patch):
|Comment by Greg Chapman [ 13/May/14 6:25 PM ]|
I'm not sure if this should also be fixed, but it would be nice if you could emit the code for a proxy of one of these non-initialized classes without forcing initialization. For example, the following raises an exception (I'm using Java 8):
The exception was ultimately caused by "IllegalStateException Toolkit not initialized", which javafx throws if you attempt to initialize a Control class outside of Application.launch.