Make comptime failures loud - #570
Draft
mbenke wants to merge 13 commits into
Draft
Conversation
Inlining restores its fuel unit after each attempt, so the budget returns to its starting value and the remaining amount never records that the evaluator hit the limit. The warning was therefore reachable only through the permanent spend in clone registration: compiling ct_let_ok.solc with --pe-fuel 1 emitted an unfolded `y := double(21)` and said nothing. Record exhaustion when the budget is found empty, and report that. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
The MAST check ran the same structural classification as the SAIL check that precedes it: a call to a pure function with comptime arguments was comptime because it *could* be folded. Nothing asked whether it had been. Anything that stopped partial evaluation short therefore passed silently with a residual call behind the annotation -- exhausted fuel being the general case of which the memory-read hole was one instance. Distinguish the two contexts the check walks. Where comptime values still arrive as parameters, structural classification remains all that can be asked. Where they do not, evaluation has had its chance, so the obligation is met only by a value. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
A comptime parameter only means something while there is still an argument to substitute for it, and '-> comptime' only while there is still a call to fold. By the time a function reaches Hull emission neither is true, so an annotation still standing there is a promise that was never kept -- the argument was passed at runtime, or the call was emitted as a call. Make that a rejection. For it to be a fair one, cloning has to erase every comptime parameter it can, not just the `string` ones: a callee that cannot be inlined -- because it reads storage, say -- was previously left with its comptime parameter intact and its literal argument passed at runtime. Widening the erasure fixes those, and what remains afterwards is genuinely unfixable. The new check runs as a second pass so that the diagnostics naming a specific call site or binding are preferred to it. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
Partial evaluation discharges a comptime binding and erases its declaration, but re-emits any assignment to it, so the emitted Hull referred to a variable that no longer existed -- while the compiler exited 0. Only yule caught it. Reject the assignment in the frontend instead, where a source span still exists, whatever the right-hand side is: erasure breaks a comptime right-hand side exactly as it breaks a runtime one. A Yul block assigning to an enclosing variable is rejected on the same grounds; names the block declares itself are excluded. An unannotated let whose variable is assigned anywhere in the body no longer inherits its initialiser's comptime classification -- what the variable holds later is not what it was initialised with. MastAssign now drops the assigned variable from the MAST checker's comptime environment, which is what an assignment does. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
The fuel warning was printed directly, so --warnings never did not silence it, --warnings deny did not escalate it, and it carried no code for a test to assert against. Give it SC0401 -- a new SC04xx range for the partial-evaluation backend -- and route it through the warning policy like the match-compiler warnings. It carries no label: the evaluator works on Mast, which has no source locations, so an inferred span would point at an arbitrary token of an arbitrary file. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
The MAST checker had a second classifier for functions that still carry comptime parameters or a comptime return -- exactly the functions the residual-annotation check rejects unconditionally. It could therefore never change whether a program was accepted, only which message a doomed one got, while the module header presented the split as load-bearing. Drop it. A discharged comptime obligation is a value, full stop. The comptime environment it was threading goes with it, and so does the stricter purity notion it needed: memory-op purity now has one meaning again, the partial evaluator's. The verifier's messages are now asserted exactly, rather than merely that some error was produced. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
runTestExpectingFailure accepts any failure, so the fixtures for the new rules would keep passing if the compiler started rejecting them for an unrelated reason. Match on the message instead, as a substring so the assertion survives the (bogus) Mast span these diagnostics still carry. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
A clone created by tryCloneComptime drops the parameters it specialises
on, and relied on the evaluator to propagate their literals from a
variable environment. That environment does not survive an opaque
assembly block or a loop -- evalStmts clears it -- and after erasure it
is the parameter's only definition, so the body kept reading a parameter
the signature no longer had. ct_param_erasure_word.solc, on this branch
already, emitted `Mul_mul$word(v, n)` with no `n` in scope: sol-core
exited 0 and yule rejected it.
Substitute the literals into the body up front instead, when the clone is
created. The traversal mirrors substYulBlock, threading the map through
each statement so a let or a pattern variable of the same name shadows as
it should, and handing asm blocks to substYulBlock itself. Reassignment
is handled by refusing to erase a parameter the body assigns to at all,
which leaves the comptime verifier (or EmitHull, for a string) to report
the function rather than a clone quietly disagreeing with itself.
The fixtures for both halves of the erasure rule are now type-checked as
Hull, since the pipeline never runs the Hull checker on its own output
and an emission bug otherwise surfaces only in yule. Compiling every
fixture under test/examples and std shows two output changes, both of
them the intended one: the dangling `n` becomes 4, and a clone that had
folded strlenLit("abcd") to 0 now folds it to 4.
Assisted-By: Claude Opus 5 <noreply@anthropic.com>
`evalFunBody` extracts a callee's return value for inlining, and it walked past every statement-expression on the way. Inlining puts the body in expression position, where a statement has nowhere to leave an effect behind, so a call whose result is discarded but whose effect is not simply vanished. Two consequences. `encodeNone` in test/examples/dispatch/generic_sum.solc lost its whole ABI encoding: the emitted body allocated a zeroed buffer and read it back without ever writing to it, agreeing with the right answer only because `None` encodes as zeros. And a comptime builtin's stub body -- each is `unimplemented(); return 0;`, because folding the real meaning is evalPrimitive's job -- inlined to its placeholder, so `keccakWordLit(<runtime word>)` compiled to `return 0` instead of being reported. Only a statement that folded away entirely may be dropped; anything residual means the body cannot be inlined, and the call survives for the comptime verifier to reject. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
The document was written before any of it was built and still said the comptime flag "is currently ignored by all later phases and lost during specialization" -- the opposite of the truth since `toMastParam` started carrying it. Option C was taken and both of its stages have shipped, so the note now says so up front, cites the modules and line numbers where each piece lives, and corrects the two predictions the implementation did not follow: expression labels reduce to literals in MastEval rather than needing if-else chains in the backend, and an assembly block is not definitionally runtime now that MastEval interprets Yul. The option analysis in sections 3-6 is kept as the record of what was weighed. Section 7 becomes the answers -- syntax, inference, fuel-bounded recursion, label collisions -- plus the two questions still open: MAST rejections carry no source span, and the immutability rule does not reach comptime-only types. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
The claim that assigning to a comptime-only parameter is caught by EmitHull's guard was inferred rather than checked, and it is wrong in both directions: a foldable assignment compiles clean, and one whose result reaches a comptime obligation is caught earlier by the late check. The guard fires only in the third case, as a panic. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
Dropping the strict purity set left comptime-asm.md documenting memOk, asmIsInterpretableWith and computeComptimePureFuns as live API, and both it and comptime-integer.md naming isComptime as the MAST checker's classifier. State the single purity notion, and why the comptime check needs no stricter one: purity guides folding, while the check inspects only its outcome, so an unfoldable memory read is rejected for not being a value rather than for being impure. The ct_asm_mem / ct_asm_mload_runtime analysis is unchanged. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
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.
Three comptime failures were silent. This branch turns each into a rejection
with a message, and pins the messages so they cannot quietly stop working.
Assigning to a comptime binding emitted structurally invalid Hull
MastEvaldrops theletwhen a comptime binding is dischargedbut always re-emits the assignment, so the body referenced an undeclared
variable. Neither comptime checker looked at an assignment target, and the
pipeline never runs
Language.Hull.TypeCheckon its own output, so onlyyulecaught it — well after a green build. The rule now lives in
Solcore.Frontend.ComptimeCheck, where source spans exist: a declaredcomptime binding (
let x : comptime word, or acomptimeparameter) isimmutable, including against assignment from inside an
assemblyblock, whichreaches enclosing variables by name. Parameters merely treated as comptime
because the result is comptime are unaffected — those are assumptions, not
declarations.
Fixing that exposed a flow-insensitivity hole:
if/for/matchdiscardthe environment their bodies produce, so
let x = 5; x = <runtime>; f(x)classified
xas comptime. An unannotatedletwhose name is assignedanywhere in the function now yields
CTDeferred.Fuel exhaustion was a bare
putStrLn, so--warnings neverdid notsilence it,
--warnings denydid not escalate it, and nothing could assert it.It is now
warning[SC0401]through the normal diagnostics path, reported froma sticky flag rather than inferred from the remaining budget.
The Mast-level checker's open/closed function split was decorative.
fcOpenwas true exactly whencheckNothingLeftToSubstituterejectsunconditionally, so the split only chose which message a doomed program got.
Collapsed to
isValue; the module header now states the one rule the passenforces.
Two correctness bugs turned up while writing the fixture for the
stringclone relaxation, both worse than anything above:
tryCloneComptimeerased a parameter from the clone's signature but left itsvalue only in the partial evaluator's variable environment. That environment
is cleared at every opaque
assemblyblock and at everyforloop, so onesloadin the body and the clone referenced a parameter that no longerexisted — again exit 0, again caught only by
yule(Undefined variable: n). The literals are now substituted into the body at clone creation, and aparameter the body assigns to is not erased at all.
evalFunBodywalked past every statement-expression while extracting acallee's return value, so inlining silently dropped any statement whose
result was discarded but whose effect was not.
encodeNoneintest/examples/dispatch/generic_sum.solclost its entire ABI encoding —allocate a zeroed buffer, never write to it, read it back — and passed its
contract test only because
Noneencodes as zeros. The same hole let acomptime builtin's stub body (
unimplemented(); return 0;) inline to itsplaceholder, so
keccakWordLit(<runtime word>)compiled toreturn 0instead of being reported.
A residual call no longer counts as comptime. The Mast-level checker
classified an expression structurally: comptime if it was a literal, a
comptime-bound variable, or a call to a pure function with comptime
arguments. But this pass runs after partial evaluation has already had its
chance at every function, so a call that is still there is one the evaluator
declined to fold — a runtime computation whatever its shape. The obligation
is now discharged only by a value: a literal, or a constructor applied to
values. That is also what makes exhausted fuel a rejection rather than
silently worse code.
This subsumes #566. That PR excluded memory-op functions from the purity
notion feeding the classifier, so an unfoldable
mloadread would stop beingclassified comptime; under the value rule any residual call is rejected,
memory or not, so
computeComptimePureFunsand thememOkflag it neededare gone (its fixture,
ct_asm_mload_runtime.solc, still passes — the fiveasmIsInterpretableWith Falseunit tests went with the function).A second pass then rejects any comptime annotation that survives to emission
at all: every function reaching this point is about to be emitted, so a
comptime parameter means the argument was never substituted, and a comptime
return means no call to it was ever folded.
Assisted-By: Claude Opus 5