Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit e2c6942

Browse files
gurrygurry
committedMar 18, 2024
Do not emit iteration note in coerce if loop does iterate
This fix does not cover all the cases when the loop iterates such as when the iterate value is a local instead of a literal
1 parent 4d4bb49 commit e2c6942

File tree

3 files changed

+572
-7
lines changed

3 files changed

+572
-7
lines changed
 

‎compiler/rustc_hir_typeck/src/coercion.rs

Lines changed: 113 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,12 @@
3636
//! ```
3737
3838
use crate::FnCtxt;
39+
use hir::{Expr, ExprField, ExprKind, LangItem, QPath};
40+
use rustc_ast::{LitKind, UnOp};
3941
use rustc_errors::{codes::*, struct_span_code_err, Applicability, Diag, MultiSpan};
4042
use rustc_hir as hir;
4143
use rustc_hir::def_id::DefId;
4244
use rustc_hir::intravisit::{self, Visitor};
43-
use rustc_hir::Expr;
4445
use rustc_hir_analysis::astconv::AstConv;
4546
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
4647
use rustc_infer::infer::{Coercion, DefineOpaqueTypes, InferOk, InferResult};
@@ -57,6 +58,7 @@ use rustc_middle::ty::relate::RelateResult;
5758
use rustc_middle::ty::visit::TypeVisitableExt;
5859
use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeAndMut};
5960
use rustc_session::parse::feature_err;
61+
use rustc_span::source_map::Spanned;
6062
use rustc_span::symbol::sym;
6163
use rustc_span::DesugaringKind;
6264
use rustc_target::spec::abi::Abi;
@@ -1694,6 +1696,23 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
16941696
let hir::ExprKind::Loop(_, _, _, loop_span) = expr.kind else {
16951697
return;
16961698
};
1699+
1700+
let hir = tcx.hir();
1701+
let parent_node = tcx.hir_node(hir.get_parent_item(expr.hir_id).into());
1702+
let parent_block = if let Some(body_id) = parent_node.body_id()
1703+
&& let hir::ExprKind::Block(block, _) = hir.body(body_id).value.kind
1704+
{
1705+
Some(block)
1706+
} else {
1707+
None
1708+
};
1709+
1710+
if let Some(block) = parent_block
1711+
&& Self::loop_iterates_atleast_once(block)
1712+
{
1713+
return;
1714+
}
1715+
16971716
let mut span: MultiSpan = vec![loop_span].into();
16981717
span.push_span_label(loop_span, "this might have zero elements to iterate on");
16991718
const MAXITER: usize = 3;
@@ -1714,15 +1733,11 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
17141733
ret_exprs.len() - MAXITER
17151734
));
17161735
}
1717-
let hir = tcx.hir();
1718-
let item = hir.get_parent_item(expr.hir_id);
17191736
let ret_msg = "return a value for the case when the loop has zero elements to iterate on";
17201737
let ret_ty_msg =
17211738
"otherwise consider changing the return type to account for that possibility";
1722-
let node = tcx.hir_node(item.into());
1723-
if let Some(body_id) = node.body_id()
1724-
&& let Some(sig) = node.fn_sig()
1725-
&& let hir::ExprKind::Block(block, _) = hir.body(body_id).value.kind
1739+
if let Some(block) = parent_block
1740+
&& let Some(sig) = parent_node.fn_sig()
17261741
&& !ty.is_never()
17271742
{
17281743
let indentation = if let None = block.expr
@@ -1787,6 +1802,97 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
17871802
}
17881803
}
17891804

