Reimplement C-str literals #113476

fee1-dead · 2023-07-08T05:45:22Z

While converting lexer tokens to ast Tokens in rustc_parse, we check the edition of the span of the token. If the edition < 2021, we split the token into two, one being the identifier and other being the str literal.

Fixes #113333

rustbot · 2023-07-08T05:45:28Z

r? @WaffleLapkin

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-07-08T05:45:32Z

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.

rustbot · 2023-07-08T06:00:40Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

petrochenkov · 2023-07-08T07:10:09Z

we pass is_rust_2021 as a boolean flag to the lexer

This sounds like a red flag, and if it is required, then the previous literal prefix future-proofing was probably done incorrectly.
I'll check in a couple of days.

compiler/rustc_codegen_llvm/src/allocator.rs

compiler/rustc_parse/src/lexer/mod.rs

klensy · 2023-07-08T09:09:23Z

Basically we pass is_rust_2021 as a boolean flag

So in edition > 2021 this will be false and broken? For example edition 2024.
Ugh, sess.edition.rust_2021() actually means edition >= 2021, thats unexpected.

fee1-dead · 2023-07-08T09:15:11Z

That flag will be true for 2024. Should I rename?

petrochenkov · 2023-07-10T17:10:06Z

we pass is_rust_2021 as a boolean flag to the lexer

This sounds like a red flag, and if it is required, then the previous literal prefix future-proofing was probably done incorrectly. I'll check in a couple of days.

The future-proofing is correct.
fn next_token in the parser pulls the next token from the lexer, applies a span to it (including hygiene information which contains edition) and report_unknown_prefix processes it and reports or not reports the error.

If this PR worked consistently with the existing future-proofing check, then it would either:

In fn next_token pull the whole C-str literal and then break it into two tokens if the edition is 2015-2018.
In fn next_token pull the prefix and then additionally pull the rest of the C-str literal and combine, if the prefix's edition is 2021+.

Both variants may require changes to the lexer, but neither of them requires passing an edition to it.
We can implement any of these alternatives that is simpler to implement in practice.

petrochenkov · 2023-07-10T17:29:46Z

That flag will be true for 2024. Should I rename?

If this is implemented in lexer (rather than as post-processing in fn next_token), then I'd rather rename it to something like enable_c_str to make the lexer configurable, but still edition-oblivious.

fee1-dead · 2023-07-16T17:23:54Z

I'll try implementing edition-based token combining then.

fee1-dead · 2023-07-16T19:00:54Z

implemented edition-based token splitting instead

compiler/rustc_parse/src/lexer/mod.rs

petrochenkov · 2023-07-25T12:04:26Z

Thanks!
@bors r+

bors · 2023-07-25T12:04:28Z

📌 Commit a0376e9 has been approved by petrochenkov

It is now in the queue for this repository.

bors · 2023-07-25T12:04:36Z

⌛ Testing commit a0376e9 with merge 23405bb...

fmease · 2023-07-25T12:11:53Z

Could you add "Fixes #113333" to the PR description since this PR doesn't need backporting?

fee1-dead · 2023-07-25T12:39:35Z

Could you add "Fixes #113333" to the PR description since this PR doesn't need backporting?

done

bors · 2023-07-25T14:18:01Z

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 23405bb to master...

rust-timer · 2023-07-25T19:37:39Z

