Community
Participate
Working Groups
Created attachment 261695 [details] Demo project According to JVMS 4.7.6 [1]: 1) A nested class or nested interface must have an `InnerClasses` attribute with information about its containing class or interface. 2) A class or interface with nested classes and/or nested interfaces must also have an `InnerClasses` attribute with information about its nested classes and/or nested interfaces. AspectJ correctly creates `InnerClasses` attributes for item 1 but not for item 2. This hinders interoperability with other compilers/libraries that rely on such information. For more details please see the description, attachments, and comments on KT-12179 [2]. The demo project attachment from KT-12179 is attached here as well for your convenience. [1] http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.6 [2] https://youtrack.jetbrains.com/issue/KT-12179
Actually, as far as I can tell, the InnerClasses attribute of the $ajcMightHaveAspect class also looks incorrect. Using ASM's ASMifier, I get: cw.visitInnerClass("example/aspect/FooAspect$ajcMightHaveAspect", null, "FooAspect", ACC_PUBLIC + ACC_ABSTRACT + ACC_INTERFACE); This appears to define inner class example.aspect.FooAspect$ajcMightHaveAspect with NO outer class (null), and an inner alias of "FooAspect". Contrast with the ASMifier results on java.util.Map$Entry: cw.visitInnerClass("java/util/Map$Entry", "java/util/Map", "Entry", ACC_PUBLIC + ACC_STATIC + ACC_ABSTRACT + ACC_INTERFACE); If FooAspect$ajcMightHaveAspect interface is indeed supposed to be an inner class, I would expect its definition to be: cw.visitInnerClass("example/aspect/FooAspect$ajcMightHaveAspect", "example/aspect/FooAspect", "ajcMightHaveAspect", ACC_PUBLIC + ACC_STATIC + ACC_ABSTRACT + ACC_INTERFACE); (nested interfaces are always static) Further, the FooAspect classfile should include the same inner class definition as the interface. OTOH, perhaps there is no reason for *$ajcMightHaveAspect to actually be an inner class (since it's basically not, apparently; note that if you call #getDeclaredClasses() from the java.util.Class instance representing this type you receive an empty array), in which case its InnerClasses info could be dropped entirely, and it would be up to other libraries to make sure they can process top-level classes that happen to contain a $ in their name. OR, is there some deliberate (hopefully documented) technical reason (hack?) for the weird bytecode limbo in which $ajcMightHaveAspect interfaces seem to live?
Thanks for the clear bug report, having a dig into it now. I don't think that handling around ajcMightHaveAspect has been properly looked at it many many (many) years and I don't recall any testcases explicitly verifying the structure.
I think I'll go with what Matt was thinking, that thing doesn't need to be an inner class. Right now having browsed the old code it looks like some attempts were made long ago to make it a kind of anonymous member type, but it was half baked. The enclosing type not updated properly and the attribute in the anonymous class not finished properly either. If using annotation style as opposed to code style, there is also another code path that can generate the interface too and that code path does not make it an inner class. So I'm inclined to remove the attribute from the generated interface. However, at the same time I don't really want to change the name, there is code out there (nasty, evil code) relying on the name. I'm not even sure I know all the places where that might be relied on - I just know the few I can see. It does indeed mean that anyone assuming $ means inner could be affected. Regression tests still all pass with it removed.
As far as I can tell, outer$inner is a convention and doesn't seem to be explicitly dictated in the JLS. I can attest that it is perfectly legal to create classes with $ and sequences of that character and have them function at runtime. So I think if you resolve the inner/nested (the JLS actually implies that "inner" classes are non-static, stating that interfaces, being explicitly static, are not inner classes per se), you can safely keep *$ajcMightHaveAspect and push supporting top-level (really all) types back to other libraries with compatibility issues... which, had they already done so, would probably have meant this issue report would never have been filed. Still, it's probably for the best to clean up the generated bytecode.
several comments above I said java.util.Class; of course I meant java.lang.Class :P
Changes are in. No longer a (half baked) inner class. If someone needs a workaround for this ahead of the next release, you should be able to translate your aspect to annotation style - that will exercise the alternate route that produces an ajcMightHaveAspect without the InnerClasses tag.
Dev build here for anyone who wants to kick it around: http://www.eclipse.org/downloads/download.php?file=/tools/aspectj/dev/aspectj-DEVELOPMENT-20160512153500.jar
