On partial uninit error point at where we need init

When a binding is declared without a value, borrowck verifies that all
codepaths have *one* assignment to them to initialize them fully. If
there are any cases where a condition can be met that leaves the binding
uninitialized or we attempt to initialize a field of an unitialized
binding, we emit E0381.

We now look at all the statements that initialize the binding, and use
them to explore branching code paths that *don't* and point at them. If
we find *no* potential places where an assignment to the binding might
be missing, we display the spans of all the existing initializers to
provide some context.

2 files changed, 204 insertions, 44 deletions

diff --git a/compiler/rustc_borrowck/src/borrowck_errors.rs b/compiler/rustc_borrowck/src/borrowck_errors.rs
index a5d9a2bc597..08ea00d71ef 100644
--- a/compiler/rustc_borrowck/src/borrowck_errors.rs
+++ b/compiler/rustc_borrowck/src/borrowck_errors.rs
@@ -33,22 +33,6 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
         err
     }
 
-    pub(crate) fn cannot_act_on_uninitialized_variable(
-        &self,
-        span: Span,
-        verb: &str,
-        desc: &str,
-    ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
-        struct_span_err!(
-            self,
-            span,
-            E0381,
-            "{} of possibly-uninitialized variable: `{}`",
-            verb,
-            desc,
-        )
-    }
-
     pub(crate) fn cannot_mutably_borrow_multiply(
         &self,
         new_loan_span: Span,
@@ -175,8 +159,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
             self,
             new_loan_span,
             E0501,
-            "cannot borrow {}{} as {} because previous closure \
-             requires unique access",
+            "cannot borrow {}{} as {} because previous closure requires unique access",
             desc_new,
             opt_via,
             kind_new,
@@ -453,9 +436,8 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
             self,
             closure_span,
             E0373,
-            "{} may outlive the current function, \
-             but it borrows {}, \
-             which is owned by the current function",
+            "{} may outlive the current function, but it borrows {}, which is owned by the current \
+             function",
             closure_kind,
             borrowed_path,
         );
@@ -479,7 +461,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
     }
 
     #[rustc_lint_diagnostics]
-    fn struct_span_err_with_code<S: Into<MultiSpan>>(
+    pub(crate) fn struct_span_err_with_code<S: Into<MultiSpan>>(
         &self,
         sp: S,
         msg: impl Into<DiagnosticMessage>,
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
index d2a54a646ec..07133525d5d 100644
--- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -2,9 +2,12 @@ use either::Either;
 use rustc_const_eval::util::CallKind;
 use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fx::FxHashSet;
-use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan};
+use rustc_errors::{
+    struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan,
+};
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
+use rustc_hir::intravisit::{walk_expr, Visitor};
 use rustc_hir::{AsyncGeneratorKind, GeneratorKind};
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_infer::traits::ObligationCause;
@@ -94,32 +97,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 return;
             }
 
