Skip to content

ClassPrinter silently drops MethodParametersAttribute in TRACE_ALL mode#30197

Draft
wenshao wants to merge 2 commits intoopenjdk:masterfrom
wenshao:fix/classprinter-missing-method-parameters-node
Draft

ClassPrinter silently drops MethodParametersAttribute in TRACE_ALL mode#30197
wenshao wants to merge 2 commits intoopenjdk:masterfrom
wenshao:fix/classprinter-missing-method-parameters-node

Conversation

@wenshao
Copy link
Contributor

@wenshao wenshao commented Mar 11, 2026

In ClassPrinterImpl.attributesToTree(), the MethodParametersAttribute case builds a MapNodeImpl representing method parameters but never adds it to the nodes list. This causes method parameter names and access flags to be silently omitted from the output when using Verbosity.TRACE_ALL.

All other attribute cases in the same switch expression correctly call nodes.add() after constructing their nodes.

  • Before:
  case MethodParametersAttribute p -> {
      Node n = map("method_parameters", ...);
      // missing: nodes.add(n);
  }
  • After:
  case MethodParametersAttribute p -> {
      Node n = map("method_parameters", ...);
      nodes.add(n);
  }

Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/30197/head:pull/30197
$ git checkout pull/30197

Update a local copy of the PR:
$ git checkout pull/30197
$ git pull https://git.openjdk.org/jdk.git pull/30197/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 30197

View PR using the GUI difftool:
$ git pr show -t 30197

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/30197.diff

…L mode

The MethodParametersAttribute case in ClassPrinterImpl.attributesToTree()
built a MapNodeImpl but never added it to the nodes list, causing method
parameter names and flags to be silently dropped from TRACE_ALL output.
All other attribute cases in the same switch correctly call nodes.add().

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@bridgekeeper
Copy link

bridgekeeper bot commented Mar 11, 2026

👋 Welcome back swen! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Mar 11, 2026

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@openjdk
Copy link

openjdk bot commented Mar 11, 2026

@wenshao The following label will be automatically applied to this pull request:

  • core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@wenshao wenshao changed the title Fix ClassPrinter missing MethodParametersAttribute output in TRACE_AL… Fix ClassPrinter missing MethodParametersAttribute output in TRACE_ALL mode Mar 11, 2026
@wenshao wenshao changed the title Fix ClassPrinter missing MethodParametersAttribute output in TRACE_ALL mode ClassPrinter silently drops MethodParametersAttribute in TRACE_ALL mode Mar 11, 2026
Update ClassPrinterTest to include MethodParametersAttribute in the
test model and verify its output in all formats (YAML, JSON, XML) and
verbosity levels (TRACE_ALL, CRITICAL_ATTRIBUTES, MEMBERS_ONLY).

Changes:
- Add MethodParametersAttribute with two params (param1 FINAL, param2)
  to the test class model
- Update YAML/JSON/XML TRACE_ALL expected output with method parameters
  content and shifted constant pool indices
- Update all attributes lists to include MethodParameters
- Update walk() counts: TRACE_ALL 588->606, CRITICAL 146->147,
  MEMBERS 42->43

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant