Skip to content

Commit

Permalink
Do not emit iteration note in coerce if loop does iterate
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gurry committed Mar 18, 2024
1 parent 4d4bb49 commit e2c6942
Show file tree
Hide file tree
Showing 3 changed files with 572 additions and 7 deletions.
120 changes: 113 additions & 7 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,12 @@
//! ```

use crate::FnCtxt;
use hir::{Expr, ExprField, ExprKind, LangItem, QPath};
use rustc_ast::{LitKind, UnOp};
use rustc_errors::{codes::*, struct_span_code_err, Applicability, Diag, MultiSpan};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::Expr;
use rustc_hir_analysis::astconv::AstConv;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::{Coercion, DefineOpaqueTypes, InferOk, InferResult};
Expand All @@ -57,6 +58,7 @@ use rustc_middle::ty::relate::RelateResult;
use rustc_middle::ty::visit::TypeVisitableExt;
use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeAndMut};
use rustc_session::parse::feature_err;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::sym;
use rustc_span::DesugaringKind;
use rustc_target::spec::abi::Abi;
Expand Down Expand Up @@ -1694,6 +1696,23 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
let hir::ExprKind::Loop(_, _, _, loop_span) = expr.kind else {
return;
};

let hir = tcx.hir();
let parent_node = tcx.hir_node(hir.get_parent_item(expr.hir_id).into());
let parent_block = if let Some(body_id) = parent_node.body_id()
&& let hir::ExprKind::Block(block, _) = hir.body(body_id).value.kind
{
Some(block)
} else {
None
};

if let Some(block) = parent_block
&& Self::loop_iterates_atleast_once(block)
{
return;
}

let mut span: MultiSpan = vec![loop_span].into();
span.push_span_label(loop_span, "this might have zero elements to iterate on");
const MAXITER: usize = 3;
Expand All @@ -1714,15 +1733,11 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
ret_exprs.len() - MAXITER
));
}
let hir = tcx.hir();
let item = hir.get_parent_item(expr.hir_id);
let ret_msg = "return a value for the case when the loop has zero elements to iterate on";
let ret_ty_msg =
"otherwise consider changing the return type to account for that possibility";
let node = tcx.hir_node(item.into());
if let Some(body_id) = node.body_id()
&& let Some(sig) = node.fn_sig()
&& let hir::ExprKind::Block(block, _) = hir.body(body_id).value.kind
if let Some(block) = parent_block
&& let Some(sig) = parent_node.fn_sig()
&& !ty.is_never()
{
let indentation = if let None = block.expr
Expand Down Expand Up @@ -1787,6 +1802,97 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
}
}

/// Returns `true` if the given `block` is a loop
/// and it will definitely iterate at least once
fn loop_iterates_atleast_once(block: &hir::Block<'tcx>) -> bool {
// Check if `block` is a for loop and extract
// the expr it iterate over as `iter_target`
let Some(Expr {
kind:
ExprKind::DropTemps(Expr {
kind:
ExprKind::Match(
Expr {
kind:
ExprKind::Call(
Expr {
kind:
ExprKind::Path(
QPath::LangItem(LangItem::IntoIterIntoIter, ..),
..,
),
..
},
[Expr { kind: iter_target, .. }],
),
..
},
..,
),
..
}),
..
}) = block.expr
else {
return false; // Block is not a for loop
};

// Peel away any ref if present
let iter_target = match iter_target {
ExprKind::AddrOf(.., deref) => deref.kind,
_ => *iter_target,
};

// Computes value of a literal expr
// Takes into account any enclosing neg unary expr if present
fn get_literal<'a>(expr_kind: &ExprKind<'a>) -> Option<i128> {
match expr_kind {
ExprKind::Lit(Spanned { node: LitKind::Int(lit, ..), .. }) => {
i128::try_from(lit.get()).ok()
}
ExprKind::Unary(
UnOp::Neg,
Expr {
kind: ExprKind::Lit(Spanned { node: LitKind::Int(lit, ..), .. }), ..
},
) => i128::try_from(lit.get()).map(|v| -1 * v).ok(),
_ => None,
}
}