-            let item_msg =
-                match self.describe_place_with_options(used_place, IncludingDowncast(true)) {
-                    Some(name) => format!("`{}`", name),
-                    None => "value".to_owned(),
-                };
-            let mut err = self.cannot_act_on_uninitialized_variable(
-                span,
-                desired_action.as_noun(),
-                &self
-                    .describe_place_with_options(moved_place, IncludingDowncast(true))
-                    .unwrap_or_else(|| "_".to_owned()),
-            );
-            err.span_label(span, format!("use of possibly-uninitialized {}", item_msg));
-
-            use_spans.var_span_label_path_only(
-                &mut err,
-                format!("{} occurs due to use{}", desired_action.as_noun(), use_spans.describe()),
-            );
-
+            let err =
+                self.report_use_of_uninitialized(mpi, used_place, desired_action, span, use_spans);
             self.buffer_error(err);
         } else {
             if let Some((reported_place, _)) = self.has_move_error(&move_out_indices) {
                 if self.prefixes(*reported_place, PrefixSet::All).any(|p| p == used_place) {
                     debug!(
-                        "report_use_of_moved_or_uninitialized place: error suppressed \
-                         mois={:?}",
+                        "report_use_of_moved_or_uninitialized place: error suppressed mois={:?}",
                         move_out_indices
                     );
                     return;
@@ -326,6 +311,99 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         }
     }
 
+    fn report_use_of_uninitialized(
+        &self,
+        mpi: MovePathIndex,
+        used_place: PlaceRef<'tcx>,
+        desired_action: InitializationRequiringAction,
+        span: Span,
+        use_spans: UseSpans<'tcx>,
+    ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
+        // We need all statements in the body where the binding was assigned to to later find all
+        // the branching code paths where the binding *wasn't* assigned to.
+        let inits = &self.move_data.init_path_map[mpi];
+        let move_path = &self.move_data.move_paths[mpi];
+        let decl_span = self.body.local_decls[move_path.place.local].source_info.span;
+        let mut spans = vec![];
+        for init_idx in inits {
+            let init = &self.move_data.inits[*init_idx];
+            let span = init.span(&self.body);
+            spans.push(span);
+        }
+
+        let (item_msg, name, desc) =
+            match self.describe_place_with_options(used_place, IncludingDowncast(true)) {
+                Some(name) => (format!("`{name}`"), format!("`{name}`"), format!("`{name}` ")),
+                None => ("value".to_string(), "the variable".to_string(), String::new()),
+            };
+        let initialized = if let InitializationRequiringAction::PartialAssignment = desired_action {
+            // The same error is emitted for bindings that are *sometimes* initialized and the ones
+            // that are *partially* initialized by assigning to a field of an uninitialized
+            // binding. We differentiate between them for more accurate wording here.
+            "fully initialized"
+        } else if spans.iter().filter(|i| !i.contains(span)).count() == 0 {
+            // We filter above to avoid misleading wording in cases like:
+            // ```
+            // let x;
+            // x += 1;
+            // ```
+            "initialized"
+        } else {
+            "initialized in all conditions"
+        };
+        let mut err = struct_span_err!(self, span, E0381, "binding {desc}isn't {initialized}");
+        use_spans.var_span_label_path_only(
+            &mut err,
+            format!("{} occurs due to use{}", desired_action.as_noun(), use_spans.describe()),
+        );
+
+        if let InitializationRequiringAction::PartialAssignment = desired_action {
+            err.help(
+                "partial initialization isn't supported, fully initialize the binding with \
+                a default value and mutate, or use `std::mem::MaybeUninit`",
+            );
+        }
+        let verb = desired_action.as_verb_in_past_tense();
+        err.span_label(span, format!("{item_msg} {verb} here but it isn't {initialized}",));
+
+        // We use the statements were the binding was initialized, and inspect the HIR to look
+        // for the branching codepaths that aren't covered, to point at them.
+        let hir_id = self.mir_hir_id();
+        let map = self.infcx.tcx.hir();
+        let body_id = map.body_owned_by(hir_id);
+        let body = map.body(body_id);
+
+        let mut visitor = ConditionVisitor { spans: &spans, name: &name, errors: vec![] };
+        visitor.visit_body(&body);
+        if visitor.errors.is_empty() {
+            for sp in &spans {
+                if *sp < span && !sp.overlaps(span) {
+                    err.span_label(*sp, "binding initialized here in some conditions");
+                }
+            }
+        }
+        for (sp, label) in visitor.errors {
+            if sp < span && !sp.overlaps(span) {
+                // When we have a case like `match-cfg-fake-edges.rs`, we don't want to mention
+                // match arms coming after the primary span because they aren't relevant:
+                // ```
+                // let x;
+                // match y {
+                //     _ if { x = 2; true } => {}
+                //     _ if {
+                //         x; //~ ERROR
+                //         false
+                //     } => {}
+                //     _ => {} // We don't want to point to this.
+                // };
+                // ```
+                err.span_label(sp, &label);
+            }
+        }
+        err.span_label(decl_span, "variable declared here");
+        err
+    }
+
     fn suggest_borrow_fn_like(
         &self,
         err: &mut DiagnosticBuilder<'tcx, ErrorGuaranteed>,
@@ -2448,3 +2526,103 @@ impl<'tcx> AnnotatedBorrowFnSignature<'tcx> {
         }
     }
 }
