Paul King created GROOVY-12157:
----------------------------------
Summary: Covariant bridge methods are emitted in hash order, not
the order they are found
Key: GROOVY-12157
URL: https://issues.apache.org/jira/browse/GROOVY-12157
Project: Groovy
Issue Type: Improvement
Reporter: Paul King
Assignee: Paul King
h2. Description
{{Verifier.addCovariantMethods}} collects the bridge methods it needs to
generate into unordered maps and then iterates them to emit the methods:
{code:java}
// Verifier.java:1679-1680
Map<String, MethodNode> absInterfaceMethods = new HashMap<>();
Map<String, MethodNode> allInterfaceMethods = new HashMap<>();
...
// Verifier.java:1719
Map<String, MethodNode> methodsToAdd = new HashMap<>();
addCovariantMethods(classNode, declaredMethods, absInterfaceMethods,
methodsToAdd, genericsSpec);
if (!methodsToAdd.isEmpty()) {
...
for (Map.Entry<String, MethodNode> entry : methodsToAdd.entrySet()) {
// we skip bridge methods implemented in current class already
MethodNode mn = declaredMethodsMap.get(entry.getKey());
if (mn == null || !mn.getDeclaringClass().equals(classNode)) {
addPropertyMethod(entry.getValue()); // <-- emits into the class
file
}
}
}
{code}
The three maps chain into each other — {{allInterfaceMethods}} is iterated to
populate {{absInterfaceMethods}} ({{:1699}}), which is iterated as
{{interfaceMethods.values()}} ({{:1750}}) to populate {{methodsToAdd}}, which
is iterated to call {{addPropertyMethod}}. So *the order of the generated
bridge methods in the emitted class file is the hash order of the
type-descriptor keys*, not the order in which the methods were discovered.
h3. Demonstration
{code:java}
interface Alpha { Object alpha() }
interface Beta { Object beta() }
interface Gamma { Object gamma() }
interface Delta { Object delta() }
interface Epsilon { Object epsilon() }
class Sample implements Alpha, Beta, Gamma, Delta, Epsilon {
String alpha() { null }
Integer beta() { null }
Long gamma() { null }
Double delta() { null }
Short epsilon() { null }
}
{code}
{{javap -p Sample.class}} shows the five declared methods in declaration order,
followed by the five generated bridges in an arbitrary order:
{noformat}
public java.lang.String alpha();
public java.lang.Integer beta();
public java.lang.Long gamma();
public java.lang.Double delta();
public java.lang.Short epsilon();
public java.lang.Object alpha(); <-- bridges, in hash order:
public java.lang.Object gamma(); alpha, gamma, epsilon, beta, delta
public java.lang.Object epsilon();
public java.lang.Object beta();
public java.lang.Object delta();
{noformat}
h3. Impact
The keys are type-descriptor {{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. Two reasons it still
matters:
* The *insertion* order into these maps comes from {{ClassNode.getMethods()}}
of the supertypes, which for reflection-configured {{ClassNode}}s is the
unspecified JDK reflection order being addressed in GROOVY-12149. Within a
colliding bucket, {{HashMap}} chain order follows insertion order, so the
emitted bridge order is coupled to that nondeterminism.
* Even where it is stable, the emitted order is arbitrary rather than
meaningful, which makes generated class files harder to diff and reason about.
h3. Proposed fix
Use insertion-ordered maps for the three that are iterated to produce output:
{code:java}
Map<String, MethodNode> absInterfaceMethods = new LinkedHashMap<>();
Map<String, MethodNode> allInterfaceMethods = new LinkedHashMap<>();
...
Map<String, MethodNode> methodsToAdd = new LinkedHashMap<>();
{code}
Bridge methods are then emitted in the order they are discovered (declaration
order in the example above: {{alpha, beta, gamma, delta, epsilon}}). The method
*set* is unchanged — only the order.
{{declaredMethodsMap}} ({{:1724}}) is lookup-only and {{defaultMethods}}
({{:323}}) feeds duplicate-default-method diagnostics only, so neither needs to
change.
h3. Test
{{src/test/groovy/org/codehaus/groovy/classgen/CovariantBridgeMethodOrderTest.groovy}}
compiles the source above via {{CompilationUnit}}, reads the emitted bytes
with an ASM {{ClassReader}} (the pattern already used by
{{Groovy11776.groovy}}), and asserts the bridge methods appear in declaration
order. It fails on the current code with {{['alpha', 'gamma', 'epsilon',
'beta', 'delta']}} and passes with the fix.
h3. Related
GROOVY-12149 (nondeterministic reflection order flows into generated bytecode)
— feeds the insertion order of these maps. GROOVY-12156 (chooseBestMethod
returns candidates in identity-hash order) — same class of defect in the static
type checker.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
