Remove invalid help diagnostics for const pointer

[chenyukang]

Partially addresses #127562

[rustbot]

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

[fee1-dead]

I have edited the PR description so that merging this PR does not close that issue. The best diagnostic improvement in that case would be suggesting addr_of! to be changed to addr_of_mut!.

@bors r+ rollup

[fee1-dead]

@bors r+ rollup

[bors]

📌 Commit b44a484 has been approved by fee1-dead

It is now in the queue for this repository.

[RalfJung]

Wait but this still suggests ill-typed code, doesn't it?

[fee1-dead]

Wait but this still suggests ill-typed code, doesn't it?

Huh. Hold on. The original issue doesn't even reproduce with latest nightly: https://rust.godbolt.org/z/c5vYcfY7d.

I'm less sure that this is actually fixing what it is supposed to be fixing.

@bors r-

[chenyukang]

Wait but this still suggests ill-typed code, doesn't it?

Huh. Hold on. The original issue doesn't even reproduce with latest nightly: https://rust.godbolt.org/z/c5vYcfY7d.

I'm less sure that this is actually fixing what it is supposed to be fixing.

@bors r-

I think godbolt does some similar span remap with test UI framework.
We can reproduce the original issue on playground and nightly command line:
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=f0b69f8ac092cab9389293dffb523cba

I will have a dig on why the ill-type code suggestion comes out when span is remapped.

[RalfJung]

It is also quite concerning that the ui test suite produces different results than playground... that's a disaster for testability of this bug. Is there an issue tracking that mismatch? Any idea how it can be fixed?

[chenyukang]

Seems the document for this is at here:
https://github.com/rust-lang/rustc-dev-guide/blob/d6e3a32a557db5902e714604def8015d6bb7e0f7/src/tests/ui.md#L114

The directory to the standard library source is replaced with $SRC_DIR. Example: /path/to/rust/library
Line and column numbers for paths in $SRC_DIR are replaced with LL:COL.
This helps ensure that changes to the layout of the standard library do not cause widespread changes to the .stderr files. Example: $SRC_DIR/alloc/src/sync.rs:53:46

so when the code logic is base on something like tcx.sess.source_map().span_to_snippet(span), it will return an Err:

Err(SourceNotAvailable { filename: Real(Remapped { local_path: None, virtual_name: "$SRC_DIR/core/src/ptr/mod.rs" }) })

then the code flow changes...

[chenyukang]

This new change will resolve this specific invalid suggestion in the UI test:
https://github.com/rust-lang/rust/pull/127675/files#diff-23edf23f7466b97eb3154b82f7b324983287c327937fd66e854b3df6759926ceR1149-R1154

@oli-obk told me we had several issues like this, I'm not sure whether we can get an more general to avoid different results between the UI test and playground.

[RalfJung]

Seems the document for this is at here: https://github.com/rust-lang/rustc-dev-guide/blob/d6e3a32a557db5902e714604def8015d6bb7e0f7/src/tests/ui.md#L114

The directory to the standard library source is replaced with $SRC_DIR. Example: /path/to/rust/library
Line and column numbers for paths in $SRC_DIR are replaced with LL:COL.
This helps ensure that changes to the layout of the standard library do not cause widespread changes to the .stderr files. Example: $SRC_DIR/alloc/src/sync.rs:53:46

so when the code logic is base on something like tcx.sess.source_map().span_to_snippet(span), it will return an Err:

Err(SourceNotAvailable { filename: Real(Remapped { local_path: None, virtual_name: "$SRC_DIR/core/src/ptr/mod.rs" }) })

then the code flow changes...

No, that just does some post-processing of the output, so that can't be it.

@oli-obk do you understand what is going on here? We should definitely have an issue about this -- when I copy some code from the playground to a ui test, it should behave the same, otherwise we aren't even testing the thing that people will be using in the wild!

[oli-obk]

idk, we do try to actually do what happens outside of the test suite, but there are some corner cases around libstd that are not easy and no one felt like it's too important to actually work on it.

[alex-semenyuk]

@chenyukang
From wg-triage. Could you please clarify the status of PR?

[chenyukang]

I think we could merge this PR first and create a new issue to track the test suite issue in #127675 (comment) ?

[chenyukang]

cc @fee1-dead

[fee1-dead]

r? compiler

[RalfJung]

The source of the problem here is that the suggestion points inside a macro. This is IMO not something that should be fixed in this specific diagnostic, it affects all daignostics and thus needs a central fix.

I don't know the diagnostic system well enough though to suggest how this could be fixed.
Cc @rust-lang/wg-diagnostics

[jieyouxu]

The "suggestion points inside a macro" problem is generally hard to solve, which is why very often suggestions just bail on Span::from_expansion. When this is not acceptable, then people sometimes use e.g.

• https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/span_encoding/struct.Span.html#method.find_ancestor_inside
• https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/span_encoding/struct.Span.html#method.find_ancestor_in_same_ctxt
• https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/span_encoding/struct.Span.html#method.find_ancestor_inside_same_ctxt
• https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/span_encoding/struct.Span.html#method.find_oldest_ancestor_in_same_ctxt

[RalfJung]

Hm, that's not what this code does though?

[RalfJung]

It is also quite concerning that the ui test suite produces different results than playground... that's a disaster for testability of this bug. Is there an issue tracking that mismatch? Any idea how it can be fixed?

I filed an issue for that: #131782

[petrochenkov]

@bors r+

[bors]

📌 Commit 7ff71e5 has been approved by petrochenkov

It is now in the queue for this repository.

[cyrgani]

This PR does not even fully solve the invalid diagnostic problem though: while addr_of! no longer suggests &mut raw const val, expanding the macro (which is recommended) does:

fn main() {
    let val = 2;
    let ptr = &raw const val;
    unsafe { *ptr = 3; }
}

Error:

error[E0594]: cannot assign to `*ptr`, which is behind a `*const` pointer
 --> src/main.rs:4:14
  |
4 |     unsafe { *ptr = 3; }
  |              ^^^^^^^^ `ptr` is a `*const` pointer, so the data it refers to cannot be written
  |
help: consider changing this to be a mutable pointer
  |
3 |     let ptr = &mut raw const val;
  |                +++

For more information about this error, try `rustc --explain E0594`.

[chenyukang]

This PR does not even fully solve the invalid diagnostic problem though: while addr_of! no longer suggests &mut raw const val, expanding the macro (which is recommended) does:

fn main() {
    let val = 2;
    let ptr = &raw const val;
    unsafe { *ptr = 3; }
}

Error:

error[E0594]: cannot assign to `*ptr`, which is behind a `*const` pointer
 --> src/main.rs:4:14
  |
4 |     unsafe { *ptr = 3; }
  |              ^^^^^^^^ `ptr` is a `*const` pointer, so the data it refers to cannot be written
  |
help: consider changing this to be a mutable pointer
  |
3 |     let ptr = &mut raw const val;
  |                +++

For more information about this error, try `rustc --explain E0594`.

Yes, the original issue is not closed, this PR only remove this part, we should not suggest code change for any span which is not written by user:

help: consider changing this to be a mutable pointer
 --> /home/user/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:2189:6
  |
21|     &mut raw const $place
  |      +++