1805+
/// Returns `true` if the given `block` is a loop
1806+
/// and it will definitely iterate at least once
1807+
fn loop_iterates_atleast_once(block: &hir::Block<'tcx>) -> bool {
1808+
// Check if `block` is a for loop and extract
1809+
// the expr it iterate over as `iter_target`
1810+
let Some(Expr {
1811+
kind:
1812+
ExprKind::DropTemps(Expr {
1813+
kind:
1814+
ExprKind::Match(
1815+
Expr {
1816+
kind:
1817+
ExprKind::Call(
1818+
Expr {
1819+
kind:
1820+
ExprKind::Path(
1821+
QPath::LangItem(LangItem::IntoIterIntoIter, ..),
1822+
..,
1823+
),
1824+
..
1825+
},
1826+
[Expr { kind: iter_target, .. }],
1827+
),
1828+
..
1829+
},
1830+
..,
1831+
),
1832+
..
1833+
}),
1834+
..
1835+
}) = block.expr
1836+
else {
1837+
return false; // Block is not a for loop
1838+
};
1839+
1840+
// Peel away any ref if present
1841+
let iter_target = match iter_target {
1842+
ExprKind::AddrOf(.., deref) => deref.kind,
1843+
_ => *iter_target,
1844+
};
1845+
1846+
// Computes value of a literal expr
1847+
// Takes into account any enclosing neg unary expr if present
1848+
fn get_literal<'a>(expr_kind: &ExprKind<'a>) -> Option<i128> {
1849+
match expr_kind {
1850+
ExprKind::Lit(Spanned { node: LitKind::Int(lit, ..), .. }) => {
1851+
i128::try_from(lit.get()).ok()
1852+
}
1853+
ExprKind::Unary(
1854+
UnOp::Neg,
1855+
Expr {
1856+
kind: ExprKind::Lit(Spanned { node: LitKind::Int(lit, ..), .. }), ..
1857+
},
1858+
) => i128::try_from(lit.get()).map(|v| -1 * v).ok(),
1859+
_ => None,
1860+
}
1861+
}
1862+
1863+
// Check if `iter_target` will be iterated over at least once.
1864+
// We support only exclusive, inclusive and infinite range
1865+
// literals and array literals
1866+
match iter_target {
1867+
// Exclusive range
1868+
ExprKind::Struct(
1869+
QPath::LangItem(LangItem::Range, ..),
1870+
[
1871+
ExprField { expr: Expr { kind: start, .. }, .. },
1872+
ExprField { expr: Expr { kind: end, .. }, .. },
1873+
],
1874+
..,
1875+
) => match (get_literal(start), get_literal(end)) {
1876+
(Some(start), Some(end)) => end > start,
1877+
_ => false,
1878+
},
1879+
// Inclusive range
1880+
ExprKind::Call(
1881+
Expr {
1882+
kind: ExprKind::Path(QPath::LangItem(LangItem::RangeInclusiveNew, ..)), ..
1883+
},
1884+
[Expr { kind: start, .. }, Expr { kind: end, .. }],
1885+
) => match (get_literal(start), get_literal(end)) {
1886+
(Some(start), Some(end)) => end >= start,
1887+
_ => false,
1888+
},
1889+
// Infinite range
1890+
ExprKind::Struct(QPath::LangItem(LangItem::RangeFrom, ..), ..) => true,
1891+
ExprKind::Array(items) => items.len() > 0,
1892+
_ => false,
1893+
}
1894+
}
1895+
17901896
fn report_return_mismatched_types<'a>(
17911897
&self,
17921898
cause: &ObligationCause<'tcx>,

