-
Notifications
You must be signed in to change notification settings - Fork 1.8k
as_pointer_underscore: reduce applicability for unnameable inferred type
#15418
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
base: master
Are you sure you want to change the base?
Conversation
|
r? @Alexendoo rustbot has assigned @Alexendoo. Use |
6e3f3e5 to
a53cbdc
Compare
|
r? clippy |
| && let ty::RawPtr(pointee, _) = ty_into.kind() | ||
| // skip fn item type, since it can not be explicitly expressed with syntax | ||
| && !matches!(pointee.kind(), ty::FnDef(..)) |
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.
The missing_transmute_annotations lint was running into basically the same unnamed types issue and someone wrote a function for it:
rust-clippy/clippy_lints/src/transmute/missing_transmute_annotations.rs
Lines 107 to 112 in 7a12684
| fn ty_cannot_be_named(ty: Ty<'_>) -> bool { | |
| matches!( | |
| ty.kind(), | |
| ty::Alias(ty::AliasTyKind::Opaque | ty::AliasTyKind::Inherent, _) | |
| ) | |
| } |
Could make sense to add ty::FnDef(..) there and use that function here as well so we have one place for checking for whether a type could be written in user code. What do you think?
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.
And looking at how this situation is handled in missing_transmute_annotations, it might make sense to handle it in the same way here and actually still emit a warning with a reduced applicability (not MachineApplicable).
I can't imagine a situation in which one would want to create a raw pointer to a fn item, but given that this is a restriction lint that one would have to enable manually, it does seem valid to emit a warning anyway even if it can't simply be written because the point still stands that as *const _ is prone to refactoring bugs.
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.
Could make sense to add
ty::FnDef(..)there and use that function here as well so we have one place for checking for whether a type could be written in user code. What do you think?
makes sense. just moved ty_cannot_be_named to clippy_utils.
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.
makes sense. just moved ty_cannot_be_named to clippy_utils.
is_suggestable function used in #15652 might be a better thing to do here? Probably we can update missing_transmute_annotations as well in a separate PR.
rust-clippy/clippy_lints/src/casts/as_underscore.rs
Lines 5 to 14 in 90d0085
| use rustc_middle::ty::IsSuggestable; | |
| use super::AS_UNDERSCORE; | |
| pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, ty: &'tcx Ty<'_>) { | |
| if matches!(ty.kind, TyKind::Infer(())) { | |
| span_lint_and_then(cx, AS_UNDERSCORE, expr.span, "using `as _` conversion", |diag| { | |
| let ty_resolved = cx.typeck_results().expr_ty(expr); | |
| if ty_resolved.is_suggestable(cx.tcx, true) { | |
| diag.span_suggestion( |
a53cbdc to
2f1aa60
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
2f1aa60 to
3a55d66
Compare
as_pointer_underscore: don't lint when casting fn item pointersas_pointer_underscore: reduce applicability for unnameable inferred type
3a55d66 to
20afee5
Compare
don't emit machine applicable suggestion when the inferred type is unsuggestable, for example fn item pointer, closure changelog: [`as_pointer_underscore`]: reduce applicability for unsuggestable inferred type Signed-off-by: Zihan <[email protected]>
20afee5 to
45b4531
Compare
|
ping @y21 :) |
| #![crate_type = "lib"] | ||
| #![no_std] |
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.
These two attrs should be redundant, or are they needed for something?
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.
Also, could the single test case in this file just be part of tests/ui/as_pointer_underscore.rs? Usually _unfixable.rs test files are for lints that emit a broken suggestion and need //@no-rustfix, but in this case there's no suggestion at all so it should be fine to live in the same file as the other lint's test cases.
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.
We are actually still emitting some helper message w/o the full type name: "use explicit type" for unsuggestable types. Should we make it silent?
Don't emit machine applicable suggestion for unsuggestable inferred type, for example fn item pointer.
Fixes: #15281
changelog: [
as_pointer_underscore]: reduce applicability for unsuggestable inferred type