fix(java): javac holds the step frame's invariant, and the budget gate stops measuring the wrong class - #409
Merged
Merged
Conversation
…te measuring the wrong class Three defects from an adversarial review of the commits that never had one. The step switch's arms return (6c2d9e6) but its `default` still broke, so the end of the switch stayed reachable and javac accepted a statement after it -- dead on all twelve MATypes, reached only by the unreachable default. Before 6c2d9e6 such a tail ran on every arm and was correct, so that commit is what turned it into a trap. C's sibling has exactly that tail (ta_MA.c:900-902); Java only avoids it because the bookkeeping lives in `update`. `default` now returns too: measured 305 bytes either way, byte-identical class file, and javac rejects the tail as unreachable -- the enforcement is free. The gate searched the concatenated javap output, so a signature was bound to whatever class printed first. Demonstrated: with `Core` dumped first and a same-shaped `peek(double)` on it, the old script measured MaStream.peek as 2 bytes and reported green. It now parses per class and refuses an ambiguous match. And `check=True` was inert: javap exits 0 when only SOME named classes resolve, naming the rest on stderr, which the script captured and discarded. A missing Core$MaStream.class therefore failed with "the signature moved -- fix the pattern", sending the reader to edit a correct regex. stderr is now a rejection in its own right. The step emitter's comment claimed the frame was "4 bytes over" the budget -- true before that same commit, false after it at 20 under, and never right again as the enum grows. It now states the invariant instead: nothing may follow the switch, and the default must keep returning or the trap comes back. Claude-Session: https://claude.ai/code/session_01N7HZcFbUwe3tB9XpKxkFPK
# Conflicts: # ta_codegen/output/java/library/src/main/java/io/github/talib/BuildStamp.java # ta_codegen/output/java/tools/TaCodegenServe.java
# Conflicts: # ta_codegen/output/java/library/src/main/java/io/github/talib/BuildStamp.java # ta_codegen/output/java/tools/TaCodegenServe.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #406, from an adversarial review of the three commits in that series that never got one. Three defects, all confirmed by reproduction rather than argument.
1. The step change turned a working tail into a silently dead one
6c2d9e6c6made every MAType arm ofmaStepImplreturn, but leftdefault: break;. That keeps the end of the switch reachable, so javac accepts a statement after it — dead on all twelve real MATypes, reached only by the unreachable default. Before that commit such a tail ran on every arm and was correct, so the commit is precisely what turned it into a trap.Not hypothetical: C's sibling
TA_MA_Updatehas exactly that tail (ta_MA.c:900-902). Java only avoids it becauseoutRangeCount++lives inupdate.defaultnow returns too. Measured 305 bytes either way, byte-identical class file, and javac now rejects the tail aserror: unreachable statement— the enforcement costs nothing. The peek frame already had this, because its default throws.2. The gate could measure the wrong class and report green
code_length()searched the concatenatedjavapoutput, so a signature bound to whichever class printed first. Demonstrated by givingCorea same-shapedpublic double peek(double)and dumping it first: the old script reportedCore$MaStream.peek 2 bytes (budget 325, -323)and exited 0.It now splits the disassembly per class and refuses an ambiguous match.
3.
check=Truewas inertjavapexits 0 when only some named classes resolve, naming the rest on stderr — which the script captured and threw away. So a missingCore$MaStream.classfailed with "the signature moved — fix the pattern in this script", sending the reader to edit a regex that was correct. stderr is now a rejection in its own right, and an absent class section is its own error.4. A comment that its own commit made false
The step emitter said the frame was "4 bytes over" the budget — true before that commit, false after it at 20 under, and never right again as the enum grows. It now states the invariant instead: nothing may follow the switch, and the default must keep returning.
Verification
Re-proved every way the gate can read green: missing class, over-budget classes, empty dir, decoy signature, reversed class order. Generator suite 943/0, clippy clean, regen-check with no exclusions, six Java suites (7,164 checks).
https://claude.ai/code/session_01N7HZcFbUwe3tB9XpKxkFPK