Skip to content

sed: support back-references in byte mode - #545

Draft
lKolabrodl wants to merge 2 commits into
uutils:mainfrom
lKolabrodl:fix/543-byte-backrefs
Draft

sed: support back-references in byte mode#545
lKolabrodl wants to merge 2 commits into
uutils:mainfrom
lKolabrodl:fix/543-byte-backrefs

Conversation

@lKolabrodl

Copy link
Copy Markdown

Closes #543.

Problem

Byte mode rejected every regular expression containing a back-reference before attempting a match. This made address and substitution expressions such as \(a\)b\1 fail whenever the C locale selected byte-oriented matching.

User impact

Scripts using back-references could work in a UTF-8 locale but fail immediately in the C locale. Invalid UTF-8 input could not use back-references at all.

Root cause

The fast byte regex engine does not implement back-references, while the fallback fancy_regex engine accepts only UTF-8 text and reports UTF-8 offsets.

Fix

  • add a byte-mode fancy_regex path that maps each input byte to one Unicode scalar
  • translate matches and capture offsets back to the original byte buffer
  • keep captures_iter streaming instead of collecting every match
  • preserve captures from invalid UTF-8 input
  • cover the reported address expression and an invalid UTF-8 substitution

Verification

  • cargo test --lib — 360 passed
  • cargo test backref — 9 related tests passed
  • cargo clippy --lib --tests -- -D warnings
  • cargo fmt --all -- --check

The full integration run on Windows reached 219 passing tests, 2 ignored tests, and 47 failures caused by existing CRLF-sensitive shell-script and fixture expectations. The focused integration tests for this change pass.

AI disclosure

AI assistance was used to help trace the byte/UTF-8 boundary, implement the offset mapping, and draft tests. The resulting code was reviewed and verified with the commands above.

@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 51.12782% with 65 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.42%. Comparing base (f1c5813) to head (68b4e4e).

Files with missing lines Patch % Lines
src/sed/fast_regex.rs 51.12% 65 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #545      +/-   ##
==========================================
- Coverage   83.04%   82.42%   -0.62%     
==========================================
  Files          13       13              
  Lines        7046     7170     +124     
  Branches      401      404       +3     
==========================================
+ Hits         5851     5910      +59     
- Misses       1192     1257      +65     
  Partials        3        3              
Flag Coverage Δ
macos_latest 83.02% <51.12%> (-0.65%) ⬇️
ubuntu_latest 83.24% <51.12%> (-0.64%) ⬇️
windows_latest 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq

codspeed-hq Bot commented Aug 30, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 0.87%

⚡ 1 improved benchmark
❌ 2 regressed benchmarks
✅ 8 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
number_fix 1.1 s 1.2 s -2.97%
genome_subst 443.7 ms 453.1 ms -2.09%
access_log_subst 2.4 s 2.3 s +2.53%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing lKolabrodl:fix/543-byte-backrefs (68b4e4e) with main (f1c5813)

Open in CodSpeed

@sylvestre

Copy link
Copy Markdown
Contributor

could you please add more tests? the code coverage isn't ideal:
https://app.codecov.io/gh/uutils/sed/pull/545?dropdown=coverage&src=pr&el=h1

@lKolabrodl

Copy link
Copy Markdown
Author

Added five focused unit tests in 51aaf56 covering non-ASCII byte offset mapping, optional capture groups, multiple invalid UTF-8 matches, zero-width iteration, and no-match paths. Locally, cargo fmt --all -- --check, cargo test fancy_byte --lib (7 passed), and cargo test backref (9 passed) all pass. Thanks!

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.

Back-references are rejected in byte (C locale) mode

2 participants