Finished benchmarking commit (23405bb): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.7%	[0.7%, 0.7%]	3
Regressions ❌ (secondary)	0.7%	[0.5%, 1.2%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[0.7%, 0.7%]	3

Max RSS (memory usage)

Results

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)	-2.9%	[-3.2%, -2.5%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.9%	[-3.2%, -2.5%]	2

Cycles

Results

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)	2.1%	[2.1%, 2.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 651.831s -> 651.438s (-0.06%)

rylev · 2023-08-05T12:50:48Z

Perf: this is noise

@rustbot label: +perf-regression-triaged

…ilstrieb Stabilize C string literals RFC: https://rust-lang.github.io/rfcs/3348-c-str-literal.html Tracking issue: rust-lang#105723 Documentation PR (reference manual): rust-lang/reference#1423 # Stabilization report Stabilizes C string and raw C string literals (`c"..."` and `cr#"..."#`), which are expressions of type [`&CStr`](https://doc.rust-lang.org/stable/core/ffi/struct.CStr.html). Both new literals require Rust edition 2021 or later. ```rust const HELLO: &core::ffi::CStr = c"Hello, world!"; ``` C strings may contain any byte other than `NUL` (`b'\x00'`), and their in-memory representation is guaranteed to end with `NUL`. ## Implementation Originally implemented by PR rust-lang#108801, which was reverted due to unintentional changes to lexer behavior in Rust editions < 2021. The current implementation landed in PR rust-lang#113476, which restricts C string literals to Rust edition >= 2021. ## Resolutions to open questions from the RFC * Adding C character literals (`c'.'`) of type `c_char` is not part of this feature. * Support for `c"..."` literals does not prevent `c'.'` literals from being added in the future. * C string literals should not be blocked on making `&CStr` a thin pointer. * It's possible to declare constant expressions of type `&'static CStr` in stable Rust (as of v1.59), so C string literals are not adding additional coupling on the internal representation of `CStr`. * The unstable `concat_bytes!` macro should not accept `c"..."` literals. * C strings have two equally valid `&[u8]` representations (with or without terminal `NUL`), so allowing them to be used in `concat_bytes!` would be ambiguous. * Adding a type to represent C strings containing valid UTF-8 is not part of this feature. * Support for a hypothetical `&Utf8CStr` may be explored in the future, should such a type be added to Rust.

Stabilize C string literals RFC: https://rust-lang.github.io/rfcs/3348-c-str-literal.html Tracking issue: rust-lang/rust#105723 Documentation PR (reference manual): rust-lang/reference#1423 # Stabilization report Stabilizes C string and raw C string literals (`c"..."` and `cr#"..."#`), which are expressions of type [`&CStr`](https://doc.rust-lang.org/stable/core/ffi/struct.CStr.html). Both new literals require Rust edition 2021 or later. ```rust const HELLO: &core::ffi::CStr = c"Hello, world!"; ``` C strings may contain any byte other than `NUL` (`b'\x00'`), and their in-memory representation is guaranteed to end with `NUL`. ## Implementation Originally implemented by PR rust-lang/rust#108801, which was reverted due to unintentional changes to lexer behavior in Rust editions < 2021. The current implementation landed in PR rust-lang/rust#113476, which restricts C string literals to Rust edition >= 2021. ## Resolutions to open questions from the RFC * Adding C character literals (`c'.'`) of type `c_char` is not part of this feature. * Support for `c"..."` literals does not prevent `c'.'` literals from being added in the future. * C string literals should not be blocked on making `&CStr` a thin pointer. * It's possible to declare constant expressions of type `&'static CStr` in stable Rust (as of v1.59), so C string literals are not adding additional coupling on the internal representation of `CStr`. * The unstable `concat_bytes!` macro should not accept `c"..."` literals. * C strings have two equally valid `&[u8]` representations (with or without terminal `NUL`), so allowing them to be used in `concat_bytes!` would be ambiguous. * Adding a type to represent C strings containing valid UTF-8 is not part of this feature. * Support for a hypothetical `&Utf8CStr` may be explored in the future, should such a type be added to Rust.

rustbot assigned WaffleLapkin Jul 8, 2023

This comment has been minimized.

Sign in to view

petrochenkov self-assigned this Jul 8, 2023

Noratrieb reviewed Jul 8, 2023

View reviewed changes

compiler/rustc_codegen_llvm/src/allocator.rs Outdated Show resolved Hide resolved

fmease reviewed Jul 8, 2023

View reviewed changes

compiler/rustc_parse/src/lexer/mod.rs Outdated Show resolved Hide resolved

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 Jul 10, 2023

workingjubilee mentioned this pull request Jul 10, 2023

Propose code string literals rust-lang/rfcs#3450

Open

fmease mentioned this pull request Jul 16, 2023

Reimplement the lexing of c"…" string literals with backward compatibility in mind #113333

Closed

fee1-dead force-pushed the c-str-lit branch from e676f99 to d98a851 Compare July 16, 2023 19:00

fee1-dead force-pushed the c-str-lit branch from d98a851 to 2ddda12 Compare July 16, 2023 19:04

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 Jul 16, 2023

petrochenkov reviewed Jul 17, 2023

View reviewed changes

compiler/rustc_parse/src/lexer/mod.rs Outdated Show resolved Hide resolved

petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 17, 2023

petrochenkov reviewed Jul 24, 2023

View reviewed changes

compiler/rustc_parse/src/lexer/mod.rs Outdated Show resolved Hide resolved

compiler/rustc_parse/src/lexer/mod.rs Show resolved Hide resolved

petrochenkov unassigned WaffleLapkin Jul 24, 2023

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 Jul 24, 2023

extract common code

a0376e9

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 25, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 25, 2023

bors merged commit 23405bb into rust-lang:master Jul 25, 2023
11 checks passed

rustbot added this to the 1.73.0 milestone Jul 25, 2023

fee1-dead deleted the c-str-lit branch July 25, 2023 16:37

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

tgross35 mentioned this pull request Aug 3, 2023

c"…" literals are handled incorrectly by proc_macro::Literal #112820

Closed

rustbot added the perf-regression-triaged The performance regression has been triaged. label Aug 5, 2023

tgross35 mentioned this pull request Oct 31, 2023

Tracking Issue for c"…" string literals #105723

Closed

12 tasks

jmillikin mentioned this pull request Nov 1, 2023

Stabilize C string literals #117472

Merged

traviscross mentioned this pull request Aug 4, 2024

Reserve guarded string literals (RFC 3593) #123951

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reimplement C-str literals #113476

Reimplement C-str literals #113476

fee1-dead commented Jul 8, 2023 •

edited

Loading

rustbot commented Jul 8, 2023

rustbot commented Jul 8, 2023

This comment has been minimized.

rustbot commented Jul 8, 2023

petrochenkov commented Jul 8, 2023 •

edited

Loading

klensy commented Jul 8, 2023 •

edited

Loading

fee1-dead commented Jul 8, 2023

petrochenkov commented Jul 10, 2023

petrochenkov commented Jul 10, 2023

fee1-dead commented Jul 16, 2023

fee1-dead commented Jul 16, 2023

petrochenkov commented Jul 25, 2023

bors commented Jul 25, 2023

bors commented Jul 25, 2023

fmease commented Jul 25, 2023

fee1-dead commented Jul 25, 2023

bors commented Jul 25, 2023

rust-timer commented Jul 25, 2023

rylev commented Aug 5, 2023

Reimplement C-str literals #113476

Reimplement C-str literals #113476

Conversation

fee1-dead commented Jul 8, 2023 • edited Loading

rustbot commented Jul 8, 2023

rustbot commented Jul 8, 2023

This comment has been minimized.

rustbot commented Jul 8, 2023

petrochenkov commented Jul 8, 2023 • edited Loading

klensy commented Jul 8, 2023 • edited Loading

fee1-dead commented Jul 8, 2023

petrochenkov commented Jul 10, 2023

petrochenkov commented Jul 10, 2023

fee1-dead commented Jul 16, 2023

fee1-dead commented Jul 16, 2023

petrochenkov commented Jul 25, 2023

bors commented Jul 25, 2023

bors commented Jul 25, 2023

fmease commented Jul 25, 2023

fee1-dead commented Jul 25, 2023

bors commented Jul 25, 2023

rust-timer commented Jul 25, 2023

Overall result: ❌ regressions - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

Binary size

rylev commented Aug 5, 2023

fee1-dead commented Jul 8, 2023 •

edited

Loading

petrochenkov commented Jul 8, 2023 •

edited

Loading

klensy commented Jul 8, 2023 •

edited

Loading