‎tests/ui/coercion/coerce-for-loop-issue-122561.rs

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
// Regression test for #122561
2+
// Checks that we do our best not to emit the note
3+
// saying the loop might run zero times if it is
4+
// clear it will certainly run at least once
5+
6+
7+
// ----- Should not emit note for these -----
8+
fn atleast_once_iter_range() -> bool {
9+
for i in 0..3 {
10+
//~^ ERROR mismatched types
11+
return false;
12+
}
13+
}
14+
15+
fn atleast_once_iter_range_neg() -> bool {
16+
for i in -3..-1 {
17+
//~^ ERROR mismatched types
18+
return false;
19+
}
20+
}
21+
22+
fn atleast_once_iter_range_inclusive() -> bool {
23+
for i in 0..=3 {
24+
//~^ ERROR mismatched types
25+
return false;
26+
}
27+
}
28+
29+
fn atleast_once_iter_range_inclusive_neg() -> bool {
30+
for i in -3..=-1 {
31+
//~^ ERROR mismatched types
32+
return false;
33+
}
34+
}
35+
36+
fn atleast_once_iter_infinite_range() -> bool {
37+
for i in 0.. {
38+
//~^ ERROR mismatched types
39+
return false;
40+
}
41+
}
42+
43+
fn atleast_once_iter_infinite_range_neg() -> bool {
44+
for i in -3.. {
45+
//~^ ERROR mismatched types
46+
return false;
47+
}
48+
}
49+
50+
fn atleast_once_iter_array() -> bool {
51+
for i in [1, 2, 3] {
52+
//~^ ERROR mismatched types
53+
return false;
54+
}
55+
}
56+
57+
fn atleast_once_iter_array_ref() -> bool {
58+
for i in &[1, 2, 3] {
59+
//~^ ERROR mismatched types
60+
return false;
61+
}
62+
}
63+
64+
65+
// ----- Should emit note for these -----
66+
fn zero_iter_range() -> bool {
67+
for i in 0..0 {
68+
//~^ ERROR mismatched types
69+
return false;
70+
}
71+
}
72+
73+
fn zero_iter_array() -> bool {
74+
for i in [] {
75+
//~^ ERROR mismatched types
76+
return false;
77+
}
78+
}
79+
80+
fn zero_iter_array_ref() -> bool {
81+
for i in &[] {
82+
//~^ ERROR mismatched types
83+
return false;
84+
}
85+
}
86+
87+
// For the following cases the loop does iterate at
88+
// least once but we aren't currently smart enough
89+
// to not emit the note for them. We might add such
90+
// smarts in the future
91+
fn atlast_once_iter_array_var() -> bool {
92+
let x = [1, 2, 3];
93+
for i in x {
94+
//~^ ERROR mismatched types
95+
return false;
96+
}
97+
}
98+
99+
fn atleast_once_iter_vec() -> bool {
100+
for i in vec![1, 2, 3] {
101+
//~^ ERROR mismatched types
102+
return false;
103+
}
104+
}
105+
106+
fn atleast_once_iter_array_iter() -> bool {
107+
for i in [1, 2, 3].iter() {
108+
//~^ ERROR mismatched types
109+
return false;
110+
}
111+
}
112+
113+
fn atleast_once_iter_func_result() -> bool {
114+
for i in get_iter() {
115+
//~^ ERROR mismatched types
116+
return false;
117+
}
118+
}
119+
120+
121+
// Helper function
122+
fn get_iter() -> impl Iterator<Item=i32> {
123+
1..
124+
}
125+
126+
fn main() {}

‎tests/ui/coercion/coerce-for-loop-issue-122561.stderr

