compiler: report an unterminated s or y command instead of aborting - #571
SichenLiang wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #571 +/- ##
==========================================
- Coverage 83.86% 83.85% -0.02%
==========================================
Files 14 14
Lines 7203 7209 +6
Branches 424 426 +2
==========================================
+ Hits 6041 6045 +4
- Misses 1157 1159 +2
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| Ok(Address::Line(number)) | ||
| } | ||
| _ => panic!("invalid context address"), | ||
| _ => compilation_error(lines, line, "expected context address"), |
There was a problem hiding this comment.
GNU says "unexpected ,'" for 2,d`, could we match it?
please check with LANG=C sed -e '2,d'
There was a problem hiding this comment.
You're right. #578 now makes 2,d report unexpected `,', so I removed the address-parser changes from this PR. It now only handles scripts ending after s or y.
| // A script that ends in the middle of an address or of an s/y delimiter must | ||
| // produce a diagnostic, not abort. | ||
| #[test] | ||
| fn incomplete_script_reports_an_error() { |
There was a problem hiding this comment.
please prefix with test_ like the rest of the file
There was a problem hiding this comment.
Renamed to test_unterminated_s_or_y_command.
| .args(&["-e", script]) | ||
| .fails() | ||
| .code_is(1) | ||
| .stderr_contains(*message); |
There was a problem hiding this comment.
please use stderr_is with the full message like the other tests in this file
There was a problem hiding this comment.
Updated. Each case now checks the complete diagnostic with stderr_is.
The substitution and transliteration compilers read the delimiter with ScriptCharProvider::current() even when the command letter was the last character on the line. This indexed past the end of the line and aborted. Check for the end of the line first and report an unterminated command, as GNU sed does. Scripts such as `s`, `1y`, and `p;s` now exit 1 with a diagnostic instead of aborting. The address-parser aborts from uutils#570 were fixed separately in uutils#578.
a4d2c85 to
05a5338
Compare
SichenLiang
changed the title
|
Rebased on current main and updated the title and description to the narrower scope. |
Merging this PR will improve performance by 3.34%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | access_log_no_subst |
1.1 s | 1.1 s | +3.45% |
| ⚡ | access_log_all_del |
1.2 s | 1.1 s | +3.34% |
| ⚡ | access_log_no_del |
1.1 s | 1.1 s | +3.23% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing SichenLiang:malformed-script-errors (05a5338) with main (95804a2)
2 participants
Fixes #570.
After #578 fixed the address-parser cases from #570, the remaining aborts are scripts that end immediately after
sory. Both command compilers tried to read a delimiter without first checking for the end of the line.They now return a compilation error with GNU sed's wording instead:
The new table test covers
s,1s,p;s,y, and{y, and checks the complete diagnostic for each case.