[gnodet]

Summary

Builds on top of #1144 (ABI-fingerprint incremental compilation). Replaces the ASM bytecode analysis with the standard java.lang.classfile API when running on JDK 24 or later, keeping ASM as a transparent fallback on older JVMs.

Base branch: This PR is stacked on feature/abi-incremental (#1144). Please review after #1144 is merged; the diff against master includes the #1144 commits.

Design

Three new classes in the incremental package:

Class	Role
ClassAnalyzer	Strategy interface: analyze(byte[]) → ClassAnalysis
AsmClassAnalyzer	Existing ASM logic, extracted from BytecodeAnalyzer
ClassfileClassAnalyzer	New impl using java.lang.classfile (JDK 24+)
Sha256	Shared hashing helper, extracted from BytecodeAnalyzer

BytecodeAnalyzer becomes a thin public facade. At class-load time it checks Runtime.version().feature() >= 24 and loads ClassfileClassAnalyzer reflectively, falling back to AsmClassAnalyzer if unavailable.

Compilation

ClassfileClassAnalyzer is compiled separately with --release 24 via a dedicated maven-compiler-plugin execution pointing at src/main/java24/. The main sources remain at the baseline Java version (17). Both outputs are bundled in the same plugin JAR.

Correctness

The new cross-validation test asmAndClassfileAnalyzersProduceIdenticalResults compiles a test class that exercises constants, generics, inheritance, exception types, and bytecode instructions, then asserts that both implementations produce identical className, abiCanonical, abiFingerprint, and referencedTypes results.

Impact

• JDK ≥ 24: uses java.lang.classfile — no ASM processing overhead.
• JDK < 24: transparent fallback to ASM, no behaviour change.
• All 98 unit tests pass (JDK 25).

[gnodet-bot]

COMMENT — Well-designed architecture with clean separation of concerns. The incremental compilation engine is solid and the maker/checker ABI-fingerprinting approach is sound. A few issues need attention before this is merge-ready.

Key findings:

1. 🔴 pom.xml: The compile-java24 execution is unconditional — it will break CI on JDK 8/21 (the default CI matrix includes ["21", "25", "8"]). Needs a JDK-gated profile.
2. ⚠️ AbiExtractor: Contains a private sha256() that duplicates Sha256.hash() — same algorithm, same truncation. Should delegate to the shared utility.
3. ⚠️ DependencyScanner: Lambda expressions are not explicitly handled. Since LambdaExpressionTree is a subtype of ExpressionTree, lambda bodies are scanned by the default TreePathScanner traversal without incrementing bodyDepth — type references inside lambda bodies will be classified as signature deps if the lambda appears in a signature-level context (e.g., a default field initializer before the initializer bodyDepth bump kicks in, or an annotation value). This is conservative (over-cascading) but worth documenting.
4. 💡 IncrementalState.copy(): The types map contains TypeInfo records with Set<String> fields. Since TypeInfo is a record and the sets may be mutable TreeSets, the copy shares those set instances. A mutation by the caller of copy() on a TypeInfo's set would corrupt the original. In practice this doesn't happen today, but it's a latent bug.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

🔴 Critical: This compile-java24 execution is unconditional. The CI matrix for this project tests on JDK 8, 21, and 25 (via apache/maven-gh-actions-shared). JDK 8 and 21 do not support --release 24, so javac will fail with an error like error: release version 24 not supported.

This execution must be gated behind a JDK-version-activated profile:

<profiles>
  <profile>
    <id>java24+</id>
    <activation>
      <jdk>[24,)</jdk>
    </activation>
    <build>
      <plugins>
        <plugin>
          <groupId>org.apache.maven.plugins</groupId>
          <artifactId>maven-compiler-plugin</artifactId>
          <executions>
            <execution>
              <id>compile-java24</id>
              <goals><goal>compile</goal></goals>
              <phase>compile</phase>
              <configuration>
                <compileSourceRoots>
                  <compileSourceRoot>${project.basedir}/src/main/java24</compileSourceRoot>
                </compileSourceRoots>
                <release>24</release>
              </configuration>
            </execution>
          </executions>
        </plugin>
      </plugins>
    </build>
  </profile>
</profiles>

The reflective fallback in BytecodeAnalyzer.selectAnalyzer() already handles the case where ClassfileClassAnalyzer is absent — it falls back to ASM. So the plugin JAR just won't contain the java24 class when built on older JDKs, which is fine.

[gnodet-bot]

⚠️ Duplication: This sha256() method is functionally identical to Sha256.hash() — same algorithm, same 16-char truncation. Use the shared utility instead:

Suggested change

	private static String sha256(String input) {
	try {
	var md = MessageDigest.getInstance("SHA-256");
	byte[] hash = md.digest(input.getBytes(StandardCharsets.UTF_8));
	var hex = new StringBuilder();
	for (byte b : hash) {
	hex.append(String.format("%02x", b));
	}
	return hex.substring(0, 16);
	} catch (java.security.NoSuchAlgorithmException e) {
	throw new RuntimeException(e);
	}
	}
	}
	private static String sha256(String input) {
	return Sha256.hash(input);
	}