+
+/// Detect whether one of the provided spans is a statement nested within the top-most visited expr
+struct ReferencedStatementsVisitor<'a>(&'a [Span], bool);
+
+impl<'a, 'v> Visitor<'v> for ReferencedStatementsVisitor<'a> {
+    fn visit_stmt(&mut self, s: &'v hir::Stmt<'v>) {
+        match s.kind {
+            hir::StmtKind::Semi(expr) if self.0.contains(&expr.span) => {
+                self.1 = true;
+            }
+            _ => {}
+        }
+    }
+}
+
+/// Given a set of spans representing statements initializing the relevant binding, visit all the
+/// function expressions looking for branching code paths that *do not* initialize the binding.
+struct ConditionVisitor<'b> {
+    spans: &'b [Span],
+    name: &'b str,
+    errors: Vec<(Span, String)>,
+}
+
+impl<'b, 'v> Visitor<'v> for ConditionVisitor<'b> {
+    fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) {
+        match ex.kind {
+            hir::ExprKind::If(cond, body, None) => {
+                // `if` expressions with no `else` that initialize the binding might be missing an
+                // `else` arm.
+                let mut v = ReferencedStatementsVisitor(self.spans, false);
+                v.visit_expr(body);
+                if v.1 {
+                    self.errors.push((
+                        cond.span,
+                        format!(
+                            "this `if` expression might be missing an `else` arm where {} is \
+                             initialized",
+                            self.name,
+                        ),
+                    ));
+                }
+            }
+            hir::ExprKind::If(cond, body, Some(other)) => {
+                // `if` expressions where the binding is only initialized in one of the two arms
+                // might be missing a binding initialization.
+                let mut a = ReferencedStatementsVisitor(self.spans, false);
+                a.visit_expr(body);
+                let mut b = ReferencedStatementsVisitor(self.spans, false);
+                b.visit_expr(other);
+                match (a.1, b.1) {
+                    (true, true) | (false, false) => {}
+                    (true, false) => {
+                        self.errors.push((
+                            cond.span,
+                            format!("{} is uninitialized if this condition isn't met", self.name,),
+                        ));
+                    }
+                    (false, true) => {
+                        self.errors.push((
+                            cond.span,
+                            format!("{} is uninitialized if this condition is met", self.name),
+                        ));
+                    }
+                }
+            }
+            hir::ExprKind::Match(_, arms, _) => {
+                // If the binding is initialized in one of the match arms, then the other match
+                // arms might be missing an initialization.
+                let results: Vec<bool> = arms
+                    .iter()
+                    .map(|arm| {
+                        let mut v = ReferencedStatementsVisitor(self.spans, false);
+                        v.visit_arm(arm);
+                        v.1
+                    })
+                    .collect();
+                if results.iter().any(|x| *x) && !results.iter().all(|x| *x) {
+                    for (arm, seen) in arms.iter().zip(results) {
+                        if !seen {
+                            self.errors.push((
+                                arm.pat.span,
+                                format!(
+                                    "{} is uninitialized if this pattern is matched",
+                                    self.name
+                                ),
+                            ));
+                        }
+                    }
+                }
+            }
+            // FIXME: should we also account for binops, particularly `&&` and `||`? `try` should
+            // also be accounted for. For now it is fine, as if we don't find *any* relevant
+            // branching code paths, we point at the places where the binding *is* initialized for
+            // *some* context. We should also specialize the output for `while` and `for` loops,
+            // but for now we can rely on their desugaring to provide appropriate output.
+            _ => {}
+        }
+        walk_expr(self, ex);
+    }
+}