On Thu, 16 Dec 2021 17:44:04 GMT, Vicente Romero <vrom...@openjdk.org> wrote:
> Hi, > > Please review this change that is fixing a bug in reflection in particular in > `sun.reflect.annotation.TypeAnnotationParser::buildAnnotatedTypes` the > current code is assuming that for inner class constructors it is always > working on type annotations on parameters, but it is also invoked to extract > type annotations applied to exception types for example. This bug affects the > behavior of method: > `java.lang.reflect.Executable::getAnnotatedExceptionTypes` which is not > behaving according to its specification. Given that this fix affects the > behavior of a method belonging to our API a CSR has been filed too. Please > review it at [JDK-8278926](https://bugs.openjdk.java.net/browse/JDK-8278926). > > TIA src/java.base/share/classes/sun/reflect/annotation/TypeAnnotationParser.java line 137: > 135: (declaringClass.getModifiers() & Modifier.STATIC) == 0) > && > 136: filter == > TypeAnnotation.TypeAnnotationTarget.METHOD_FORMAL_PARAMETER) { > 137: offset = true; Currently, we have a few parallel systems for determining if certain parameters are synthetic. This guessing by being inner classes or enums is one, and detecting the flags from `MethodParameters` attribute (used for handling signatures) is another. (In fact, both are present in `Executable` in `handleParameterNumberMismatch` and `getAllGenericParameterTypes`) Detecting `MethodParameters` attribute should be more reliable, especially for other code that may include methods with any number of synthetic or mandated parameters (presumably in the beginning of the parameter list). However, javac will not produce that attribute without `-parameters` flag, so most class files compiled to be missing this attribute. IMO a longer-term solution is to opt-in Javac to emit critical method parameter information by default (ex. emit the parameter flags without names) whenever it emits parameter signatures or annotations. And then the system for skipping synthetic/mandated parameters in the two places may possibly be unified. ------------- PR: https://git.openjdk.java.net/jdk/pull/6869