On Tue, 7 Oct 2025 07:59:25 GMT, Alan Bateman <[email protected]> wrote:
> The APIs are in Field and Lookup so having the API method as the top frame is
> useful. It would be possible to reduce the filter to `{
> "java.lang.reflect.ReflectAccess",
> "java.lang.invoke.MethodHandles$Lookup::unreflectField" }` with
> determineStackTraceOffset returning 6 but it's too fiddly and requires
> knowing about two "faraway places" when doing any refactoring. Mutating final
> fields is the slow path so performance is not a concern. So I think the
> trade-off to keep it as maintainable as possible is okay. The test checks the
> top frame and also scans the StackFilter to ensure the class is visible and
> that any filter value with a method name at least names a method that is
> declared in the class.
We shouldn't push it as high as 6, that's fragile, but the offer(...) method
could be skipped immediately if the offset is bumped. Class filters avoid
specifying individual methods, which are more likely to be refactored.
I can see the argument for not having the user's method as the top frame. A
user may get a quick hint (instead of looking at the line number) if they see
something like setInt(...), but this doesn’t work as well with tooling when you
want to group stack traces by top frame, for example in a tree view. You
typically want to see the application frame and then expand the nodes. If
setInt, setFloat, setLong, etc. appear as the top nodes, users have to click
and expand every setter, instead of seeing an aggregated list directly of
packages, classes, or methods where finals are modified.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/25115#issuecomment-3516700622
