A fault is never converted to a boolean - #425
Merged
Merged
Conversation
The plan was to make `test` abort in a condition like `[[ ]]` does.
Probing every position first showed `[[ ]]` was the wrong reference: it
aborts inside an `if` and reads as false inside a `||`, so it did not
agree with itself, and `(( ))` had the same split.
statement if/while/! && / || (left)
[[ ]] exit 2 abort reads false
test exit 2 reads false reads false
(( )) exit 2 abort reads false
The `||` row is the one that does damage. All three printed a
conclusion drawn from a comparison that never happened:
x=abc; [[ "$x" -eq 1 ]] || echo "concluded: not one"
That is the silent coercion the number rules exist to refuse, reached
through control flow instead of arithmetic.
The rule now is one sentence: a fault is never converted to a boolean.
Where something consumes the result as a boolean there is no true or
false to give it, so the statement aborts. Where nothing does — a
standalone statement, or a chain's RIGHT operand, whose value simply
becomes the chain's value — it is exit 2 with the message and execution
continues.
`ExecResult::fault` carries the distinction, because a command cannot
say "I could not decide" through an exit code that callers already read
as false. It is set by the three fault sources and read at the two
places a boolean is demanded: a condition's command, and a chain's left
operand. `accumulate_result` assigns it like `code`, so a fault reaching
an outer chain through an inner chain's tail is still seen.
A command that ran and failed is not a fault, and that boundary is
tested: `grep` matching nothing and `false` still select `else` and
still drive `||`. Only an operand that cannot be compared aborts.
The controls carry the weight here. Every abort test is paired with a
sound comparison in the same position, because a rule that aborts too
eagerly would pass every abort test and destroy ordinary control flow.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… into fix/fault-never-becomes-boolean # Conflicts: # CHANGELOG.md
tobert
added a commit
that referenced
this pull request
Aug 31, 2026
Independent of #421-#425 — this one is off main and touches only the REPL binary and its presentation tests. An earlier fix moved parse and lexer failures out of the `Error: execution failed` / `Caused by:` wrapper so their diagnostic leads. It stopped there, and `error_presentation_tests.rs` pinned the remaining classes to record that boundary rather than to endorse it. Everything else that fails during execution was still buried. A validation failure produces the same `line:col [code]: message` shape a parse error does, and it arrived three lines down: ``` Error: execution failed Caused by: validation failed: error [E012]: bare variable in for loop iterates once … → wrap it in $(...) … ``` The suggested rewrite — the part that tells the reader what to do — was the fifth line of output. Assume the context is truncated and the whole message is gone. It now reads: ``` validation failed: error [E012]: bare variable in for loop iterates once … → wrap it in $(...) … ``` `execution failed` named the phase and nothing else, and as the outermost context anyhow makes it the headline. Removed. Callers print the diagnostic directly rather than letting `main` re-wrap it with an `Error:` prefix and a `Caused by:` split, the same way the parse path already prints. The boundary that remains is between a generic context and a specific one. `Failed to read script: <path>` names the file and the action, which the io error under it does not, so it stays and is still pinned. The rule is not "no context", it is "no context that displaces the message with less information than the message". Untouched and still passing: parse, lexer, command-not-found, a nonzero exit, and a builtin's own error, none of which went through the wrapper. Exit codes are unchanged. Gates: test --all, clippy (0 warnings), rustdoc, no-default-features, wasi, and insta all green.
…es-boolean # Conflicts: # CHANGELOG.md
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
1 participant
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.
Stacked on #424 — review that one first; this PR's diff is against it, not against main.
The plan was to make
testabort in a condition the way[[ ]]does. Probing every position first showed[[ ]]was the wrong reference. It aborts inside anifand reads as false inside a||, so it did not agree with itself, and(( ))had the same split.The
||row is the one that does damage. All three printed a conclusion drawn from a comparison that never happened:That is the silent coercion the number rules exist to refuse, reached through control flow instead of arithmetic.
The rule is now one sentence: a fault is never converted to a boolean. Where something consumes the result as a boolean — an
if/whilecondition, a!, or the left operand of&&/||— there is no true or false to give it, so the statement aborts. Where nothing does, including a chain's right operand whose value simply becomes the chain's value, it is exit 2 with the message and execution continues.ExecResult::faultcarries the distinction, because a command cannot say "I could not decide" through an exit code callers already read as false. It is set by the three fault sources and read at the two places a boolean is demanded.A command that ran and failed is not a fault:
grepmatching nothing andfalsestill selectelseand still drive||. Every abort test is paired with a sound comparison in the same position, since a rule that aborted too eagerly would pass every abort test while destroying ordinary control flow.Known follow-up, deliberately not in this PR: an abort still prints
Error: execution failedwith the real message underCaused by:. Parse and lexer failures were moved out of that wrapper previously anderror_presentation_tests.rspins the remaining classes as an explicit scope boundary, so moving faults out belongs in that work, not here.Gates: test --all, clippy (0 warnings), rustdoc, no-default-features, wasi, and insta all green.