syntax: Keep string literals in ABIs and `asm!` more precisely #66271

petrochenkov · 2019-11-10T15:01:59Z

As a result we don't lose spans when extern functions or blocks are passed to proc macros, and also escape all string literals consistently.
Continuation of #60679, which did a similar thing with all literals besides those in ABIs and asm!.

TODO: Add tests.

Fixes #60493
Fixes #64561
r? @Centril

Centril

Looks good so far! Waiting for the tests for the remainder of the review. :)

src/libsyntax/ast.rs

Centril · 2019-11-11T07:44:34Z

src/libsyntax_ext/deriving/generic/mod.rs

@@ -737,7 +737,6 @@ impl<'a> TraitDef<'a> {
                                         self,
                                         type_ident,
                                         generics,
-                                         sym::Rust,


Centril · 2019-11-11T07:55:45Z

src/libsyntax/parse/parser/expr.rs

    pub(super) fn parse_lit(&mut self) -> PResult<'a, Lit> {
+        self.parse_opt_lit().ok_or_else(|| {
+            let msg = format!("unexpected token: {}", self.this_token_descr());
+            self.span_fatal(self.token.span, &msg)


Hmm; should we perhaps recover here instead with e.g. LitKind::Err(Option<Symbol>)? cc @estebank

This is ok as is. Recovery of parse_lit would have to be aware of what the caller expects after the Lit. If anything this should be a struct_span_err instead and let the caller handle it.

I generally didn't change any diagnostics in this PR.

I can try turning this into an "expected literal, found X" error.

Nah, infinite recursion happens in attribute parsing if something non-span_fatal is returned (e.g. self.unexpected()), I remember I've seen this before.
Leaving for some future PR.

src/libsyntax/parse/parser/mod.rs

src/libsyntax_ext/asm.rs

Revert weird renaming of the former `LitError::report`

petrochenkov · 2019-11-16T18:34:41Z

Updated.

Centril · 2019-11-16T20:44:43Z

r=me with the typo fixed

petrochenkov · 2019-11-16T22:13:33Z

@bors r=Centril

bors · 2019-11-16T22:13:34Z

📌 Commit 28aec1b has been approved by Centril

@Centril

syntax: Keep string literals in ABIs and `asm!` more precisely As a result we don't lose spans when `extern` functions or blocks are passed to proc macros, and also escape all string literals consistently. Continuation of rust-lang#60679, which did a similar thing with all literals besides those in ABIs and `asm!`. TODO: Add tests. Fixes rust-lang#60493 Fixes rust-lang#64561 r? @Centril

bors · 2019-11-17T04:27:03Z

⌛ Testing commit 28aec1b with merge f80bed9be489ceab0c403381dd4014f2fb96b7ab...

@Centril

syntax: Keep string literals in ABIs and `asm!` more precisely As a result we don't lose spans when `extern` functions or blocks are passed to proc macros, and also escape all string literals consistently. Continuation of rust-lang#60679, which did a similar thing with all literals besides those in ABIs and `asm!`. TODO: Add tests. Fixes rust-lang#60493 Fixes rust-lang#64561 r? @Centril

JohnTitor · 2019-11-17T04:37:48Z

@bors retry rolled up

@ghost

Rollup of 11 pull requests Successful merges: - #65739 (Improve documentation of `Vec::split_off(...)`) - #66271 (syntax: Keep string literals in ABIs and `asm!` more precisely) - #66344 (rustc_plugin: Remove `Registry::register_attribute`) - #66381 (find_deprecation: deprecation attr may be ill-formed meta.) - #66395 (Centralize panic macro documentation) - #66456 (Move `DIAGNOSTICS` usage to `rustc_driver`) - #66465 (add missing 'static lifetime in docs) - #66466 (miri panic_unwind: fix hack for SEH platforms) - #66469 (Use "field is never read" instead of "field is never used") - #66471 (Add test for issue 63116) - #66477 (Clarify transmute_copy documentation example) Failed merges: r? @ghost

bors · 2019-11-17T07:44:27Z

⌛ Testing commit 28aec1b with merge 8831d76...

Rustup to rustc 1.41.0-nightly (35ef33a 2019-11-21) I don't have the right fix for the fmtstr tests, and I'm also hitting problems caused by messense/rustc-test#3 List of rustups: - rust-lang/rust#66271 (syntax: Keep string literals in ABIs and `asm!` more precisely) - rust-lang/rust#65355 (Stabilize `!` in Rust 1.41.0) - rust-lang/rust#66515 (Reduce size of `hir::Expr` by boxing more of `hir::InlineAsm`) - rust-lang/rust#66389 (Specific labels when referring to "expected" and "found" types) - rust-lang/rust#66074 ([mir-opt] Turn on the `ConstProp` pass by default) changelog: none

Updates to account for changes in rust-lang/rust#66271

rust-highfive assigned Centril Nov 10, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 10, 2019

This was referenced Nov 10, 2019

String literals in asm!() and ABIs are not escaped #60493

Closed

Ommiting "C" in extern "C" fn in derive input causes spans to get lost. #64561

Closed

This comment has been minimized.

Sign in to view

Centril reviewed Nov 11, 2019

View reviewed changes

Centril 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 Nov 11, 2019

petrochenkov added 4 commits November 16, 2019 21:21

ast: Keep extern qualifiers in functions more precisely

266f547

ast: Keep string literals in ABIs precisely

00bc449

parse: Support parsing optional literals

b85a3da

Revert weird renaming of the former `LitError::report`

parse: Use string literal parsing in the asm macro

a699f17

petrochenkov force-pushed the abism branch from 1f89985 to f413457 Compare November 16, 2019 18:33

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 Nov 16, 2019

petrochenkov added 2 commits November 17, 2019 01:11

Address review comments

11580ce

Add some more tests

28aec1b

petrochenkov force-pushed the abism branch from f413457 to 28aec1b Compare November 16, 2019 22:11

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 Nov 16, 2019

Centril mentioned this pull request Nov 17, 2019

Rollup of 11 pull requests #66483

Closed

JohnTitor mentioned this pull request Nov 17, 2019

Rollup of 11 pull requests #66485

Merged

bors merged commit 28aec1b into rust-lang:master Nov 17, 2019

Manishearth mentioned this pull request Nov 22, 2019

Rustup to rustc 1.41.0-nightly (35ef33a8 2019-11-21) rust-lang/rust-clippy#4825

Merged

calebcartwright added a commit to calebcartwright/rustfmt that referenced this pull request Jan 16, 2020

refactor: update ABI/Extern handling

e3e04af

Updates to account for changes in rust-lang/rust#66271

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

syntax: Keep string literals in ABIs and `asm!` more precisely #66271

syntax: Keep string literals in ABIs and `asm!` more precisely #66271

petrochenkov commented Nov 10, 2019

This comment has been minimized.

Centril left a comment

Centril Nov 11, 2019

Centril Nov 11, 2019

estebank Nov 13, 2019

petrochenkov Nov 16, 2019

petrochenkov Nov 16, 2019 •

edited

Loading

petrochenkov commented Nov 16, 2019

Centril commented Nov 16, 2019

petrochenkov commented Nov 16, 2019

bors commented Nov 16, 2019

bors commented Nov 17, 2019

JohnTitor commented Nov 17, 2019

bors commented Nov 17, 2019

syntax: Keep string literals in ABIs and asm! more precisely #66271

syntax: Keep string literals in ABIs and asm! more precisely #66271

Conversation

petrochenkov commented Nov 10, 2019

This comment has been minimized.

Centril left a comment

Choose a reason for hiding this comment

Centril Nov 11, 2019

Choose a reason for hiding this comment

Centril Nov 11, 2019

Choose a reason for hiding this comment

estebank Nov 13, 2019

Choose a reason for hiding this comment

petrochenkov Nov 16, 2019

Choose a reason for hiding this comment

petrochenkov Nov 16, 2019 • edited Loading

Choose a reason for hiding this comment

petrochenkov commented Nov 16, 2019

Centril commented Nov 16, 2019

petrochenkov commented Nov 16, 2019

bors commented Nov 16, 2019

bors commented Nov 17, 2019

JohnTitor commented Nov 17, 2019

bors commented Nov 17, 2019

syntax: Keep string literals in ABIs and `asm!` more precisely #66271

syntax: Keep string literals in ABIs and `asm!` more precisely #66271

petrochenkov Nov 16, 2019 •

edited

Loading