From 4ad272b282dc28c1366cf0516315f00b13d1621e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 6 Feb 2022 12:15:39 -0800 Subject: [PATCH 1/7] implement tainted_by_errors in mir borrowck --- .../src/diagnostics/bound_region_errors.rs | 16 ++++---- .../src/diagnostics/conflict_errors.rs | 21 ++++++---- .../src/diagnostics/move_errors.rs | 2 +- .../src/diagnostics/mutability_errors.rs | 2 +- .../src/diagnostics/outlives_suggestion.rs | 2 +- .../src/diagnostics/region_errors.rs | 35 +++++++--------- compiler/rustc_borrowck/src/lib.rs | 40 ++++++++++++++----- compiler/rustc_borrowck/src/nll.rs | 1 + compiler/rustc_middle/src/mir/query.rs | 1 + 9 files changed, 68 insertions(+), 52 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs index 96326ef2d5a07..ac9950241bfe3 100644 --- a/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs @@ -55,7 +55,7 @@ impl<'tcx> UniverseInfo<'tcx> { found, TypeError::RegionsPlaceholderMismatch, ); - err.buffer(&mut mbcx.errors_buffer); + mbcx.buffer_error(err); } UniverseInfoInner::TypeOp(ref type_op_info) => { type_op_info.report_error(mbcx, placeholder, error_element, cause); @@ -64,11 +64,9 @@ impl<'tcx> UniverseInfo<'tcx> { // FIXME: This error message isn't great, but it doesn't show // up in the existing UI tests. Consider investigating this // some more. - mbcx.infcx - .tcx - .sess - .struct_span_err(cause.span, "higher-ranked subtype error") - .buffer(&mut mbcx.errors_buffer); + mbcx.buffer_error( + mbcx.infcx.tcx.sess.struct_span_err(cause.span, "higher-ranked subtype error"), + ); } } } @@ -149,7 +147,7 @@ trait TypeOpInfo<'tcx> { { adjusted } else { - self.fallback_error(tcx, cause.span).buffer(&mut mbcx.errors_buffer); + mbcx.buffer_error(self.fallback_error(tcx, cause.span)); return; }; @@ -178,9 +176,9 @@ trait TypeOpInfo<'tcx> { let nice_error = self.nice_error(tcx, cause, placeholder_region, error_region); if let Some(nice_error) = nice_error { - nice_error.buffer(&mut mbcx.errors_buffer); + mbcx.buffer_error(nice_error); } else { - self.fallback_error(tcx, span).buffer(&mut mbcx.errors_buffer); + mbcx.buffer_error(self.fallback_error(tcx, span)); } } } diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index ba111d394ec26..e32963faa7a4b 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -77,6 +77,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if move_out_indices.is_empty() { let root_place = PlaceRef { projection: &[], ..used_place }; + self.set_tainted_by_errors(); if !self.uninitialized_error_reported.insert(root_place) { debug!( "report_use_of_moved_or_uninitialized place: error about {:?} suppressed", @@ -104,7 +105,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { format!("{} occurs due to use{}", desired_action.as_noun(), use_spans.describe()), ); - err.buffer(&mut self.errors_buffer); + self.buffer_error(err); } else { if let Some((reported_place, _)) = self.move_error_reported.get(&move_out_indices) { if self.prefixes(*reported_place, PrefixSet::All).any(|p| p == used_place) { @@ -216,6 +217,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { place_name, partially_str, loop_message ), ); + self.set_tainted_by_errors(); if self.fn_self_span_reported.insert(fn_span) { err.span_note( // Check whether the source is accessible @@ -297,6 +299,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } // Avoid pointing to the same function in multiple different // error messages. + self.set_tainted_by_errors(); if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span) { err.span_note( @@ -449,6 +452,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } + self.set_tainted_by_errors(); if let Some((_, mut old_err)) = self.move_error_reported.insert(move_out_indices, (used_place, err)) { @@ -503,7 +507,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { Some(borrow_span), None, ); - err.buffer(&mut self.errors_buffer); + self.buffer_error(err); } pub(crate) fn report_use_while_mutably_borrowed( @@ -1012,6 +1016,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { return; } + self.set_tainted_by_errors(); self.access_place_error_reported.insert(( Place { local: root_place.local, projection: root_place_projection }, borrow_span, @@ -1021,7 +1026,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if self.body.local_decls[borrowed_local].is_ref_to_thread_local() { let err = self.report_thread_local_value_does_not_live_long_enough(drop_span, borrow_span); - err.buffer(&mut self.errors_buffer); + self.buffer_error(err); return; } @@ -1113,7 +1118,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ), }; - err.buffer(&mut self.errors_buffer); + self.buffer_error(err); } fn report_local_value_does_not_live_long_enough( @@ -1295,7 +1300,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { None, ); - err.buffer(&mut self.errors_buffer); + self.buffer_error(err); } fn report_thread_local_value_does_not_live_long_enough( @@ -1810,7 +1815,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { loan.kind.describe_mutability(), ); - err.buffer(&mut self.errors_buffer); + self.buffer_error(err); return; } @@ -1836,7 +1841,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { self.explain_deref_coercion(loan, &mut err); - err.buffer(&mut self.errors_buffer); + self.buffer_error(err); } fn explain_deref_coercion(&mut self, loan: &BorrowData<'tcx>, err: &mut DiagnosticBuilder<'_>) { @@ -1938,7 +1943,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } err.span_label(span, msg); - err.buffer(&mut self.errors_buffer); + self.buffer_error(err); } fn classify_drop_access_kind(&self, place: PlaceRef<'tcx>) -> StorageDeadOrDrop<'tcx> { diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs index 692c20d7dfe34..2934d921868a8 100644 --- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs @@ -264,7 +264,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { }; self.add_move_hints(error, &mut err, err_span); - err.buffer(&mut self.errors_buffer); + self.buffer_error(err); } fn report_cannot_move_from_static( diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index e6c057cc8eea5..5963904aa0b51 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -626,7 +626,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } } - err.buffer(&mut self.errors_buffer); + self.buffer_error(err); } /// User cannot make signature of a trait mutable without changing the diff --git a/compiler/rustc_borrowck/src/diagnostics/outlives_suggestion.rs b/compiler/rustc_borrowck/src/diagnostics/outlives_suggestion.rs index 723b57ed970ad..21f00af5c0cef 100644 --- a/compiler/rustc_borrowck/src/diagnostics/outlives_suggestion.rs +++ b/compiler/rustc_borrowck/src/diagnostics/outlives_suggestion.rs @@ -256,6 +256,6 @@ impl OutlivesSuggestionBuilder { diag.sort_span = mir_span.shrink_to_hi(); // Buffer the diagnostic - diag.buffer(&mut mbcx.errors_buffer); + mbcx.buffer_error(diag); } } diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs index df23eaf24bc6d..31c977cc78d3a 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs @@ -168,14 +168,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let type_test_span = type_test.locations.span(&self.body); if let Some(lower_bound_region) = lower_bound_region { - self.infcx - .construct_generic_bound_failure( - type_test_span, - None, - type_test.generic_kind, - lower_bound_region, - ) - .buffer(&mut self.errors_buffer); + self.buffer_error(self.infcx.construct_generic_bound_failure( + type_test_span, + None, + type_test.generic_kind, + lower_bound_region, + )); } else { // FIXME. We should handle this case better. It // indicates that we have e.g., some region variable @@ -186,27 +184,22 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { // to report it; we could probably handle it by // iterating over the universal regions and reporting // an error that multiple bounds are required. - self.infcx - .tcx - .sess - .struct_span_err( - type_test_span, - &format!("`{}` does not live long enough", type_test.generic_kind), - ) - .buffer(&mut self.errors_buffer); + self.buffer_error(self.infcx.tcx.sess.struct_span_err( + type_test_span, + &format!("`{}` does not live long enough", type_test.generic_kind), + )); } } RegionErrorKind::UnexpectedHiddenRegion { span, hidden_ty, member_region } => { let named_ty = self.regioncx.name_regions(self.infcx.tcx, hidden_ty); let named_region = self.regioncx.name_regions(self.infcx.tcx, member_region); - unexpected_hidden_region_diagnostic( + self.buffer_error(unexpected_hidden_region_diagnostic( self.infcx.tcx, span, named_ty, named_region, - ) - .buffer(&mut self.errors_buffer); + )); } RegionErrorKind::BoundUniversalRegionError { @@ -285,7 +278,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { if let (Some(f), Some(o)) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) { let nice = NiceRegionError::new_from_span(self.infcx, cause.span, o, f); if let Some(diag) = nice.try_report_from_nll() { - diag.buffer(&mut self.errors_buffer); + self.buffer_error(diag); return; } } @@ -375,7 +368,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } } - diag.buffer(&mut self.errors_buffer); + self.buffer_error(diag); } /// Report a specialized error when `FnMut` closures return a reference to a captured variable. diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 5597a8b091554..4368a894f219e 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -313,6 +313,7 @@ fn do_mir_borrowck<'a, 'tcx>( move_error_reported: BTreeMap::new(), uninitialized_error_reported: Default::default(), errors_buffer, + tainted_by_errors: false, regioncx: regioncx.clone(), used_mut: Default::default(), used_mut_upvars: SmallVec::new(), @@ -346,6 +347,7 @@ fn do_mir_borrowck<'a, 'tcx>( move_error_reported: BTreeMap::new(), uninitialized_error_reported: Default::default(), errors_buffer, + tainted_by_errors: false, regioncx: Rc::clone(®ioncx), used_mut: Default::default(), used_mut_upvars: SmallVec::new(), @@ -398,7 +400,7 @@ fn do_mir_borrowck<'a, 'tcx>( diag.message = initial_diag.styled_message().clone(); diag.span = initial_diag.span.clone(); - diag.buffer(&mut mbcx.errors_buffer); + mbcx.buffer_error(diag); }, ); initial_diag.cancel(); @@ -423,7 +425,7 @@ fn do_mir_borrowck<'a, 'tcx>( mbcx.gather_used_muts(temporary_used_locals, unused_mut_locals); debug!("mbcx.used_mut: {:?}", mbcx.used_mut); - let used_mut = mbcx.used_mut; + let used_mut = std::mem::take(&mut mbcx.used_mut); for local in mbcx.body.mut_vars_and_args_iter().filter(|local| !used_mut.contains(local)) { let local_decl = &mbcx.body.local_decls[local]; let lint_root = match &mbcx.body.source_scopes[local_decl.source_info.scope].local_data { @@ -461,8 +463,8 @@ fn do_mir_borrowck<'a, 'tcx>( } // Buffer any move errors that we collected and de-duplicated. - for (_, (_, diag)) in mbcx.move_error_reported { - diag.buffer(&mut mbcx.errors_buffer); + for (_, (_, diag)) in std::mem::take(&mut mbcx.move_error_reported) { + mbcx.buffer_error(diag); } if !mbcx.errors_buffer.is_empty() { @@ -477,6 +479,7 @@ fn do_mir_borrowck<'a, 'tcx>( concrete_opaque_types: opaque_type_values, closure_requirements: opt_closure_req, used_mut_upvars: mbcx.used_mut_upvars, + tainted_by_errors: mbcx.tainted_by_errors, }; let body_with_facts = if return_body_with_facts { @@ -573,6 +576,8 @@ struct MirBorrowckCtxt<'cx, 'tcx> { uninitialized_error_reported: FxHashSet>, /// Errors to be reported buffer errors_buffer: Vec, + /// Set to true if we emit an error during borrowck + tainted_by_errors: bool, /// This field keeps track of all the local variables that are declared mut and are mutated. /// Used for the warning issued by an unused mutable local variable. used_mut: FxHashSet, @@ -1028,6 +1033,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if conflict_error || mutability_error { debug!("access_place: logging error place_span=`{:?}` kind=`{:?}`", place_span, kind); + self.set_tainted_by_errors(); self.access_place_error_reported.insert((place_span.0, place_span.1)); } } @@ -1107,12 +1113,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { error_reported = true; match kind { ReadKind::Copy => { - this.report_use_while_mutably_borrowed(location, place_span, borrow) - .buffer(&mut this.errors_buffer); + let err = this + .report_use_while_mutably_borrowed(location, place_span, borrow); + this.buffer_error(err); } ReadKind::Borrow(bk) => { - this.report_conflicting_borrow(location, place_span, bk, borrow) - .buffer(&mut this.errors_buffer); + let err = + this.report_conflicting_borrow(location, place_span, bk, borrow); + this.buffer_error(err); } } Control::Break @@ -1162,8 +1170,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { error_reported = true; match kind { WriteKind::MutableBorrow(bk) => { - this.report_conflicting_borrow(location, place_span, bk, borrow) - .buffer(&mut this.errors_buffer); + let err = + this.report_conflicting_borrow(location, place_span, bk, borrow); + this.buffer_error(err); } WriteKind::StorageDeadOrDrop => this .report_borrowed_value_does_not_live_long_enough( @@ -1570,7 +1579,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { yield_span, ); - err.buffer(&mut self.errors_buffer); + self.buffer_error(err); } } @@ -2299,6 +2308,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { fn is_upvar_field_projection(&self, place_ref: PlaceRef<'tcx>) -> Option { path_utils::is_upvar_field_projection(self.infcx.tcx, &self.upvars, place_ref, self.body()) } + + pub fn buffer_error(&mut self, t: DiagnosticBuilder<'_>) { + self.tainted_by_errors = true; + t.buffer(&mut self.errors_buffer); + } + + pub fn set_tainted_by_errors(&mut self) { + self.tainted_by_errors = true; + } } /// The degree of overlap between 2 places for borrow-checking. diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 6ffab16577908..ac505a6071ded 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -418,6 +418,7 @@ pub(super) fn dump_annotation<'a, 'tcx>( err.note(&format!("Inferred opaque type values:\n{:#?}", opaque_type_values)); } + // FIXME(compiler-errors): Maybe we need to set tainted here err.buffer(errors_buffer); } diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index 4b8eb3fbd9607..b1aad01118ad4 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -214,6 +214,7 @@ pub struct BorrowCheckResult<'tcx> { pub concrete_opaque_types: VecMap, Ty<'tcx>>, pub closure_requirements: Option>, pub used_mut_upvars: SmallVec<[Field; 8]>, + pub tainted_by_errors: bool, } /// The result of the `mir_const_qualif` query. From 77dae2d25d900a8e7493c548498cdc94bdcee3d9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 6 Feb 2022 12:16:49 -0800 Subject: [PATCH 2/7] skip const eval if we have an error in borrowck --- .../src/const_eval/eval_queries.rs | 3 +++ .../src/interpret/eval_context.rs | 6 +++++- compiler/rustc_middle/src/ty/context.rs | 15 ++++++++++++++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 9dc34260de766..7235b8416ad65 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -287,6 +287,9 @@ pub fn eval_to_allocation_raw_provider<'tcx>( if let Some(error_reported) = tcx.typeck_opt_const_arg(def).tainted_by_errors { return Err(ErrorHandled::Reported(error_reported)); } + if tcx.mir_borrowck_opt_const_arg(def).tainted_by_errors { + return Err(ErrorHandled::Reported(ErrorReported {})); + } } if !tcx.is_mir_available(def.did) { tcx.sess.delay_span_bug( diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 0a8112da2aba8..00c066a2851c3 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -514,10 +514,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { if let Some(def) = def.as_local() { if self.tcx.has_typeck_results(def.did) { if let Some(error_reported) = self.tcx.typeck_opt_const_arg(def).tainted_by_errors { - throw_inval!(AlreadyReported(error_reported)) + throw_inval!(AlreadyReported(error_reported)); + } + if self.tcx.mir_borrowck_opt_const_arg(def).tainted_by_errors { + throw_inval!(AlreadyReported(rustc_errors::ErrorReported {})); } } } + trace!("load mir(instance={:?}, promoted={:?})", instance, promoted); if let Some(promoted) = promoted { return Ok(&self.tcx.promoted_mir_opt_const_arg(def)[promoted]); diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index a119655a90b20..4a3efb5c1b8ef 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -8,7 +8,9 @@ use crate::lint::{struct_lint_level, LintDiagnosticBuilder, LintLevelSource}; use crate::middle::resolve_lifetime::{self, LifetimeScopeForPath}; use crate::middle::stability; use crate::mir::interpret::{self, Allocation, ConstValue, Scalar}; -use crate::mir::{Body, Field, Local, Place, PlaceElem, ProjectionKind, Promoted}; +use crate::mir::{ + Body, BorrowCheckResult, Field, Local, Place, PlaceElem, ProjectionKind, Promoted, +}; use crate::thir::Thir; use crate::traits; use crate::ty::query::{self, TyCtxtAt}; @@ -1061,6 +1063,17 @@ impl<'tcx> TyCtxt<'tcx> { } } + pub fn mir_borrowck_opt_const_arg( + self, + def: ty::WithOptConstParam, + ) -> &'tcx BorrowCheckResult<'tcx> { + if let Some(param_did) = def.const_param_did { + self.mir_borrowck_const_arg((def.did, param_did)) + } else { + self.mir_borrowck(def.did) + } + } + pub fn alloc_steal_thir(self, thir: Thir<'tcx>) -> &'tcx Steal> { self.arena.alloc(Steal::new(thir)) } From 06eb912e761d39c63490ab5e688abcf2f94fc61d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 5 Feb 2022 20:51:33 -0800 Subject: [PATCH 3/7] fix tests, add new tests checking borrowck CFTE ICE --- .../const-generic-default-wont-borrowck.rs | 6 ++++++ .../const-generic-default-wont-borrowck.stderr | 9 +++++++++ .../const-mut-refs/issue-76510.32bit.stderr | 17 +++-------------- .../const-mut-refs/issue-76510.64bit.stderr | 17 +++-------------- .../ui/consts/const-mut-refs/issue-76510.rs | 1 - src/test/ui/consts/issue-78655.rs | 2 +- src/test/ui/consts/issue-78655.stderr | 11 +---------- 7 files changed, 23 insertions(+), 40 deletions(-) create mode 100644 src/test/ui/const-generics/const-generic-default-wont-borrowck.rs create mode 100644 src/test/ui/const-generics/const-generic-default-wont-borrowck.stderr diff --git a/src/test/ui/const-generics/const-generic-default-wont-borrowck.rs b/src/test/ui/const-generics/const-generic-default-wont-borrowck.rs new file mode 100644 index 0000000000000..bb5a2f1766f1b --- /dev/null +++ b/src/test/ui/const-generics/const-generic-default-wont-borrowck.rs @@ -0,0 +1,6 @@ +struct X; + +fn main() {} diff --git a/src/test/ui/const-generics/const-generic-default-wont-borrowck.stderr b/src/test/ui/const-generics/const-generic-default-wont-borrowck.stderr new file mode 100644 index 0000000000000..6c25019b0ceb3 --- /dev/null +++ b/src/test/ui/const-generics/const-generic-default-wont-borrowck.stderr @@ -0,0 +1,9 @@ +error[E0381]: borrow of possibly-uninitialized variable: `s` + --> $DIR/const-generic-default-wont-borrowck.rs:2:26 + | +LL | let s: &'static str; s.len() + | ^^^^^^^ use of possibly-uninitialized `*s` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0381`. diff --git a/src/test/ui/consts/const-mut-refs/issue-76510.32bit.stderr b/src/test/ui/consts/const-mut-refs/issue-76510.32bit.stderr index a9411fb0e3d4c..61b00be345fee 100644 --- a/src/test/ui/consts/const-mut-refs/issue-76510.32bit.stderr +++ b/src/test/ui/consts/const-mut-refs/issue-76510.32bit.stderr @@ -19,18 +19,7 @@ error[E0596]: cannot borrow data in a `&` reference as mutable LL | const S: &'static mut str = &mut " hello "; | ^^^^^^^^^^^^^^ cannot borrow as mutable -error[E0080]: it is undefined behavior to use this value - --> $DIR/issue-76510.rs:5:1 - | -LL | const S: &'static mut str = &mut " hello "; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered mutable reference in a `const` - | - = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. - = note: the raw bytes of the constant (size: 8, align: 4) { - ╾─alloc3──╼ 07 00 00 00 │ ╾──╼.... - } - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors -Some errors have detailed explanations: E0080, E0596, E0658, E0764. -For more information about an error, try `rustc --explain E0080`. +Some errors have detailed explanations: E0596, E0658, E0764. +For more information about an error, try `rustc --explain E0596`. diff --git a/src/test/ui/consts/const-mut-refs/issue-76510.64bit.stderr b/src/test/ui/consts/const-mut-refs/issue-76510.64bit.stderr index 9ad5f20d57cb2..61b00be345fee 100644 --- a/src/test/ui/consts/const-mut-refs/issue-76510.64bit.stderr +++ b/src/test/ui/consts/const-mut-refs/issue-76510.64bit.stderr @@ -19,18 +19,7 @@ error[E0596]: cannot borrow data in a `&` reference as mutable LL | const S: &'static mut str = &mut " hello "; | ^^^^^^^^^^^^^^ cannot borrow as mutable -error[E0080]: it is undefined behavior to use this value - --> $DIR/issue-76510.rs:5:1 - | -LL | const S: &'static mut str = &mut " hello "; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered mutable reference in a `const` - | - = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. - = note: the raw bytes of the constant (size: 16, align: 8) { - ╾───────alloc3────────╼ 07 00 00 00 00 00 00 00 │ ╾──────╼........ - } - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors -Some errors have detailed explanations: E0080, E0596, E0658, E0764. -For more information about an error, try `rustc --explain E0080`. +Some errors have detailed explanations: E0596, E0658, E0764. +For more information about an error, try `rustc --explain E0596`. diff --git a/src/test/ui/consts/const-mut-refs/issue-76510.rs b/src/test/ui/consts/const-mut-refs/issue-76510.rs index 892f6c98116c2..143d2fb6b9a3a 100644 --- a/src/test/ui/consts/const-mut-refs/issue-76510.rs +++ b/src/test/ui/consts/const-mut-refs/issue-76510.rs @@ -6,7 +6,6 @@ const S: &'static mut str = &mut " hello "; //~^ ERROR: mutable references are not allowed in the final value of constants //~| ERROR: mutation through a reference is not allowed in constants //~| ERROR: cannot borrow data in a `&` reference as mutable -//~| ERROR: it is undefined behavior to use this value const fn trigger() -> [(); unsafe { let s = transmute::<(*const u8, usize), &ManuallyDrop>((S.as_ptr(), 3)); diff --git a/src/test/ui/consts/issue-78655.rs b/src/test/ui/consts/issue-78655.rs index 066764bc46fc4..b85e612992549 100644 --- a/src/test/ui/consts/issue-78655.rs +++ b/src/test/ui/consts/issue-78655.rs @@ -1,4 +1,4 @@ -const FOO: *const u32 = { //~ ERROR encountered dangling pointer in final constant +const FOO: *const u32 = { let x; &x //~ ERROR borrow of possibly-uninitialized variable: `x` }; diff --git a/src/test/ui/consts/issue-78655.stderr b/src/test/ui/consts/issue-78655.stderr index cf3fe18f802fb..734266a3453b5 100644 --- a/src/test/ui/consts/issue-78655.stderr +++ b/src/test/ui/consts/issue-78655.stderr @@ -4,15 +4,6 @@ error[E0381]: borrow of possibly-uninitialized variable: `x` LL | &x | ^^ use of possibly-uninitialized `x` -error: encountered dangling pointer in final constant - --> $DIR/issue-78655.rs:1:1 - | -LL | / const FOO: *const u32 = { -LL | | let x; -LL | | &x -LL | | }; - | |__^ - error: could not evaluate constant pattern --> $DIR/issue-78655.rs:7:9 | @@ -25,6 +16,6 @@ error: could not evaluate constant pattern LL | let FOO = FOO; | ^^^ -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0381`. From 8b7b0a0e49cf3e313d0432d1f6e8ce20253fef0a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 6 Feb 2022 13:53:38 -0800 Subject: [PATCH 4/7] always cache result from mir_borrowck --- compiler/rustc_middle/src/query/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index fc2750d230395..98d54edf92b80 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -836,13 +836,14 @@ rustc_queries! { /// additional requirements that the closure's creator must verify. query mir_borrowck(key: LocalDefId) -> &'tcx mir::BorrowCheckResult<'tcx> { desc { |tcx| "borrow-checking `{}`", tcx.def_path_str(key.to_def_id()) } - cache_on_disk_if(tcx) { tcx.is_typeck_child(key.to_def_id()) } + cache_on_disk_if { true } } query mir_borrowck_const_arg(key: (LocalDefId, DefId)) -> &'tcx mir::BorrowCheckResult<'tcx> { desc { |tcx| "borrow-checking the const argument`{}`", tcx.def_path_str(key.0.to_def_id()) } + cache_on_disk_if { true } } /// Gets a complete map from all types to their inherent impls. From 29c2bb51c0b82bed1322a18566fb659c1846d136 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 7 Feb 2022 22:37:32 -0800 Subject: [PATCH 5/7] rework borrowck errors so that it's harder to not set tainted --- .../rustc_borrowck/src/borrowck_errors.rs | 2 +- .../src/diagnostics/conflict_errors.rs | 14 +- compiler/rustc_borrowck/src/lib.rs | 159 ++++++++++++------ compiler/rustc_borrowck/src/nll.rs | 6 +- .../src/const_eval/eval_queries.rs | 4 +- .../src/interpret/eval_context.rs | 6 +- compiler/rustc_middle/src/mir/query.rs | 2 +- 7 files changed, 119 insertions(+), 74 deletions(-) diff --git a/compiler/rustc_borrowck/src/borrowck_errors.rs b/compiler/rustc_borrowck/src/borrowck_errors.rs index 5702203d7c4ff..7140cda8e4e51 100644 --- a/compiler/rustc_borrowck/src/borrowck_errors.rs +++ b/compiler/rustc_borrowck/src/borrowck_errors.rs @@ -327,7 +327,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { verb: &str, optional_adverb_for_moved: &str, moved_path: Option, - ) -> DiagnosticBuilder<'cx> { + ) -> DiagnosticBuilder<'tcx> { let moved_path = moved_path.map(|mp| format!(": `{}`", mp)).unwrap_or_default(); struct_span_err!( diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index e32963faa7a4b..7b8b5974fe758 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -77,7 +77,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if move_out_indices.is_empty() { let root_place = PlaceRef { projection: &[], ..used_place }; - self.set_tainted_by_errors(); if !self.uninitialized_error_reported.insert(root_place) { debug!( "report_use_of_moved_or_uninitialized place: error about {:?} suppressed", @@ -107,7 +106,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { self.buffer_error(err); } else { - if let Some((reported_place, _)) = self.move_error_reported.get(&move_out_indices) { + 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 \ @@ -217,7 +216,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { place_name, partially_str, loop_message ), ); - self.set_tainted_by_errors(); if self.fn_self_span_reported.insert(fn_span) { err.span_note( // Check whether the source is accessible @@ -299,7 +297,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } // Avoid pointing to the same function in multiple different // error messages. - self.set_tainted_by_errors(); if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span) { err.span_note( @@ -452,13 +449,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - self.set_tainted_by_errors(); - if let Some((_, mut old_err)) = - self.move_error_reported.insert(move_out_indices, (used_place, err)) - { - // Cancel the old error so it doesn't ICE. - old_err.cancel(); - } + self.buffer_move_error(move_out_indices, (used_place, err)); } } @@ -1016,7 +1007,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { return; } - self.set_tainted_by_errors(); self.access_place_error_reported.insert(( Place { local: root_place.local, projection: root_place_projection }, borrow_span, diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 4368a894f219e..459b03b0fad65 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -175,10 +175,13 @@ fn do_mir_borrowck<'a, 'tcx>( } } + let mut errors = error::BorrowckErrors::new(); + // Gather the upvars of a closure, if any. let tables = tcx.typeck_opt_const_arg(def); if let Some(ErrorReported) = tables.tainted_by_errors { infcx.set_tainted_by_errors(); + errors.set_tainted_by_errors(); } let upvars: Vec<_> = tables .closure_min_captures_flattened(def.did.to_def_id()) @@ -205,7 +208,6 @@ fn do_mir_borrowck<'a, 'tcx>( let location_table_owned = LocationTable::new(body); let location_table = &location_table_owned; - let mut errors_buffer = Vec::new(); let (move_data, move_errors): (MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>) = match MoveData::gather_moves(&body, tcx, param_env) { Ok(move_data) => (move_data, Vec::new()), @@ -263,7 +265,7 @@ fn do_mir_borrowck<'a, 'tcx>( ®ioncx, &opt_closure_req, &opaque_type_values, - &mut errors_buffer, + &mut errors, ); // The various `flow_*` structures can be large. We drop `flow_inits` here @@ -310,10 +312,7 @@ fn do_mir_borrowck<'a, 'tcx>( access_place_error_reported: Default::default(), reservation_error_reported: Default::default(), reservation_warnings: Default::default(), - move_error_reported: BTreeMap::new(), uninitialized_error_reported: Default::default(), - errors_buffer, - tainted_by_errors: false, regioncx: regioncx.clone(), used_mut: Default::default(), used_mut_upvars: SmallVec::new(), @@ -324,9 +323,10 @@ fn do_mir_borrowck<'a, 'tcx>( region_names: RefCell::default(), next_region_name: RefCell::new(1), polonius_output: None, + errors, }; promoted_mbcx.report_move_errors(move_errors); - errors_buffer = promoted_mbcx.errors_buffer; + errors = promoted_mbcx.errors; }; } @@ -344,10 +344,7 @@ fn do_mir_borrowck<'a, 'tcx>( access_place_error_reported: Default::default(), reservation_error_reported: Default::default(), reservation_warnings: Default::default(), - move_error_reported: BTreeMap::new(), uninitialized_error_reported: Default::default(), - errors_buffer, - tainted_by_errors: false, regioncx: Rc::clone(®ioncx), used_mut: Default::default(), used_mut_upvars: SmallVec::new(), @@ -358,6 +355,7 @@ fn do_mir_borrowck<'a, 'tcx>( region_names: RefCell::default(), next_region_name: RefCell::new(1), polonius_output, + errors, }; // Compute and report region errors, if any. @@ -462,24 +460,13 @@ fn do_mir_borrowck<'a, 'tcx>( }) } - // Buffer any move errors that we collected and de-duplicated. - for (_, (_, diag)) in std::mem::take(&mut mbcx.move_error_reported) { - mbcx.buffer_error(diag); - } - - if !mbcx.errors_buffer.is_empty() { - mbcx.errors_buffer.sort_by_key(|diag| diag.sort_span); - - for diag in mbcx.errors_buffer.drain(..) { - mbcx.infcx.tcx.sess.diagnostic().emit_diagnostic(&diag); - } - } + let tainted_by_errors = mbcx.emit_errors(); let result = BorrowCheckResult { concrete_opaque_types: opaque_type_values, closure_requirements: opt_closure_req, used_mut_upvars: mbcx.used_mut_upvars, - tainted_by_errors: mbcx.tainted_by_errors, + tainted_by_errors, }; let body_with_facts = if return_body_with_facts { @@ -556,28 +543,9 @@ struct MirBorrowckCtxt<'cx, 'tcx> { /// for the activation of the borrow. reservation_warnings: FxHashMap, Span, Location, BorrowKind, BorrowData<'tcx>)>, - /// This field keeps track of move errors that are to be reported for given move indices. - /// - /// There are situations where many errors can be reported for a single move out (see #53807) - /// and we want only the best of those errors. - /// - /// The `report_use_of_moved_or_uninitialized` function checks this map and replaces the - /// diagnostic (if there is one) if the `Place` of the error being reported is a prefix of the - /// `Place` of the previous most diagnostic. This happens instead of buffering the error. Once - /// all move errors have been reported, any diagnostics in this map are added to the buffer - /// to be emitted. - /// - /// `BTreeMap` is used to preserve the order of insertions when iterating. This is necessary - /// when errors in the map are being re-added to the error buffer so that errors with the - /// same primary span come out in a consistent order. - move_error_reported: BTreeMap, (PlaceRef<'tcx>, DiagnosticBuilder<'cx>)>, /// This field keeps track of errors reported in the checking of uninitialized variables, /// so that we don't report seemingly duplicate errors. uninitialized_error_reported: FxHashSet>, - /// Errors to be reported buffer - errors_buffer: Vec, - /// Set to true if we emit an error during borrowck - tainted_by_errors: bool, /// This field keeps track of all the local variables that are declared mut and are mutated. /// Used for the warning issued by an unused mutable local variable. used_mut: FxHashSet, @@ -609,6 +577,8 @@ struct MirBorrowckCtxt<'cx, 'tcx> { /// Results of Polonius analysis. polonius_output: Option>, + + errors: error::BorrowckErrors<'tcx>, } // Check that: @@ -1032,8 +1002,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if conflict_error || mutability_error { debug!("access_place: logging error place_span=`{:?}` kind=`{:?}`", place_span, kind); - - self.set_tainted_by_errors(); self.access_place_error_reported.insert((place_span.0, place_span.1)); } } @@ -2055,10 +2023,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { | WriteKind::MutableBorrow(BorrowKind::Shared) | WriteKind::MutableBorrow(BorrowKind::Shallow), ) => { - if let (Err(_), true) = ( - self.is_mutable(place.as_ref(), is_local_mutation_allowed), - self.errors_buffer.is_empty(), - ) { + if self.is_mutable(place.as_ref(), is_local_mutation_allowed).is_err() + && !self.has_buffered_errors() + { // rust-lang/rust#46908: In pure NLL mode this code path should be // unreachable, but we use `delay_span_bug` because we can hit this when // dereferencing a non-Copy raw pointer *and* have `-Ztreat-err-as-bug` @@ -2308,14 +2275,102 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { fn is_upvar_field_projection(&self, place_ref: PlaceRef<'tcx>) -> Option { path_utils::is_upvar_field_projection(self.infcx.tcx, &self.upvars, place_ref, self.body()) } +} + +mod error { + use super::*; + + pub struct BorrowckErrors<'tcx> { + /// This field keeps track of move errors that are to be reported for given move indices. + /// + /// There are situations where many errors can be reported for a single move out (see #53807) + /// and we want only the best of those errors. + /// + /// The `report_use_of_moved_or_uninitialized` function checks this map and replaces the + /// diagnostic (if there is one) if the `Place` of the error being reported is a prefix of the + /// `Place` of the previous most diagnostic. This happens instead of buffering the error. Once + /// all move errors have been reported, any diagnostics in this map are added to the buffer + /// to be emitted. + /// + /// `BTreeMap` is used to preserve the order of insertions when iterating. This is necessary + /// when errors in the map are being re-added to the error buffer so that errors with the + /// same primary span come out in a consistent order. + buffered_move_errors: + BTreeMap, (PlaceRef<'tcx>, DiagnosticBuilder<'tcx>)>, + /// Errors to be reported buffer + buffered: Vec, + /// Set to Some if we emit an error during borrowck + tainted_by_errors: Option, + } + + impl BorrowckErrors<'_> { + pub fn new() -> Self { + BorrowckErrors { + buffered_move_errors: BTreeMap::new(), + buffered: Default::default(), + tainted_by_errors: None, + } + } + + pub fn buffer_error(&mut self, t: DiagnosticBuilder<'_>) { + self.tainted_by_errors = Some(ErrorReported {}); + t.buffer(&mut self.buffered); + } - pub fn buffer_error(&mut self, t: DiagnosticBuilder<'_>) { - self.tainted_by_errors = true; - t.buffer(&mut self.errors_buffer); + pub fn set_tainted_by_errors(&mut self) { + self.tainted_by_errors = Some(ErrorReported {}); + } } - pub fn set_tainted_by_errors(&mut self) { - self.tainted_by_errors = true; + impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { + pub fn buffer_error(&mut self, t: DiagnosticBuilder<'_>) { + self.errors.buffer_error(t); + } + + pub fn buffer_move_error( + &mut self, + move_out_indices: Vec, + place_and_err: (PlaceRef<'tcx>, DiagnosticBuilder<'tcx>), + ) -> bool { + if let Some((_, mut diag)) = + self.errors.buffered_move_errors.insert(move_out_indices, place_and_err) + { + // Cancel the old diagnostic so we don't ICE + diag.cancel(); + false + } else { + true + } + } + + pub fn emit_errors(&mut self) -> Option { + // Buffer any move errors that we collected and de-duplicated. + for (_, (_, diag)) in std::mem::take(&mut self.errors.buffered_move_errors) { + // We have already set tainted for this error, so just buffer it. + diag.buffer(&mut self.errors.buffered); + } + + if !self.errors.buffered.is_empty() { + self.errors.buffered.sort_by_key(|diag| diag.sort_span); + + for diag in self.errors.buffered.drain(..) { + self.infcx.tcx.sess.diagnostic().emit_diagnostic(&diag); + } + } + + self.errors.tainted_by_errors + } + + pub fn has_buffered_errors(&self) -> bool { + self.errors.buffered.is_empty() + } + + pub fn has_move_error( + &self, + move_out_indices: &[MoveOutIndex], + ) -> Option<&(PlaceRef<'tcx>, DiagnosticBuilder<'cx>)> { + self.errors.buffered_move_errors.get(move_out_indices) + } } } diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index ac505a6071ded..7fc1fe1130b14 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -1,7 +1,6 @@ //! The entry point of the NLL borrow checker. use rustc_data_structures::vec_map::VecMap; -use rustc_errors::Diagnostic; use rustc_index::vec::IndexVec; use rustc_infer::infer::InferCtxt; use rustc_middle::mir::{create_dump_file, dump_enabled, dump_mir, PassWhere}; @@ -373,7 +372,7 @@ pub(super) fn dump_annotation<'a, 'tcx>( regioncx: &RegionInferenceContext<'tcx>, closure_region_requirements: &Option>, opaque_type_values: &VecMap, Ty<'tcx>>, - errors_buffer: &mut Vec, + errors: &mut crate::error::BorrowckErrors<'tcx>, ) { let tcx = infcx.tcx; let base_def_id = tcx.typeck_root_def_id(body.source.def_id()); @@ -418,8 +417,7 @@ pub(super) fn dump_annotation<'a, 'tcx>( err.note(&format!("Inferred opaque type values:\n{:#?}", opaque_type_values)); } - // FIXME(compiler-errors): Maybe we need to set tainted here - err.buffer(errors_buffer); + errors.buffer_error(err); } fn for_each_region_constraint( diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 7235b8416ad65..df08b541801dd 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -287,8 +287,8 @@ pub fn eval_to_allocation_raw_provider<'tcx>( if let Some(error_reported) = tcx.typeck_opt_const_arg(def).tainted_by_errors { return Err(ErrorHandled::Reported(error_reported)); } - if tcx.mir_borrowck_opt_const_arg(def).tainted_by_errors { - return Err(ErrorHandled::Reported(ErrorReported {})); + if let Some(error_reported) = tcx.mir_borrowck_opt_const_arg(def).tainted_by_errors { + return Err(ErrorHandled::Reported(error_reported)); } } if !tcx.is_mir_available(def.did) { diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 00c066a2851c3..022127de65a67 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -516,8 +516,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { if let Some(error_reported) = self.tcx.typeck_opt_const_arg(def).tainted_by_errors { throw_inval!(AlreadyReported(error_reported)); } - if self.tcx.mir_borrowck_opt_const_arg(def).tainted_by_errors { - throw_inval!(AlreadyReported(rustc_errors::ErrorReported {})); + if let Some(error_reported) = + self.tcx.mir_borrowck_opt_const_arg(def).tainted_by_errors + { + throw_inval!(AlreadyReported(error_reported)); } } } diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index b1aad01118ad4..5c22bfd09a396 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -214,7 +214,7 @@ pub struct BorrowCheckResult<'tcx> { pub concrete_opaque_types: VecMap, Ty<'tcx>>, pub closure_requirements: Option>, pub used_mut_upvars: SmallVec<[Field; 8]>, - pub tainted_by_errors: bool, + pub tainted_by_errors: Option, } /// The result of the `mir_const_qualif` query. From a431174c237755d6d46b7f50c147af755212b8ab Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 7 Feb 2022 22:00:15 -0800 Subject: [PATCH 6/7] add tainted_by_errors to mir::Body --- .../src/transform/check_consts/check.rs | 8 ++++-- .../src/transform/check_consts/qualifs.rs | 4 +-- .../src/transform/promote_consts.rs | 1 + compiler/rustc_middle/src/mir/mod.rs | 6 ++++ compiler/rustc_middle/src/mir/query.rs | 2 +- .../rustc_middle/src/ty/structural_impls.rs | 1 + compiler/rustc_mir_build/src/build/mod.rs | 7 +++-- .../rustc_mir_transform/src/const_prop.rs | 1 + compiler/rustc_mir_transform/src/lib.rs | 28 +++++++++---------- compiler/rustc_mir_transform/src/shim.rs | 2 ++ 10 files changed, 37 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index 6799514a4490d..12a8b8c6d7790 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -120,7 +120,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { fn in_return_place( &mut self, ccx: &'mir ConstCx<'mir, 'tcx>, - error_occured: Option, + tainted_by_errors: Option, ) -> ConstQualifs { // Find the `Return` terminator if one exists. // @@ -134,7 +134,9 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { .map(|(bb, _)| bb); let return_block = match return_block { - None => return qualifs::in_any_value_of_ty(ccx, ccx.body.return_ty(), error_occured), + None => { + return qualifs::in_any_value_of_ty(ccx, ccx.body.return_ty(), tainted_by_errors); + } Some(bb) => bb, }; @@ -166,7 +168,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> { needs_non_const_drop: self.needs_non_const_drop(ccx, RETURN_PLACE, return_loc), has_mut_interior: self.has_mut_interior(ccx, RETURN_PLACE, return_loc), custom_eq, - error_occured, + tainted_by_errors, } } } diff --git a/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs b/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs index 91610b15eb999..c3fa98b000f4a 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs @@ -17,14 +17,14 @@ use super::ConstCx; pub fn in_any_value_of_ty<'tcx>( cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>, - error_occured: Option, + tainted_by_errors: Option, ) -> ConstQualifs { ConstQualifs { has_mut_interior: HasMutInterior::in_any_value_of_ty(cx, ty), needs_drop: NeedsDrop::in_any_value_of_ty(cx, ty), needs_non_const_drop: NeedsNonConstDrop::in_any_value_of_ty(cx, ty), custom_eq: CustomEq::in_any_value_of_ty(cx, ty), - error_occured, + tainted_by_errors, } } diff --git a/compiler/rustc_const_eval/src/transform/promote_consts.rs b/compiler/rustc_const_eval/src/transform/promote_consts.rs index ac282a5ecc82c..92d1f5bceefe4 100644 --- a/compiler/rustc_const_eval/src/transform/promote_consts.rs +++ b/compiler/rustc_const_eval/src/transform/promote_consts.rs @@ -974,6 +974,7 @@ pub fn promote_candidates<'tcx>( vec![], body.span, body.generator_kind(), + body.tainted_by_errors, ); let promoter = Promoter { diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 33fb1e570b1c0..0688d7d2569f5 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -13,6 +13,7 @@ use crate::ty::subst::{Subst, SubstsRef}; use crate::ty::{self, List, Ty, TyCtxt}; use crate::ty::{AdtDef, InstanceDef, Region, ScalarInt, UserTypeAnnotationIndex}; +use rustc_errors::ErrorReported; use rustc_hir::def::{CtorKind, Namespace}; use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX}; use rustc_hir::{self, GeneratorKind}; @@ -284,6 +285,8 @@ pub struct Body<'tcx> { predecessor_cache: PredecessorCache, is_cyclic: GraphIsCyclicCache, + + pub tainted_by_errors: Option, } impl<'tcx> Body<'tcx> { @@ -297,6 +300,7 @@ impl<'tcx> Body<'tcx> { var_debug_info: Vec>, span: Span, generator_kind: Option, + tainted_by_errors: Option, ) -> Self { // We need `arg_count` locals, and one for the return place. assert!( @@ -329,6 +333,7 @@ impl<'tcx> Body<'tcx> { is_polymorphic: false, predecessor_cache: PredecessorCache::new(), is_cyclic: GraphIsCyclicCache::new(), + tainted_by_errors, }; body.is_polymorphic = body.has_param_types_or_consts(); body @@ -356,6 +361,7 @@ impl<'tcx> Body<'tcx> { is_polymorphic: false, predecessor_cache: PredecessorCache::new(), is_cyclic: GraphIsCyclicCache::new(), + tainted_by_errors: None, }; body.is_polymorphic = body.has_param_types_or_consts(); body diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index 5c22bfd09a396..6e2b060e7ddcf 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -228,7 +228,7 @@ pub struct ConstQualifs { pub needs_drop: bool, pub needs_non_const_drop: bool, pub custom_eq: bool, - pub error_occured: Option, + pub tainted_by_errors: Option, } /// After we borrow check a closure, we are left with various diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index 1c5bc7860db2d..39efc006d9d0f 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -253,6 +253,7 @@ TrivialTypeFoldableAndLiftImpls! { crate::ty::UniverseIndex, crate::ty::Variance, ::rustc_span::Span, + ::rustc_errors::ErrorReported, } /////////////////////////////////////////////////////////////////////////// diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 88d994e32fe0c..10807d4327643 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -104,8 +104,8 @@ fn mir_build(tcx: TyCtxt<'_>, def: ty::WithOptConstParam) -> Body<'_ let span_with_body = span_with_body.unwrap_or_else(|| tcx.hir().span(id)); tcx.infer_ctxt().enter(|infcx| { - let body = if let Some(ErrorReported) = typeck_results.tainted_by_errors { - build::construct_error(&infcx, def, id, body_id, body_owner_kind) + let body = if let Some(error_reported) = typeck_results.tainted_by_errors { + build::construct_error(&infcx, def, id, body_id, body_owner_kind, error_reported) } else if body_owner_kind.is_fn_or_closure() { // fetch the fully liberated fn signature (that is, all bound // types/lifetimes replaced) @@ -715,6 +715,7 @@ fn construct_error<'a, 'tcx>( hir_id: hir::HirId, body_id: hir::BodyId, body_owner_kind: hir::BodyOwnerKind, + err: ErrorReported, ) -> Body<'tcx> { let tcx = infcx.tcx; let span = tcx.hir().span(hir_id); @@ -769,6 +770,7 @@ fn construct_error<'a, 'tcx>( vec![], span, generator_kind, + Some(err), ); body.generator.as_mut().map(|gen| gen.yield_ty = Some(ty)); body @@ -857,6 +859,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.var_debug_info, self.fn_span, self.generator_kind, + self.typeck_results.tainted_by_errors, ) } diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 98de64cd97c9c..0d314a109bac7 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -145,6 +145,7 @@ impl<'tcx> MirPass<'tcx> for ConstProp { Default::default(), body.span, body.generator_kind(), + body.tainted_by_errors, ); // FIXME(oli-obk, eddyb) Optimize locals (or even local paths) to hold diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 991d10a8547ff..e7d5bab8fd9d4 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -252,8 +252,11 @@ fn mir_promoted<'tcx>( // Ensure that we compute the `mir_const_qualif` for constants at // this point, before we steal the mir-const result. // Also this means promotion can rely on all const checks having been done. - let _ = tcx.mir_const_qualif_opt_const_arg(def); + let const_qualifs = tcx.mir_const_qualif_opt_const_arg(def); let mut body = tcx.mir_const(def).steal(); + if let Some(error_reported) = const_qualifs.tainted_by_errors { + body.tainted_by_errors = Some(error_reported); + } let mut required_consts = Vec::new(); let mut required_consts_visitor = RequiredConstsVisitor::new(&mut required_consts); @@ -358,13 +361,7 @@ fn mir_drops_elaborated_and_const_checked<'tcx>( return tcx.mir_drops_elaborated_and_const_checked(def); } - // (Mir-)Borrowck uses `mir_promoted`, so we have to force it to - // execute before we can steal. - if let Some(param_did) = def.const_param_did { - tcx.ensure().mir_borrowck_const_arg((def.did, param_did)); - } else { - tcx.ensure().mir_borrowck(def.did); - } + let mir_borrowck = tcx.mir_borrowck_opt_const_arg(def); let is_fn_like = tcx.hir().get_by_def_id(def.did).fn_kind().is_some(); if is_fn_like { @@ -379,6 +376,9 @@ fn mir_drops_elaborated_and_const_checked<'tcx>( let (body, _) = tcx.mir_promoted(def); let mut body = body.steal(); + if let Some(error_reported) = mir_borrowck.tainted_by_errors { + body.tainted_by_errors = Some(error_reported); + } // IMPORTANT pm::run_passes(tcx, &mut body, &[&remove_false_edges::RemoveFalseEdges]); @@ -544,15 +544,13 @@ fn promoted_mir<'tcx>( return tcx.arena.alloc(IndexVec::new()); } - if let Some(param_did) = def.const_param_did { - tcx.ensure().mir_borrowck_const_arg((def.did, param_did)); - } else { - tcx.ensure().mir_borrowck(def.did); - } - let (_, promoted) = tcx.mir_promoted(def); - let mut promoted = promoted.steal(); + let tainted_by_errors = tcx.mir_borrowck_opt_const_arg(def).tainted_by_errors; + let mut promoted = tcx.mir_promoted(def).1.steal(); for body in &mut promoted { + if let Some(error_reported) = tainted_by_errors { + body.tainted_by_errors = Some(error_reported); + } run_post_borrowck_cleanup_passes(tcx, body); } diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index 919171db39e31..a4927c467cfc3 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -235,6 +235,8 @@ fn new_body<'tcx>( vec![], span, None, + // FIXME(compiler-errors): is this correct? + None, ) } From 67ad0ffdf8a979535eac7da6bec8626a87fb2843 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 7 Feb 2022 22:21:23 -0800 Subject: [PATCH 7/7] use body.tainted_by_error to skip loading MIR --- .../src/const_eval/eval_queries.rs | 23 ---------------- .../src/const_eval/machine.rs | 8 ++++++ .../src/interpret/eval_context.rs | 26 +++++++------------ compiler/rustc_middle/src/query/mod.rs | 3 +-- src/test/ui/consts/const-fn-error.rs | 1 - src/test/ui/consts/const-fn-error.stderr | 18 +++---------- 6 files changed, 21 insertions(+), 58 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index df08b541801dd..bfb9c40be57df 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -6,7 +6,6 @@ use crate::interpret::{ ScalarMaybeUninit, StackPopCleanup, }; -use rustc_errors::ErrorReported; use rustc_hir::def::DefKind; use rustc_middle::mir; use rustc_middle::mir::interpret::ErrorHandled; @@ -281,28 +280,6 @@ pub fn eval_to_allocation_raw_provider<'tcx>( let cid = key.value; let def = cid.instance.def.with_opt_param(); - - if let Some(def) = def.as_local() { - if tcx.has_typeck_results(def.did) { - if let Some(error_reported) = tcx.typeck_opt_const_arg(def).tainted_by_errors { - return Err(ErrorHandled::Reported(error_reported)); - } - if let Some(error_reported) = tcx.mir_borrowck_opt_const_arg(def).tainted_by_errors { - return Err(ErrorHandled::Reported(error_reported)); - } - } - if !tcx.is_mir_available(def.did) { - tcx.sess.delay_span_bug( - tcx.def_span(def.did), - &format!("no MIR body is available for {:?}", def.did), - ); - return Err(ErrorHandled::Reported(ErrorReported {})); - } - if let Some(error_reported) = tcx.mir_const_qualif_opt_const_arg(def).error_occured { - return Err(ErrorHandled::Reported(error_reported)); - } - } - let is_static = tcx.is_static(def.did); let mut ecx = InterpCx::new( diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 89717b75f1281..e157b58405212 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -1,3 +1,5 @@ +use rustc_errors::ErrorReported; +use rustc_hir::def::DefKind; use rustc_middle::mir; use rustc_middle::ty::{self, Ty}; use std::borrow::Borrow; @@ -243,6 +245,12 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, ty::InstanceDef::Item(def) => { if ecx.tcx.is_ctfe_mir_available(def.did) { Ok(ecx.tcx.mir_for_ctfe_opt_const_arg(def)) + } else if ecx.tcx.def_kind(def.did) == DefKind::AssocConst { + ecx.tcx.sess.delay_span_bug( + rustc_span::DUMMY_SP, + "This is likely a const item that is missing from its impl", + ); + throw_inval!(AlreadyReported(ErrorReported {})); } else { let path = ecx.tcx.def_path_str(def.did); Err(ConstEvalErrKind::NeedsRfc(format!("calling extern function `{}`", path)) diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 022127de65a67..1b86bcfa8c9ad 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -509,26 +509,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { instance: ty::InstanceDef<'tcx>, promoted: Option, ) -> InterpResult<'tcx, &'tcx mir::Body<'tcx>> { - // do not continue if typeck errors occurred (can only occur in local crate) let def = instance.with_opt_param(); - if let Some(def) = def.as_local() { - if self.tcx.has_typeck_results(def.did) { - if let Some(error_reported) = self.tcx.typeck_opt_const_arg(def).tainted_by_errors { - throw_inval!(AlreadyReported(error_reported)); - } - if let Some(error_reported) = - self.tcx.mir_borrowck_opt_const_arg(def).tainted_by_errors - { - throw_inval!(AlreadyReported(error_reported)); - } - } - } - trace!("load mir(instance={:?}, promoted={:?})", instance, promoted); - if let Some(promoted) = promoted { - return Ok(&self.tcx.promoted_mir_opt_const_arg(def)[promoted]); + let body = if let Some(promoted) = promoted { + &self.tcx.promoted_mir_opt_const_arg(def)[promoted] + } else { + M::load_mir(self, instance)? + }; + // do not continue if typeck errors occurred (can only occur in local crate) + if let Some(err) = body.tainted_by_errors { + throw_inval!(AlreadyReported(err)); } - M::load_mir(self, instance) + Ok(body) } /// Call this on things you got out of the MIR (so it is as generic as the current diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 98d54edf92b80..fc2750d230395 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -836,14 +836,13 @@ rustc_queries! { /// additional requirements that the closure's creator must verify. query mir_borrowck(key: LocalDefId) -> &'tcx mir::BorrowCheckResult<'tcx> { desc { |tcx| "borrow-checking `{}`", tcx.def_path_str(key.to_def_id()) } - cache_on_disk_if { true } + cache_on_disk_if(tcx) { tcx.is_typeck_child(key.to_def_id()) } } query mir_borrowck_const_arg(key: (LocalDefId, DefId)) -> &'tcx mir::BorrowCheckResult<'tcx> { desc { |tcx| "borrow-checking the const argument`{}`", tcx.def_path_str(key.0.to_def_id()) } - cache_on_disk_if { true } } /// Gets a complete map from all types to their inherent impls. diff --git a/src/test/ui/consts/const-fn-error.rs b/src/test/ui/consts/const-fn-error.rs index 948c162e8946d..065944ea7eaad 100644 --- a/src/test/ui/consts/const-fn-error.rs +++ b/src/test/ui/consts/const-fn-error.rs @@ -6,7 +6,6 @@ const fn f(x: usize) -> usize { //~^ ERROR mutable references //~| ERROR calls in constant functions //~| ERROR calls in constant functions - //~| ERROR E0080 //~| ERROR `for` is not allowed in a `const fn` sum += i; } diff --git a/src/test/ui/consts/const-fn-error.stderr b/src/test/ui/consts/const-fn-error.stderr index df24585e5551a..e4b62f20a3318 100644 --- a/src/test/ui/consts/const-fn-error.stderr +++ b/src/test/ui/consts/const-fn-error.stderr @@ -5,7 +5,7 @@ LL | / for i in 0..x { LL | | LL | | LL | | -... | +LL | | LL | | sum += i; LL | | } | |_____^ @@ -34,19 +34,7 @@ error[E0015]: calls in constant functions are limited to constant functions, tup LL | for i in 0..x { | ^^^^ -error[E0080]: evaluation of constant value failed - --> $DIR/const-fn-error.rs:5:14 - | -LL | for i in 0..x { - | ^^^^ - | | - | calling non-const function ` as IntoIterator>::into_iter` - | inside `f` at $DIR/const-fn-error.rs:5:14 -... -LL | let a : [i32; f(X)]; - | ---- inside `main::{constant#0}` at $DIR/const-fn-error.rs:18:19 - -error: aborting due to 5 previous errors +error: aborting due to 4 previous errors -Some errors have detailed explanations: E0015, E0080, E0658. +Some errors have detailed explanations: E0015, E0658. For more information about an error, try `rustc --explain E0015`.