// Check if `iter_target` will be iterated over at least once.
// We support only exclusive, inclusive and infinite range
// literals and array literals
match iter_target {
// Exclusive range
ExprKind::Struct(
QPath::LangItem(LangItem::Range, ..),
[
ExprField { expr: Expr { kind: start, .. }, .. },
ExprField { expr: Expr { kind: end, .. }, .. },
],
..,
) => match (get_literal(start), get_literal(end)) {
(Some(start), Some(end)) => end > start,
_ => false,
},
// Inclusive range
ExprKind::Call(
Expr {
kind: ExprKind::Path(QPath::LangItem(LangItem::RangeInclusiveNew, ..)), ..
},
[Expr { kind: start, .. }, Expr { kind: end, .. }],
) => match (get_literal(start), get_literal(end)) {
(Some(start), Some(end)) => end >= start,
_ => false,
},
// Infinite range
ExprKind::Struct(QPath::LangItem(LangItem::RangeFrom, ..), ..) => true,
ExprKind::Array(items) => items.len() > 0,
_ => false,
}
}

fn report_return_mismatched_types<'a>(
&self,
cause: &ObligationCause<'tcx>,
Expand Down
126 changes: 126 additions & 0 deletions tests/ui/coercion/coerce-for-loop-issue-122561.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// Regression test for #122561
// Checks that we do our best not to emit the note
// saying the loop might run zero times if it is
// clear it will certainly run at least once


// ----- Should not emit note for these -----
fn atleast_once_iter_range() -> bool {
for i in 0..3 {
//~^ ERROR mismatched types
return false;
}
}

fn atleast_once_iter_range_neg() -> bool {
for i in -3..-1 {
//~^ ERROR mismatched types
return false;
}
}

fn atleast_once_iter_range_inclusive() -> bool {
for i in 0..=3 {
//~^ ERROR mismatched types
return false;
}
}

fn atleast_once_iter_range_inclusive_neg() -> bool {
for i in -3..=-1 {
//~^ ERROR mismatched types
return false;
}
}

fn atleast_once_iter_infinite_range() -> bool {
for i in 0.. {
//~^ ERROR mismatched types
return false;
}
}

fn atleast_once_iter_infinite_range_neg() -> bool {
for i in -3.. {
//~^ ERROR mismatched types
return false;
}
}

fn atleast_once_iter_array() -> bool {
for i in [1, 2, 3] {
//~^ ERROR mismatched types
return false;
}
}

fn atleast_once_iter_array_ref() -> bool {
for i in &[1, 2, 3] {
//~^ ERROR mismatched types
return false;
}
}


// ----- Should emit note for these -----
fn zero_iter_range() -> bool {
for i in 0..0 {
//~^ ERROR mismatched types
return false;
}
}

fn zero_iter_array() -> bool {
for i in [] {
//~^ ERROR mismatched types
return false;
}
}

fn zero_iter_array_ref() -> bool {
for i in &[] {
//~^ ERROR mismatched types
return false;
}
}

// For the following cases the loop does iterate at
// least once but we aren't currently smart enough
// to not emit the note for them. We might add such
// smarts in the future
fn atlast_once_iter_array_var() -> bool {
let x = [1, 2, 3];
for i in x {
//~^ ERROR mismatched types
return false;
}
}

fn atleast_once_iter_vec() -> bool {
for i in vec![1, 2, 3] {
//~^ ERROR mismatched types
return false;
}
}

fn atleast_once_iter_array_iter() -> bool {
for i in [1, 2, 3].iter() {
//~^ ERROR mismatched types
return false;
}
}

fn atleast_once_iter_func_result() -> bool {
for i in get_iter() {
//~^ ERROR mismatched types
return false;
}
}


// Helper function
fn get_iter() -> impl Iterator<Item=i32> {
1..
}

fn main() {}
Loading

0 comments on commit e2c6942

Please sign in to comment.