Paul King created GROOVY-12159:
----------------------------------

             Summary: Synthetic class-literal fields and accessors are emitted 
in hash order, not the order the literals were encountered
                 Key: GROOVY-12159
                 URL: https://issues.apache.org/jira/browse/GROOVY-12159
             Project: Groovy
          Issue Type: Improvement
            Reporter: Paul King
            Assignee: Paul King


h2. Description

{{AsmClassGenerator}} collects the class literals that cannot be emitted as an 
{{ldc}} into an unordered map, and iterates it to generate the synthetic helper 
members:

{code:java}
// AsmClassGenerator.java:284
private final Map<String,ClassNode> referencedClasses = new HashMap<>();

// AsmClassGenerator.java:1857 (visitClassExpression)
String staticFieldName = getStaticFieldName(type);
referencedClasses.put(staticFieldName, type);

// AsmClassGenerator.java:1774 (createSyntheticStaticFields)
for (Map.Entry<String, ClassNode> entry : referencedClasses.entrySet()) {
    ...   // <-- emits the $class$... field and its $get$$class$... accessor
}
{code}

The keys are the synthetic field names, so *the emitted {{$class$...}} fields 
and their {{$get$$class$...}} accessors appear in the hash order of those 
names* rather than in the order the class literals were encountered. Both 
iteration sites are affected: {{createSyntheticStaticFields}} ({{:1774}}) for 
classes and {{createInterfaceSyntheticStaticFields}} ({{:1758}}) for interfaces.

h3. Reachability

This path is narrow. {{visitClassExpression}} emits an {{ldc}} — and never 
touches the map — unless {{BytecodeHelper.isClassLiteralPossible(type)}} is 
false and the type is not in the same compilation unit. That predicate only 
checks for {{public}}:

{code:java}
// BytecodeHelper.java:660
public static boolean isClassLiteralPossible(ClassNode classNode) {
    return Modifier.isPublic(classNode.getModifiers());
}
{code}

So the map is populated only when a class literal names a *non-public type from 
another compilation unit*. Ordinary code never reaches it. This is the least 
impactful of the related ordering issues below, and worth fixing mainly for 
consistency and because the fix is a single word.

h3. Demonstration

Compilation unit 1:
{code:java}
package p
import groovy.transform.PackageScope
@PackageScope class Ha {}
@PackageScope class Hb {}
@PackageScope class Hc {}
@PackageScope class Hd {}
@PackageScope class He {}
{code}

Compilation unit 2, compiled against the first:
{code:java}
class User {
    def f() { [p.Ha, p.Hb, p.Hc, p.Hd, p.He] }
}
{code}

Reading the emitted {{User.class}} with an ASM {{ClassReader}}, the synthetic 
members appear as:

{noformat}
fields   : $class$p$Ha, $class$p$Hc, $class$p$Hb, $class$p$He, $class$p$Hd
accessors: $get$$class$p$Ha, $get$$class$p$Hc, $get$$class$p$Hb, 
$get$$class$p$He, $get$$class$p$Hd
{noformat}

rather than the encounter order {{Ha, Hb, Hc, Hd, He}}.

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, and because within a colliding bucket 
{{HashMap}} chain order follows insertion order, which is the order the 
literals are visited.

h3. Proposed fix

Use an insertion-ordered map:

{code:java}
private final Map<String,ClassNode> referencedClasses = new LinkedHashMap<>();
{code}

The synthetic fields and accessors are then emitted in the order the class 
literals were encountered. The member *set* is unchanged — only the order. The 
map is already {{clear()}}ed per class in {{visitClass}} ({{:355}}), so 
ordering remains scoped to each class.

h3. Test

{{src/test/groovy/org/codehaus/groovy/classgen/SyntheticClassLiteralFieldOrderTest.groovy}}
 compiles the {{@PackageScope}} types into a temp directory as one compilation 
unit, compiles the referencing class as a second unit with that directory on 
the classpath, then reads the emitted bytes with an ASM {{ClassReader}} and 
asserts both the {{$class$...}} fields and the {{$get$$class$...}} accessors 
appear in encounter order. It fails on the current code with {{['Ha', 'Hc', 
'Hb', 'He', 'Hd']}} 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), 
{{AnnotationCollectorTransform.makeListOfAnnotations}} (annotation member order 
for precompiled collectors), and GROOVY-12156 ({{chooseBestMethod}} candidates 
in identity-hash order).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to