Auto merge of #141883 - oli-obk:remove-check-mod-loops, r=nnethercote

Remove check_mod_loops query and run the checks per-body instead

This analysis is older than my first rustc contribution I believe. It was never querified. Ideally we'd merge it into the analysis happening within typeck anyway (typeck just uses span_delayed_bug instead of erroring), but I didn't want to do that within this PR that also moves things around and subtly changes diagnostic ordering.

Author: bors

compiler/rustc_hir_typeck/messages.ftl

@@ -17,6 +17,24 @@ hir_typeck_base_expression_double_dot_enable_default_field_values =
     add `#![feature(default_field_values)]` to the crate attributes to enable default values on `struct` fields
 hir_typeck_base_expression_double_dot_remove = remove the `..` as all the fields are already present
 
+hir_typeck_break_inside_closure =
+    `{$name}` inside of a closure
+    .label = cannot `{$name}` inside of a closure
+    .closure_label = enclosing closure
+
+hir_typeck_break_inside_coroutine =
+    `{$name}` inside `{$kind}` {$source}
+    .label = cannot `{$name}` inside `{$kind}` {$source}
+    .coroutine_label = enclosing `{$kind}` {$source}
+
+hir_typeck_break_non_loop =
+    `break` with value from a `{$kind}` loop
+    .label = can only break with a value inside `loop` or breakable block
+    .label2 = you can't `break` with a value in a `{$kind}` loop
+    .suggestion = use `break` on its own without a value inside this `{$kind}` loop
+    .break_expr_suggestion = alternatively, you might have meant to use the available loop label
+
+
 hir_typeck_candidate_trait_note = `{$trait_name}` defines an item `{$item_name}`{$action_or_ty ->
     [NONE] {""}
     [implement] , perhaps you need to implement it
@@ -64,6 +82,12 @@ hir_typeck_const_select_must_be_fn = this argument must be a function item
     .note = expected a function item, found {$ty}
     .help = consult the documentation on `const_eval_select` for more information
 
+hir_typeck_continue_labeled_block =
+    `continue` pointing to a labeled block
+    .label = labeled blocks cannot be `continue`'d
+    .block_label = labeled block the `continue` points to
+
+
 hir_typeck_convert_to_str = try converting the passed type into a `&str`
 
 hir_typeck_convert_using_method = try using `{$sugg}` to convert `{$found}` to `{$expected}`
@@ -182,6 +206,19 @@ hir_typeck_option_result_asref = use `{$def_path}::as_ref` to convert `{$expecte
 hir_typeck_option_result_cloned = use `{$def_path}::cloned` to clone the value inside the `{$def_path}`
 hir_typeck_option_result_copied = use `{$def_path}::copied` to copy the value inside the `{$def_path}`
 
+hir_typeck_outside_loop =
+    `{$name}` outside of a loop{$is_break ->
+        [true] {" or labeled block"}
+        *[false] {""}
+    }
+    .label = cannot `{$name}` outside of a loop{$is_break ->
+        [true] {" or labeled block"}
+        *[false] {""}
+    }
+
+hir_typeck_outside_loop_suggestion = consider labeling this block to be able to break within it
+
+
 hir_typeck_params_not_allowed =
     referencing function parameters is not allowed in naked functions
     .help = follow the calling convention in asm block to use parameters
@@ -254,6 +291,13 @@ hir_typeck_union_pat_dotdot = `..` cannot be used in union patterns
 
 hir_typeck_union_pat_multiple_fields = union patterns should have exactly one field
 
+hir_typeck_unlabeled_cf_in_while_condition =
+    `break` or `continue` with no label in the condition of a `while` loop
+    .label = unlabeled `{$cf_type}` in the condition of a `while` loop
+
+hir_typeck_unlabeled_in_labeled_block =
+    unlabeled `{$cf_type}` inside of a labeled block
+    .label = `{$cf_type}` statements that would diverge to or through a labeled block need to bear a label
 hir_typeck_use_is_empty =
     consider using the `is_empty` method on `{$expr_ty}` to determine if it contains anything
 

compiler/rustc_hir_typeck/src/errors.rs

@@ -2,11 +2,14 @@
 
 use std::borrow::Cow;
 
+use rustc_ast::Label;
 use rustc_errors::codes::*;
 use rustc_errors::{
     Applicability, Diag, DiagArgValue, DiagCtxtHandle, DiagSymbolList, Diagnostic,
     EmissionGuarantee, IntoDiagArg, Level, MultiSpan, Subdiagnostic,
 };
+use rustc_hir as hir;
+use rustc_hir::ExprKind;
 use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
 use rustc_middle::ty::{self, Ty};
 use rustc_span::edition::{Edition, LATEST_STABLE_EDITION};
@@ -721,6 +724,131 @@ pub(crate) struct TrivialCast<'tcx> {
     pub cast_ty: Ty<'tcx>,
 }
 
+pub(crate) struct BreakNonLoop<'a> {
+    pub span: Span,
+    pub head: Option<Span>,
+    pub kind: &'a str,
+    pub suggestion: String,
+    pub loop_label: Option<Label>,
+    pub break_label: Option<Label>,
+    pub break_expr_kind: &'a ExprKind<'a>,
+    pub break_expr_span: Span,
+}
+
+impl<'a, G: EmissionGuarantee> Diagnostic<'_, G> for BreakNonLoop<'a> {
+    #[track_caller]
+    fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
+        let mut diag = Diag::new(dcx, level, fluent::hir_typeck_break_non_loop);
+        diag.span(self.span);
+        diag.code(E0571);
+        diag.arg("kind", self.kind);
+        diag.span_label(self.span, fluent::hir_typeck_label);
+        if let Some(head) = self.head {
+            diag.span_label(head, fluent::hir_typeck_label2);
+        }
+        diag.span_suggestion(
+            self.span,
+            fluent::hir_typeck_suggestion,
+            self.suggestion,
+            Applicability::MaybeIncorrect,
+        );
+        if let (Some(label), None) = (self.loop_label, self.break_label) {
+            match self.break_expr_kind {
+                ExprKind::Path(hir::QPath::Resolved(
+                    None,
+                    hir::Path { segments: [segment], res: hir::def::Res::Err, .. },
+                )) if label.ident.to_string() == format!("'{}", segment.ident) => {
+                    // This error is redundant, we will have already emitted a
+                    // suggestion to use the label when `segment` wasn't found
+                    // (hence the `Res::Err` check).
+                    diag.downgrade_to_delayed_bug();
+                }
+                _ => {
+                    diag.span_suggestion(
+                        self.break_expr_span,
+                        fluent::hir_typeck_break_expr_suggestion,
+                        label.ident,
+                        Applicability::MaybeIncorrect,
+                    );
+                }
+            }
+        }
+        diag
+    }
+}
+
+#[derive(Diagnostic)]
+#[diag(hir_typeck_continue_labeled_block, code = E0696)]
+pub(crate) struct ContinueLabeledBlock {
+    #[primary_span]
+    #[label]
+    pub span: Span,
+    #[label(hir_typeck_block_label)]
+    pub block_span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(hir_typeck_break_inside_closure, code = E0267)]
+pub(crate) struct BreakInsideClosure<'a> {
+    #[primary_span]
+    #[label]
+    pub span: Span,
+    #[label(hir_typeck_closure_label)]
+    pub closure_span: Span,
+    pub name: &'a str,
+}
+
+#[derive(Diagnostic)]
+#[diag(hir_typeck_break_inside_coroutine, code = E0267)]
+pub(crate) struct BreakInsideCoroutine<'a> {
+    #[primary_span]
+    #[label]
+    pub span: Span,
+    #[label(hir_typeck_coroutine_label)]
+    pub coroutine_span: Span,
+    pub name: &'a str,
+    pub kind: &'a str,
+    pub source: &'a str,
+}
+
+#[derive(Diagnostic)]
+#[diag(hir_typeck_outside_loop, code = E0268)]
+pub(crate) struct OutsideLoop<'a> {
+    #[primary_span]
+    #[label]
+    pub spans: Vec<Span>,
+    pub name: &'a str,
+    pub is_break: bool,
+    #[subdiagnostic]
+    pub suggestion: Option<OutsideLoopSuggestion>,
+}
+#[derive(Subdiagnostic)]
+#[multipart_suggestion(hir_typeck_outside_loop_suggestion, applicability = "maybe-incorrect")]
+pub(crate) struct OutsideLoopSuggestion {
+    #[suggestion_part(code = "'block: ")]
+    pub block_span: Span,
+    #[suggestion_part(code = " 'block")]
+    pub break_spans: Vec<Span>,
+}
+
+#[derive(Diagnostic)]
+#[diag(hir_typeck_unlabeled_in_labeled_block, code = E0695)]
+pub(crate) struct UnlabeledInLabeledBlock<'a> {
+    #[primary_span]
+    #[label]
+    pub span: Span,
+    pub cf_type: &'a str,
+}
+
+#[derive(Diagnostic)]
+#[diag(hir_typeck_unlabeled_cf_in_while_condition, code = E0590)]
+pub(crate) struct UnlabeledCfInWhileCondition<'a> {
+    #[primary_span]
+    #[label]
+    pub span: Span,
+    pub cf_type: &'a str,
+}
+
 #[derive(Diagnostic)]
 #[diag(hir_typeck_no_associated_item, code = E0599)]
 pub(crate) struct NoAssociatedItem {

compiler/rustc_hir_typeck/src/lib.rs

@@ -28,6 +28,7 @@ mod fallback;
 mod fn_ctxt;
 mod gather_locals;
 mod intrinsicck;
+mod loops;
 mod method;
 mod naked_functions;
 mod op;
@@ -149,6 +150,7 @@ fn typeck_with_inspect<'tcx>(
         };
 
         check_abi(tcx, id, span, fn_sig.abi());
+        loops::check(tcx, def_id, body);
 
         // Compute the function signature from point of view of inside the fn.
         let mut fn_sig = tcx.liberate_late_bound_regions(def_id.to_def_id(), fn_sig);
@@ -189,6 +191,8 @@ fn typeck_with_inspect<'tcx>(
             tcx.type_of(def_id).instantiate_identity()
         };
 
+        loops::check(tcx, def_id, body);
+
         let expected_type = fcx.normalize(body.value.span, expected_type);
 
         let wf_code = ObligationCauseCode::WellFormed(Some(WellFormedLoc::Ty(def_id)));

compiler/rustc_passes/src/loops.rs renamed to compiler/rustc_hir_typeck/src/loops.rs

@@ -3,11 +3,11 @@ use std::fmt;
 
 use Context::*;
 use rustc_hir as hir;
-use rustc_hir::def_id::{LocalDefId, LocalModDefId};
+use rustc_hir::def::DefKind;
+use rustc_hir::def_id::LocalDefId;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::{Destination, Node};
 use rustc_middle::hir::nested_filter;
-use rustc_middle::query::Providers;
 use rustc_middle::span_bug;
 use rustc_middle::ty::TyCtxt;
 use rustc_span::hygiene::DesugaringKind;
@@ -73,51 +73,32 @@ struct CheckLoopVisitor<'tcx> {
     block_breaks: BTreeMap<Span, BlockInfo>,
 }
 
-fn check_mod_loops(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
+pub(crate) fn check<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &'tcx hir::Body<'tcx>) {
     let mut check =
         CheckLoopVisitor { tcx, cx_stack: vec![Normal], block_breaks: Default::default() };
-    tcx.hir_visit_item_likes_in_module(module_def_id, &mut check);
+    let cx = match tcx.def_kind(def_id) {
+        DefKind::AnonConst => AnonConst,
+        _ => Fn,
+    };
+    check.with_context(cx, |v| v.visit_body(body));
     check.report_outside_loop_error();
 }
 
-pub(crate) fn provide(providers: &mut Providers) {
-    *providers = Providers { check_mod_loops, ..*providers };
-}
-
 impl<'hir> Visitor<'hir> for CheckLoopVisitor<'hir> {
     type NestedFilter = nested_filter::OnlyBodies;
 
     fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
         self.tcx
     }
 
-    fn visit_anon_const(&mut self, c: &'hir hir::AnonConst) {
-        self.with_context(AnonConst, |v| intravisit::walk_anon_const(v, c));
+    fn visit_anon_const(&mut self, _: &'hir hir::AnonConst) {
+        // Typecked on its own.
     }
 
     fn visit_inline_const(&mut self, c: &'hir hir::ConstBlock) {
         self.with_context(ConstBlock, |v| intravisit::walk_inline_const(v, c));
     }
 
-    fn visit_fn(
-        &mut self,
-        fk: hir::intravisit::FnKind<'hir>,
-        fd: &'hir hir::FnDecl<'hir>,
-        b: hir::BodyId,
-        _: Span,
-        id: LocalDefId,
-    ) {
-        self.with_context(Fn, |v| intravisit::walk_fn(v, fk, fd, b, id));
-    }
-
-    fn visit_trait_item(&mut self, trait_item: &'hir hir::TraitItem<'hir>) {
-        self.with_context(Fn, |v| intravisit::walk_trait_item(v, trait_item));
-    }
-
-    fn visit_impl_item(&mut self, impl_item: &'hir hir::ImplItem<'hir>) {
-        self.with_context(Fn, |v| intravisit::walk_impl_item(v, impl_item));
-    }
-
     fn visit_expr(&mut self, e: &'hir hir::Expr<'hir>) {
         match e.kind {
             hir::ExprKind::If(cond, then, else_opt) => {

compiler/rustc_interface/src/passes.rs

@@ -954,7 +954,6 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
                 tcx.ensure_ok().exportable_items(LOCAL_CRATE);
                 tcx.ensure_ok().stable_order_of_exportable_impls(LOCAL_CRATE);
                 tcx.par_hir_for_each_module(|module| {
-                    tcx.ensure_ok().check_mod_loops(module);
                     tcx.ensure_ok().check_mod_attrs(module);
                     tcx.ensure_ok().check_mod_unstable_api_usage(module);
                 });

compiler/rustc_middle/src/query/mod.rs

@@ -1115,11 +1115,6 @@ rustc_queries! {
         desc { |tcx| "checking for unstable API usage in {}", describe_as_module(key, tcx) }
     }
 
-    /// Checks the loops in the module.
-    query check_mod_loops(key: LocalModDefId) {
-        desc { |tcx| "checking loops in {}", describe_as_module(key, tcx) }
-    }
-
     query check_mod_privacy(key: LocalModDefId) {
         desc { |tcx| "checking privacy in {}", describe_as_module(key.to_local_def_id(), tcx) }
     }