Skip to content

Commit f562117

Browse files
flip1995flip1995
authored
Rollup merge of rust-lang#5724 - ebroto:5697_const_result_option, r=Manishearth
redundant_pattern_matching: avoid non-`const fn` calls in const contexts changelog: Avoid suggesting non-`const fn` calls in const contexts in [`redundant_pattern_matching`] Fixes rust-lang#5697
2 parents 68c0b24 + a7743e9 commit f562117

7 files changed

+318
-46
lines changed

‎clippy_lints/src/redundant_pattern_matching.rs‎

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1-
use crate::utils::{match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
1+
use crate::utils::{in_constant, match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
22
use if_chain::if_chain;
33
use rustc_ast::ast::LitKind;
44
use rustc_errors::Applicability;
5-
use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind, QPath};
5+
use rustc_hir::{Arm, Expr, ExprKind, HirId, MatchSource, PatKind, QPath};
66
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_middle::ty;
8+
use rustc_mir::const_eval::is_const_fn;
79
use rustc_session::{declare_lint_pass, declare_tool_lint};
10+
use rustc_span::source_map::Symbol;
811

912
declare_clippy_lint! {
1013
/// **What it does:** Lint for redundant pattern matching over `Result` or
@@ -64,26 +67,37 @@ fn find_sugg_for_if_let<'a, 'tcx>(
6467
arms: &[Arm<'_>],
6568
keyword: &'static str,
6669
) {
70+
fn find_suggestion(cx: &LateContext<'_, '_>, hir_id: HirId, path: &QPath<'_>) -> Option<&'static str> {
71+
if match_qpath(path, &paths::RESULT_OK) && can_suggest(cx, hir_id, sym!(result_type), "is_ok") {
72+
return Some("is_ok()");
73+
}
74+
if match_qpath(path, &paths::RESULT_ERR) && can_suggest(cx, hir_id, sym!(result_type), "is_err") {
75+
return Some("is_err()");
76+
}
77+
if match_qpath(path, &paths::OPTION_SOME) && can_suggest(cx, hir_id, sym!(option_type), "is_some") {
78+
return Some("is_some()");
79+
}
80+
if match_qpath(path, &paths::OPTION_NONE) && can_suggest(cx, hir_id, sym!(option_type), "is_none") {
81+
return Some("is_none()");
82+
}
83+
None
84+
}
85+
86+
let hir_id = expr.hir_id;
6787
let good_method = match arms[0].pat.kind {
6888
PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
6989
if let PatKind::Wild = patterns[0].kind {
70-
if match_qpath(path, &paths::RESULT_OK) {
71-
"is_ok()"
72-
} else if match_qpath(path, &paths::RESULT_ERR) {
73-
"is_err()"
74-
} else if match_qpath(path, &paths::OPTION_SOME) {
75-
"is_some()"
76-
} else {
77-
return;
78-
}
90+
find_suggestion(cx, hir_id, path)
7991
} else {
80-
return;
92+
None
8193
}
8294
},
83-
84-
PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
85-
86-
_ => return,
95+
PatKind::Path(ref path) => find_suggestion(cx, hir_id, path),
96+
_ => None,
97+
};
98+
let good_method = match good_method {
99+
Some(method) => method,
100+
None => return,
87101
};
88102

89103
// check that `while_let_on_iterator` lint does not trigger
@@ -128,6 +142,7 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
128142
if arms.len() == 2 {
129143
let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);
130144

145+
let hir_id = expr.hir_id;
131146
let found_good_method = match node_pair {
132147
(
133148
PatKind::TupleStruct(ref path_left, ref patterns_left, _),
@@ -142,6 +157,8 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
142157
&paths::RESULT_ERR,
143158
"is_ok()",
144159
"is_err()",
160+
|| can_suggest(cx, hir_id, sym!(result_type), "is_ok"),
161+
|| can_suggest(cx, hir_id, sym!(result_type), "is_err"),
145162
)
146163
} else {
147164
None
@@ -160,6 +177,8 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
160177
&paths::OPTION_NONE,
161178
"is_some()",
162179
"is_none()",
180+
|| can_suggest(cx, hir_id, sym!(option_type), "is_some"),
181+
|| can_suggest(cx, hir_id, sym!(option_type), "is_none"),
163182
)
164183
} else {
165184
None
@@ -188,6 +207,7 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
188207
}
189208
}
190209

210+
#[allow(clippy::too_many_arguments)]
191211
fn find_good_method_for_match<'a>(
192212
arms: &[Arm<'_>],
193213
path_left: &QPath<'_>,
@@ -196,6 +216,8 @@ fn find_good_method_for_match<'a>(
196216
expected_right: &[&str],
197217
should_be_left: &'a str,
198218
should_be_right: &'a str,
219+
can_suggest_left: impl Fn() -> bool,
220+
can_suggest_right: impl Fn() -> bool,
199221
) -> Option<&'a str> {
200222
let body_node_pair = if match_qpath(path_left, expected_left) && match_qpath(path_right, expected_right) {
201223
(&(*arms[0].body).kind, &(*arms[1].body).kind)
@@ -207,10 +229,32 @@ fn find_good_method_for_match<'a>(
207229

208230
match body_node_pair {
209231
(ExprKind::Lit(ref lit_left), ExprKind::Lit(ref lit_right)) => match (&lit_left.node, &lit_right.node) {
210-
(LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
211-
(LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
232+
(LitKind::Bool(true), LitKind::Bool(false)) if can_suggest_left() => Some(should_be_left),
233+
(LitKind::Bool(false), LitKind::Bool(true)) if can_suggest_right() => Some(should_be_right),
212234
_ => None,
213235
},
214236
_ => None,
215237
}
216238
}
239+
240+
fn can_suggest(cx: &LateContext<'_, '_>, hir_id: HirId, diag_item: Symbol, name: &str) -> bool {
241+
if !in_constant(cx, hir_id) {
242+
return true;
243+
}
244+
245+
// Avoid suggesting calls to non-`const fn`s in const contexts, see #5697.
246+
cx.tcx
247+
.get_diagnostic_item(diag_item)
248+
.and_then(|def_id| {
249+
cx.tcx.inherent_impls(def_id).iter().find_map(|imp| {
250+
cx.tcx
251+
.associated_items(*imp)
252+
.in_definition_order()
253+
.find_map(|item| match item.kind {
254+
ty::AssocKind::Fn if item.ident.name.as_str() == name => Some(item.def_id),
255+
_ => None,
256+
})
257+
})
258+
})
259+
.map_or(false, |def_id| is_const_fn(cx.tcx, def_id))
260+
}

‎tests/ui/redundant_pattern_matching.fixed‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// run-rustfix
22

3+
#![feature(const_if_match)]
4+
#![feature(const_loop)]
35
#![warn(clippy::all)]
46
#![warn(clippy::redundant_pattern_matching)]
57
#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)]
@@ -67,6 +69,7 @@ fn main() {
6769
takes_bool(x);
6870

6971
issue5504();
72+
issue5697();
7073

7174
let _ = if gen_opt().is_some() {
7275
1
@@ -117,3 +120,42 @@ fn issue5504() {
117120
if m!().is_some() {}
118121
while m!().is_some() {}
119122
}
123+
124+
// None of these should be linted because none of the suggested methods
125+
// are `const fn` without toggling a feature.
126+
const fn issue5697() {
127+
if let Ok(_) = Ok::<i32, i32>(42) {}
128+
129+
if let Err(_) = Err::<i32, i32>(42) {}
130+
131+
if let Some(_) = Some(42) {}
132+
133+
if let None = None::<()> {}
134+
135+
while let Ok(_) = Ok::<i32, i32>(10) {}
136+
137+
while let Err(_) = Ok::<i32, i32>(10) {}
138+
139+
while let Some(_) = Some(42) {}
140+
141+
while let None = None::<()> {}
142+
143+
match Ok::<i32, i32>(42) {
144+
Ok(_) => true,
145+
Err(_) => false,
146+
};
147+
148+
match Err::<i32, i32>(42) {
149+
Ok(_) => false,
150+
Err(_) => true,
151+
};
152+
match Some(42) {
153+
Some(_) => true,
154+
None => false,
155+
};
156+
157+
match None::<()> {
158+
Some(_) => false,
159+
None => true,
160+
};
161+
}

‎tests/ui/redundant_pattern_matching.rs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// run-rustfix
22

3+
#![feature(const_if_match)]
4+
#![feature(const_loop)]
35
#![warn(clippy::all)]
46
#![warn(clippy::redundant_pattern_matching)]
57
#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)]
@@ -88,6 +90,7 @@ fn main() {
8890
takes_bool(x);
8991

9092
issue5504();
93+
issue5697();
9194

9295
let _ = if let Some(_) = gen_opt() {
9396
1
@@ -138,3 +141,42 @@ fn issue5504() {
138141
if let Some(_) = m!() {}
139142
while let Some(_) = m!() {}
140143
}
144+
145+
// None of these should be linted because none of the suggested methods
146+
// are `const fn` without toggling a feature.
147+
const fn issue5697() {
148+
if let Ok(_) = Ok::<i32, i32>(42) {}
149+
150+
if let Err(_) = Err::<i32, i32>(42) {}
151+
152+
if let Some(_) = Some(42) {}
153+
154+
if let None = None::<()> {}
155+
156+
while let Ok(_) = Ok::<i32, i32>(10) {}
157+
158+
while let Err(_) = Ok::<i32, i32>(10) {}
159+
160+
while let Some(_) = Some(42) {}
161+
162+
while let None = None::<()> {}
163+
164+
match Ok::<i32, i32>(42) {
165+
Ok(_) => true,
166+
Err(_) => false,
167+
};
168+
169+
match Err::<i32, i32>(42) {
170+
Ok(_) => false,
171+
Err(_) => true,
172+
};
173+
match Some(42) {
174+
Some(_) => true,
175+
None => false,
176+
};
177+
178+
match None::<()> {
179+
Some(_) => false,
180+
None => true,
181+
};
182+
}

0 commit comments

Comments
 (0)