RFR: JDK-8213103: RoundEnvironment.getElementsAnnotatedWith(Class) crashes with -source 8 (original) (raw)

Jan Lahoda jan.lahoda at oracle.com
Thu Nov 1 12:34:10 UTC 2018

Hi Joe,

Thanks for the comments. An updated webrev that uses Feature.MODULES instead of Elements.getAllModules, and also updates the copyright year as noted by Vicente, is here: http://cr.openjdk.java.net/~jlahoda/8213103/webrev.01/

(As a side note, Elements.getAllModules() should be OK performance-wise for this purpose, it basically reads an existing/pre-filed field.)

For changing the method to throw IAE, while I agree this situation may point out an unintended difference between the compile- and run-time environments, sometimes the difference may be intentional (runtime dependencies need to be transitive, while compile time don't need to be) and may be benign, as is in the case where I've noticed this problem. So given the method didn't throw the exception in such cases before, I'd rather keep the behavior compatible.

Thanks, Jan

On 31.10.2018 18:27, joe darcy wrote:

Hi Jan,

On 10/31/2018 3:50 AM, Jan Lahoda wrote: Hi,

When RoundEnvironment.getElementsAnnotatedWith(Class) converts the Class to TypeElement, it uses: eltUtils.getTypeElement(eltUtils.getModuleElement(), )

But this fails with -source 8, as there are no module (so getModuleElement returns null, and getTypeElement checks the module parameter is non-null). The proposed fix is to avoid this code. Bug: https://bugs.openjdk.java.net/browse/JDK-8213103 Webrev: http://cr.openjdk.java.net/~jlahoda/8213103/webrev.00/index.html Any feedback is welcome, Thanks for looking at this issue. Stepping back a bit, the API functionality being supported is RoundEnvironment.getElementsAnnotatedWith​(Class<? extends_ _Annotation> a) and related methods which take in annotation types indicated using Class objects (a runtime core reflection representation) and convert them internally to TypeElement's (a compile-time representation) by extracting the name of the annotation type and then doing various look-ups. If the compile-time and runtime environments and configured to be consistent with each other, then a runtime annotation type should be able to get back a corresponding compile-time representation. However, it is possible for the environments to not be consistent, leading to this bug and the earlier change 8190886: "package-info handling in RoundEnvironment.getElementsAnnotatedWith" which this proposed fix amends. The proposed "return null" arm in @@ -285,13 +285,15 @@ // differ from the single module being compiled. String name = annotation.getCanonicalName(); TypeElement annotationElement = eltUtils.getTypeElement(name); if (annotationElement != null) return annotationElement; - else { + else if (!eltUtils.getAllModuleElements().isEmpty()) { String moduleName = Objects.requireNonNullElse(annotation.getModule().getName(), ""); return eltUtils.getTypeElement(eltUtils.getModuleElement(moduleName), name); + } else { + return null; } } private Element mirrorAsElement(AnnotationMirror annotationMirror) { return annotationMirror.getAnnotationType().asElement(); is basically the handling for an inconsistent environment pre-modules. The preceding lines are an, in retrospect, incomplete handling of an inconsistent state with modules. I recommend the check for "if modules are supported" be implemented using a Source-based predicate so that any future dead code will get removed once modules are in all supported source levels. In that route is not chosen, is getAllModuleElements() a good-performing predicate for this purpose? Would something like "if (getModuleElement​("java.base") != null )" be better? If the annotation type indicated by the Class object cannot be turned into an compile-time annotation type, arguably that is an erroneous situation that should cause an exception. The IllegalArgumentException clauses of RoundEnvironment.getElementsAnnotatedWith​(Class<? extends Annotation> a) RoundEnvironment.getElementsAnnotatedWithAny​(Set<Class<? extends_ _Annotation>> annotations) could be expanded to cover this situation and, in the implementation, return null replaced by throwing an informative exception. What do you think? Cheers, -Joe

More information about the compiler-dev mailing list