Skip to content

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

Merged
bors merged 6 commits into
rust-lang:masterfrom
petrochenkov:abism
Nov 17, 2019
Merged

syntax: Keep string literals in ABIs and asm! more precisely#66271
bors merged 6 commits into
rust-lang:masterfrom
petrochenkov:abism

Conversation

@petrochenkov

@petrochenkov petrochenkov commented Nov 10, 2019

Copy link
Copy Markdown
Contributor

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

@rust-highfive 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
Closed
Closed
@bors

This comment has been minimized.

Sign in to view
Centril
Centril reviewed Nov 11, 2019
View reviewed changes

@Centril Centril left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/libsyntax/ast.rs Outdated
Comment thread src/libsyntax_ext/deriving/generic/mod.rs Outdated

@Centril Centril Nov 11, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦀 😢

Comment thread src/libsyntax/parse/parser/expr.rs Outdated

@Centril Centril Nov 11, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@estebank estebank Nov 13, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@petrochenkov petrochenkov Nov 16, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

@petrochenkov petrochenkov Nov 16, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/libsyntax/parse/parser/mod.rs Outdated
Comment thread src/libsyntax_ext/asm.rs Outdated
@Centril 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

petrochenkov commented Nov 16, 2019

Copy link
Copy Markdown
Contributor Author

Updated.

@petrochenkov 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
@Centril

Centril commented Nov 16, 2019

Copy link
Copy Markdown
Contributor

r=me with the typo fixed

@petrochenkov

petrochenkov commented Nov 16, 2019

Copy link
Copy Markdown
Contributor Author

@bors r=Centril

@bors

bors commented Nov 16, 2019

Copy link
Copy Markdown
Collaborator

📌 Commit 28aec1b has been approved by Centril

@bors 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 added a commit to Centril/rust that referenced this pull request Nov 17, 2019
@Centril
Rollup merge of rust-lang#66271 - petrochenkov:abism, r=Centril
b25e706 
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
@Centril Centril mentioned this pull request Nov 17, 2019
Closed
@bors

bors commented Nov 17, 2019

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 28aec1b with merge f80bed9be489ceab0c403381dd4014f2fb96b7ab...

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 17, 2019
@JohnTitor
Rollup merge of rust-lang#66271 - petrochenkov:abism, r=Centril
b83d50d 
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 JohnTitor mentioned this pull request Nov 17, 2019
Merged
@JohnTitor

JohnTitor commented Nov 17, 2019

Copy link
Copy Markdown
Member

@bors retry rolled up

bors added a commit that referenced this pull request Nov 17, 2019
@bors
Auto merge of #66485 - JohnTitor:rollup-vbwhg6r, r=JohnTitor
8831d76 
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

bors commented Nov 17, 2019

Copy link
Copy Markdown
Collaborator

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

@bors bors merged commit 28aec1b into rust-lang:master Nov 17, 2019
@Manishearth Manishearth mentioned this pull request Nov 22, 2019
Merged
bors added a commit to rust-lang/rust-clippy that referenced this pull request Nov 23, 2019
@bors
Auto merge of #4825 - Manishearth:rustup, r=Manishearth
19769b5 
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
bors added a commit to rust-lang/rust-clippy that referenced this pull request Nov 23, 2019
@bors
Auto merge of #4825 - Manishearth:rustup, r=Manishearth
3870050 
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
bors added a commit to rust-lang/rust-clippy that referenced this pull request Nov 23, 2019
@bors
Auto merge of #4825 - Manishearth:rustup, r=flip1995
60e8413 
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
calebcartwright added a commit to calebcartwright/rustfmt that referenced this pull request Jan 16, 2020
@calebcartwright
refactor: update ABI/Extern handling
e3e04af 
Updates to account for changes in
rust-lang/rust#66271
@petrochenkov petrochenkov deleted the abism branch February 22, 2025 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estebank estebank estebank left review comments

+1 more reviewer

@Centril Centril Centril left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@Centril Centril

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Ommiting "C" in extern "C" fn in derive input causes spans to get lost. String literals in asm!() and ABIs are not escaped

6 participants

@petrochenkov @bors @Centril @JohnTitor @estebank @rust-highfive