Auto merge of #66815 - mark-i-m:simplify-borrow_check-errors, r=Dylan…

…-DPC

Reorganize borrow check diagnostic code

Currently borrow checker diagnostics are split across many different modules in different places in the `librustc_mir` crate. This moves them all to a `diagnostics` module. This also reduces the nesting of the modules a bit (sooo much nesting).

I am also thinking of moving stuff out of the `nll` module since we only have one borrow checker now (:tada:), and maybe it even makes sense to split out all of this stuff to a `librustc_borrow_check`, but those are for the future. Feel free to ping me here or on zulip and let me know what you think...

cc @nikomatsakis @matthewjasper @eddyb

src/librustc_mir/borrow_check/conflict_errors.rs → src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs

@@ -14,18 +14,22 @@ use rustc_errors::{Applicability, DiagnosticBuilder};
 use syntax_pos::Span;
 use syntax::source_map::DesugaringKind;
 
-use super::nll::explain_borrow::BorrowExplanation;
-use super::nll::region_infer::{RegionName, RegionNameSource};
-use super::prefixes::IsPrefixOf;
-use super::WriteKind;
-use super::borrow_set::BorrowData;
-use super::MirBorrowckCtxt;
-use super::{InitializationRequiringAction, PrefixSet};
-use super::error_reporting::{IncludingDowncast, UseSpans};
 use crate::dataflow::drop_flag_effects;
 use crate::dataflow::indexes::{MovePathIndex, MoveOutIndex};
 use crate::util::borrowck_errors;
 
+use crate::borrow_check::{
+    prefixes::IsPrefixOf,
+    WriteKind,
+    borrow_set::BorrowData,
+    MirBorrowckCtxt, InitializationRequiringAction, PrefixSet
+};
+
+use super::{
+    IncludingDowncast, UseSpans, RegionName, RegionNameSource,
+    explain_borrow::BorrowExplanation,
+};
+
 #[derive(Debug)]
 struct MoveSite {
     /// Index of the "move out" that we found. The `MoveData` can
@@ -46,7 +50,7 @@ enum StorageDeadOrDrop<'tcx> {
 }
 
 impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
-    pub(super) fn report_use_of_moved_or_uninitialized(
+    pub(in crate::borrow_check) fn report_use_of_moved_or_uninitialized(
         &mut self,
         location: Location,
         desired_action: InitializationRequiringAction,
@@ -269,7 +273,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         }
     }
 
