Skip to content

Parse $'' and $"" as redirect targets, and process substitutions after <<< - #19

Merged
webpro merged 1 commit into
webpro-nl:mainfrom
gkurt:redirect-targets
Nov 1, 2026
Merged

webpro merged 1 commit into
webpro-nl:mainfrom
gkurt:redirect-targets

Conversation

@gkurt

@gkurt gkurt commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Bash accepts all of these (bash -n passes), but unbash reports "expected redirect target":

  • cat > $'', cat > $"", cat <<< $'': an empty ANSI-C or locale string didn't mark the word as quoted, so it looked like no target. The heredoc delimiter scanner already does this check.
  • cat <<< <(f), cat <<< >(f), cat <<<<(f): the here-string branch returned before the process-substitution check the other redirects use. It's now a shared helper.

Tests added to test/error-recovery.test.ts.

@webpro webpro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, both fixes are right and the tests cover both cases.

Bash 3.2 and 5.3 accept every input in the description. Against main, all fixtures parse to byte-identical ASTs and printer output, and a 149-input corpus covering every redirect operator, $'...' and $"..." in command, argument, assignment, case, test, loop and array positions, nested substitutions, heredoc bodies, fd prefixes and compound-command redirects changes only for the intended inputs. A welcome side effect: an unterminated $', $" or <( target now reports one error instead of two.

Trade-offs weighed before approving:

  • The quoted lookahead costs one charCodeAt and two compares per $ in a slow-path word, the same check readHereDocDelimiter already makes. Measured with bench/worker.ts over separate builds of main and this branch, every fixture group stays within 1% of main, inside the 1.3 to 4% run-to-run spread; a synthetic input that is 24% $ by bytes runs 1.4% slower, about 0.8 ns per $. The large fixtures are 1.2 to 2.3% $ by bytes, so the expected cost on real scripts is about 0.1%. Evaluating the lookahead only when the processed word text is empty, the only case the flag is read for a $ word, would take it off the hot path, but the explicit form reads better and 0.1% does not justify the special case.
  • The <<< branch still repeats the generic redirect tail (skip blanks, process-substitution check, read the target, build the token) apart from the operator string. Setting op = "<<<" and falling through to that tail removes the duplication and the helper, with identical output on the same corpus and fixtures. Not required for this PR.
  • cat <<< <(f)x is one word in Bash but lexes as a target plus an argument, exactly as cat < <(f)x does on main. Out of scope here.

Two optional test notes: literal [source, target] pairs would be more direct than deriving the target from the input, and echo <<< $"" and cat <<<>(f) would complete the lists; both parse.

@pkg-pr-new

pkg-pr-new Bot commented Nov 1, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/unbash@19

commit: 6b4151d

@webpro
webpro merged commit 758e1b9 into webpro-nl:main Nov 1, 2026
3 checks passed
@webpro

webpro commented Nov 1, 2026

Copy link
Copy Markdown
Member

🚀 This pull request is included in v6.0.3. See Release 6.0.3 for release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants