Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using &raw const instead of &raw mut does not offer a suggestion fix #127562

Closed
cyrgani opened this issue Jul 10, 2024 · 8 comments · Fixed by #134397
Closed

Using &raw const instead of &raw mut does not offer a suggestion fix #127562

cyrgani opened this issue Jul 10, 2024 · 8 comments · Fixed by #134397
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cyrgani
Copy link
Contributor

cyrgani commented Jul 10, 2024

Code

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

Current output

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

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

Desired output

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 = &raw mut val;
  |                    ~~~

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

Rationale and extra context

The current error message does not include a suggestion for how to fix it.

Other cases

No response

Rust Version

1.85.0-nightly (2024-12-14 0aeaa5eb22180fdf12a8)

Anything else?

EDIT 1: updated since addr_of! and addr_of_mut! diagnostics are not incorrect anymore due to #127675
EDIT 2: updated as #134224 removed the invalid suggestion

@cyrgani cyrgani added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 10, 2024
@chenyukang
Copy link
Member

chenyukang commented Jul 13, 2024

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

    let ptr = &mut val;

@chenyukang
Copy link
Member

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.

@tesuji
Copy link
Contributor

tesuji commented Jul 13, 2024

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.

@chenyukang
Copy link
Member

chenyukang commented Jul 13, 2024

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.

@cyrgani
Copy link
Contributor Author

cyrgani commented Jul 14, 2024

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

@RalfJung
Copy link
Member

&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.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 15, 2024
… r=fee1-dead

Remove invalid help diagnostics for const pointer

Partially addresses rust-lang#127562
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 19, 2024
… r=petrochenkov

Remove invalid help diagnostics for const pointer

Partially addresses rust-lang#127562
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 20, 2024
Rollup merge of rust-lang#127675 - chenyukang:yukang-fix-127562-addr, r=petrochenkov

Remove invalid help diagnostics for const pointer

Partially addresses rust-lang#127562
@cyrgani cyrgani changed the title Using ptr::addr_of! instead of ptr::addr_of_mut! produces invalid help message Using &raw const instead of &raw mut produces invalid help message Oct 21, 2024
@estebank estebank added A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-papercut Diagnostics: An error or lint that needs small tweaks. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. A-raw-pointers Area: raw pointers, MaybeUninit, NonNull labels Oct 22, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 13, 2024
…=jieyouxu

rustc_borrowck: Stop suggesting the invalid syntax `&mut raw const`

A legitimate suggestion would be to change from

    &raw const val

to

    &raw mut val

But until we have figured out how to make that happen we should at least
stop suggesting invalid syntax.

I recommend review commit-by-commit.

Part of rust-lang#127562
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 14, 2024
…=jieyouxu

rustc_borrowck: Stop suggesting the invalid syntax `&mut raw const`

A legitimate suggestion would be to change from

    &raw const val

to

    &raw mut val

But until we have figured out how to make that happen we should at least
stop suggesting invalid syntax.

I recommend review commit-by-commit.

Part of rust-lang#127562
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 14, 2024
…=jieyouxu

rustc_borrowck: Stop suggesting the invalid syntax `&mut raw const`

A legitimate suggestion would be to change from

    &raw const val

to

    &raw mut val

But until we have figured out how to make that happen we should at least
stop suggesting invalid syntax.

I recommend review commit-by-commit.

Part of rust-lang#127562
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 14, 2024
Rollup merge of rust-lang#134244 - Enselic:no-mut-hint-for-raw-ref, r=jieyouxu

rustc_borrowck: Stop suggesting the invalid syntax `&mut raw const`

A legitimate suggestion would be to change from

    &raw const val

to

    &raw mut val

But until we have figured out how to make that happen we should at least
stop suggesting invalid syntax.

I recommend review commit-by-commit.

Part of rust-lang#127562
@Enselic
Copy link
Member

Enselic commented Dec 14, 2024

With #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).

@cyrgani cyrgani changed the title Using &raw const instead of &raw mut produces invalid help message Using &raw const instead of &raw mut does not offer a suggestion fix Dec 15, 2024
@cyrgani
Copy link
Contributor Author

cyrgani commented Dec 15, 2024

@rustbot label: -D-invalid-suggestion

@rustbot rustbot removed the D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. label Dec 15, 2024
jhpratt added a commit to jhpratt/rust that referenced this issue Dec 17, 2024
rustc_borrowck: Suggest changing `&raw const` to `&raw mut` if applicable

Closes rust-lang#127562

For reference, here is the diff compared to the original error reported in that issue before rust-lang#134244 stopped suggesting the invalid syntax:

```
diff --git a/tests/ui/borrowck/no-invalid-mut-suggestion-for-raw-pointer-issue-127562.stderr b/tests/ui/borrowck/no-invalid-mut-suggestion-for-raw-pointer-issue-127562.stderr
index 0da5d15cf7f..dbe834b6b78 100644
--- a/tests/ui/borrowck/no-invalid-mut-suggestion-for-raw-pointer-issue-127562.stderr
+++ b/tests/ui/borrowck/no-invalid-mut-suggestion-for-raw-pointer-issue-127562.stderr
`@@` -6,8 +6,8 `@@` LL |     unsafe { *ptr = 3; }
    |
 help: consider changing this to be a mutable pointer
    |
-LL |     let ptr = &mut raw const val;
-   |                +++
+LL |     let ptr = &raw mut val;
+   |                    ~~~

 error: aborting due to 1 previous error
```
@bors bors closed this as completed in 40ce4e0 Dec 17, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 17, 2024
Rollup merge of rust-lang#134397 - Enselic:raw-mut, r=compiler-errors

rustc_borrowck: Suggest changing `&raw const` to `&raw mut` if applicable

Closes rust-lang#127562

For reference, here is the diff compared to the original error reported in that issue before rust-lang#134244 stopped suggesting the invalid syntax:

```
diff --git a/tests/ui/borrowck/no-invalid-mut-suggestion-for-raw-pointer-issue-127562.stderr b/tests/ui/borrowck/no-invalid-mut-suggestion-for-raw-pointer-issue-127562.stderr
index 0da5d15cf7f..dbe834b6b78 100644
--- a/tests/ui/borrowck/no-invalid-mut-suggestion-for-raw-pointer-issue-127562.stderr
+++ b/tests/ui/borrowck/no-invalid-mut-suggestion-for-raw-pointer-issue-127562.stderr
``@@`` -6,8 +6,8 ``@@`` LL |     unsafe { *ptr = 3; }
    |
 help: consider changing this to be a mutable pointer
    |
-LL |     let ptr = &mut raw const val;
-   |                +++
+LL |     let ptr = &raw mut val;
+   |                    ~~~

 error: aborting due to 1 previous error
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants