DefaultParameterNameDiscoverer should be updated to not use LocalVariableTableParameterNameDiscoverer when running as a native image.
Comment From: snicoll
I think this should behave the same way on the JVM. the check should be on AOT not native.
Comment From: sdeleuze
Could be great for consistency indeed even if technically on JVM + AOT we have to the capability to use it. Let's validate the choice in today's Framework meeting.
Comment From: sdeleuze
I updated the PR accordingly.
Comment From: jhoeller
An interesting case in terms of automatic exclusion indeed.
We certainly don't want to support that parameter name discovery strategy in a native image (where the underlying class files are generally not available), and it's not recommended in any other scenario either... since you can always compile with -parameters instead, with no need to parse class files then. In such a recommended setup, StandardReflectionParameterNameDiscoverer will always be able to resolve the parameter names first, so LocalVariableTableParameterNameDiscoverer will never actually be reached.
From that perspective, for a setup following Java 8+ recommendations, LocalVariableTableParameterNameDiscoverer could even be removed completely. It's only really there for backwards compatibility with older setups that got migrated without -parameters. Baking that assumption into our AOT arrangement - namely that you need to compile with -parameters when you are optimizing for AOT, completely avoiding unnecessary class file parsing - seems sensible.
Comment From: jhoeller
I'm afraid we'll have to return to a NativeDetector check here since AotDetector is in the higher-level aot package whereas DefaultParameterNameDiscoverer is a very low-level core component. Since there is no significant difference to be expected in practice, I don't think the cycle-free narrower check matters. After all, LocalVariableTableParameterNameDiscoverer is effectively not applicable in a native image due to its class file parsing approach, so it arguably does make sense to exclude it on that basis and rely on -parameters usage by convention.
Comment From: jhoeller
Reopening this one after a team discussion: It is ultimately preferable to deprecate LocalVariableTableParameterNameDiscoverer completely, not using it by default in any setup anymore, since that class file parsing strategy has been long superseded by the Java 8 -parameters flag on javac already.
We have considered doing this before and meant to address the native image impact of it in 6.0 but somehow missed this last week. So let's fix this glitch now, in time for the Boot 3.0 GA release, with a note in the upgrade wiki page.
Comment From: jhoeller
As a lenient measure for the transition period, we'll keep LocalVariableTableParameterNameDiscoverer active on the JVM but log a warning for each successful parameter name resolution. This will only be logged when parameter names actually need to be inspected, and only when StandardReflectionParameterNameDiscoverer did not return anything, suggesting that compilation with -parameters has been missed somewhere.