Skip to content

Conversation

@dtolnay
Copy link
Member

@dtolnay dtolnay commented Apr 11, 2024

This PR changes the behavior of proc_macro::Literal::byte_character (#115268), byte_string, and c_string (#119750) to improve their choice of escape sequences. 3 categories of changes are made:

  1. Never use \x00. Always prefer \0, which is supported in all the same places.

  2. Never escape \' inside double quotes and \" inside single quotes.

  3. Never use \x for valid UTF-8 in literals that permit \u.

The second commit adds tests covering these cases, asserting the old behavior.

The third commit implements the behavior change and simultaneously updates the tests to assert the new behavior.

@rustbot
Copy link
Collaborator

rustbot commented Apr 11, 2024

r? @fee1-dead

rustbot has assigned @fee1-dead.
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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 11, 2024
@dtolnay dtolnay added the A-proc-macros Area: Procedural macros label Apr 11, 2024
@petrochenkov
Copy link
Contributor

I remember we talked about something related in #95343, but don't remember details.
cc #60495 as well.

@rust-log-analyzer

This comment has been minimized.

@fee1-dead
Copy link
Member

Given the FCP here and that this is mostly an improvement / not a bug fix I'd say we should wait until it gets stabilized (on beta?)

@fee1-dead fee1-dead added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 12, 2024
@dtolnay
Copy link
Member Author

dtolnay commented Apr 12, 2024

If anything, having an official non-core::str::Utf8Chunks implementation checked in is an advantage because I'll need to use that in proc-macro2. Proc-macro2 is not going to be able to adopt Utf8Chunks as soon as it goes into beta.

@fee1-dead
Copy link
Member

I don't think we should add burden of maintainability to a first-party library to ease the burden of maintainability of a third-party library.

@dtolnay
Copy link
Member Author

dtolnay commented Jun 13, 2024

@rustbot ready

The new implementation compiles with stable Rust 1.79, which contains the stabilization of Utf8Chunks.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jun 13, 2024
Copy link
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@fee1-dead
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 14, 2024

📌 Commit 7ddc89e has been approved by fee1-dead

It is now in the queue for this repository.

@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 Jun 14, 2024
@bors bors merged commit 20ca54b into rust-lang:master Jun 14, 2024
@rustbot rustbot added this to the 1.81.0 milestone Jun 14, 2024
@dtolnay dtolnay deleted the literal branch August 29, 2024 03:29
@dtolnay dtolnay removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-proc-macros Area: Procedural macros T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants