Skip to content

Commit

Permalink
Remove note about iteration count in coerce
Browse files Browse the repository at this point in the history
and replace it with a simple note suggesting returning a value.

The type mismatch error was never due to how many times
the loop iterates. It is more because of the peculiar structure
of what the for loop desugars to. So the note talking about
iteration count didn't make sense
  • Loading branch information
gurry committed Mar 29, 2024
1 parent 760e567 commit 3ca3dca
Show file tree
Hide file tree
Showing 14 changed files with 450 additions and 200 deletions.
172 changes: 50 additions & 122 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,9 @@
use crate::errors::SuggestBoxingForReturnImplTrait;
use crate::FnCtxt;
use rustc_errors::{codes::*, struct_span_code_err, Applicability, Diag, MultiSpan};
use rustc_errors::{codes::*, struct_span_code_err, Applicability, Diag};
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::Expr;
use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::{Coercion, DefineOpaqueTypes, InferOk, InferResult};
Expand Down Expand Up @@ -95,22 +93,6 @@ impl<'a, 'tcx> Deref for Coerce<'a, 'tcx> {

type CoerceResult<'tcx> = InferResult<'tcx, (Vec<Adjustment<'tcx>>, Ty<'tcx>)>;

struct CollectRetsVisitor<'tcx> {
ret_exprs: Vec<&'tcx hir::Expr<'tcx>>,
}

impl<'tcx> Visitor<'tcx> for CollectRetsVisitor<'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
match expr.kind {
hir::ExprKind::Ret(_) => self.ret_exprs.push(expr),
// `return` in closures does not return from the outer function
hir::ExprKind::Closure(_) => return,
_ => {}
}
intravisit::walk_expr(self, expr);
}
}