Or better yet, just call Sha256.hash() directly at the call site (line 55) and remove this method entirely.

[gnodet-bot]

⚠️ Same duplication: This sha256(byte[]) method also duplicates the hashing logic. The Sha256 utility class only has hash(String), but this method hashes byte[] and returns the full 64-char hex (not truncated to 16 chars like Sha256.hash()). The inconsistency is intentional here (source content hashes vs ABI fingerprints), but it would be cleaner to centralize both variants in Sha256 with clear naming:

• Sha256.fingerprint(String) → 16-char truncated (for ABI)
• Sha256.digest(byte[]) → full hex (for content hashing)

[gnodet-bot]

💡 Lambda handling gap: visitMethod correctly increments bodyDepth for method bodies. But LambdaExpressionTree is not overridden — lambda bodies are traversed by the default scanner without adjusting bodyDepth.

This matters when a lambda appears in a signature-level position that the scanner hasn't entered a body for yet. For example, a constant field initializer with a lambda:

public static final Comparator<Foo> CMP = (a, b) -> new Helper().compare(a, b);

Here Helper would be classified as an impl dep (field initializers bump bodyDepth via visitVariable), so the lambda case is partially covered. But in annotation values or other edge cases, the missing override could misclassify.

This is conservative (over-cascading, not under-cascading) so it won't cause missed recompilations, but adding:

@Override
public Void visitLambdaExpression(LambdaExpressionTree node, Void p) {
    bodyDepth++;
    var result = super.visitLambdaExpression(node, p);
    bodyDepth--;
    return result;
}

would make the classification more precise.

[gnodet-bot]

💡 Shallow copy of mutable sets in copy(): TypeInfo is a record with Set<String> fields. When copy() does copy.types.putAll(this.types), the TypeInfo instances are shared between original and copy since records are immutable references but their contained sets may be mutable TreeSets.

If any code path ever calls removeTypesForSource() on the copy and then modifies the sets of a remaining TypeInfo, the original's data would be corrupted. The current code doesn't hit this because TypeInfo sets are never mutated after construction, but it's a latent correctness hazard.

Consider either:

1. Making TypeInfo sets defensive copies in the constructor: Set.copyOf(signatureDeps)
2. Or deep-copying in copy()

[gnodet-bot]

💡 JAR identity based on size+mtime: This caching strategy can produce false positives when:

1. A JAR is rebuilt deterministically (same content, same mtime via SOURCE_DATE_EPOCH or similar)
2. A JAR is replaced atomically and the FS rounds mtime to the same second

Both are edge cases. For most Maven builds this is fine because JAR rebuilds update mtimes. Just noting that an alternative like a content hash (MD5/SHA-1 of the JAR header) would be more robust if this ever becomes an issue.

[gnodet]

Folded into #1144 directly.