Replace ASCII control chars with Unicode Control Pictures #127528

estebank · 2024-07-09T17:07:00Z

Replace ASCII control chars like CR with Unicode Control Pictures like ␍:

error: bare CR not allowed in doc-comment
  --> $DIR/lex-bare-cr-string-literal-doc-comment.rs:3:32
   |
LL | /// doc comment with bare CR: '␍'
   |                                ^

Centralize the checking of unicode char width for the purposes of CLI display in one place. Account for the new replacements. Remove unneeded tracking of "zero-width" unicode chars, as we calculate these in the SourceMap as needed now.

rustbot · 2024-07-09T17:07:07Z

r? @pnkfelix

rustbot has assigned @pnkfelix.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

estebank · 2024-07-09T17:08:53Z

If we're concerned by legibility or by replacing with characters that might not be well supported in all terminals, we can also replace these with something like [CR]:

error: bare CR not allowed in doc-comment
  --> $DIR/lex-bare-cr-string-literal-doc-comment.rs:3:32
   |
LL | /// doc comment with bare CR: '[CR]'
   |                                ^^^^

rustbot · 2024-07-09T17:21:42Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

bors · 2024-07-15T22:48:23Z

☔ The latest upstream changes (presumably #127777) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2024-07-16T19:18:59Z

☔ The latest upstream changes (presumably #127819) made this pull request unmergeable. Please resolve the merge conflicts.

``` error: bare CR not allowed in doc-comment --> $DIR/lex-bare-cr-string-literal-doc-comment.rs:3:32 | LL | /// doc comment with bare CR: '␍' | ^ ```

No longer track "zero-width" chars in `SourceMap`, read directly from the line when calculating the `display_col` of a `BytePos`. Move `char_width` to `rustc_span` and use it from the emitter. This change allows the following to properly align in terminals (depending on the font, the replaced control codepoints are rendered as 1 or 2 width, on my terminal they are rendered as 1, on VSCode text they are rendered as 2): ``` error: this file contains an unclosed delimiter --> $DIR/issue-68629.rs:5:17 | LL | ␜␟ts␀![{i | -- unclosed delimiter | | | unclosed delimiter LL | ␀␀ fn rݻoa>rݻm | ^ ```

We already point these out quite aggressively, telling people not to use them, but would normally be rendered as nothing. Having them visible will make it easier for people to actually deal with them. ``` error: unicode codepoint changing visible direction of text present in literal --> $DIR/unicode-control-codepoints.rs:26:22 | LL | println!("{:?}", '�'); | ^-^ | || | |'\u{202e}' | this literal contains an invisible unicode text flow control codepoint | = note: these kind of unicode codepoints change the way text flows on applications that support them, but can cause confusion because they change the order of characters on the screen = help: if their presence wasn't intentional, you can remove them help: if you want to keep them but make them visible in your source code, you can escape them | LL | println!("{:?}", '\u{202e}'); | ~~~~~~~~ ``` vs the previous ``` error: unicode codepoint changing visible direction of text present in literal --> $DIR/unicode-control-codepoints.rs:26:22 | LL | println!("{:?}", ''); | ^- | || | |'\u{202e}' | this literal contains an invisible unicode text flow control codepoint | = note: these kind of unicode codepoints change the way text flows on applications that support them, but can cause confusion because they change the order of characters on the screen = help: if their presence wasn't intentional, you can remove them help: if you want to keep them but make them visible in your source code, you can escape them | LL | println!("{:?}", '\u{202e}'); | ~~~~~~~~ ```

estebank · 2024-07-19T18:13:26Z

tests/ui/parser/trailing-carriage-return-in-string.stderr

 error: unknown character escape: `\r`
  --> $DIR/trailing-carriage-return-in-string.rs:10:25
   |
-LL |     let bad = "This is \ a test";
+LL |     let bad = "This is \␍ a test";
   |                         ^ unknown character escape


This is confusing, because the problem is that \ escape is followed by a non-printable character. This is parsed as \␍, which is kind-of \\r.

oli-obk · 2024-07-24T16:29:52Z

@bors rollup

bors · 2024-07-24T16:29:54Z

📌 Commit 9dffe95 has been approved by oli-obk

It is now in the queue for this repository.

…i-obk Replace ASCII control chars with Unicode Control Pictures Replace ASCII control chars like `CR` with Unicode Control Pictures like `␍`: ``` error: bare CR not allowed in doc-comment --> $DIR/lex-bare-cr-string-literal-doc-comment.rs:3:32 | LL | /// doc comment with bare CR: '␍' | ^ ``` Centralize the checking of unicode char width for the purposes of CLI display in one place. Account for the new replacements. Remove unneeded tracking of "zero-width" unicode chars, as we calculate these in the `SourceMap` as needed now.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#126548 (Improved clarity of documentation for std::fs::create_dir_all) - rust-lang#127528 (Replace ASCII control chars with Unicode Control Pictures) - rust-lang#127717 (Fix malformed suggestion for repeated maybe unsized bounds) - rust-lang#128046 (Fix some `#[cfg_attr(not(doc), repr(..))]`) - rust-lang#128122 (Mark `missing_fragment_specifier` as `FutureReleaseErrorReportInDeps`) - rust-lang#128135 (std: use duplicate thread local state in tests) - rust-lang#128140 (Remove Unnecessary `.as_str()` Conversions) r? `@ghost` `@rustbot` modify labels: rollup

matthiaskrgr · 2024-07-24T20:23:10Z

@bors r-
#128148 (comment)

estebank · 2024-07-24T21:06:46Z

@bors r=oli-obk

bors · 2024-07-24T21:06:48Z

📌 Commit 9bd7680 has been approved by oli-obk

It is now in the queue for this repository.

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#127054 (Reorder trait bound modifiers *after* `for<...>` binder in trait bounds) - rust-lang#127528 (Replace ASCII control chars with Unicode Control Pictures) - rust-lang#127872 (Migrate `pointer-auth-link-with-c`, `c-dynamic-rlib` and `c-dynamic-dylib` `run-make` tests to rmake) - rust-lang#128111 (Do not use question as label) - rust-lang#128160 (Don't ICE when auto trait has assoc ty in old solver) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#127528 - estebank:ascii-control-chars, r=oli-obk Replace ASCII control chars with Unicode Control Pictures Replace ASCII control chars like `CR` with Unicode Control Pictures like `␍`: ``` error: bare CR not allowed in doc-comment --> $DIR/lex-bare-cr-string-literal-doc-comment.rs:3:32 | LL | /// doc comment with bare CR: '␍' | ^ ``` Centralize the checking of unicode char width for the purposes of CLI display in one place. Account for the new replacements. Remove unneeded tracking of "zero-width" unicode chars, as we calculate these in the `SourceMap` as needed now.

Kobzol · 2024-07-25T07:59:34Z

@rust-timer build e3343bd

rust-timer · 2024-07-25T11:01:50Z

Finished benchmarking commit (e3343bd): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.2%, 2.9%]	32
Regressions ❌ (secondary)	0.5%	[0.3%, 2.2%]	17
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.8%	[0.2%, 2.9%]	32

Max RSS (memory usage)

Results (primary -3.2%, secondary -6.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.2%	[-3.2%, -3.2%]	1
Improvements ✅ (secondary)	-6.1%	[-6.1%, -6.1%]	1
All ❌✅ (primary)	-3.2%	[-3.2%, -3.2%]	1

Cycles

Results (primary 1.2%, secondary 1.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.2%	[1.0%, 1.5%]	3
Regressions ❌ (secondary)	1.0%	[1.0%, 1.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.2%	[1.0%, 1.5%]	3

Binary size

Results (secondary -0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.0%	[-0.0%, -0.0%]	1
All ❌✅ (primary)	-	-	0

Bootstrap: 771.74s -> 769.585s (-0.28%)
Artifact size: 328.91 MiB -> 328.93 MiB (0.01%)

…rs, r=oli-obk" This reverts commit cce2db0, reversing changes made to cfc5f25.

bugadani · 2024-08-01T15:27:30Z

This change seems to have a (hopefully unintended) side effect: at least rust-lld on windows seem to end lines with CRLF, so every line of the rust-lld output ends with an ugly ␍.

estebank · 2024-08-01T18:21:12Z

@bugadani I'd noticed that in one test but didn't think too much of it at the time. Yes, we should clean that up.

…felix Change output normalization logic to be linear against size of output Modify the rendered output normalization routine to scan each character *once* and construct a `String` to be printed out to the terminal *once*, instead of using `String::replace` in a loop multiple times. The output doesn't change, but the time spent to prepare a diagnostic is now faster (or rather, closer to what it was before rust-lang#127528).

rustbot assigned pnkfelix Jul 9, 2024

This comment has been minimized.

Sign in to view

estebank force-pushed the ascii-control-chars branch from 0089595 to 4db25f7 Compare July 9, 2024 17:21

estebank force-pushed the ascii-control-chars branch from 4db25f7 to 42ed400 Compare July 16, 2024 00:07

Replace ASCII control chars with Unicode Control Pictures

89f273f

``` error: bare CR not allowed in doc-comment --> $DIR/lex-bare-cr-string-literal-doc-comment.rs:3:32 | LL | /// doc comment with bare CR: '␍' | ^ ```

estebank force-pushed the ascii-control-chars branch from 42ed400 to ac6eb65 Compare July 18, 2024 19:45

estebank added 2 commits July 18, 2024 20:08

estebank force-pushed the ascii-control-chars branch from aa4f805 to 9dffe95 Compare July 18, 2024 20:08

estebank commented Jul 19, 2024

View reviewed changes

oli-obk approved these changes Jul 24, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2024

matthiaskrgr mentioned this pull request Jul 24, 2024

Rollup of 7 pull requests #128148

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 24, 2024

Fix ddltool-failed test

9bd7680

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 24, 2024

matthiaskrgr mentioned this pull request Jul 25, 2024

Rollup of 5 pull requests #128169

Merged

bors merged commit cce2db0 into rust-lang:master Jul 25, 2024
6 checks passed

rustbot added this to the 1.82.0 milestone Jul 25, 2024

This comment has been minimized.

Sign in to view

rustbot added the perf-regression Performance regression. label Jul 25, 2024

oli-obk added a commit to oli-obk/rust that referenced this pull request Jul 25, 2024

Revert "Rollup merge of rust-lang#127528 - estebank:ascii-control-cha…

6679c62

…rs, r=oli-obk" This reverts commit cce2db0, reversing changes made to cfc5f25.

oli-obk mentioned this pull request Jul 25, 2024

Revert Replace ASCII control chars with Unicode Control Pictures #128179

Closed

estebank mentioned this pull request Jul 29, 2024

Change output normalization logic to be linear against size of output #128200

Merged

This was referenced Aug 15, 2024

Special way to underline with empty spans rust-lang/annotate-snippets-rs#108

Open

Bare CR allowed in code causing terrible spans #16480

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace ASCII control chars with Unicode Control Pictures #127528

Replace ASCII control chars with Unicode Control Pictures #127528

estebank commented Jul 9, 2024

rustbot commented Jul 9, 2024

estebank commented Jul 9, 2024

This comment has been minimized.

rustbot commented Jul 9, 2024

bors commented Jul 15, 2024

bors commented Jul 16, 2024

estebank Jul 19, 2024

oli-obk commented Jul 24, 2024

bors commented Jul 24, 2024

matthiaskrgr commented Jul 24, 2024

estebank commented Jul 24, 2024

bors commented Jul 24, 2024

Kobzol commented Jul 25, 2024

This comment has been minimized.

rust-timer commented Jul 25, 2024

bugadani commented Aug 1, 2024

estebank commented Aug 1, 2024

Replace ASCII control chars with Unicode Control Pictures #127528

Replace ASCII control chars with Unicode Control Pictures #127528

Conversation

estebank commented Jul 9, 2024

rustbot commented Jul 9, 2024

estebank commented Jul 9, 2024

This comment has been minimized.

rustbot commented Jul 9, 2024

bors commented Jul 15, 2024

bors commented Jul 16, 2024

estebank Jul 19, 2024

Choose a reason for hiding this comment

oli-obk commented Jul 24, 2024

bors commented Jul 24, 2024

matthiaskrgr commented Jul 24, 2024

estebank commented Jul 24, 2024

bors commented Jul 24, 2024

Kobzol commented Jul 25, 2024

This comment has been minimized.

rust-timer commented Jul 25, 2024

Overall result: ❌ regressions - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

Binary size

bugadani commented Aug 1, 2024

estebank commented Aug 1, 2024