From 43850e0bee0a23147b706206295c0e160ac3544f Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 6 Aug 2018 21:06:00 +0200 Subject: [PATCH] Special case error message for thread-local statics. --- .../borrow_check/error_reporting.rs | 48 +++++++++++++++++-- src/librustc_mir/borrow_check/mod.rs | 9 +--- src/librustc_mir/diagnostics.rs | 23 +++++++++ src/librustc_mir/util/borrowck_errors.rs | 16 +++++++ ...-thread-local-static-borrow-outlives-fn.rs | 2 +- src/test/ui/issue-17954.ast.nll.stderr | 14 ------ src/test/ui/issue-17954.mir.stderr | 14 ------ src/test/ui/issue-17954.nll.stderr | 12 +++++ src/test/ui/issue-17954.rs | 15 ++---- ...ue-17954.ast.stderr => issue-17954.stderr} | 2 +- 10 files changed, 104 insertions(+), 51 deletions(-) delete mode 100644 src/test/ui/issue-17954.ast.nll.stderr delete mode 100644 src/test/ui/issue-17954.mir.stderr create mode 100644 src/test/ui/issue-17954.nll.stderr rename src/test/ui/{issue-17954.ast.stderr => issue-17954.stderr} (92%) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index aabed6686858f..b55a7ff068c47 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -412,6 +412,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { .insert((root_place.clone(), borrow_span)); let mut err = match &self.describe_place(&borrow.borrowed_place) { + Some(_) if self.is_place_thread_local(root_place) => { + self.report_thread_local_value_does_not_live_long_enough( + drop_span, + borrow_span, + ) + } Some(name) => self.report_local_value_does_not_live_long_enough( context, name, @@ -455,9 +461,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { context, name, scope_tree, borrow, drop_span, borrow_span ); - let tcx = self.tcx; - let mut err = - tcx.path_does_not_live_long_enough(borrow_span, &format!("`{}`", name), Origin::Mir); + let mut err = self.tcx.path_does_not_live_long_enough( + borrow_span, &format!("`{}`", name), Origin::Mir); + err.span_label(borrow_span, "borrowed value does not live long enough"); err.span_label( drop_span, @@ -468,6 +474,27 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err } + fn report_thread_local_value_does_not_live_long_enough( + &mut self, + drop_span: Span, + borrow_span: Span, + ) -> DiagnosticBuilder<'cx> { + debug!( + "report_thread_local_value_does_not_live_long_enough(\ + {:?}, {:?}\ + )", + drop_span, borrow_span + ); + + let mut err = self.tcx.thread_local_value_does_not_live_long_enough( + borrow_span, Origin::Mir); + + err.span_label(borrow_span, + "thread-local variables cannot be borrowed beyond the end of the function"); + err.span_label(drop_span, "end of enclosing function is here"); + err + } + fn report_temporary_value_does_not_live_long_enough( &mut self, context: Context, @@ -856,6 +883,21 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { }, } } + + /// Check if a place is a thread-local static. + pub fn is_place_thread_local(&self, place: &Place<'tcx>) -> bool { + if let Place::Static(statik) = place { + let attrs = self.tcx.get_attrs(statik.def_id); + let is_thread_local = attrs.iter().any(|attr| attr.check_name("thread_local")); + + debug!("is_place_thread_local: attrs={:?} is_thread_local={:?}", + attrs, is_thread_local); + is_thread_local + } else { + debug!("is_place_thread_local: no"); + false + } + } } // The span(s) associated to a use of a place. diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 320d3a4720321..967a8d9033096 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1487,15 +1487,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // FIXME: allow thread-locals to borrow other thread locals? let (might_be_alive, will_be_dropped) = match root_place { Place::Promoted(_) => (true, false), - Place::Static(statik) => { + Place::Static(_) => { // Thread-locals might be dropped after the function exits, but // "true" statics will never be. - let is_thread_local = self - .tcx - .get_attrs(statik.def_id) - .iter() - .any(|attr| attr.check_name("thread_local")); - + let is_thread_local = self.is_place_thread_local(&root_place); (true, is_thread_local) } Place::Local(_) => { diff --git a/src/librustc_mir/diagnostics.rs b/src/librustc_mir/diagnostics.rs index 3c751d52b0664..e72ef798612ae 100644 --- a/src/librustc_mir/diagnostics.rs +++ b/src/librustc_mir/diagnostics.rs @@ -2251,6 +2251,29 @@ unsafe { b.resume() }; ``` "##, +E0712: r##" +This error occurs because a borrow of a thread-local variable was made inside a +function which outlived the lifetime of the function. + +Example of erroneous code: + +```compile_fail,E0712 +#![feature(nll)] +#![feature(thread_local)] + +#[thread_local] +static FOO: u8 = 3; + +fn main() { + let a = &FOO; // error: thread-local variable borrowed past end of function + + std::thread::spawn(move || { + println!("{}", a); + }); +} +``` +"##, + } register_diagnostics! { diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index 0a53361df6e95..7be6241b3f91e 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -675,6 +675,22 @@ pub trait BorrowckErrors<'cx>: Sized + Copy { self.cancel_if_wrong_origin(err, o) } + + fn thread_local_value_does_not_live_long_enough( + self, + span: Span, + o: Origin, + ) -> DiagnosticBuilder<'cx> { + let err = struct_span_err!( + self, + span, + E0712, + "thread-local variable borrowed past end of function{OGN}", + OGN = o + ); + + self.cancel_if_wrong_origin(err, o) + } } impl<'cx, 'gcx, 'tcx> BorrowckErrors<'cx> for TyCtxt<'cx, 'gcx, 'tcx> { diff --git a/src/test/compile-fail/borrowck/borrowck-thread-local-static-borrow-outlives-fn.rs b/src/test/compile-fail/borrowck/borrowck-thread-local-static-borrow-outlives-fn.rs index f2e6d51d064d1..7aa02558446e3 100644 --- a/src/test/compile-fail/borrowck/borrowck-thread-local-static-borrow-outlives-fn.rs +++ b/src/test/compile-fail/borrowck/borrowck-thread-local-static-borrow-outlives-fn.rs @@ -19,5 +19,5 @@ static FOO: u8 = 3; fn assert_static(_t: &'static u8) {} fn main() { assert_static(&FOO); //[ast]~ ERROR [E0597] - //[mir]~^ ERROR [E0597] + //[mir]~^ ERROR [E0712] } diff --git a/src/test/ui/issue-17954.ast.nll.stderr b/src/test/ui/issue-17954.ast.nll.stderr deleted file mode 100644 index 5cab7ca9e7a3f..0000000000000 --- a/src/test/ui/issue-17954.ast.nll.stderr +++ /dev/null @@ -1,14 +0,0 @@ -error[E0597]: `FOO` does not live long enough - --> $DIR/issue-17954.rs:20:13 - | -LL | let a = &FOO; - | ^^^^ borrowed value does not live long enough -... -LL | } - | - `FOO` dropped here while still borrowed - | - = note: borrowed value must be valid for the static lifetime... - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/issue-17954.mir.stderr b/src/test/ui/issue-17954.mir.stderr deleted file mode 100644 index 5cab7ca9e7a3f..0000000000000 --- a/src/test/ui/issue-17954.mir.stderr +++ /dev/null @@ -1,14 +0,0 @@ -error[E0597]: `FOO` does not live long enough - --> $DIR/issue-17954.rs:20:13 - | -LL | let a = &FOO; - | ^^^^ borrowed value does not live long enough -... -LL | } - | - `FOO` dropped here while still borrowed - | - = note: borrowed value must be valid for the static lifetime... - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/issue-17954.nll.stderr b/src/test/ui/issue-17954.nll.stderr new file mode 100644 index 0000000000000..67a5e3eaec787 --- /dev/null +++ b/src/test/ui/issue-17954.nll.stderr @@ -0,0 +1,12 @@ +error[E0712]: thread-local variable borrowed past end of function + --> $DIR/issue-17954.rs:17:13 + | +LL | let a = &FOO; + | ^^^^ thread-local variables cannot be borrowed beyond the end of the function +... +LL | } + | - end of enclosing function is here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0712`. diff --git a/src/test/ui/issue-17954.rs b/src/test/ui/issue-17954.rs index b5e550e5be1c9..ce554a7254812 100644 --- a/src/test/ui/issue-17954.rs +++ b/src/test/ui/issue-17954.rs @@ -8,9 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// revisions: ast mir -//[mir]compile-flags: -Z borrowck=mir - #![feature(thread_local)] #[thread_local] @@ -18,16 +15,12 @@ static FOO: u8 = 3; fn main() { let a = &FOO; - //[mir]~^ ERROR `FOO` does not live long enough [E0597] - //[mir]~| does not live long enough - //[mir]~| NOTE borrowed value must be valid for the static lifetime - //[ast]~^^^^ ERROR borrowed value does not live long enough - //[ast]~| does not live long enough - //[ast]~| NOTE borrowed value must be valid for the static lifetime + //~^ ERROR borrowed value does not live long enough + //~| does not live long enough + //~| NOTE borrowed value must be valid for the static lifetime std::thread::spawn(move || { println!("{}", a); }); } -//[mir]~^ `FOO` dropped here while still borrowed -//[ast]~^^ temporary value only lives until here +//~^ NOTE temporary value only lives until here diff --git a/src/test/ui/issue-17954.ast.stderr b/src/test/ui/issue-17954.stderr similarity index 92% rename from src/test/ui/issue-17954.ast.stderr rename to src/test/ui/issue-17954.stderr index 677d2cbfffc47..76858a9b097b2 100644 --- a/src/test/ui/issue-17954.ast.stderr +++ b/src/test/ui/issue-17954.stderr @@ -1,5 +1,5 @@ error[E0597]: borrowed value does not live long enough - --> $DIR/issue-17954.rs:20:14 + --> $DIR/issue-17954.rs:17:14 | LL | let a = &FOO; | ^^^ temporary value does not live long enough