Detect NulInCStr error earlier.

[nnethercote]

By making it an EscapeError instead of a LitError. This makes it like the other errors produced when checking string literals contents, e.g. for invalid escape sequences or bare CR chars.

NOTE: this means these errors are issued earlier, before expansion, which changes behaviour. It will be possible to move the check back to the later point if desired. If that happens, it's likely that all the string literal contents checks will be delayed together.

One nice thing about this: the old approach had some code in report_lit_error to calculate the span of the nul char from a range. This code used a hardwired +2 to account for the c" at the start of a C string literal, but this should have changed to a +3 for raw C string literals to account for the cr", which meant that the caret in cr" nul error messages was one short of where it should have been. The new approach doesn't need any of this and avoids the off-by-one error.

r? @fee1-dead

[nnethercote]

@fee1-dead: some additional context in this zulip thread.

In short, the delayed C NUL str check is inconsistent with all other string literal checks. If it ships in its current state, we're stuck with that behaviour permanently. If we move it earlier right now before it ships, we have the option to delay it (and all other string literal checks) later on (as implemented in #118699). So if we do this in the next few days, we avoid a one-way door shutting.

cc @joshtriplett

[nnethercote]

Ugh, the refusal of git and GitHub to show "binary" files is really annoying for this one.

[fee1-dead]

Currently on my phone right now, but do we have a test for c"\0" on an earlier edition passed to a macro which then consumes it?

[nnethercote]

Ugh, the CI failure is due to a rust-analyzer sync problem #118861, which is blocked by #119124, which is awaiting review.

[nnethercote]

Ugh, the refusal of git and GitHub to show "binary" files is really annoying for this one.

To clarify: the test contains 15 C string literals. Prior to this commit, only the first five have ERROR annotations. This commit adds ERROR annotations to the other ten. And the .stderr file changes from having five errors to fifteen errors.

[nnethercote]

Currently on my phone right now, but do we have a test for c"\0" on an earlier edition passed to a macro which then consumes it?

I don't think so. Is that interesting? It would produce output like this:

error: expected one of `!`, `.`, `::`, `;`, `?`, `else`, `{`, `}`, or an operator, found `"\0"`
 --> a2.rs:7:10
  |
7 |     _ = c"\0";
  |          ^^^^ expected one of 9 possible tokens

[fee1-dead]

I meant something like this:

macro_rules! hi {
    (c $t:tt) => { $t };
}

fn main() {
    println!(hi!(c"hello!\0"))
}

do we already have that in our test suite?

[nnethercote]

I don't see anything in tests/ui/ like that, no.

[traviscross]

@rustbot labels +I-lang-nominated

Nominating this for us to discuss as there is some time pressure here. If we want to save this space, we should do it before C string literals stabilize in Rust 1.76.

[joshtriplett]

Following up on this: the decision made was to defer the stabilization by a release, so that we don't need to rush this change in over the holidays.

[rustbot]

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

[nnethercote]

The rust-analyzer problem has been fixed, and I have fixed the conflicts. @fee1-dead, this is ready for review. Unfortunately, git and GitHub both refuse to show the changed test file because it contains NUL chars and so is "binary".

Here's the before:

// edition: 2021

fn main() {
    c"\0";
    //~^ ERROR null characters in C string literals

    c"\u{00}";
    //~^ ERROR null characters in C string literals

    c"^@";
    //~^ ERROR null characters in C string literals

    c"\x00";
    //~^ ERROR null characters in C string literals

    cr"^@";
    //~^ ERROR null characters in C string literals
}

macro_rules! empty {
    ($($tt:tt)*) => {};
}

// The cfg consumes the literals before nul checking occurs.
#[cfg(FALSE)]
fn test() {
    c"\0";
    c"\u{00}";
    c"^@";
    c"\x00";
    cr"^@";
}

// The macro consumes the literals before nul checking occurs.
fn test_empty() {
    empty!(c"\0");
    empty!(c"\u{00}");
    empty!(c"^@");
    empty!(c"\x00");
    empty!(cr"^@");
}

and the after:

// edition: 2021

// The null char check for C string literals was originally implemented after
// expansion, which meant the first five strings in this file triggered errors,
// and the remaining ten did not. But this is different to all the other
// content checks done on string literals, such as checks for invalid escapes
// and bare CR chars. So the check was moved earlier. The check can be moved
// back to after expansion at a later date if necessary, because that would be
// a backward compatible change. (In contrast, moving the check from after
// expansion to lexing time would be a backward incompatible change, because it
// could break code that was previously accepted.)

fn main() {
    c"\0";     //~ ERROR null characters in C string literals
    c"\u{00}"; //~ ERROR null characters in C string literals
    c"^@";     //~ ERROR null characters in C string literals
    c"\x00";   //~ ERROR null characters in C string literals
    cr"^@";    //~ ERROR null characters in C string literals
}

macro_rules! empty {
    ($($tt:tt)*) => {};
}

// The cfg does not consume the literals before nul checking occurs.
#[cfg(FALSE)]
fn test() {
    c"\0";     //~ ERROR null characters in C string literals
    c"\u{00}"; //~ ERROR null characters in C string literals
    c"^@";     //~ ERROR null characters in C string literals
    c"\x00";   //~ ERROR null characters in C string literals
    cr"^@";    //~ ERROR null characters in C string literals
}

// The macro does not consume the literals before nul checking occurs.
fn test_empty() {
    empty!(c"\0");     //~ ERROR null characters in C string literals
    empty!(c"\u{00}"); //~ ERROR null characters in C string literals
    empty!(c"^@");     //~ ERROR null characters in C string literals
    empty!(c"\x00");   //~ ERROR null characters in C string literals
    empty!(cr"^@");    //~ ERROR null characters in C string literals
}

as rendered by vim.

[petrochenkov]

Blocked on the decisions in #118699, but I'm fine with temporarily landing this if it unblocks stabilizing C-strings sooner.
@rustbot blocked

[nnethercote]

Blocked on the decisions in #118699, but I'm fine with temporarily landing this if it unblocks stabilizing C-strings sooner.

That's exactly the point of this PR: to unblock stabilization of C-string literals. Can you unblock?

[petrochenkov]

@bors r+

[bors]

📌 Commit 9018d2c has been approved by petrochenkov

It is now in the queue for this repository.

[ehuss]

@nnethercote Just want to double check if you can post a reference PR update as mentioned at https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/rfc.203349.3A.20mixed.20utf8.20literals/near/409285306 to update the grammar and remove the sentence about post-lexing validation?

[nnethercote]

@nnethercote Just want to double check if you can post a reference PR update

Thanks for the reminder, done in rust-lang/reference#1450.