Fwd: Mandated/implicit parameters

[-]

Your patch looks nice. I suggest, in addition, to write the attribute
when any synthetic parameter is present as well (such as the first two
String and int of enum constructors). In addition, I would recommend a
test for core reflection to ensure the newly emitted attributes allow
core reflection to correctly describe the parameterized types of inner
class constructors, etc.

Once you can find a related issue on the bug tracker (or report one at
https://bugs.java.com/ if you can't find a fitting one), you can
submit this patch for that bug tracker issue.

For class files compiled by older Javac, unfortunately, they indeed do
not have necessary data for core reflection to correctly interpret if
an argument is synthetic or mandated. However, any javac beyond 8
should have the -parameters attribute, which they could turn on for a
workaround.

For the ClassReader, I think you may need a `index++;` before the
`continue;`, as that would better match the behavior of
LocalVariableTable register-based index assigning.


On Tue, Jun 7, 2022 at 10:00 AM Hannes Greule <hannesgreule at outlook.de> wrote:
>
>  > So, your approach would fix all these issues reported.
>
> That's interesting, I didn't think  of other related issues before.
>
> However, this would only fix the issue for newly compiled class files
> and only for compile targets >= Java 8 from my understanding.
>
> How should we go forward from here? I think I got a working fix for it
> on this branch[1] and I already ran the langtools_javac tests.
> There are two failing tests, one of them[2] was assuming a wrong
> constant pool index (as the MethodParameter attribute wasn't written
> before). I updated the test accordingly.
>
> The other one[3] seems to be an issue with the ClassReader here[4].
>
> [1]
> https://github.com/SirYwell/jdk/tree/fix/enforce-methodparam_attr-if-mandated
> [2]
> https://github.com/openjdk/jdk/blob/master/test/langtools/tools/javac/annotations/typeAnnotations/classfile/AnnotatedExtendsTest.java#L62
> [3]
> https://github.com/openjdk/jdk/blob/master/test/langtools/tools/javac/6889255/T6889255.java
> [4]
> https://github.com/openjdk/jdk/blob/062db59eeb8ba6389aaa3c622dbc109a92d580ca/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java#L1069-L1071
>