Enable ClassRule-based JVM transform selection - #8
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b36c6160e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .toList(); | ||
| List<L1Class> renameClasses = remapClasses; | ||
| List<L1Class> renameClasses = remapClasses.stream() | ||
| .filter(clazz -> pctx.isTransformEnabledForClass(ID, clazz)) |
There was a problem hiding this comment.
When a rule enables renamer for a subclass but leaves its application superclass or interface unrenamed, this filter puts only the subclass in renameClasses. buildMemberMap() can still union the subclass method with the excluded ancestor via unionAppAncestors(), but it only emits a member mapping for classes in renameClasses, so the subclass override is renamed while the ancestor method keeps its original name. In that scoped-renamer inheritance case, virtual dispatch through the excluded base/interface no longer reaches the subclass override.
Useful? React with 👍 / 👎.
| Map<String, Integer> lambdaIndexes = lambdaKeyIndexes(pctx); | ||
| Set<String> unkeyedInboundTargets = unkeyedInboundTargets(pctx); | ||
| for (L1Class clazz : pctx.classMap().values()) { | ||
| if (!pctx.isTransformEnabledForClass(ID, clazz)) continue; |
There was a problem hiding this comment.
With keyDispatch scoped to only part of an application hierarchy, this filter allows an enabled subclass override to receive a hidden long descriptor while an excluded application superclass/interface method with the same slot remains unchanged. Because overridesExternalMethod() only protects external ABI, not excluded in-jar ancestors, a selected Sub.m() can become m(...,J) and stop overriding excluded Base.m(), so virtual calls through the base type dispatch to the wrong implementation.
Useful? React with 👍 / 👎.
| for (L1Class clazz : input.classes()) { | ||
| ctx.setCurrentL1Class(clazz); | ||
| ctx.setCurrentL1Method(null); | ||
| if (!ctx.isTransformEnabledForClass(pass.id(), clazz)) continue; |
There was a problem hiding this comment.
This per-class gate is not enough for controlFlowFlattening: the first enabled class still calls ControlFlowFlatteningPass.transformClass(), and prepareClassKeyTables() builds metadata from all pctx.classMap().values() with application code before ensureClassKeyTable() adds synthetic fields and <clinit> initialization. As a result, a config that enables CFF only for pkg.Target can still mutate excluded or unmatched application classes during class-key-table preparation.
Useful? React with 👍 / 👎.
1 participant
Summary
This PR makes
ClassRuleeffective for JVM transform selection instead of only being parsed and stored.It adds class-scoped transform resolution, wires the pipeline to schedule transforms enabled by either top-level config or matching rules, and applies the same effective class predicate to scoped global-preparation paths such as renamer and keyDispatch.
Changes
transformsare the defaultexclude: truedisables JVM transforms for the matched classPipelineContextso rule matching remains stable after renaming.ClassRuleIntegrationTestcoverage for rule-only scheduling, exclude behavior, rule ordering, inner-class matching, renamer reservation, main-entry behavior, and keyDispatch inbound boundaries.