/// Coercing a mutable reference to an immutable works, while
/// coercing `&T` to `&mut T` should be forbidden.
fn coerce_mutbls<'tcx>(
Expand Down Expand Up @@ -1593,7 +1575,6 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {

let mut err;
let mut unsized_return = false;
let mut visitor = CollectRetsVisitor { ret_exprs: vec![] };
match *cause.code() {
ObligationCauseCode::ReturnNoExpression => {
err = struct_span_code_err!(
Expand All @@ -1619,11 +1600,6 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
if !fcx.tcx.features().unsized_locals {
unsized_return = self.is_return_ty_definitely_unsized(fcx);
}
if let Some(expression) = expression
&& let hir::ExprKind::Loop(loop_blk, ..) = expression.kind
{
intravisit::walk_block(&mut visitor, loop_blk);
}
}
ObligationCauseCode::ReturnValue(id) => {
err = self.report_return_mismatched_types(
Expand Down Expand Up @@ -1732,15 +1708,8 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
None,
Some(coercion_error),
);
if visitor.ret_exprs.len() > 0 {
self.note_unreachable_loop_return(
&mut err,
fcx.tcx,
&expr,
&visitor.ret_exprs,
expected,
);
}

self.note_unreachable_loop_return(&mut err, fcx.tcx, &expr, expected);
}

let reported = err.emit_unless(unsized_return);
Expand Down Expand Up @@ -1819,102 +1788,61 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
err: &mut Diag<'_>,
tcx: TyCtxt<'tcx>,
expr: &hir::Expr<'tcx>,
ret_exprs: &Vec<&'tcx hir::Expr<'tcx>>,
ty: Ty<'tcx>,
) {
let hir::ExprKind::Loop(_, _, _, loop_span) = expr.kind else {
let hir::ExprKind::Loop(_, _, _, _) = expr.kind else {
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;
let iter = ret_exprs.iter().take(MAXITER);
for ret_expr in iter {
span.push_span_label(
ret_expr.span,
"if the loop doesn't execute, this value would never get returned",
);
}
err.span_note(
span,
"the function expects a value to always be returned, but loops might run zero times",
);
if MAXITER < ret_exprs.len() {
err.note(format!(
"if the loop doesn't execute, {} other values would never get returned",
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
&& !ty.is_never()
{
let indentation = if let None = block.expr
&& let [.., last] = &block.stmts
{
tcx.sess.source_map().indentation_before(last.span).unwrap_or_else(String::new)
} else if let Some(expr) = block.expr {
tcx.sess.source_map().indentation_before(expr.span).unwrap_or_else(String::new)
let body_def_id = hir.enclosing_body_owner(expr.hir_id);
let body_id = hir.body_owned_by(body_def_id);
let body = hir.body(body_id);

let span = match body.value.kind {
// Regular block as in a function body
hir::ExprKind::Block(block, _) => {
if let None = block.expr
&& let [.., last] = &block.stmts
{
Some(last.span)
} else if let Some(expr) = block.expr {
Some(expr.span)
} else {
None
}
}
// Anon const body (there's no enclosing block in this case)
hir::ExprKind::DropTemps(expr) => Some(expr.span),
_ => None,
};

if let Some(span) = span {
let (msg, suggestion) = if ty.is_never() {
(
"consider adding a diverging expression here",
"`loop {}` or `panic!(\"...\")`".to_string(),
)
} else {
String::new()
("consider returning a value here", format!("`{ty}` value"))
};
if let None = block.expr
&& let [.., last] = &block.stmts
{
err.span_suggestion_verbose(
last.span.shrink_to_hi(),
ret_msg,
format!("\n{indentation}/* `{ty}` value */"),
Applicability::MaybeIncorrect,
);
} else if let Some(expr) = block.expr {
err.span_suggestion_verbose(
expr.span.shrink_to_hi(),
ret_msg,
format!("\n{indentation}/* `{ty}` value */"),
Applicability::MaybeIncorrect,
);
}
let mut sugg = match sig.decl.output {
hir::FnRetTy::DefaultReturn(span) => {
vec![(span, " -> Option<()>".to_string())]
}
hir::FnRetTy::Return(ty) => {
vec![
(ty.span.shrink_to_lo(), "Option<".to_string()),
(ty.span.shrink_to_hi(), ">".to_string()),
]
}

let src_map = tcx.sess.source_map();
let suggestion = if src_map.is_multiline(expr.span) {
let indentation = src_map.indentation_before(span).unwrap_or_else(String::new);
format!("\n{indentation}/* {suggestion} */")
} else {
// If the entire expr is on a single line
// put the suggestion also on the same line
format!(" /* {suggestion} */")
};
for ret_expr in ret_exprs {
match ret_expr.kind {
hir::ExprKind::Ret(Some(expr)) => {
sugg.push((expr.span.shrink_to_lo(), "Some(".to_string()));
sugg.push((expr.span.shrink_to_hi(), ")".to_string()));
}
hir::ExprKind::Ret(None) => {
sugg.push((ret_expr.span.shrink_to_hi(), " Some(())".to_string()));
}
_ => {}
}
}
if let None = block.expr
&& let [.., last] = &block.stmts
{
sugg.push((last.span.shrink_to_hi(), format!("\n{indentation}None")));
} else if let Some(expr) = block.expr {
sugg.push((expr.span.shrink_to_hi(), format!("\n{indentation}None")));
}
err.multipart_suggestion(ret_ty_msg, sugg, Applicability::MaybeIncorrect);
} else {
err.help(format!("{ret_msg}, {ret_ty_msg}"));

err.span_suggestion_verbose(
span.shrink_to_hi(),
msg,
suggestion,
Applicability::MaybeIncorrect,
);
}
}

Expand Down
2 changes: 1 addition & 1 deletion library/stdarch
Submodule stdarch updated 68 files
+1 −1 .cirrus.yml
+2 −2 .github/workflows/main.yml
+3 −5 .gitignore
+1 −2 Cargo.toml
+16 −0 ci/docker/wasm32-wasi/Dockerfile
+0 −13 ci/docker/wasm32-wasip1/Dockerfile
+350 −350 crates/core_arch/src/aarch64/neon/generated.rs
+7 −28 crates/core_arch/src/aarch64/neon/mod.rs
+7 −29 crates/core_arch/src/arm_shared/barrier/mod.rs
+10 −34 crates/core_arch/src/arm_shared/crc.rs
+14 −56 crates/core_arch/src/arm_shared/crypto.rs
+5 −31 crates/core_arch/src/arm_shared/hints.rs
+5 −30 crates/core_arch/src/arm_shared/mod.rs
+2,039 −2,053 crates/core_arch/src/arm_shared/neon/generated.rs
+1 −1 crates/core_arch/src/arm_shared/neon/load_tests.rs
+427 −1,612 crates/core_arch/src/arm_shared/neon/mod.rs
+1 −1 crates/core_arch/src/arm_shared/neon/shift_and_insert_tests.rs
+1 −1 crates/core_arch/src/arm_shared/neon/store_tests.rs
+49 −49 crates/core_arch/src/arm_shared/neon/table_lookup_tests.rs
+1 −1 crates/core_arch/src/arm_shared/test_support.rs
+0 −1 crates/core_arch/src/lib.rs
+0 −7,027 crates/core_arch/src/loongarch64/lasx/generated.rs
+0 −21 crates/core_arch/src/loongarch64/lasx/mod.rs
+0 −14,690 crates/core_arch/src/loongarch64/lasx/tests.rs
+0 −57 crates/core_arch/src/loongarch64/lasx/types.rs
+0 −6,843 crates/core_arch/src/loongarch64/lsx/generated.rs
+0 −21 crates/core_arch/src/loongarch64/lsx/mod.rs
+0 −7,132 crates/core_arch/src/loongarch64/lsx/tests.rs
+0 −41 crates/core_arch/src/loongarch64/lsx/types.rs
+0 −9 crates/core_arch/src/loongarch64/mod.rs
+5 −24 crates/core_arch/src/mod.rs
+180 −3 crates/core_arch/src/powerpc/altivec.rs
+0 −185 crates/core_arch/src/powerpc/macros.rs
+2 −1 crates/core_arch/src/powerpc/mod.rs
+0 −5 crates/core_arch/src/powerpc64/mod.rs
+0 −156 crates/core_arch/src/powerpc64/vsx.rs
+0 −22 crates/core_arch/src/wasm32/mod.rs
+27 −28 crates/core_arch/src/x86/avx.rs
+3 −3 crates/core_arch/src/x86/avx512f.rs
+9 −9 crates/core_arch/src/x86/sse.rs
+11 −12 crates/core_arch/src/x86/sse2.rs
+12 −12 crates/core_arch/src/x86/sse41.rs
+1 −2 crates/core_arch/src/x86_64/sse2.rs
+6 −6 crates/intrinsic-test/src/main.rs
+1 −2 crates/simd-test-macro/src/lib.rs
+1 −1 crates/std_detect/src/detect/arch/aarch64.rs
+27 −15 crates/std_detect/src/detect/arch/loongarch.rs
+1 −1 crates/std_detect/src/detect/arch/mod.rs
+0 −3 crates/std_detect/src/detect/arch/x86.rs
+1 −2 crates/std_detect/src/detect/mod.rs
+9 −30 crates/std_detect/src/detect/os/linux/loongarch.rs
+7 −15 crates/std_detect/src/detect/os/x86.rs
+1 −6 crates/std_detect/tests/cpu-detection.rs
+0 −2 crates/std_detect/tests/macro_trailing_commas.rs
+0 −1 crates/std_detect/tests/x86-specific.rs
+0 −10 crates/stdarch-gen-loongarch/Cargo.toml
+0 −33 crates/stdarch-gen-loongarch/README.md
+0 −3,685 crates/stdarch-gen-loongarch/lasx.spec
+0 −5,342 crates/stdarch-gen-loongarch/lasxintrin.h
+0 −3,585 crates/stdarch-gen-loongarch/lsx.spec
+0 −5,185 crates/stdarch-gen-loongarch/lsxintrin.h
+0 −1,525 crates/stdarch-gen-loongarch/src/main.rs
+1 −1 crates/stdarch-gen/Cargo.toml
+1 −1 crates/stdarch-gen/README.md
+0 −0 crates/stdarch-gen/neon.spec
+12 −18 crates/stdarch-gen/src/main.rs
+1 −1 crates/stdarch-test/src/disassembly.rs
+1 −1 crates/stdarch-test/src/lib.rs
2 changes: 1 addition & 1 deletion src/tools/cargo
Submodule cargo updated 208 files
94 changes: 94 additions & 0 deletions tests/ui/coercion/coerce-loop-issue-122561.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// Regression test for #122561

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

fn for_finite() -> String {
for i in 0..5 {
//~^ ERROR mismatched types
return String::from("test");
}
}

fn for_never_type() -> ! {
for i in 0..0 {
//~^ ERROR mismatched types
}
}

// Entire function on a single line.
// Tests that we format the suggestion
// correctly in this case
fn for_single_line() -> bool { for i in 0.. { return false; } }
//~^ ERROR mismatched types

// Loop in an anon const in function args
// Tests that we:
// a. deal properly with this complex case
// b. format the suggestion correctly so
// that it's readable
fn for_in_arg(a: &[(); for x in 0..2 {}]) -> bool {
//~^ ERROR `for` is not allowed in a `const`
//~| ERROR mismatched types
true
}

fn while_true() -> bool {
while true {
//~^ ERROR mismatched types
//~| WARN denote infinite loops with `loop { ... }` [while_true]
return true;
}
}

fn while_false() -> bool {
while false {
//~^ ERROR mismatched types
return true;
}
}

fn while_true_never_type() -> ! {
while true {
//~^ ERROR mismatched types
//~| WARN denote infinite loops with `loop { ... }` [while_true]
}
}

// No type mismatch error in this case
fn loop_() -> bool {
loop {
return true;
}
}

const C: i32 = {
for i in 0.. {
//~^ ERROR `for` is not allowed in a `const`
//~| ERROR mismatched types
}
};

fn main() {
let _ = [10; {
for i in 0..5 {
//~^ ERROR `for` is not allowed in a `const`
//~| ERROR mismatched types
}
}];

let _ = [10; {
while false {
//~^ ERROR mismatched types
}
}];


let _ = |a: &[(); for x in 0..2 {}]| {};
//~^ ERROR `for` is not allowed in a `const`
//~| ERROR mismatched types
}
Loading

0 comments on commit 3ca3dca

Please sign in to comment.