rust icon indicating copy to clipboard operation
rust copied to clipboard

Using `ptr::addr_of!` instead of `ptr::addr_of_mut!` produces invalid help message

Open cyrgani opened this issue 1 year ago • 6 comments

Code

fn main() {
    let val = 2;
    let ptr = std::ptr::addr_of!(val);
    unsafe { *ptr = 3; }
}

Current output

error[E0594]: cannot assign to `*ptr`, which is behind a `*const` pointer
 --> a.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
 --> /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
  |      +++

Desired output

I'm not sure whether the ideal solution would be possible to implement, so I offer three different options:

  1. Suggest replacing std::ptr::addr_of! with std::ptr::addr_of_mut! This seems to be the best option, but it might be impossible as the macro was already expanded.

  2. Removing the help suggestion without a replacement This would be less helpful, but avoid a faulty error message.

  3. Suggesting the appropriate syntax for a raw mutable reference instead Since &mut raw const is invalid syntax and should be &raw mut instead, this should be suggested.

Rationale and extra context

The current help message is confusing as it shows syntax from the expanded addr_of! macro inside std, which the user cannot change reasonably anyway. Additionally, the suggestion is wrong:

#![feature(raw_ref_op)]

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

gives

error: expected one of `!`, `.`, `::`, `;`, `?`, `else`, `{`, or an operator, found keyword `const`
 --> a.rs:5:24
  |
5 |     let ptr = &mut raw const val;
  |                        ^^^^^ expected one of 8 possible tokens

whereas the correct syntax would be &raw mut val.

Other cases

No response

Rust Version

rustc 1.79.0 (129f3b996 2024-06-10)
binary: rustc
commit-hash: 129f3b9964af4d4a709d1383930ade12dfe7c081
commit-date: 2024-06-10
host: x86_64-unknown-linux-gnu
release: 1.79.0
LLVM version: 18.1.7

Anything else?

No response

cyrgani avatar Jul 10 '24 08:07 cyrgani

I think the current suggestion is suggesting the code like this:

    let ptr = &mut val;

chenyukang avatar Jul 13 '24 03:07 chenyukang

oh, the suggestion's code comes from here:

https://github.com/chenyukang/rust/blob/c8ed5c50edfd1b721c0541fa614864179251aaac/library/core/src/ptr/mod.rs#L2208

I think we should not suggest to change the library code here.

chenyukang avatar Jul 13 '24 03:07 chenyukang

I have playground link that represents the problem above: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=fee632ddd4f51a0390ac6918ef4eff96

However, when I add that code as a regression tests in tests/ui directory, the diagnostic suddenly changed to

error[E0594]: cannot assign to `*ptr`, which is behind a `*const` pointer
  --> $DIR/dont_suggest_raw_pointer_syntax-issue-127562.rs:9:9
   |
LL |         *ptr = 3;
   |         ^^^^^^^^ `ptr` is a `*const` pointer, so the data it refers to cannot be written
   |
help: consider specifying this binding's type
   |
LL |     let ptr: *mut i32 = std::ptr::addr_of!(val);
   |            ++++++++++

I don't have any ideas about this differences.

tesuji avatar Jul 13 '24 03:07 tesuji

binding's type

This is because when run the test UI, the span here is remaped $SRC_DIR/core/src/ptr/mod.rs:LL:COL (#4): https://github.com/chenyukang/rust/blob/b44a48416d06524da2ea247f1d1973d85ef514f8/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs#L1452

so the here we get an error and didn't continue the code branch: https://github.com/chenyukang/rust/blob/b44a48416d06524da2ea247f1d1973d85ef514f8/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs#L1453

it's weird, maybe another issue need to be fixed.

chenyukang avatar Jul 13 '24 05:07 chenyukang

Given that &raw syntax might get stabilized soon by #127679, suggesting the correct &raw mut syntax would become a more feasible option.

cyrgani avatar Jul 14 '24 08:07 cyrgani

&mut raw const $place

This is completely invalid syntax so just suggesting nothing instead as a first step would already be a good idea.

Suggesting anything that involves references is a really bad idea -- code that uses raw pointers should keep using raw pointers throughout with no intermediate references.

RalfJung avatar Jul 14 '24 16:07 RalfJung

With https://github.com/rust-lang/rust/pull/134244 we now at least don't suggest invalid syntax.

But it would be even nicer to suggest changing &raw const val to &raw mut val (which when applied will then suggest changing to let mut val).

Enselic avatar Dec 14 '24 09:12 Enselic

@rustbot label: -D-invalid-suggestion

cyrgani avatar Dec 15 '24 14:12 cyrgani