-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Rework ptr-to-ref conversion suggestion for method calls #123007
Rework ptr-to-ref conversion suggestion for method calls #123007
Conversation
r? @nnethercote rustbot has assigned @nnethercote. Use |
&& let ty::RawPtr(ty, ptr_mutbl) = *rcvr_ty.kind() | ||
&& let Ok(pick) = self.lookup_probe_for_diagnostic( | ||
item_name, | ||
Ty::new_ref(tcx, ty::Region::new_error_misc(tcx), ty, ptr_mutbl), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this is the right lifetime to use.
r? compiler |
☔ The latest upstream changes (presumably #123036) made this pull request unmergeable. Please resolve the merge conflicts. |
d3da599
to
b5e7a1c
Compare
☔ The latest upstream changes (presumably #123429) made this pull request unmergeable. Please resolve the merge conflicts. |
b5e7a1c
to
18f4f3d
Compare
TBH, I would change it to be
or
In general, we can rely on links to have information for things that are nice to know but nor absolutely required. Most people barely read the errors, even fewer read the linked docs. If we reliably had |
cd3bf4f
to
c78d878
Compare
This comment has been minimized.
This comment has been minimized.
c78d878
to
57aa6b1
Compare
☔ The latest upstream changes (presumably #123339) made this pull request unmergeable. Please resolve the merge conflicts. |
57aa6b1
to
41676ff
Compare
☔ The latest upstream changes (presumably #123576) made this pull request unmergeable. Please resolve the merge conflicts. |
41676ff
to
0d20a84
Compare
☔ The latest upstream changes (presumably #123569) made this pull request unmergeable. Please resolve the merge conflicts. |
0d20a84
to
c214d4b
Compare
☔ The latest upstream changes (presumably #123708) made this pull request unmergeable. Please resolve the merge conflicts. |
c214d4b
to
e53c449
Compare
e53c449
to
3a2a3ae
Compare
@bors r+ |
…ef, r=estebank Rework ptr-to-ref conversion suggestion for method calls If we have a value `z` of type `*const u8` and try to call `z.to_string()`, the upstream compiler will show you a note suggesting to call `<*const u8>::as_ref` first. This PR extends that: - The note will only be shown when the method would exist on the corresponding reference type - It can now suggest any of `<*const u8>::as_ref`, `<*mut u8>::as_ref` and `<*mut u8>::as_mut`, depending on what the method needs. I didn't introduce a `help` message because that's not a good idea with `unsafe` functions (and you'd also need to unwrap the `Option<&_>` somehow). People should check the safety requirements. For the simplest case ```rust fn main() { let x = 8u8; let z: *const u8 = &x; // issue rust-lang#21596 println!("{}", z.to_string()); //~ ERROR E0599 } ``` the output changes like this: ```diff error[E0599]: `*const u8` doesn't implement `std::fmt::Display` --> $DIR/suggest-convert-ptr-to-ref.rs:5:22 | LL | println!("{}", z.to_string()); | ^^^^^^^^^ `*const u8` cannot be formatted with the default formatter | - = note: try using `<*const T>::as_ref()` to get a reference to the type behind the pointer: https://doc.rust-lang.org/std/primitive.pointer.html#method.as_ref - = note: using `<*const T>::as_ref()` on a pointer which is unaligned or points to invalid or uninitialized memory is undefined behavior +note: the method `to_string` exists on the type `&u8` + --> $SRC_DIR/alloc/src/string.rs:LL:COL + = note: try using the unsafe method `<*const T>::as_ref` to get an optional reference to the value behind the pointer: https://doc.rust-lang.org/std/primitive.pointer.html#method.as_ref = note: the following trait bounds were not satisfied: `*const u8: std::fmt::Display` which is required by `*const u8: ToString` ``` I removed the separate note about the safety requirements because it was incomplete and the linked doc page already has the information you need. Fixes rust-lang#83695, but that's more of a side effect. The upstream compiler already suggests the right method name here.
💔 Test failed - checks-actions |
@bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (f13f37f): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis 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.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 676.971s -> 676.139s (-0.12%) |
If we have a value
z
of type*const u8
and try to callz.to_string()
, the upstream compiler will show you a note suggesting to call<*const u8>::as_ref
first.This PR extends that:
<*const u8>::as_ref
,<*mut u8>::as_ref
and<*mut u8>::as_mut
, depending on what the method needs.I didn't introduce a
help
message because that's not a good idea withunsafe
functions (and you'd also need to unwrap theOption<&_>
somehow).People should check the safety requirements.
For the simplest case
the output changes like this:
I removed the separate note about the safety requirements because it was incomplete and the linked doc page already has the information you need.
Fixes #83695, but that's more of a side effect. The upstream compiler already suggests the right method name here.