Skip to content

Commit 2da0b2c

Browse files
phtcostaphtcosta
committed
docs(gh62): round-6 consistency fixes — 5 critical residuals (refs #62)
Rigorous consistency sweep against the round-6 redesign surfaced five critical residual references to round-5 nominal-family closures that contradicted the new demand-driven D8 scope: 1. tasks.md §4.3 TargetGrammarTest.targetNameStandaloneBinding: was COVERED "after round-5 family flip"; round-6 dropped standalone TargetPC (zero demand). Now @disabled SILENT-GAP. 2. tasks.md §4.5 ArgsGrammarTest.argsNameStandaloneBinding: same fix. 3. tasks.md §4.14 NamedReferenceGrammarTest.resolvesAgainstAspectTable: referenced AspectDescriptor.namedPointcuts() (which does not exist). Renamed to resolvesAgainstCommonPointcut + added unresolvedReferenceFallsBackWithWarn (§4.D in-change closure). 4. tasks.md §4.19 CompositionGrammarTest.negationBeyondWithin: referenced !handler/!cflow/!if specialization (round-5 family, zero demand). Renamed to negativeTargetArgsParserSpecialization (§4.N). Dropped callTargetArgsIfFourWayBinding (depended on dropped reflective API substrate). 5. tasks.md §4.20 JoinPointReflectiveApiGrammarTest: declared all rows COVERED via "§4.R below" (which no longer exists). All methods now @disabled SILENT-GAP, deferred to Follow-up (ledger entry "Full shipped org.aspectj.lang.* runtime substrate" covers these rows). Plus minor alignment fixes: - design.md §Context intro: "Three rounds of cross-LLM review" -> "Four rounds" (adds round-6) with one-line round-6 summary. - design.md mapping table R5: WithinGrammarTest -> WithinFamilyGrammarTest (consistent with tasks.md §4.6). - design.md mapping table R6: CallGrammarTest -> CallPointcutGrammarTest + callOwnerSubtypeExpansion -> callTSubtypeInOwner (consistent with tasks.md §4.1 enumeration). - tasks.md §4.22 sparseSwitch comment: refer to round-6 closures (after-throwing/staticinit/within positive) instead of dropped Reflective API + Matcher Correctness. - tasks.md §6.S.1 APK selection: cite the 3 specific __STATICSIG corpus sites by filename for reviewer auditability. Anchor verification (PointcutMatcher.java:106-108, :109-114, :153-157, :161-167; DexWeaver.java:560-566) confirmed literally against HEAD. openspec validate --strict PASS.
1 parent 06ddf4f commit 2da0b2c

2 files changed

Lines changed: 10 additions & 10 deletions

File tree

‎rv-android/openspec/changes/gh62-aspectj-grammar-coverage/design.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
The dexlib2 instrumenter has been built incrementally against the demand of the JCA spec set since `gh52` (initial port, 2026-Q1). Each subsequent gap — `gh56` wide-param binding, `gh57` static-analysis overhaul, `gh59` wide-slot tracking, `gh61` RegisterShifter frame growth + `Object+` in `call()` params — was framed and solved as a *single visible bug*. Production code that is silently wrong but parses-and-compiles is invisible to that posture.
66

7-
Three rounds of cross-LLM artifact review (Codex GPT-5, DeepSeek, Gemini 2.5 Pro, Claude Opus 4.7) — the third of which audited the round-4 state of this change — inverted that lens. The reviews surfaced **four** classes of silent failure in the dexlib2 parser/matcher/emitter (round-5 added the fourth, WRONG-DATA, after opus47 identified that `ThisJoinPointEmitter.signatureFor()` returns the raw pointcut expression string as a `Signature` payload — the emitter populates wrong data, not empty data; this still classifies as `SILENT-GAP` under the 4-value vocabulary because the divergence is invisible without an external oracle, but the Evidence column for affected rows now precisely describes the wrong-payload mechanism instead of the inaccurate "advice body not populated" framing):
7+
Four rounds of cross-LLM artifact review (Codex GPT-5, DeepSeek, Gemini 2.5 Pro, Claude Opus 4.7) — round-3 audited the round-4 state, round-5 expanded D8 by nominal AspectJ family, and round-6 (opus47_deep + four supporting reports) empirically refuted three round-5 premises and reframed D8 by corpus demand — inverted the reactive lens. The reviews surfaced **four** classes of silent failure in the dexlib2 parser/matcher/emitter:
88

99
1. **Matcher always-match for unmodelled designators.** `PointcutMatcher.java:109-114` treats `IfPC`, `NamedRefPC`, and `WithinPC` as always-match. The parser routes every unmodelled *identifier-named* designator to `NamedRefPC` (`PointcutExpressionParser.java:131-132`), so `this`, `withincode`, `cflow`, `cflowbelow`, `handler`, `initialization`, `preinitialization`, `get`, `set`, and `adviceexecution` all silently match every join point. The AspectJ 5 `@*` family (`@annotation`, `@target`, `@this`, `@args`, `@within`, `@withincode`) reaches a *different* failure mode: `PointcutExpressionParser.isIdentPart()` rejects `@` as part of an identifier, so the parser raises `PointcutParseException` before the `NamedRefPC` fallback fires. Same end-state verdict (SILENT-GAP — the exception is caught upstream and the pointcut is treated as inert) but a distinct code path. Matrix rows for the `@*` family MUST cite the parser-crash anchor (`PointcutExpressionParser.isIdentPart()`), NOT the matcher always-match anchor at `PointcutMatcher.java:109-114`.
1010
2. **Malformed-descriptor exact-match for partially-modelled forms.** `T+` in `call()` owner position (`PointcutMatcher.java:153-157`), `T+` in return type, `*` wildcard in method name, and trailing-mixed `(T, ..)` varargs all produce descriptors like `Ljava/io/OutputStream+;` or `Ljava/lang/..;` that never match anything.
@@ -104,8 +104,8 @@ Relevant PRD references: **FR02** (APK instrumentation — round-6 D8 ships eigh
104104
| INV-INS-92 (bidirectional: tests → matrix) | `grammar-tests/` test classpath + matrix | `testEnabledTestsResolveToCoveredOrExplicitNoOpRow` + `testDisabledTestsResolveToSilentGapRow` + `testSkipCountEqualsSilentGapCount`. Closure atomicity enforced by `MatrixIntegrityTest` running in CI at commit time — a closure that flips the test annotation without also flipping the matrix row breaks the build (see D6) |
105105
| INV-INS-93 (demand counts via portable Java; `.aj`+`.mop` scan) | `DemandCounter` | `testDemandCountsReproducible` |
106106
| `condition(...)` MOP guard emit (R4) | `advice-emitter/.../ConditionGuardEmitter.java` (NEW) + `EmitterDispatch` wiring | `ConditionGrammarTest.conditionShortCircuitsMonitorInvoke` + `MatrixIntegrityTest.testDemandDrivenClosuresAreCovered` |
107-
| Positive `within(typePattern)` matcher (R5) | `pointcut-engine/.../WithinPC` (NEW or refactor) reusing `matchesTypePattern` from `NotWithinPC:343-359` | `WithinGrammarTest.withinPositiveFiltersClassDef` + `MatrixIntegrityTest.testDemandDrivenClosuresAreCovered` |
108-
| `T+` in owner + method-name glob + `!target/!args` matcher fixes (R6) | `pointcut-engine/.../PointcutMatcher` owner subtype expansion (§4.O) + method-name `startsWith` (§4.X) + `PointcutExpressionParser.parseUnary` negation specialization (§4.N) | `CallGrammarTest.callOwnerSubtypeExpansion`, `CallGrammarTest.methodNamePrefixGlob`, `CompositionGrammarTest.negativeTargetArgsParserSpecialization` + `MatrixIntegrityTest.testDemandDrivenClosuresAreCovered` |
107+
| Positive `within(typePattern)` matcher (R5) | `pointcut-engine/.../WithinPC` (NEW or refactor) reusing `matchesTypePattern` from `NotWithinPC:343-359` | `WithinFamilyGrammarTest.withinPositiveFiltersClassDef` + `MatrixIntegrityTest.testDemandDrivenClosuresAreCovered` |
108+
| `T+` in owner + method-name glob + `!target/!args` matcher fixes (R6) | `pointcut-engine/.../PointcutMatcher` owner subtype expansion (§4.O) + method-name `startsWith` (§4.X) + `PointcutExpressionParser.parseUnary` negation specialization (§4.N) | `CallPointcutGrammarTest.callTSubtypeInOwner`, `CallPointcutGrammarTest.methodNamePrefixGlob`, `CompositionGrammarTest.negativeTargetArgsParserSpecialization` + `MatrixIntegrityTest.testDemandDrivenClosuresAreCovered` |
109109
| `__STATICSIG` macro + `staticinitialization` synthesis + `after throwing` install (R7) | `advice-emitter/.../StaticSigEmitter.java` (NEW) + `StaticInitSynthesizer.java` (NEW) + `DexWeaver.applyPlan` TRY_CATCH_WRAP install | `StaticSigGrammarTest.staticSigMacroExpandsToInlineSignature`, `StaticInitializationGrammarTest.synthesizesClinitWhenAbsent`, `AfterThrowingGrammarTest.installsTryRangeAndHandler` + `MatrixIntegrityTest.testDemandDrivenClosuresAreCovered` |
110110
| `NamedRefPC` resolver via `commonPointcut` (R8) | `pointcut-engine/.../NamedRefPC` + `AspectDescriptor.getCommonPointcut()` plumbed through `PointcutMatcher.Context` | `NamedReferenceGrammarTest.resolvesAgainstCommonPointcut`, `NamedReferenceGrammarTest.unresolvedReferenceFallsBackWithWarn` + `MatrixIntegrityTest.testDemandDrivenClosuresAreCovered` |
111111
| INV-INS-94 (demand-driven closures COVERED) | Eight demand-driven closures above | `testDemandDrivenClosuresAreCovered` |

0 commit comments

Comments
 (0)