Skip to content

Commit f6c682f

Browse files
committed
Properly resolve associated constants of primitive types
Associated constants of primitive types are resolved as `Res::Err` in the HIR. To force a resolution, the primitive type must be recognized as certain. Since they are not ADT, they don't have a `DefId`. By allowing the `Certainty` type to embed a `TypeKind` which is either a `PrimTy` or a `DefId`, we allow primitives types to be resolved, and the associated constants as well. Also, in method calls, if the receiver is not certain, consider that the overall call is not either.
1 parent 23574ee commit f6c682f

File tree

6 files changed

+107
-30
lines changed

6 files changed

+107
-30
lines changed

clippy_utils/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#![feature(assert_matches)]
77
#![feature(unwrap_infallible)]
88
#![feature(array_windows)]
9+
#![feature(try_trait_v2)]
910
#![recursion_limit = "512"]
1011
#![allow(
1112
clippy::missing_errors_doc,

clippy_utils/src/ty/type_certainty/certainty.rs

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,22 @@
11
use rustc_hir::def_id::DefId;
22
use std::fmt::Debug;
3+
use std::ops::{ControlFlow, FromResidual, Try};
4+
5+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
6+
pub enum TypeKind {
7+
PrimTy,
8+
AdtDef(DefId),
9+
}
310

