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.
This commit is contained in:
Esteban Küber
parent
3e51277fe6
commit
29e2aa12ff
106 changed files with 947 additions and 453 deletions
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue