You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "nbauma109 (via GitHub)" <gi...@apache.org> on 2026/10/22 06:14:59 UTC
[PR] Add support for the RuntimeVisibleTypeAnnotations and RuntimeInvisibleTypeAnnotations attributes [commons-bcel]
nbauma109 opened a new pull request, #539:
URL: https://github.com/apache/commons-bcel/pull/539
Parses and writes the `RuntimeVisibleTypeAnnotations` and `RuntimeInvisibleTypeAnnotations` attributes ([JVMS 4.7.20](https://docs.oracle.com/javase/specs/jvms/se21/html/jvms-4.html#jvms-4.7.20)), which `Attribute.readAttribute` returns as `Unknown` today.
## Why
Annotations written on types (JSR 308) are stored apart from the declaration annotations: JSpecify's `@Nullable` in `void put(@Nullable Object key)`, `List<@Nullable String>`, `<K extends @Nullable Object>`. A user of BCEL can only get them by parsing the bytes of an `Unknown` attribute itself: target type, variable-size target info, type path, then the annotation with its element values. For example, a decompiler built on BCEL loses every `@Nullable` of a library such as Guava, whose packages are `@NullMarked`, and the recompiled sources are rejected by Error Prone.
## Changes
- `Const`: `ATTR_RUNTIME_VISIBLE_TYPE_ANNOTATIONS = 29`, `ATTR_RUNTIME_INVISIBLE_TYPE_ANNOTATIONS = 30`, `KNOWN_ATTRIBUTES = 31`, and the two attribute names.
- `TypeAnnotations` (abstract, `Iterable<TypeAnnotationEntry>`, like `Annotations`), `RuntimeVisibleTypeAnnotations`, `RuntimeInvisibleTypeAnnotations`.
- `TypeAnnotationEntry`: the target type (`TARGET_*` constants for the table 4.7.20-A), the target info as stored in the class file (its size depends on the target type; the `localvar_target` table has a variable size), the type path (`getTypePathLength`, `getTypePathKind`, `getTypePathArgumentIndex`), and the `AnnotationEntry` read by the existing code. An undefined target type throws `ClassFormatException`. `dump` writes what `read` read.
- `Attribute.readAttribute`, `Visitor.visitTypeAnnotations` (a default method: existing `Visitor` implementations are unaffected), `DescendingVisitor`, `EmptyVisitor`, `StringRepresentation`, and an entry in `changes.xml`.
The target info is exposed as bytes, not as one class per kind of target, to keep the API small. The meaning of its bytes is documented on `getTargetInfo()`: index of the type parameter, of the bound, or of the formal parameter.
## Tests
`TypeAnnotationsTest` reads `src/test/resources/typeannotations/TypeAnnotated.class`, compiled with `javac --release 8` from the `.java` file next to it. It covers a field, the type path of a type argument, an invisible annotation, a method return and a parameter, the bound of a class type parameter, dump then re-read, `copy`, the visitor and `StringRepresentation`. Two more tests cover the `localvar_target` table (read then dump gives the same bytes) and an undefined target type.
The default `mvn` goal (tests, rat, japicmp, checkstyle, pmd, spotbugs, javadoc) passes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for the RuntimeVisibleTypeAnnotations and RuntimeInvisibleTypeAnnotations attributes [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #539:
URL: https://github.com/apache/commons-bcel/pull/539
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for the RuntimeVisibleTypeAnnotations and RuntimeInvisibleTypeAnnotations attributes [commons-bcel]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #539:
URL: https://github.com/apache/commons-bcel/pull/539#issuecomment-6340231172
@nbauma109
Please comment on or address these the following.
## Copying
**`RuntimeVisibleTypeAnnotations.copy(ConstantPool)` and `RuntimeInvisibleTypeAnnotations.copy(ConstantPool)` do not fulfill their documented contract.** Both methods promise a _deep_ copy, but return a shallow clone, ignore the supplied constant pool, and share the original annotation array and mutable `AnnotationEntry` objects. Changing shared contents through the copy can therefore change the original. Existing ordinary-annotation classes have a similar defect, but repeating it here also causes a regression: these attributes previously used `Unknown.copy(ConstantPool)`, which copies its bytes and installs the supplied pool. The new methods should copy mutable contents independently and bind their constant-pool references to the supplied pool. Those references are described in [JVMS §4.7.20](https://docs.oracle.com/javase/specs/jvms/se25/html/jvms-4.html#jvms-4.7.20); the deep-copy requirement comes from BCEL’s Javadoc. The current test compares strings using the original poo
l, so it needs additional checks for independent mutation and a different pool.
## Visting
**`DescendingVisitor.visitTypeAnnotations(TypeAnnotations)` leaves the new visitor support incomplete.** It visits the attribute but does not visit the contained `AnnotationEntry` objects, unlike `DescendingVisitor.visitAnnotation(Annotations)`. With the new fixture, there are five type-annotation attribute callbacks and zero annotation-entry callbacks, despite six annotations being present. Previously, these annotations were inside `Unknown` attributes and were not traversed either, so this is missing functionality rather than a regression. The new method should traverse the wrapped annotation entries, with tests checking their callbacks and visitor stack context. The contained annotation structure is defined in [JVMS 4.7.20](https://docs.oracle.com/javase/specs/jvms/se25/html/jvms-4.html#jvms-4.7.20); the traversal expectation comes from BCEL’s existing visitor behavior.
## TypeAnnotationEntry
**The new `TypeAnnotationEntry` API accepts malformed input, and some new Javadoc is inaccurate.** The public constructor documents `typePath` as a pair of bytes per step, but accepts a one-byte array. `dump(DataOutputStream)` then writes a path length of zero followed by that extra byte, shifting the following annotation fields. Dumping succeeds, but reading the result throws `EOFException`. This affects invalid caller input to the new API; valid parsed entries round-tripped successfully. Validation should reject malformed arrays before writing. [JVMS 4.7.20.2](https://docs.oracle.com/javase/specs/jvms/se25/html/jvms-4.html#jvms-4.7.20.2) defines the two-byte path entries, and [4.7.20.1](https://docs.oracle.com/javase/specs/jvms/se25/html/jvms-4.html#jvms-4.7.20.1) defines the target-info layouts.
## Javadoc
The new Javadoc also needs corrections: `getTypePathLength()` says an empty path identifies the “whole type,” but for nested types it identifies the outermost part where an annotation is allowed; `TARGET_CLASS_EXTENDS` omits interface `extends` clauses; and the new attribute class summaries omit expression types and type-parameter declarations. These cases are described in [JVMS 4.7.20](https://docs.oracle.com/javase/specs/jvms/se25/html/jvms-4.html#jvms-4.7.20).
Could you address each point, or explain the intended BCEL contract if any of these behaviors is intentional?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for the RuntimeVisibleTypeAnnotations and RuntimeInvisibleTypeAnnotations attributes [commons-bcel]
Posted by "nbauma109 (via GitHub)" <gi...@apache.org>.
nbauma109 commented on PR #539:
URL: https://github.com/apache/commons-bcel/pull/539#issuecomment-6336734403
@garydgregory The `28-ea` job of this pull request fails for a reason unrelated to the change: master fails it too, since JaCoCo 0.8.15 cannot instrument Java 28 class files (`Unsupported class file major version 72`). #538 fixes it. Once #538 is merged and this branch is rebased onto master, `28-ea` is expected to pass.
On my fork every other job passes (21 of 22, all operating systems and Java 8 to 27): https://github.com/nbauma109/commons-bcel/pull/103
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for the RuntimeVisibleTypeAnnotations and RuntimeInvisibleTypeAnnotations attributes [commons-bcel]
Posted by "nbauma109 (via GitHub)" <gi...@apache.org>.
nbauma109 commented on PR #539:
URL: https://github.com/apache/commons-bcel/pull/539#issuecomment-6340458694
@garydgregory All four points are addressed in 601010f.
**Copying.** `copy(ConstantPool)` of both attributes now sets the given constant pool on the copy and replaces the entries with deep copies (`TypeAnnotationEntry.copy(ConstantPool)`, which rebuilds each entry and its `AnnotationEntry` against the given pool, keeping the indexes as `Unknown.copy` does). The test checks the pool of the attribute and of the copied annotations, that the entries and annotation objects differ, that changing the copy (adding an element value pair, replacing its table) leaves the original unchanged, and a copy of the invisible attribute.
**Visiting.** `DescendingVisitor.visitTypeAnnotations` visits the `AnnotationEntry` of each entry, while the attribute is on the stack. The test counts 5 attribute callbacks and 6 annotation entry callbacks on the fixture, and checks through `predecessor()` that each annotation is visited within its attribute class.
**TypeAnnotationEntry.** The constructor throws `IllegalArgumentException` for a type path that is not a sequence of pairs of bytes or has more than 255 steps, and for a target info whose size does not match the target type (the `localvar_target` table is checked against its length); an undefined target type still throws `ClassFormatException`. Tests cover each case, and the 255-step limit being accepted.
**Javadoc.** `getTypePathLength()` now says an empty path is the outermost part of the type where an annotation is allowed (`@A Outer.Inner`, whereas `Outer.@A Inner` needs a step); `TARGET_CLASS_EXTENDS` covers the `extends` of an interface; the attribute summaries list the declarations, the supertypes, the type parameters and their bounds, and the expressions and local variables of a `Code` attribute, and the classes that hold the attributes.
The default `mvn` goal passes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org