-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Note expr being cast when encounter NonScalar cast error #140787
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
Conversation
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.
Most of the time this results in a longer message but no extra information, e.g.:
LL | fn main() { let u = 0u32 as (); }
| ^^^^^^^^^^ an `as` expression can only be used to convert between primitive types or to coerce to a specific trait object
|
= note: casting expr `0u32` as `()`
It's obvious that 0u32
is cast as ()
, it's right there above. Is it possible to only add the note when the type being cast to is partially/fully unspecified?
Also, in the issue-73886.rs
we see some cases where the type is partially unspecified but the error message doesn't give any extra information:
= note: casting expr `&&[0]` as `&[_]`
= note: casting expr `7u32` as `Option<_>`
It would be nice if the _
were actually expanded here.
BTW, two thumbs up for adding the new test in the first commit and then doing the rest of the changes in the second commit. That makes reviewing easier.
Thanks for review :)
Indeed, it doesn't seem necessary to add duplicate notes for some simple types, and we can add notes when expressions are composite types such as references, for example, are added. However, the type may not be what this PR needs to address, but rather should indicate which expression is being converted.
In
Yes, this was handed to me the last time you reviewed my PR, and I've been doing this ever since. |
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.
@rustbot ready
|
||
if let Ok(snippet) = fcx.tcx.sess.source_map().span_to_snippet(self.expr_span) | ||
&& matches!(self.expr.kind, ExprKind::AddrOf(..)) | ||
{ | ||
err.note(format!("casting reference expression `{}`", snippet)); | ||
} | ||
|
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.
Now, it will emit note only when the expression is &...
.
This comment has been minimized.
This comment has been minimized.
Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>
Never mind.
I think it is a useful hint!
I'm concerned that simply suggesting parentheses may be inconsistent with the original intent of the code writer. There are a variety of situations that trigger such hints, such as &[0] as &&[_], which may require further categorization to refine the suggestions. The current use of NOTE seems to be sufficient without being unnecessarily intrusive. Specific suggestion information can be left to be enhanced later. :) @rustbot ready |
tests/ui/coercion/issue-73886.stderr
Outdated
@@ -3,6 +3,8 @@ error[E0605]: non-primitive cast: `&&[i32; 1]` as `&[_]` | |||
| | |||
LL | let _ = &&[0] as &[_]; | |||
| ^^^^^^^^^^^^^ an `as` expression can only be used to convert between primitive types or to coerce to a specific trait object | |||
| | |||
= note: casting reference expression `&&[0]`, where `&` binds tighter than `as` |
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 think because
works better than where
. Otherwise, this looks good.
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.
Good suggestion. I will revise it later.
Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>
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.
@rustbot ready
LL | ..._>) = &my_fn as _; | ||
| ^^^^^^^^^^^ an `as` expression can only be used to convert between primitive types or to coerce to a specific trait object | ||
| | ||
= note: casting reference expression `&my_fn` because `&` binds tighter than `as` |
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.
because
is indeed better.
@bors r+ rollup |
Rollup of 8 pull requests Successful merges: - #140787 (Note expr being cast when encounter NonScalar cast error) - #141112 (std: note that `std::str::from_utf8*` functions are aliases to `<str>::from_utf8*` methods) - #141646 (Document what `distcheck` is intended to exercise) - #141740 (Hir item kind field order) - #141793 (`tests/ui`: A New Order [1/N]) - #141805 (Update `compiler-builtins` to 0.1.160) - #141815 (Enable non-leaf Frame Pointers for mingw-w64 Arm64 Windows) - #141819 (Fixes for building windows-gnullvm hosts) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #140787 - xizheyin:issue-140491, r=nnethercote Note expr being cast when encounter NonScalar cast error Fixes #140491 I added note for `expr` so that it doesn't treat `&x as T` as `&(x as T)` but `(&x) as T`. But I'm not sure if I want to add note for all NonScalar, maybe for specific `expr_ty`? r? compiler
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#140787 (Note expr being cast when encounter NonScalar cast error) - rust-lang#141112 (std: note that `std::str::from_utf8*` functions are aliases to `<str>::from_utf8*` methods) - rust-lang#141646 (Document what `distcheck` is intended to exercise) - rust-lang#141740 (Hir item kind field order) - rust-lang#141793 (`tests/ui`: A New Order [1/N]) - rust-lang#141805 (Update `compiler-builtins` to 0.1.160) - rust-lang#141815 (Enable non-leaf Frame Pointers for mingw-w64 Arm64 Windows) - rust-lang#141819 (Fixes for building windows-gnullvm hosts) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #140491
I added note for
expr
so that it doesn't treat&x as T
as&(x as T)
but(&x) as T
. But I'm not sure if I want to add note for all NonScalar, maybe for specificexpr_ty
?r? compiler