411
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
512
pub enum Certainty {
613
/// Determining the type requires contextual information.
714
Uncertain,
815

916
/// The type can be determined purely from subexpressions. If the argument is `Some(..)`, the
10-
/// specific `DefId` is known. Such arguments are needed to handle path segments whose `res` is
11-
/// `Res::Err`.
12-
Certain(Option<DefId>),
17+
/// specific primitive type or `DefId` is known. Such arguments are needed to handle path
18+
/// segments whose `res` is `Res::Err`.
19+
Certain(Option<TypeKind>),
1320

1421
/// The heuristic believes that more than one `DefId` applies to a type---this is a bug.
1522
Contradiction,
@@ -23,7 +30,7 @@ pub trait TryJoin: Sized {
2330
fn try_join(self, other: Self) -> Option<Self>;
2431
}
2532

26-
impl Meet for Option<DefId> {
33+
impl Meet for Option<TypeKind> {
2734
fn meet(self, other: Self) -> Self {
2835
match (self, other) {
2936
(None, _) | (_, None) => None,
@@ -32,11 +39,11 @@ impl Meet for Option<DefId> {
3239
}
3340
}
3441

35-
impl TryJoin for Option<DefId> {
42+
impl TryJoin for Option<TypeKind> {
3643
fn try_join(self, other: Self) -> Option<Self> {
3744
match (self, other) {
3845
(Some(lhs), Some(rhs)) => (lhs == rhs).then_some(Some(lhs)),
39-
(Some(def_id), _) | (_, Some(def_id)) => Some(Some(def_id)),
46+
(Some(ty_kind), _) | (_, Some(ty_kind)) => Some(Some(ty_kind)),
4047
(None, None) => Some(None),
4148
}
4249
}
@@ -79,29 +86,37 @@ impl Certainty {
7986
/// Join two `Certainty`s after clearing their `DefId`s. This method should be used when `self`
8087
/// or `other` do not necessarily refer to types, e.g., when they are aggregations of other
8188
/// `Certainty`s.
82-
pub fn join_clearing_def_ids(self, other: Self) -> Self {
83-
self.clear_def_id().join(other.clear_def_id())
89+
pub fn join_clearing_types(self, other: Self) -> Self {
90+
self.clear_type().join(other.clear_type())
8491
}
8592

86-
pub fn clear_def_id(self) -> Certainty {
93+
pub fn clear_type(self) -> Certainty {
8794
if matches!(self, Certainty::Certain(_)) {
8895
Certainty::Certain(None)
8996
} else {
9097
self
9198
}
9299
}
93100

101+
pub fn with_prim_ty(self) -> Certainty {
102+
if matches!(self, Certainty::Certain(_)) {
103+
Certainty::Certain(Some(TypeKind::PrimTy))
104+
} else {
105+
self
106+
}
107+
}
108+
94109
pub fn with_def_id(self, def_id: DefId) -> Certainty {
95110
if matches!(self, Certainty::Certain(_)) {
96-
Certainty::Certain(Some(def_id))
111+
Certainty::Certain(Some(TypeKind::AdtDef(def_id)))
97112
} else {
98113
self
99114
}
100115
}
101116

102117
pub fn to_def_id(self) -> Option<DefId> {
103118
match self {
104-
Certainty::Certain(inner) => inner,
119+
Certainty::Certain(Some(TypeKind::AdtDef(def_id))) => Some(def_id),
105120
_ => None,
106121
}
107122
}
@@ -120,3 +135,28 @@ pub fn meet(iter: impl Iterator<Item = Certainty>) -> Certainty {
120135
pub fn join(iter: impl Iterator<Item = Certainty>) -> Certainty {
121136
iter.fold(Certainty::Uncertain, Certainty::join)
122137
}
138+
139+
pub struct NoCertainty(Certainty);
140+
141+
impl FromResidual<NoCertainty> for Certainty {
142+
fn from_residual(residual: NoCertainty) -> Self {
143+
residual.0
144+
}
145+
}
146+
147+
impl Try for Certainty {
148+
type Output = Certainty;
149+
150+
type Residual = NoCertainty;
151+
152+
fn from_output(output: Self::Output) -> Self {
153+
output
154+
}
155+
156+
fn branch(self) -> ControlFlow<Self::Residual, Self::Output> {
157+
match self {
158+
Certainty::Certain(_) => ControlFlow::Continue(self),
159+
_ => ControlFlow::Break(NoCertainty(self)),
160+
}
161+
}
162+
}

clippy_utils/src/ty/type_certainty/mod.rs

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use rustc_middle::ty::{self, AdtDef, GenericArgKind, Ty};
2222
use rustc_span::Span;
2323

2424
mod certainty;
25-
use certainty::{Certainty, Meet, join, meet};
25+
use certainty::{Certainty, Meet, TypeKind, join, meet};
2626

2727
pub fn expr_type_is_certain(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
2828
expr_type_certainty(cx, expr, false).is_certain()
@@ -46,11 +46,12 @@ fn expr_type_certainty(cx: &LateContext<'_>, expr: &Expr<'_>, in_arg: bool) -> C
4646
} else {
4747
Certainty::Uncertain
4848
};
49-
lhs.join_clearing_def_ids(rhs)
49+
lhs.join_clearing_types(rhs)
5050
},
5151

5252
ExprKind::MethodCall(method, receiver, args, _) => {
53-
let mut receiver_type_certainty = expr_type_certainty(cx, receiver, false);
53+
let mut receiver_type_certainty = expr_type_certainty(cx, receiver, false)?;
54+
5455
// Even if `receiver_type_certainty` is `Certain(Some(..))`, the `Self` type in the method
5556
// identified by `type_dependent_def_id(..)` can differ. This can happen as a result of a `deref`,
5657
// for example. So update the `DefId` in `receiver_type_certainty` (if any).
@@ -66,7 +67,7 @@ fn expr_type_certainty(cx: &LateContext<'_>, expr: &Expr<'_>, in_arg: bool) -> C
6667
.chain(args.iter().map(|arg| expr_type_certainty(cx, arg, true))),
6768
)
6869
} else {
69-
Certainty::Uncertain
70+
return Certainty::Uncertain;
7071
};
7172
lhs.join(rhs)
7273
},
@@ -117,12 +118,13 @@ fn expr_type_certainty(cx: &LateContext<'_>, expr: &Expr<'_>, in_arg: bool) -> C
117118
_ => Certainty::Uncertain,
118119
};
119120

120-
let expr_ty = cx.typeck_results().expr_ty(expr);
121-
if let Some(def_id) = adt_def_id(expr_ty) {
122-
certainty.with_def_id(def_id)
123-
} else {
124-
certainty.clear_def_id()
125-
}
121+
let result = match cx.typeck_results().expr_ty(expr).kind() {
122+
ty::Adt(adt_def, _) => certainty.with_def_id(adt_def.did()),
123+
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) => certainty.with_prim_ty(),
124+
_ => certainty.clear_type(),
125+
};
126+
127+
result
126128
}
127129

128130
struct CertaintyVisitor<'cx, 'tcx> {
@@ -209,7 +211,11 @@ fn qpath_certainty(cx: &LateContext<'_>, qpath: &QPath<'_>, resolves_to_type: bo
209211
.map_or(Certainty::Uncertain, |def_id| {
210212
let generics = cx.tcx.generics_of(def_id);
211213
if generics.is_empty() {
212-
Certainty::Certain(if resolves_to_type { Some(def_id) } else { None })
214+
Certainty::Certain(if resolves_to_type {
215+
Some(TypeKind::AdtDef(def_id))
216+
} else {
217+
None
218+
})
213219
} else {
214220
Certainty::Uncertain
215221
}
@@ -230,7 +236,7 @@ fn param_certainty(cx: &LateContext<'_>, param: &Param<'_>) -> Certainty {
230236
let body_params = cx.tcx.hir_body_owned_by(owner_did).params;
231237
std::iter::zip(body_params, inputs)
232238
.find(|(p, _)| p.hir_id == param.hir_id)
233-
.map_or(Certainty::Uncertain, |(_, ty)| type_certainty(cx, ty).clear_def_id())
239+
.map_or(Certainty::Uncertain, |(_, ty)| type_certainty(cx, ty).clear_type())
234240
}
235241

236242
fn path_segment_certainty(
@@ -263,7 +269,7 @@ fn path_segment_certainty(
263269
.args
264270
.map_or(Certainty::Uncertain, |args| generic_args_certainty(cx, args));
265271
// See the comment preceding `qpath_certainty`. `def_id` could refer to a type or a value.
266-
let certainty = lhs.join_clearing_def_ids(rhs);
272+
let certainty = lhs.join_clearing_types(rhs);
267273
if resolves_to_type {
268274
if let DefKind::TyAlias = cx.tcx.def_kind(def_id) {
269275
adt_def_id(cx.tcx.type_of(def_id).instantiate_identity())
@@ -279,9 +285,7 @@ fn path_segment_certainty(
279285
}
280286
},
281287

282-
Res::PrimTy(_) | Res::SelfTyParam { .. } | Res::SelfTyAlias { .. } | Res::SelfCtor(_) => {
283-
Certainty::Certain(None)
284-
},
288+
Res::PrimTy(_) => Certainty::Certain(Some(TypeKind::PrimTy)),
285289

286290
// `get_parent` because `hir_id` refers to a `Pat`, and we're interested in the node containing the `Pat`.
287291
Res::Local(hir_id) => match cx.tcx.parent_hir_node(hir_id) {
@@ -299,13 +303,13 @@ fn path_segment_certainty(
299303
if resolves_to_type {
300304
certainty
301305
} else {
302-
certainty.clear_def_id()
306+
certainty.clear_type()
303307
}
304308
},
305309
_ => Certainty::Uncertain,
306310
},
307311

308-
_ => Certainty::Uncertain,
312+
_ => Certainty::Certain(None),
309313
};
310314
debug_assert!(resolves_to_type || certainty.to_def_id().is_none());
311315
certainty

tests/ui/unnecessary_cast.fixed

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,4 +310,17 @@ mod fixable {
310310
let _ = threshold;
311311
//~^ unnecessary_cast
312312
}
313+
314+
fn with_prim_ty() {
315+
let threshold = 20;
316+
let threshold = if threshold == 0 {
317+
i64::MAX
318+
} else if threshold <= 60 {
319+
10
320+
} else {
321+
0
322+
};
323+
let _ = threshold;
324+
//~^ unnecessary_cast
325+
}
313326
}

tests/ui/unnecessary_cast.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,4 +310,17 @@ mod fixable {
310310
let _ = threshold as i64;
311311
//~^ unnecessary_cast
312312
}
313+
314+
fn with_prim_ty() {
315+
let threshold = 20;
316+
let threshold = if threshold == 0 {
317+
i64::MAX
318+
} else if threshold <= 60 {
319+
10
320+
} else {
321+
0
322+
};
323+
let _ = threshold as i64;
324+
//~^ unnecessary_cast
325+
}
313326
}

tests/ui/unnecessary_cast.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,5 +283,11 @@ error: casting to the same type is unnecessary (`i64` -> `i64`)
283283
LL | let _ = threshold as i64;
284284
| ^^^^^^^^^^^^^^^^ help: try: `threshold`
285285

286-
error: aborting due to 47 previous errors
286+
error: casting to the same type is unnecessary (`i64` -> `i64`)
287+
--> tests/ui/unnecessary_cast.rs:323:17
288+
|
289+
LL | let _ = threshold as i64;
290+
| ^^^^^^^^^^^^^^^^ help: try: `threshold`
291+
292+
error: aborting due to 48 previous errors
287293

0 commit comments

Comments
 (0)