-    pub(super) fn report_move_out_while_borrowed(
+    pub(in crate::borrow_check) fn report_move_out_while_borrowed(
         &mut self,
         location: Location,
         (place, span): (&Place<'tcx>, Span),
@@ -326,7 +330,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         err.buffer(&mut self.errors_buffer);
     }
 
-    pub(super) fn report_use_while_mutably_borrowed(
+    pub(in crate::borrow_check) fn report_use_while_mutably_borrowed(
         &mut self,
         location: Location,
         (place, _span): (&Place<'tcx>, Span),
@@ -368,7 +372,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         err
     }
 
-    pub(super) fn report_conflicting_borrow(
+    pub(in crate::borrow_check) fn report_conflicting_borrow(
         &mut self,
         location: Location,
         (place, span): (&Place<'tcx>, Span),
@@ -614,7 +618,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
     ///
     /// >  cannot borrow `a.u` (via `a.u.z.c`) as immutable because it is also borrowed as
     /// >  mutable (via `a.u.s.b`) [E0502]
-    pub(super) fn describe_place_for_conflicting_borrow(
+    pub(in crate::borrow_check) fn describe_place_for_conflicting_borrow(
         &self,
         first_borrowed_place: &Place<'tcx>,
         second_borrowed_place: &Place<'tcx>,
@@ -722,7 +726,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
     /// short a lifetime. (But sometimes it is more useful to report
     /// it as a more direct conflict between the execution of a
     /// `Drop::drop` with an aliasing borrow.)
-    pub(super) fn report_borrowed_value_does_not_live_long_enough(
+    pub(in crate::borrow_check) fn report_borrowed_value_does_not_live_long_enough(
         &mut self,
         location: Location,
         borrow: &BorrowData<'tcx>,
@@ -1478,7 +1482,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         result
     }
 
-    pub(super) fn report_illegal_mutation_of_borrowed(
+    pub(in crate::borrow_check) fn report_illegal_mutation_of_borrowed(
         &mut self,
         location: Location,
         (place, span): (&Place<'tcx>, Span),
@@ -1537,7 +1541,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
     /// assigned; `err_place` is a place providing a reason why
     /// `place` is not mutable (e.g., the non-`mut` local `x` in an
     /// assignment to `x.f`).
-    pub(super) fn report_illegal_reassignment(
+    pub(in crate::borrow_check) fn report_illegal_reassignment(
         &mut self,
         _location: Location,
         (place, span): (&Place<'tcx>, Span),
@@ -2080,7 +2084,7 @@ enum AnnotatedBorrowFnSignature<'tcx> {
 impl<'tcx> AnnotatedBorrowFnSignature<'tcx> {
     /// Annotate the provided diagnostic with information about borrow from the fn signature that
     /// helps explain.
-    pub(super) fn emit(
+    pub(in crate::borrow_check) fn emit(
         &self,
         cx: &mut MirBorrowckCtxt<'_, 'tcx>,
         diag: &mut DiagnosticBuilder<'_>,

src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs → src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs

@@ -1,8 +1,7 @@
 use std::collections::VecDeque;
 
 use crate::borrow_check::borrow_set::BorrowData;
-use crate::borrow_check::error_reporting::UseSpans;
-use crate::borrow_check::nll::region_infer::{Cause, RegionName};
+use crate::borrow_check::nll::region_infer::Cause;
 use crate::borrow_check::nll::ConstraintDescription;
 use crate::borrow_check::{MirBorrowckCtxt, WriteKind};
 use rustc::mir::{
@@ -17,7 +16,7 @@ use rustc_errors::DiagnosticBuilder;
 use syntax_pos::Span;
 use syntax_pos::symbol::Symbol;
 
-mod find_use;
+use super::{UseSpans, find_use, RegionName};
 
 #[derive(Debug)]
 pub(in crate::borrow_check) enum BorrowExplanation {

src/librustc_mir/borrow_check/error_reporting.rs → src/librustc_mir/borrow_check/diagnostics/mod.rs

@@ -1,3 +1,5 @@
+//! Borrow checker diagnostics.
+
 use rustc::hir;
 use rustc::hir::def::Namespace;
 use rustc::hir::def_id::DefId;
@@ -17,6 +19,22 @@ use super::borrow_set::BorrowData;
 use super::MirBorrowckCtxt;
 use crate::dataflow::move_paths::{InitLocation, LookupResult};
 
+mod find_use;
+mod var_name;
+mod region_name;
+mod outlives_suggestion;
+
+mod conflict_errors;
+mod move_errors;
+mod mutability_errors;
+mod region_errors;
+mod explain_borrow;
+
+crate use mutability_errors::AccessKind;
+crate use region_name::{RegionName, RegionNameSource, RegionErrorNamingCtx};
+crate use region_errors::{ErrorReportingCtx, ErrorConstraintInfo};
+crate use outlives_suggestion::OutlivesSuggestionBuilder;
+
 pub(super) struct IncludingDowncast(pub(super) bool);
 
 impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

src/librustc_mir/borrow_check/move_errors.rs → src/librustc_mir/borrow_check/diagnostics/move_errors.rs

@@ -5,7 +5,7 @@ use syntax_pos::Span;
 
 use crate::borrow_check::MirBorrowckCtxt;
 use crate::borrow_check::prefixes::PrefixSet;
-use crate::borrow_check::error_reporting::UseSpans;
+use crate::borrow_check::diagnostics::UseSpans;
 use crate::dataflow::move_paths::{
     IllegalMoveOrigin, IllegalMoveOriginKind,
     LookupResult, MoveError, MovePathIndex,

src/librustc_mir/borrow_check/mutability_errors.rs → src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs

@@ -8,18 +8,18 @@ use syntax_pos::Span;
 use syntax_pos::symbol::kw;
 
 use crate::borrow_check::MirBorrowckCtxt;
-use crate::borrow_check::error_reporting::BorrowedContentSource;
+use crate::borrow_check::diagnostics::BorrowedContentSource;
 use crate::util::collect_writes::FindAssignments;
 use rustc_errors::Applicability;
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
-pub(super) enum AccessKind {
+pub(crate) enum AccessKind {
     MutableBorrow,
     Mutate,
 }
 
 impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
-    pub(super) fn report_mutability_error(
+    pub(crate) fn report_mutability_error(
         &mut self,
         access_place: &Place<'tcx>,
         span: Span,

src/librustc_mir/borrow_check/nll/region_infer/error_reporting/outlives_suggestion.rs → src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs

@@ -13,12 +13,10 @@ use syntax_pos::symbol::Symbol;
 
 use smallvec::SmallVec;
 
-use crate::borrow_check::nll::region_infer::{
-    error_reporting::{
-        region_name::{RegionName, RegionNameSource},
-        ErrorConstraintInfo, ErrorReportingCtx, RegionErrorNamingCtx,
-    },
-    RegionInferenceContext,
+use crate::borrow_check::nll::region_infer::RegionInferenceContext;
+
+use super::{
+    RegionName, RegionNameSource, ErrorConstraintInfo, ErrorReportingCtx, RegionErrorNamingCtx,
 };
 
 /// The different things we could suggest.

src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs → src/librustc_mir/borrow_check/diagnostics/region_errors.rs

@@ -19,14 +19,7 @@ use syntax::symbol::kw;
 use syntax_pos::Span;
 use syntax_pos::symbol::Symbol;
 
-use self::outlives_suggestion::OutlivesSuggestionBuilder;
-
-pub mod outlives_suggestion;
-
-mod region_name;
-mod var_name;
-
-crate use self::region_name::{RegionName, RegionNameSource, RegionErrorNamingCtx};
+use super::{OutlivesSuggestionBuilder, RegionName, RegionNameSource, RegionErrorNamingCtx};
 
 impl ConstraintDescription for ConstraintCategory {
     fn description(&self) -> &'static str {
@@ -61,36 +54,36 @@ enum Trace {
 /// Various pieces of state used when reporting borrow checker errors.
 pub struct ErrorReportingCtx<'a, 'b, 'tcx> {
     /// The region inference context used for borrow chekcing this MIR body.
-    region_infcx: &'b RegionInferenceContext<'tcx>,
+    pub(super) region_infcx: &'b RegionInferenceContext<'tcx>,
 
     /// The inference context used for type checking.
-    infcx: &'b InferCtxt<'a, 'tcx>,
+    pub(super) infcx: &'b InferCtxt<'a, 'tcx>,
 
     /// The MIR def we are reporting errors on.
-    mir_def_id: DefId,
+    pub(super) mir_def_id: DefId,
 
     /// The MIR body we are reporting errors on (for convenience).
-    body: &'b Body<'tcx>,
+    pub(super) body: &'b Body<'tcx>,
 
     /// User variable names for MIR locals (where applicable).
-    local_names: &'b IndexVec<Local, Option<Symbol>>,
+    pub(super) local_names: &'b IndexVec<Local, Option<Symbol>>,
 
     /// Any upvars for the MIR body we have kept track of during borrow checking.
-    upvars: &'b [Upvar],
+    pub(super) upvars: &'b [Upvar],
 }
 
 /// Information about the various region constraints involved in a borrow checker error.
 #[derive(Clone, Debug)]
 pub struct ErrorConstraintInfo {
     // fr: outlived_fr
-    fr: RegionVid,
-    fr_is_local: bool,
-    outlived_fr: RegionVid,
-    outlived_fr_is_local: bool,
+    pub(super) fr: RegionVid,
+    pub(super) fr_is_local: bool,
+    pub(super) outlived_fr: RegionVid,
+    pub(super) outlived_fr_is_local: bool,
 
     // Category and span for best blame constraint
-    category: ConstraintCategory,
-    span: Span,
+    pub(super) category: ConstraintCategory,
+    pub(super) span: Span,
 }
 
 impl<'tcx> RegionInferenceContext<'tcx> {
@@ -368,7 +361,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     /// ```
     ///
     /// Here we would be invoked with `fr = 'a` and `outlived_fr = `'b`.
-    pub(super) fn report_error<'a>(
+    pub(in crate::borrow_check) fn report_error<'a>(
         &'a self,
         body: &Body<'tcx>,
         local_names: &IndexVec<Local, Option<Symbol>>,

src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs → src/librustc_mir/borrow_check/diagnostics/region_name.rs

@@ -1,12 +1,5 @@
 use std::fmt::{self, Display};
 
-use crate::borrow_check::nll::region_infer::{
-    RegionInferenceContext,
-    error_reporting::ErrorReportingCtx,
-};
-use crate::borrow_check::nll::universal_regions::DefiningTy;
-use crate::borrow_check::nll::ToRegionVid;
-use crate::borrow_check::Upvar;
 use rustc::hir;
 use rustc::hir::def::{Res, DefKind};
 use rustc::hir::def_id::DefId;
@@ -21,6 +14,15 @@ use syntax::symbol::kw;
 use rustc_data_structures::fx::FxHashMap;
 use syntax_pos::{Span, symbol::Symbol, DUMMY_SP};
 
+use crate::borrow_check::{
+    nll::region_infer::RegionInferenceContext,
+    nll::universal_regions::DefiningTy,
+    nll::ToRegionVid,
+    Upvar,
+};
+
+use super::region_errors::ErrorReportingCtx;
+
 /// A name for a particular region used in emitting diagnostics. This name could be a generated
 /// name like `'1`, a name used by the user like `'a`, or a name like `'static`.
 #[derive(Debug, Clone)]

src/librustc_mir/borrow_check/mod.rs

@@ -46,17 +46,14 @@ use self::flows::Flows;
 use self::location::LocationTable;
 use self::prefixes::PrefixSet;
 use self::MutateMode::{JustWrite, WriteAndRead};
-use self::mutability_errors::AccessKind;
+use self::diagnostics::AccessKind;
 
 use self::path_utils::*;
 
 crate mod borrow_set;
-mod error_reporting;
+mod diagnostics;
 mod flows;
 mod location;
-mod conflict_errors;
-mod move_errors;
-mod mutability_errors;
 mod path_utils;
 crate mod place_ext;
 crate mod places_conflict;

src/librustc_mir/borrow_check/nll/mod.rs

@@ -32,17 +32,17 @@ use crate::util as mir_util;
 use crate::util::pretty;
 
 mod constraint_generation;
-pub mod explain_borrow;
 mod facts;
 mod invalidation;
-crate mod region_infer;
 mod renumber;
-crate mod type_check;
-mod universal_regions;
 
-mod constraints;
 mod member_constraints;
 
+crate mod constraints;
+crate mod universal_regions;
+crate mod type_check;
+crate mod region_infer;
+
 use self::facts::AllFacts;
 use self::region_infer::RegionInferenceContext;
 use self::universal_regions::UniversalRegions;