Paul King created GROOVY-12160:
----------------------------------
Summary: @AnnotationCollector emits collected annotation members
in hash order when the collector is precompiled
Key: GROOVY-12160
URL: https://issues.apache.org/jira/browse/GROOVY-12160
Project: Groovy
Issue Type: Improvement
Reporter: Paul King
Assignee: Paul King
h2. Description
When a collector annotation is precompiled, {{AnnotationCollectorTransform}}
reads its preset members back from the generated {{CollectorHelper}} and copies
them onto the annotations the collector expands to. That copy goes through an
unordered map:
{code:java}
// AnnotationCollectorTransform.java:316-337 (makeListOfAnnotations)
for (Object[] inner : data) {
Class<?> anno = (Class<?>) inner[0];
AnnotationNode toAdd = new AnnotationNode(ClassHelper.make(anno));
ret.add(toAdd);
Map<String,Object> member = (Map<String, Object>) inner[1];
if (member.isEmpty()) {
continue;
}
Map<String, Expression> generated = new HashMap<>(member.size());
for (Map.Entry<String, Object> entry : member.entrySet()) {
generated.put(entry.getKey(), makeExpression(entry.getValue()));
}
copyMembers(generated, toAdd); // iterates `generated` ->
toAdd.addMember(...)
}
{code}
{{copyMembers}} ({{:274}}) iterates that map into {{AnnotationNode.addMember}},
and those members are written to the class file's annotation attribute in that
order. So *the members of the expanded annotation are emitted in the hash order
of their names*.
Declaration order is intact everywhere else in the chain and is lost only here:
{{serialize(AnnotationNode)}} ({{:184}}) builds the helper's map by iterating
{{AnnotationNode.getMembers()}}, which is a {{LinkedHashMap}}, so the order the
members were declared on the collector survives into the precompiled
{{CollectorHelper}}.
h3. Reachability
Only when the collector is *precompiled* — i.e. resolved from the classpath, so
the members are read reflectively from its {{CollectorHelper}} via
{{getTargetListFromClass}} ({{:280-290}}). A collector declared in the same
compilation unit takes the AST-based path, which preserves order. The
precompiled case is the normal one for any collector shipped in a library,
including Groovy's own {{@Immutable}} and {{@Canonical}}.
h3. Demonstration
Compilation unit 1 (the collector, presetting five members):
{code:java}
import groovy.transform.AnnotationCollector
import java.lang.annotation.*
@Retention(RetentionPolicy.RUNTIME) @Target(ElementType.TYPE)
@interface Marker {
String alpha() default ''
String beta() default ''
String gamma() default ''
String delta() default ''
String epsilon() default ''
}
@Marker(alpha='a', beta='b', gamma='c', delta='d', epsilon='e')
@AnnotationCollector
@interface Coll {}
{code}
Compilation unit 2, compiled against the first:
{code:java}
@Coll
class Holder {}
{code}
Reading the emitted {{Holder.class}} with an ASM {{ClassReader}}, the members
of the {{@Marker}} annotation appear as:
{noformat}
delta, epsilon, beta, gamma, alpha
{noformat}
rather than the declared order {{alpha, beta, gamma, delta, epsilon}}.
h3. Impact
The keys are {{String}}s and {{String.hashCode}} is stable, so for a given key
set the iteration order is reproducible across JVM runs — this is not by itself
a live reproducible-builds failure. It still matters because the emitted order
is arbitrary rather than meaningful, it lands in the
{{RuntimeVisibleAnnotations}} attribute of the class file, and within a
colliding bucket {{HashMap}} chain order follows insertion order. Of the
related ordering issues below, this one has the most ordinary trigger: any
classpath collector with more than one preset member.
h3. Proposed fix
Use an insertion-ordered map:
{code:java}
Map<String, Expression> generated = new LinkedHashMap<>(member.size());
{code}
The collected members are then emitted in the order they were declared on the
collector. The member *set* and their values are unchanged — only the order.
h3. Test
{{src/test/groovy/org/codehaus/groovy/transform/AnnotationCollectorMemberOrderTest.groovy}}
compiles the annotation and collector into a temp directory as one compilation
unit, applies the collector from a second unit with that directory on the
classpath (so the precompiled path is exercised), then reads the emitted bytes
with an ASM {{ClassReader}} and asserts the annotation's members appear in
declaration order. It fails on the current code with {{['delta', 'epsilon',
'beta', 'gamma', 'alpha']}} and passes with the fix.
h3. Related
GROOVY-12149 (nondeterministic reflection order flows into generated bytecode).
Same class of defect: {{Verifier.addCovariantMethods}} (covariant bridge
methods emitted in hash order),
{{AutoImplementASTTransformation.getAllCorrectedMethodsMap}}
({{@AutoImplement}} methods emitted in hash order), {{AsmClassGenerator}}
{{referencedClasses}} ({{$class$}} synthetic fields and accessors), and
GROOVY-12156 ({{chooseBestMethod}} candidates in identity-hash order).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