Lines changed: 333 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,333 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/coerce-for-loop-issue-122561.rs:9:5
3+
|
4+
LL | fn atleast_once_iter_range() -> bool {
5+
| ---- expected `bool` because of return type
6+
LL | / for i in 0..3 {
7+
LL | |
8+
LL | | return false;
9+
LL | | }
10+
| |_____^ expected `bool`, found `()`
11+
12+
error[E0308]: mismatched types
13+
--> $DIR/coerce-for-loop-issue-122561.rs:16:5
14+
|
15+
LL | fn atleast_once_iter_range_neg() -> bool {
16+
| ---- expected `bool` because of return type
17+
LL | / for i in -3..-1 {
18+
LL | |
19+
LL | | return false;
20+
LL | | }
21+
| |_____^ expected `bool`, found `()`
22+
23+
error[E0308]: mismatched types
24+
--> $DIR/coerce-for-loop-issue-122561.rs:23:5
25+
|
26+
LL | fn atleast_once_iter_range_inclusive() -> bool {
27+
| ---- expected `bool` because of return type
28+
LL | / for i in 0..=3 {
29+
LL | |
30+
LL | | return false;
31+
LL | | }
32+
| |_____^ expected `bool`, found `()`
33+
34+
error[E0308]: mismatched types
35+
--> $DIR/coerce-for-loop-issue-122561.rs:30:5
36+
|
37+
LL | fn atleast_once_iter_range_inclusive_neg() -> bool {
38+
| ---- expected `bool` because of return type
39+
LL | / for i in -3..=-1 {
40+
LL | |
41+
LL | | return false;
42+
LL | | }
43+
| |_____^ expected `bool`, found `()`
44+
45+
error[E0308]: mismatched types
46+
--> $DIR/coerce-for-loop-issue-122561.rs:37:5
47+
|
48+
LL | fn atleast_once_iter_infinite_range() -> bool {
49+
| ---- expected `bool` because of return type
50+
LL | / for i in 0.. {
51+
LL | |
52+
LL | | return false;
53+
LL | | }
54+
| |_____^ expected `bool`, found `()`
55+
56+
error[E0308]: mismatched types
57+
--> $DIR/coerce-for-loop-issue-122561.rs:44:5
58+
|
59+
LL | fn atleast_once_iter_infinite_range_neg() -> bool {
60+
| ---- expected `bool` because of return type
61+
LL | / for i in -3.. {
62+
LL | |
63+
LL | | return false;
64+
LL | | }
65+
| |_____^ expected `bool`, found `()`
66+
67+
error[E0308]: mismatched types
68+
--> $DIR/coerce-for-loop-issue-122561.rs:51:5
69+
|
70+
LL | fn atleast_once_iter_array() -> bool {
71+
| ---- expected `bool` because of return type
72+
LL | / for i in [1, 2, 3] {
73+
LL | |
74+
LL | | return false;
75+
LL | | }
76+
| |_____^ expected `bool`, found `()`
77+
78+
error[E0308]: mismatched types
79+
--> $DIR/coerce-for-loop-issue-122561.rs:58:5
80+
|
81+
LL | fn atleast_once_iter_array_ref() -> bool {
82+
| ---- expected `bool` because of return type
83+
LL | / for i in &[1, 2, 3] {
84+
LL | |
85+
LL | | return false;
86+
LL | | }
87+
| |_____^ expected `bool`, found `()`
88+
89+
error[E0308]: mismatched types
90+
--> $DIR/coerce-for-loop-issue-122561.rs:67:5
91+
|
92+
LL | fn zero_iter_range() -> bool {
93+
| ---- expected `bool` because of return type
94+
LL | / for i in 0..0 {
95+
LL | |
96+
LL | | return false;
97+
LL | | }
98+
| |_____^ expected `bool`, found `()`
99+
|
100+
note: the function expects a value to always be returned, but loops might run zero times
101+
--> $DIR/coerce-for-loop-issue-122561.rs:67:5
102+
|
103+
LL | for i in 0..0 {
104+
| ^^^^^^^^^^^^^ this might have zero elements to iterate on
105+
LL |
106+
LL | return false;
107+
| ------------ if the loop doesn't execute, this value would never get returned
108+
help: return a value for the case when the loop has zero elements to iterate on
109+
|
110+
LL ~ }
111+
LL + /* `bool` value */
112+
|
113+
help: otherwise consider changing the return type to account for that possibility
114+
|
115+
LL ~ fn zero_iter_range() -> Option<bool> {
116+
LL | for i in 0..0 {
117+
LL |
118+
LL ~ return Some(false);
119+
LL ~ }
120+
LL + None
121+
|
122+
123+
error[E0308]: mismatched types
124+
--> $DIR/coerce-for-loop-issue-122561.rs:74:5
125+
|
126+
LL | fn zero_iter_array() -> bool {
127+
| ---- expected `bool` because of return type
128+
LL | / for i in [] {
129+
LL | |
130+
LL | | return false;
131+
LL | | }
132+
| |_____^ expected `bool`, found `()`
133+
|
134+
note: the function expects a value to always be returned, but loops might run zero times
135+
--> $DIR/coerce-for-loop-issue-122561.rs:74:5
136+
|
137+
LL | for i in [] {
138+
| ^^^^^^^^^^^ this might have zero elements to iterate on
139+
LL |
140+
LL | return false;
141+
| ------------ if the loop doesn't execute, this value would never get returned
142+
help: return a value for the case when the loop has zero elements to iterate on
143+
|
144+
LL ~ }
145+
LL + /* `bool` value */
146+
|
147+
help: otherwise consider changing the return type to account for that possibility
148+
|
149+
LL ~ fn zero_iter_array() -> Option<bool> {
150+
LL | for i in [] {
151+
LL |
152+
LL ~ return Some(false);
153+
LL ~ }
154+
LL + None
155+
|
156+
157+
error[E0308]: mismatched types
158+
--> $DIR/coerce-for-loop-issue-122561.rs:81:5
159+
|
160+
LL | fn zero_iter_array_ref() -> bool {
161+
| ---- expected `bool` because of return type
162+
LL | / for i in &[] {
163+
LL | |
164+
LL | | return false;
165+
LL | | }
166+
| |_____^ expected `bool`, found `()`
167+
|
168+
note: the function expects a value to always be returned, but loops might run zero times
169+
--> $DIR/coerce-for-loop-issue-122561.rs:81:5
170+
|
171+
LL | for i in &[] {
172+
| ^^^^^^^^^^^^ this might have zero elements to iterate on
173+
LL |
174+
LL | return false;
175+
| ------------ if the loop doesn't execute, this value would never get returned
176+
help: return a value for the case when the loop has zero elements to iterate on
177+
|
178+
LL ~ }
179+
LL + /* `bool` value */
180+
|
181+
help: otherwise consider changing the return type to account for that possibility
182+
|
183+
LL ~ fn zero_iter_array_ref() -> Option<bool> {
184+
LL | for i in &[] {
185+
LL |
186+
LL ~ return Some(false);
187+
LL ~ }
188+
LL + None
189+
|
190+
191+
error[E0308]: mismatched types
192+
--> $DIR/coerce-for-loop-issue-122561.rs:93:5
193+
|
194+
LL | fn atlast_once_iter_array_var() -> bool {
195+
| ---- expected `bool` because of return type
196+
LL | let x = [1, 2, 3];
197+
LL | / for i in x {
198+
LL | |
199+
LL | | return false;
200+
LL | | }
201+
| |_____^ expected `bool`, found `()`
202+
|
203+
note: the function expects a value to always be returned, but loops might run zero times
204+
--> $DIR/coerce-for-loop-issue-122561.rs:93:5
205+
|
206+
LL | for i in x {
207+
| ^^^^^^^^^^ this might have zero elements to iterate on
208+
LL |
209+
LL | return false;
210+
| ------------ if the loop doesn't execute, this value would never get returned
211+
help: return a value for the case when the loop has zero elements to iterate on
212+
|
213+
LL ~ }
214+
LL + /* `bool` value */
215+
|
216+
help: otherwise consider changing the return type to account for that possibility
217+
|
218+
LL ~ fn atlast_once_iter_array_var() -> Option<bool> {
219+
LL | let x = [1, 2, 3];
220+
LL | for i in x {
221+
LL |
222+
LL ~ return Some(false);
223+
LL ~ }
224+
LL + None
225+
|
226+
227+
error[E0308]: mismatched types
228+
--> $DIR/coerce-for-loop-issue-122561.rs:100:5
229+
|
230+
LL | fn atleast_once_iter_vec() -> bool {
231+
| ---- expected `bool` because of return type
232+
LL | / for i in vec![1, 2, 3] {
233+
LL | |
234+
LL | | return false;
235+
LL | | }
236+
| |_____^ expected `bool`, found `()`
237+
|
238+
note: the function expects a value to always be returned, but loops might run zero times
239+
--> $DIR/coerce-for-loop-issue-122561.rs:100:5
240+
|
241+
LL | | }
242+
| |_________^ this might have zero elements to iterate on
243+
...
244+
LL | / for i in vec![1, 2, 3] {
245+
LL |
246+
LL | return false;
247+
| ------------ if the loop doesn't execute, this value would never get returned
248+
help: return a value for the case when the loop has zero elements to iterate on
249+
|
250+
LL ~ }
251+
LL + /* `bool` value */
252+
|
253+
help: otherwise consider changing the return type to account for that possibility
254+
|
255+
LL ~ fn atleast_once_iter_vec() -> Option<bool> {
256+
LL | for i in vec![1, 2, 3] {
257+
LL |
258+
LL ~ return Some(false);
259+
LL ~ }
260+
LL + None
261+
|
262+
263+
error[E0308]: mismatched types
264+
--> $DIR/coerce-for-loop-issue-122561.rs:107:5
265+
|
266+
LL | fn atleast_once_iter_array_iter() -> bool {
267+
| ---- expected `bool` because of return type
268+
LL | / for i in [1, 2, 3].iter() {
269+
LL | |
270+
LL | | return false;
271+
LL | | }
272+
| |_____^ expected `bool`, found `()`
273+
|
274+
note: the function expects a value to always be returned, but loops might run zero times
275+
--> $DIR/coerce-for-loop-issue-122561.rs:107:5
276+
|
277+
LL | for i in [1, 2, 3].iter() {
278+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ this might have zero elements to iterate on
279+
LL |
280+
LL | return false;
281+
| ------------ if the loop doesn't execute, this value would never get returned
282+
help: return a value for the case when the loop has zero elements to iterate on
283+
|
284+
LL ~ }
285+
LL + /* `bool` value */
286+
|
287+
help: otherwise consider changing the return type to account for that possibility
288+
|
289+
LL ~ fn atleast_once_iter_array_iter() -> Option<bool> {
290+
LL | for i in [1, 2, 3].iter() {
291+
LL |
292+
LL ~ return Some(false);
293+
LL ~ }
294+
LL + None
295+
|
296+
297+
error[E0308]: mismatched types
298+
--> $DIR/coerce-for-loop-issue-122561.rs:114:5
299+
|
300+
LL | fn atleast_once_iter_func_result() -> bool {
301+
| ---- expected `bool` because of return type
302+
LL | / for i in get_iter() {
303+
LL | |
304+
LL | | return false;
305+
LL | | }
306+
| |_____^ expected `bool`, found `()`
307+
|
308+
note: the function expects a value to always be returned, but loops might run zero times
309+
--> $DIR/coerce-for-loop-issue-122561.rs:114:5
310+
|
311+
LL | for i in get_iter() {
312+
| ^^^^^^^^^^^^^^^^^^^ this might have zero elements to iterate on
313+
LL |
314+
LL | return false;
315+
| ------------ if the loop doesn't execute, this value would never get returned
316+
help: return a value for the case when the loop has zero elements to iterate on
317+
|
318+
LL ~ }
319+
LL + /* `bool` value */
320+
|
321+
help: otherwise consider changing the return type to account for that possibility
322+
|
323+
LL ~ fn atleast_once_iter_func_result() -> Option<bool> {
324+
LL | for i in get_iter() {
325+
LL |
326+
LL ~ return Some(false);
327+
LL ~ }
328+
LL + None
329+
|
330+
331+
error: aborting due to 15 previous errors
332+
333+
For more information about this error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)
Please sign in to comment.