Handle `Span`s for byte and raw strings and add more detail #81307

estebank · 2021-01-23T20:49:47Z

CC #81208.

rust-highfive · 2021-01-23T20:49:51Z

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

estebank · 2021-01-23T20:50:16Z

cc @wesleywiser

petrochenkov · 2021-01-23T20:57:48Z

r? @petrochenkov

compiler/rustc_lexer/src/unescape.rs

petrochenkov · 2021-01-24T13:32:12Z

I knew something was wrong here, the ICE doesn't happen because #s are counted incorrectly, the ICE happens because emit_unescape_error performs arithmetic on the overridden span (StringReader::override_span) which may be entirely arbitrary, but happens to point to the full byte string literal in this case.
So this change fixing the issue is a pure accident, and for other override_spans (e.g. from proc macros) it will still ICE.

petrochenkov · 2021-01-24T14:17:45Z

The linked issue is fixed in #81337 by avoiding reparsing values in key-value attributes for the second time (that's where the overridden span is used).

The issue with emit_unescape_error doing arithmetic on arbitrary spans still remain.

estebank · 2021-01-25T01:40:27Z

@petrochenkov addressed your comments and see that you fixed the underlying issue. While I was in this file I updated the diagnostics to bring the up to our current standard of quality.

petrochenkov · 2021-01-27T23:19:38Z

compiler/rustc_parse/src/lexer/unescape_error_reporting.rs

+            } else {
+                diag.help(
+                    "for more information, visit \
+                     <https://static.rust-lang.org/doc/master/reference.html#literals>",


In which cases we link to the reference from error messages?
In theory, there's a reference page for every error, but in practice the diagnostics are already too noisy, so perhaps it's not a good idea to add such links just in case, without a strong specific reason.

The case where this one triggers when using a "fake" escape, like \y. We could enumerate all the possible ones that are valid, but feared that would be too verbose. Every other case handled in this document provides enough information inline. I'm more worried about the link being stale. The reason I'm adding it is because these escapes are hard to search for.

compiler/rustc_parse/src/lexer/unescape_error_reporting.rs

compiler/rustc_parse/src/lexer/mod.rs

petrochenkov · 2021-01-27T23:28:27Z

The PR name and description need an update.

petrochenkov · 2021-02-04T14:20:26Z

Thanks!
@bors r+

bors · 2021-02-04T14:20:28Z

📌 Commit 3b5d018 has been approved by petrochenkov

…etrochenkov Handle `Span`s for byte and raw strings and add more detail CC rust-lang#81208.

Rollup of 15 pull requests Successful merges: - rust-lang#79554 (Generic associated types in trait paths) - rust-lang#80726 (relax adt unsizing requirements) - rust-lang#81307 (Handle `Span`s for byte and raw strings and add more detail ) - rust-lang#81318 (rustdoc-json: Fix has_body) - rust-lang#81456 (Make remote-test-server easier to use with new targets) - rust-lang#81497 (rustdoc: Move `display_fn` struct inside `display_fn`) - rust-lang#81500 (Remove struct_type from union output) - rust-lang#81542 (Expose correct symlink API on WASI) - rust-lang#81676 (Add more information to the error code for 'crate not found') - rust-lang#81682 (Add additional bitset benchmarks) - rust-lang#81730 (Make `Allocator` object-safe) - rust-lang#81763 (Cleanup rustdoc pass descriptions a bit) - rust-lang#81767 (Update LayoutError/LayoutErr stability attributes) - rust-lang#81771 (Indicate change in RSS from start to end of pass in time-passes output) - rust-lang#81781 (Fix `install-awscli.sh` error in CI) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned matthewjasper Jan 23, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 23, 2021

estebank added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 23, 2021

rust-highfive assigned petrochenkov and unassigned matthewjasper Jan 23, 2021

cjgillot reviewed Jan 24, 2021

View reviewed changes

compiler/rustc_lexer/src/unescape.rs Outdated Show resolved Hide resolved

estebank force-pushed the invalid-byte-str-span branch from 1a7560f to 708ee39 Compare January 25, 2021 01:36

estebank force-pushed the invalid-byte-str-span branch 2 times, most recently from adbcd26 to 234e55c Compare January 26, 2021 18:23

petrochenkov reviewed Jan 27, 2021

View reviewed changes

petrochenkov 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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 27, 2021

This comment has been minimized.

Sign in to view

estebank force-pushed the invalid-byte-str-span branch from 234e55c to 64d5f03 Compare February 3, 2021 17:42

estebank changed the title ~~Properly handle Spans for byte and raw strings~~ Handle Spans for byte and raw strings and add more detail Feb 3, 2021

Handle Spans for byte and raw strings and add more detail

3b5d018

estebank force-pushed the invalid-byte-str-span branch from 64d5f03 to 3b5d018 Compare February 3, 2021 21:35

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 4, 2021

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 Feb 4, 2021

m-ou-se added a commit to m-ou-se/rust that referenced this pull request Feb 5, 2021

Rollup merge of rust-lang#81307 - estebank:invalid-byte-str-span, r=p…

e9fec38

…etrochenkov Handle `Span`s for byte and raw strings and add more detail CC rust-lang#81208.

This was referenced Feb 5, 2021

Rollup of 14 pull requests #81780

Closed

Rollup of 15 pull requests #81784

Merged

bors merged commit 8d49ca1 into rust-lang:master Feb 5, 2021

rustbot added this to the 1.51.0 milestone Feb 5, 2021

petrochenkov mentioned this pull request Feb 7, 2021

parser: Collect tokens for values in key-value attributes #81337

Merged

estebank deleted the invalid-byte-str-span branch November 9, 2023 05:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle `Span`s for byte and raw strings and add more detail #81307

Handle `Span`s for byte and raw strings and add more detail #81307

estebank commented Jan 23, 2021 •

edited

Loading

rust-highfive commented Jan 23, 2021

estebank commented Jan 23, 2021

petrochenkov commented Jan 23, 2021

petrochenkov commented Jan 24, 2021

petrochenkov commented Jan 24, 2021

estebank commented Jan 25, 2021

petrochenkov Jan 27, 2021

estebank Feb 3, 2021

petrochenkov commented Jan 27, 2021

This comment has been minimized.

petrochenkov commented Feb 4, 2021

bors commented Feb 4, 2021

Handle Spans for byte and raw strings and add more detail #81307

Handle Spans for byte and raw strings and add more detail #81307

Conversation

estebank commented Jan 23, 2021 • edited Loading

rust-highfive commented Jan 23, 2021

estebank commented Jan 23, 2021

petrochenkov commented Jan 23, 2021

petrochenkov commented Jan 24, 2021

petrochenkov commented Jan 24, 2021

estebank commented Jan 25, 2021

petrochenkov Jan 27, 2021

Choose a reason for hiding this comment

estebank Feb 3, 2021

Choose a reason for hiding this comment

petrochenkov commented Jan 27, 2021

This comment has been minimized.

petrochenkov commented Feb 4, 2021

bors commented Feb 4, 2021

Handle `Span`s for byte and raw strings and add more detail #81307

Handle `Span`s for byte and raw strings and add more detail #81307

estebank commented Jan 23, 2021 •

edited

Loading