Skip to content

Commit 691aeaa

Browse files
borsbors
committed
Auto merge of #102875 - Dylan-DPC:rollup-zwcq8h9, r=Dylan-DPC
Rollup of 6 pull requests Successful merges: - #99696 (Uplift `clippy::for_loops_over_fallibles` lint into rustc) - #102055 (Move some tests to more reasonable directories) - #102786 (Remove tuple candidate, nothing special about it) - #102794 (Make tests capture the error printed by a Result return) - #102853 (Skip chained OpaqueCast when building captures.) - #102868 (Rename `AssocItemKind::TyAlias` to `AssocItemKind::Type`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
2 parents 8dfb407 + 81b9d0b commit 691aeaa

File tree

67 files changed

+478
-621
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

67 files changed

+478
-621
lines changed

Diff for: ‎compiler/rustc_ast/src/ast.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -2953,7 +2953,7 @@ pub enum AssocItemKind {
29532953
/// An associated function.
29542954
Fn(Box<Fn>),
29552955
/// An associated type.
2956-
TyAlias(Box<TyAlias>),
2956+
Type(Box<TyAlias>),
29572957
/// A macro expanding to associated items.
29582958
MacCall(P<MacCall>),
29592959
}
@@ -2963,7 +2963,7 @@ impl AssocItemKind {
29632963
match *self {
29642964
Self::Const(defaultness, ..)
29652965
| Self::Fn(box Fn { defaultness, .. })
2966-
| Self::TyAlias(box TyAlias { defaultness, .. }) => defaultness,
2966+
| Self::Type(box TyAlias { defaultness, .. }) => defaultness,
29672967
Self::MacCall(..) => Defaultness::Final,
29682968
}
29692969
}
@@ -2974,7 +2974,7 @@ impl From<AssocItemKind> for ItemKind {
29742974
match assoc_item_kind {
29752975
AssocItemKind::Const(a, b, c) => ItemKind::Const(a, b, c),
29762976
AssocItemKind::Fn(fn_kind) => ItemKind::Fn(fn_kind),
2977-
AssocItemKind::TyAlias(ty_alias_kind) => ItemKind::TyAlias(ty_alias_kind),
2977+
AssocItemKind::Type(ty_alias_kind) => ItemKind::TyAlias(ty_alias_kind),
29782978
AssocItemKind::MacCall(a) => ItemKind::MacCall(a),
29792979
}
29802980
}
@@ -2987,7 +2987,7 @@ impl TryFrom<ItemKind> for AssocItemKind {
29872987
Ok(match item_kind {
29882988
ItemKind::Const(a, b, c) => AssocItemKind::Const(a, b, c),
29892989
ItemKind::Fn(fn_kind) => AssocItemKind::Fn(fn_kind),
2990-
ItemKind::TyAlias(ty_alias_kind) => AssocItemKind::TyAlias(ty_alias_kind),
2990+
ItemKind::TyAlias(ty_kind) => AssocItemKind::Type(ty_kind),
29912991
ItemKind::MacCall(a) => AssocItemKind::MacCall(a),
29922992
_ => return Err(item_kind),
29932993
})

Diff for: ‎compiler/rustc_ast/src/mut_visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,7 @@ pub fn noop_flat_map_assoc_item<T: MutVisitor>(
11061106
visit_fn_sig(sig, visitor);
11071107
visit_opt(body, |body| visitor.visit_block(body));
11081108
}
1109-
AssocItemKind::TyAlias(box TyAlias {
1109+
AssocItemKind::Type(box TyAlias {
11101110
defaultness,
11111111
generics,
11121112
where_clauses,

Diff for: ‎compiler/rustc_ast/src/visit.rs

+7-9
Original file line numberDiff line numberDiff line change
@@ -244,14 +244,12 @@ pub trait Visitor<'ast>: Sized {
244244

245245
#[macro_export]
246246
macro_rules! walk_list {
247-
($visitor: expr, $method: ident, $list: expr) => {
248-
for elem in $list {
249-
$visitor.$method(elem)
250-
}
251-
};
252-
($visitor: expr, $method: ident, $list: expr, $($extra_args: expr),*) => {
253-
for elem in $list {
254-
$visitor.$method(elem, $($extra_args,)*)
247+
($visitor: expr, $method: ident, $list: expr $(, $($extra_args: expr),* )?) => {
248+
{
249+
#[cfg_attr(not(bootstrap), allow(for_loops_over_fallibles))]
250+
for elem in $list {
251+
$visitor.$method(elem $(, $($extra_args,)* )?)
252+
}
255253
}
256254
}
257255
}
@@ -685,7 +683,7 @@ pub fn walk_assoc_item<'a, V: Visitor<'a>>(visitor: &mut V, item: &'a AssocItem,
685683
let kind = FnKind::Fn(FnCtxt::Assoc(ctxt), ident, sig, vis, generics, body.as_deref());
686684
visitor.visit_fn(kind, span, id);
687685
}
688-
AssocItemKind::TyAlias(box TyAlias { generics, bounds, ty, .. }) => {
686+
AssocItemKind::Type(box TyAlias { generics, bounds, ty, .. }) => {
689687
visitor.visit_generics(generics);
690688
walk_list!(visitor, visit_param_bound, bounds, BoundKind::Bound);
691689
walk_list!(visitor, visit_ty, ty);

Diff for: ‎compiler/rustc_ast_lowering/src/item.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
804804
);
805805
(generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Provided(body_id)), true)
806806
}
807-
AssocItemKind::TyAlias(box TyAlias {
807+
AssocItemKind::Type(box TyAlias {
808808
ref generics,
809809
where_clauses,
810810
ref bounds,
@@ -850,7 +850,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
850850
fn lower_trait_item_ref(&mut self, i: &AssocItem) -> hir::TraitItemRef {
851851
let kind = match &i.kind {
852852
AssocItemKind::Const(..) => hir::AssocItemKind::Const,
853-
AssocItemKind::TyAlias(..) => hir::AssocItemKind::Type,
853+
AssocItemKind::Type(..) => hir::AssocItemKind::Type,
854854
AssocItemKind::Fn(box Fn { sig, .. }) => {
855855
hir::AssocItemKind::Fn { has_self: sig.decl.has_self() }
856856
}
@@ -898,7 +898,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
898898

899899
(generics, hir::ImplItemKind::Fn(sig, body_id))
900900
}
901-
AssocItemKind::TyAlias(box TyAlias { generics, where_clauses, ty, .. }) => {
901+
AssocItemKind::Type(box TyAlias { generics, where_clauses, ty, .. }) => {
902902
let mut generics = generics.clone();
903903
add_ty_alias_where_clause(&mut generics, *where_clauses, false);
904904
self.lower_generics(
@@ -941,7 +941,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
941941
span: self.lower_span(i.span),
942942
kind: match &i.kind {
943943
AssocItemKind::Const(..) => hir::AssocItemKind::Const,
944-
AssocItemKind::TyAlias(..) => hir::AssocItemKind::Type,
944+
AssocItemKind::Type(..) => hir::AssocItemKind::Type,
945945
AssocItemKind::Fn(box Fn { sig, .. }) => {
946946
hir::AssocItemKind::Fn { has_self: sig.decl.has_self() }
947947
}

Diff for: ‎compiler/rustc_ast_passes/src/ast_validation.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1556,7 +1556,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
15561556
});
15571557
}
15581558
}
1559-
AssocItemKind::TyAlias(box TyAlias {
1559+
AssocItemKind::Type(box TyAlias {
15601560
generics,
15611561
where_clauses,
15621562
where_predicates_split,
@@ -1595,7 +1595,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
15951595
}
15961596

15971597
match item.kind {
1598-
AssocItemKind::TyAlias(box TyAlias { ref generics, ref bounds, ref ty, .. })
1598+
AssocItemKind::Type(box TyAlias { ref generics, ref bounds, ref ty, .. })
15991599
if ctxt == AssocCtxt::Trait =>
16001600
{
16011601
self.visit_vis(&item.vis);

Diff for: ‎compiler/rustc_ast_passes/src/feature_gate.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
517517
fn visit_assoc_item(&mut self, i: &'a ast::AssocItem, ctxt: AssocCtxt) {
518518
let is_fn = match i.kind {
519519
ast::AssocItemKind::Fn(_) => true,
520-
ast::AssocItemKind::TyAlias(box ast::TyAlias { ref ty, .. }) => {
520+
ast::AssocItemKind::Type(box ast::TyAlias { ref ty, .. }) => {
521521
if let (Some(_), AssocCtxt::Trait) = (ty, ctxt) {
522522
gate_feature_post!(
523523
&self,

Diff for: ‎compiler/rustc_ast_pretty/src/pprust/state/item.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ impl<'a> State<'a> {
516516
ast::AssocItemKind::Const(def, ty, body) => {
517517
self.print_item_const(ident, None, ty, body.as_deref(), vis, *def);
518518
}
519-
ast::AssocItemKind::TyAlias(box ast::TyAlias {
519+
ast::AssocItemKind::Type(box ast::TyAlias {
520520
defaultness,
521521
generics,
522522
where_clauses,

Diff for: ‎compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ impl<'a> TraitDef<'a> {
566566
tokens: None,
567567
},
568568
attrs: ast::AttrVec::new(),
569-
kind: ast::AssocItemKind::TyAlias(Box::new(ast::TyAlias {
569+
kind: ast::AssocItemKind::Type(Box::new(ast::TyAlias {
570570
defaultness: ast::Defaultness::Final,
571571
generics: Generics::default(),
572572
where_clauses: (

Diff for: ‎compiler/rustc_lint/src/for_loops_over_fallibles.rs

+183
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
use crate::{LateContext, LateLintPass, LintContext};
2+
3+
use hir::{Expr, Pat};
4+
use rustc_errors::{Applicability, DelayDm};
5+
use rustc_hir as hir;
6+
use rustc_infer::traits::TraitEngine;
7+
use rustc_infer::{infer::TyCtxtInferExt, traits::ObligationCause};
8+
use rustc_middle::ty::{self, List};
9+
use rustc_span::{sym, Span};
10+
use rustc_trait_selection::traits::TraitEngineExt;
11+
12+
declare_lint! {
13+
/// The `for_loops_over_fallibles` lint checks for `for` loops over `Option` or `Result` values.
14+
///
15+
/// ### Example
16+
///
17+
/// ```rust
18+
/// let opt = Some(1);
19+
/// for x in opt { /* ... */}
20+
/// ```
21+
///
22+
/// {{produces}}
23+
///
24+
/// ### Explanation
25+
///
26+
/// Both `Option` and `Result` implement `IntoIterator` trait, which allows using them in a `for` loop.
27+
/// `for` loop over `Option` or `Result` will iterate either 0 (if the value is `None`/`Err(_)`)
28+
/// or 1 time (if the value is `Some(_)`/`Ok(_)`). This is not very useful and is more clearly expressed
29+
/// via `if let`.
30+
///
31+
/// `for` loop can also be accidentally written with the intention to call a function multiple times,
32+
/// while the function returns `Some(_)`, in these cases `while let` loop should be used instead.
33+
///
34+
/// The "intended" use of `IntoIterator` implementations for `Option` and `Result` is passing them to
35+
/// generic code that expects something implementing `IntoIterator`. For example using `.chain(option)`
36+
/// to optionally add a value to an iterator.
37+
pub FOR_LOOPS_OVER_FALLIBLES,
38+
Warn,
39+
"for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
40+
}
41+
42+
declare_lint_pass!(ForLoopsOverFallibles => [FOR_LOOPS_OVER_FALLIBLES]);
43+
44+
impl<'tcx> LateLintPass<'tcx> for ForLoopsOverFallibles {
45+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
46+
let Some((pat, arg)) = extract_for_loop(expr) else { return };
47+
48+
let ty = cx.typeck_results().expr_ty(arg);
49+
50+
let &ty::Adt(adt, substs) = ty.kind() else { return };
51+
52+
let (article, ty, var) = match adt.did() {
53+
did if cx.tcx.is_diagnostic_item(sym::Option, did) => ("an", "Option", "Some"),
54+
did if cx.tcx.is_diagnostic_item(sym::Result, did) => ("a", "Result", "Ok"),
55+
_ => return,
56+
};
57+
58+
let msg = DelayDm(|| {
59+
format!(
60+
"for loop over {article} `{ty}`. This is more readably written as an `if let` statement",
61+
)
62+
});
63+
64+
cx.struct_span_lint(FOR_LOOPS_OVER_FALLIBLES, arg.span, msg, |lint| {
65+
if let Some(recv) = extract_iterator_next_call(cx, arg)
66+
&& let Ok(recv_snip) = cx.sess().source_map().span_to_snippet(recv.span)
67+
{
68+
lint.span_suggestion(
69+
recv.span.between(arg.span.shrink_to_hi()),
70+
format!("to iterate over `{recv_snip}` remove the call to `next`"),
71+
".by_ref()",
72+
Applicability::MaybeIncorrect
73+
);
74+
} else {
75+
lint.multipart_suggestion_verbose(
76+
format!("to check pattern in a loop use `while let`"),
77+
vec![
78+
// NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts
79+
(expr.span.with_hi(pat.span.lo()), format!("while let {var}(")),
80+
(pat.span.between(arg.span), format!(") = ")),
81+
],
82+
Applicability::MaybeIncorrect
83+
);
84+
}
85+
86+
if suggest_question_mark(cx, adt, substs, expr.span) {
87+
lint.span_suggestion(
88+
arg.span.shrink_to_hi(),
89+
"consider unwrapping the `Result` with `?` to iterate over its contents",
90+
"?",
91+
Applicability::MaybeIncorrect,
92+
);
93+
}
94+
95+
lint.multipart_suggestion_verbose(
96+
"consider using `if let` to clear intent",
97+
vec![
98+
// NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts
99+
(expr.span.with_hi(pat.span.lo()), format!("if let {var}(")),
100+
(pat.span.between(arg.span), format!(") = ")),
101+
],
102+
Applicability::MaybeIncorrect,
103+
)
104+
})
105+
}
106+
}
107+
108+
fn extract_for_loop<'tcx>(expr: &Expr<'tcx>) -> Option<(&'tcx Pat<'tcx>, &'tcx Expr<'tcx>)> {
109+
if let hir::ExprKind::DropTemps(e) = expr.kind
110+
&& let hir::ExprKind::Match(iterexpr, [arm], hir::MatchSource::ForLoopDesugar) = e.kind
111+
&& let hir::ExprKind::Call(_, [arg]) = iterexpr.kind
112+
&& let hir::ExprKind::Loop(block, ..) = arm.body.kind
113+
&& let [stmt] = block.stmts
114+
&& let hir::StmtKind::Expr(e) = stmt.kind
115+
&& let hir::ExprKind::Match(_, [_, some_arm], _) = e.kind
116+
&& let hir::PatKind::Struct(_, [field], _) = some_arm.pat.kind
117+
{
118+
Some((field.pat, arg))
119+
} else {
120+
None
121+
}
122+
}
123+
124+
fn extract_iterator_next_call<'tcx>(
125+
cx: &LateContext<'_>,
126+
expr: &Expr<'tcx>,
127+
) -> Option<&'tcx Expr<'tcx>> {
128+
// This won't work for `Iterator::next(iter)`, is this an issue?
129+
if let hir::ExprKind::MethodCall(_, recv, _, _) = expr.kind
130+
&& cx.typeck_results().type_dependent_def_id(expr.hir_id) == cx.tcx.lang_items().next_fn()
131+
{
132+
Some(recv)
133+
} else {
134+
return None
135+
}
136+
}
137+
138+
fn suggest_question_mark<'tcx>(
139+
cx: &LateContext<'tcx>,
140+
adt: ty::AdtDef<'tcx>,
141+
substs: &List<ty::GenericArg<'tcx>>,
142+
span: Span,
143+
) -> bool {
144+
let Some(body_id) = cx.enclosing_body else { return false };
145+
let Some(into_iterator_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator) else { return false };
146+
147+
if !cx.tcx.is_diagnostic_item(sym::Result, adt.did()) {
148+
return false;
149+
}
150+
151+
// Check that the function/closure/constant we are in has a `Result` type.
152+
// Otherwise suggesting using `?` may not be a good idea.
153+
{
154+
let ty = cx.typeck_results().expr_ty(&cx.tcx.hir().body(body_id).value);
155+
let ty::Adt(ret_adt, ..) = ty.kind() else { return false };
156+
if !cx.tcx.is_diagnostic_item(sym::Result, ret_adt.did()) {
157+
return false;
158+
}
159+
}
160+
161+
let ty = substs.type_at(0);
162+
let infcx = cx.tcx.infer_ctxt().build();
163+
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(infcx.tcx);
164+
165+
let cause = ObligationCause::new(
166+
span,
167+
body_id.hir_id,
168+
rustc_infer::traits::ObligationCauseCode::MiscObligation,
169+
);
170+
fulfill_cx.register_bound(
171+
&infcx,
172+
ty::ParamEnv::empty(),
173+
// Erase any region vids from the type, which may not be resolved
174+
infcx.tcx.erase_regions(ty),
175+
into_iterator_did,
176+
cause,
177+
);
178+
179+
// Select all, including ambiguous predicates
180+
let errors = fulfill_cx.select_all_or_error(&infcx);
181+
182+
errors.is_empty()
183+
}

Diff for: ‎compiler/rustc_lint/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ mod early;
5252
mod enum_intrinsics_non_enums;
5353
mod errors;
5454
mod expect;
55+
mod for_loops_over_fallibles;
5556
pub mod hidden_unicode_codepoints;
5657
mod internal;
5758
mod late;
@@ -86,6 +87,7 @@ use rustc_span::Span;
8687
use array_into_iter::ArrayIntoIter;
8788
use builtin::*;
8889
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
90+
use for_loops_over_fallibles::*;
8991
use hidden_unicode_codepoints::*;
9092
use internal::*;
9193
use let_underscore::*;
@@ -188,6 +190,7 @@ macro_rules! late_lint_mod_passes {
188190
$macro!(
189191
$args,
190192
[
193+
ForLoopsOverFallibles: ForLoopsOverFallibles,
191194
HardwiredLints: HardwiredLints,
192195
ImproperCTypesDeclarations: ImproperCTypesDeclarations,
193196
ImproperCTypesDefinitions: ImproperCTypesDefinitions,

Diff for: ‎compiler/rustc_lint/src/nonstandard_style.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ impl EarlyLintPass for NonCamelCaseTypes {
187187
}
188188

189189
fn check_trait_item(&mut self, cx: &EarlyContext<'_>, it: &ast::AssocItem) {
190-
if let ast::AssocItemKind::TyAlias(..) = it.kind {
190+
if let ast::AssocItemKind::Type(..) = it.kind {
191191
self.check_case(cx, "associated type", &it.ident);
192192
}
193193
}

0 commit comments

Comments
 (0)