From 12014d29b89557836a22fa46d74101371d81daf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Tue, 12 Jan 2021 16:03:29 +0200 Subject: [PATCH 01/21] Add Box::downcast() for dyn Any + Send + Sync --- library/alloc/src/boxed.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 33b812ec59ff9..b4dd5d92c6a13 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -1364,6 +1364,39 @@ impl Box { } } +impl Box { + #[inline] + #[stable(feature = "box_send_sync_any_downcast", since = "1.51.0")] + /// Attempt to downcast the box to a concrete type. + /// + /// # Examples + /// + /// ``` + /// use std::any::Any; + /// + /// fn print_if_string(value: Box) { + /// if let Ok(string) = value.downcast::() { + /// println!("String ({}): {}", string.len(), string); + /// } + /// } + /// + /// let my_string = "Hello World".to_string(); + /// print_if_string(Box::new(my_string)); + /// print_if_string(Box::new(0i8)); + /// ``` + pub fn downcast(self) -> Result, Self> { + if self.is::() { + unsafe { + let (raw, alloc): (*mut (dyn Any + Send + Sync), _) = + Box::into_raw_with_allocator(self); + Ok(Box::from_raw_in(raw as *mut T, alloc)) + } + } else { + Err(self) + } + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl fmt::Display for Box { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { From 8db27821c9295e936f53306774cc3318ad43e567 Mon Sep 17 00:00:00 2001 From: C Date: Wed, 16 Dec 2020 01:34:11 +0000 Subject: [PATCH 02/21] refactor: moved new out of ZipImpl --- library/core/src/iter/adapters/zip.rs | 50 +++++++++++++++++++-------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 98b8dca961407..1a26d3060e46f 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -19,8 +19,9 @@ pub struct Zip { } impl Zip { pub(in crate::iter) fn new(a: A, b: B) -> Zip { - ZipImpl::new(a, b) + ZipNew::new(a, b) } + fn super_nth(&mut self, mut n: usize) -> Option<(A::Item, B::Item)> { while let Some(x) = Iterator::next(self) { if n == 0 { @@ -32,6 +33,39 @@ impl Zip { } } +#[doc(hidden)] +trait ZipNew { + fn new(a: A, b: B) -> Self; +} + +#[doc(hidden)] +impl ZipNew for Zip +where + A: Iterator, + B: Iterator, +{ + default fn new(a: A, b: B) -> Self { + Zip { + a, + b, + index: 0, // unused + len: 0, // unused + } + } +} + +#[doc(hidden)] +impl ZipNew for Zip +where + A: TrustedRandomAccess + Iterator, + B: TrustedRandomAccess + Iterator, +{ + fn new(a: A, b: B) -> Self { + let len = cmp::min(a.size(), b.size()); + Zip { a, b, index: 0, len } + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl Iterator for Zip where @@ -82,7 +116,6 @@ where #[doc(hidden)] trait ZipImpl { type Item; - fn new(a: A, b: B) -> Self; fn next(&mut self) -> Option; fn size_hint(&self) -> (usize, Option); fn nth(&mut self, n: usize) -> Option; @@ -104,14 +137,6 @@ where B: Iterator, { type Item = (A::Item, B::Item); - default fn new(a: A, b: B) -> Self { - Zip { - a, - b, - index: 0, // unused - len: 0, // unused - } - } #[inline] default fn next(&mut self) -> Option<(A::Item, B::Item)> { @@ -183,11 +208,6 @@ where A: TrustedRandomAccess + Iterator, B: TrustedRandomAccess + Iterator, { - fn new(a: A, b: B) -> Self { - let len = cmp::min(a.size(), b.size()); - Zip { a, b, index: 0, len } - } - #[inline] fn next(&mut self) -> Option<(A::Item, B::Item)> { if self.index < self.len { From bfb0e4a64b4142ed4055613d19af148bcb91395d Mon Sep 17 00:00:00 2001 From: C Date: Wed, 16 Dec 2020 01:38:37 +0000 Subject: [PATCH 03/21] refactor: moved next_back out of ZipImpl --- library/core/src/iter/adapters/zip.rs | 138 ++++++++++++-------------- 1 file changed, 64 insertions(+), 74 deletions(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 1a26d3060e46f..c9578346be5c3 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -105,10 +105,73 @@ impl DoubleEndedIterator for Zip where A: DoubleEndedIterator + ExactSizeIterator, B: DoubleEndedIterator + ExactSizeIterator, +{ + #[inline] + default fn next_back(&mut self) -> Option<(A::Item, B::Item)> { + let a_sz = self.a.len(); + let b_sz = self.b.len(); + if a_sz != b_sz { + // Adjust a, b to equal length + if a_sz > b_sz { + for _ in 0..a_sz - b_sz { + self.a.next_back(); + } + } else { + for _ in 0..b_sz - a_sz { + self.b.next_back(); + } + } + } + match (self.a.next_back(), self.b.next_back()) { + (Some(x), Some(y)) => Some((x, y)), + (None, None) => None, + _ => unreachable!(), + } + } +} + +#[stable(feature = "rust1", since = "1.0.0")] +impl DoubleEndedIterator for Zip +where + A: TrustedRandomAccess + DoubleEndedIterator + ExactSizeIterator, + B: TrustedRandomAccess + DoubleEndedIterator + ExactSizeIterator, { #[inline] fn next_back(&mut self) -> Option<(A::Item, B::Item)> { - ZipImpl::next_back(self) + let a_side_effect = A::may_have_side_effect(); + let b_side_effect = B::may_have_side_effect(); + if a_side_effect || b_side_effect { + let sz_a = self.a.size(); + let sz_b = self.b.size(); + // Adjust a, b to equal length, make sure that only the first call + // of `next_back` does this, otherwise we will break the restriction + // on calls to `self.next_back()` after calling `get_unchecked()`. + if sz_a != sz_b { + let sz_a = self.a.size(); + if a_side_effect && sz_a > self.len { + for _ in 0..sz_a - cmp::max(self.len, self.index) { + self.a.next_back(); + } + } + let sz_b = self.b.size(); + if b_side_effect && sz_b > self.len { + for _ in 0..sz_b - self.len { + self.b.next_back(); + } + } + } + } + if self.index < self.len { + self.len -= 1; + let i = self.len; + // SAFETY: `i` is smaller than the previous value of `self.len`, + // which is also smaller than or equal to `self.a.len()` and `self.b.len()` + unsafe { + Some((self.a.__iterator_get_unchecked(i), self.b.__iterator_get_unchecked(i))) + } + } else { + None + } } } @@ -119,10 +182,6 @@ trait ZipImpl { fn next(&mut self) -> Option; fn size_hint(&self) -> (usize, Option); fn nth(&mut self, n: usize) -> Option; - fn next_back(&mut self) -> Option - where - A: DoubleEndedIterator + ExactSizeIterator, - B: DoubleEndedIterator + ExactSizeIterator; // This has the same safety requirements as `Iterator::__iterator_get_unchecked` unsafe fn get_unchecked(&mut self, idx: usize) -> ::Item where @@ -150,33 +209,6 @@ where self.super_nth(n) } - #[inline] - default fn next_back(&mut self) -> Option<(A::Item, B::Item)> - where - A: DoubleEndedIterator + ExactSizeIterator, - B: DoubleEndedIterator + ExactSizeIterator, - { - let a_sz = self.a.len(); - let b_sz = self.b.len(); - if a_sz != b_sz { - // Adjust a, b to equal length - if a_sz > b_sz { - for _ in 0..a_sz - b_sz { - self.a.next_back(); - } - } else { - for _ in 0..b_sz - a_sz { - self.b.next_back(); - } - } - } - match (self.a.next_back(), self.b.next_back()) { - (Some(x), Some(y)) => Some((x, y)), - (None, None) => None, - _ => unreachable!(), - } - } - #[inline] default fn size_hint(&self) -> (usize, Option) { let (a_lower, a_upper) = self.a.size_hint(); @@ -262,48 +294,6 @@ where self.super_nth(n - delta) } - #[inline] - fn next_back(&mut self) -> Option<(A::Item, B::Item)> - where - A: DoubleEndedIterator + ExactSizeIterator, - B: DoubleEndedIterator + ExactSizeIterator, - { - let a_side_effect = A::may_have_side_effect(); - let b_side_effect = B::may_have_side_effect(); - if a_side_effect || b_side_effect { - let sz_a = self.a.size(); - let sz_b = self.b.size(); - // Adjust a, b to equal length, make sure that only the first call - // of `next_back` does this, otherwise we will break the restriction - // on calls to `self.next_back()` after calling `get_unchecked()`. - if sz_a != sz_b { - let sz_a = self.a.size(); - if a_side_effect && sz_a > self.len { - for _ in 0..sz_a - cmp::max(self.len, self.index) { - self.a.next_back(); - } - } - let sz_b = self.b.size(); - if b_side_effect && sz_b > self.len { - for _ in 0..sz_b - self.len { - self.b.next_back(); - } - } - } - } - if self.index < self.len { - self.len -= 1; - let i = self.len; - // SAFETY: `i` is smaller than the previous value of `self.len`, - // which is also smaller than or equal to `self.a.len()` and `self.b.len()` - unsafe { - Some((self.a.__iterator_get_unchecked(i), self.b.__iterator_get_unchecked(i))) - } - } else { - None - } - } - #[inline] unsafe fn get_unchecked(&mut self, idx: usize) -> ::Item { let idx = self.index + idx; From 059733959b6c33e7234f944611850b61e009cf48 Mon Sep 17 00:00:00 2001 From: C Date: Wed, 16 Dec 2020 02:03:20 +0000 Subject: [PATCH 04/21] refactor: removed ZipImpl --- library/core/src/iter/adapters/zip.rs | 67 +++++---------------------- 1 file changed, 12 insertions(+), 55 deletions(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index c9578346be5c3..1d9560bd12d04 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -17,7 +17,12 @@ pub struct Zip { index: usize, len: usize, } -impl Zip { + +impl Zip +where + A: Iterator, + B: Iterator, +{ pub(in crate::iter) fn new(a: A, b: B) -> Zip { ZipNew::new(a, b) } @@ -66,40 +71,6 @@ where } } -#[stable(feature = "rust1", since = "1.0.0")] -impl Iterator for Zip -where - A: Iterator, - B: Iterator, -{ - type Item = (A::Item, B::Item); - - #[inline] - fn next(&mut self) -> Option { - ZipImpl::next(self) - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - ZipImpl::size_hint(self) - } - - #[inline] - fn nth(&mut self, n: usize) -> Option { - ZipImpl::nth(self, n) - } - - #[inline] - unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item - where - Self: TrustedRandomAccess, - { - // SAFETY: `ZipImpl::__iterator_get_unchecked` has same safety - // requirements as `Iterator::__iterator_get_unchecked`. - unsafe { ZipImpl::get_unchecked(self, idx) } - } -} - #[stable(feature = "rust1", since = "1.0.0")] impl DoubleEndedIterator for Zip where @@ -175,22 +146,8 @@ where } } -// Zip specialization trait -#[doc(hidden)] -trait ZipImpl { - type Item; - fn next(&mut self) -> Option; - fn size_hint(&self) -> (usize, Option); - fn nth(&mut self, n: usize) -> Option; - // This has the same safety requirements as `Iterator::__iterator_get_unchecked` - unsafe fn get_unchecked(&mut self, idx: usize) -> ::Item - where - Self: Iterator + TrustedRandomAccess; -} - -// General Zip impl -#[doc(hidden)] -impl ZipImpl for Zip +#[stable(feature = "rust1", since = "1.0.0")] +impl Iterator for Zip where A: Iterator, B: Iterator, @@ -226,7 +183,7 @@ where (lower, upper) } - default unsafe fn get_unchecked(&mut self, _idx: usize) -> ::Item + default unsafe fn __iterator_get_unchecked(&mut self, _idx: usize) -> ::Item where Self: TrustedRandomAccess, { @@ -234,8 +191,8 @@ where } } -#[doc(hidden)] -impl ZipImpl for Zip +#[stable(feature = "rust1", since = "1.0.0")] +impl Iterator for Zip where A: TrustedRandomAccess + Iterator, B: TrustedRandomAccess + Iterator, @@ -295,7 +252,7 @@ where } #[inline] - unsafe fn get_unchecked(&mut self, idx: usize) -> ::Item { + unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> ::Item { let idx = self.index + idx; // SAFETY: the caller must uphold the contract for // `Iterator::__iterator_get_unchecked`. From f2bb202185e871ef8b741a7bf6192656e3b12895 Mon Sep 17 00:00:00 2001 From: C Date: Wed, 16 Dec 2020 02:07:13 +0000 Subject: [PATCH 05/21] refactor: updated zip nth() to iterator nth() When the iterator nth() method was updated it was not in zip. Zip needs to implement nth() for the trusted length specialised implementation. --- library/core/src/iter/adapters/zip.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 1d9560bd12d04..7fbbf9481a5a8 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -27,14 +27,9 @@ where ZipNew::new(a, b) } - fn super_nth(&mut self, mut n: usize) -> Option<(A::Item, B::Item)> { - while let Some(x) = Iterator::next(self) { - if n == 0 { - return Some(x); - } - n -= 1; - } - None + fn super_nth(&mut self, n: usize) -> Option<(A::Item, B::Item)> { + self.advance_by(n).ok()?; + self.next() } } From a398994cb214c9ba022cb8e6cd97c3fee4bca6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 26 Jan 2021 16:53:56 -0800 Subject: [PATCH 06/21] Account for existing `_` field pattern when suggesting `..` Follow up to #80017. --- compiler/rustc_typeck/src/check/pat.rs | 22 +++++++++++++------ .../ui/pattern/pat-tuple-underfield.stderr | 4 ++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_typeck/src/check/pat.rs b/compiler/rustc_typeck/src/check/pat.rs index 79234f076acd1..12256058b8715 100644 --- a/compiler/rustc_typeck/src/check/pat.rs +++ b/compiler/rustc_typeck/src/check/pat.rs @@ -1041,12 +1041,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { vec![(left, "(".to_string()), (right.shrink_to_hi(), ")".to_string())], Applicability::MachineApplicable, ); - } else if fields.len() > subpats.len() { - let after_fields_span = if pat_span == DUMMY_SP { - pat_span - } else { - pat_span.with_hi(pat_span.hi() - BytePos(1)).shrink_to_hi() - }; + } else if fields.len() > subpats.len() && pat_span != DUMMY_SP { + let after_fields_span = pat_span.with_hi(pat_span.hi() - BytePos(1)).shrink_to_hi(); let all_fields_span = match subpats { [] => after_fields_span, [field] => field.span, @@ -1055,7 +1051,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Check if all the fields in the pattern are wildcards. let all_wildcards = subpats.iter().all(|pat| matches!(pat.kind, PatKind::Wild)); + let first_tail_wildcard = + subpats.iter().enumerate().fold(None, |acc, (pos, pat)| match (acc, &pat.kind) { + (None, PatKind::Wild) => Some(pos), + (Some(_), PatKind::Wild) => acc, + _ => None, + }); + let tail_span = match first_tail_wildcard { + None => after_fields_span, + Some(0) => subpats[0].span.to(after_fields_span), + Some(pos) => subpats[pos - 1].span.shrink_to_hi().to(after_fields_span), + }; + // FIXME: heuristic-based suggestion to check current types for where to add `_`. let mut wildcard_sugg = vec!["_"; fields.len() - subpats.len()].join(", "); if !subpats.is_empty() { wildcard_sugg = String::from(", ") + &wildcard_sugg; @@ -1080,7 +1088,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } else { err.span_suggestion_verbose( - after_fields_span, + tail_span, "use `..` to ignore the rest of the fields", String::from(", .."), Applicability::MaybeIncorrect, diff --git a/src/test/ui/pattern/pat-tuple-underfield.stderr b/src/test/ui/pattern/pat-tuple-underfield.stderr index 76323d9a7bf56..70c21dbafe9fc 100644 --- a/src/test/ui/pattern/pat-tuple-underfield.stderr +++ b/src/test/ui/pattern/pat-tuple-underfield.stderr @@ -122,8 +122,8 @@ LL | Point4( a , _ , _, _) => {} | ^^^^^^ help: use `..` to ignore the rest of the fields | -LL | Point4( a , _ , ..) => {} - | ^^^^ +LL | Point4( a, ..) => {} + | ^^^^ error: aborting due to 8 previous errors From 3f679fef23cd006c336e2a4f7c6707e119d22206 Mon Sep 17 00:00:00 2001 From: Ramon de C Valle Date: Fri, 20 Nov 2020 20:11:54 -0800 Subject: [PATCH 07/21] Fix rustc sysroot in systems using CAS Change filesearch::get_or_default_sysroot() to check if sysroot is found using env::args().next() if rustc in argv[0] is a symlink; otherwise, or if it is not found, use env::current_exe() to imply sysroot. This makes the rustc binary able to locate Rust libraries in systems using content-addressable storage (CAS). --- compiler/rustc_session/src/filesearch.rs | 52 ++++++++++++++++++++---- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_session/src/filesearch.rs b/compiler/rustc_session/src/filesearch.rs index 55ee4e52082e5..13080c2bcd0de 100644 --- a/compiler/rustc_session/src/filesearch.rs +++ b/compiler/rustc_session/src/filesearch.rs @@ -113,6 +113,8 @@ pub fn make_target_lib_path(sysroot: &Path, target_triple: &str) -> PathBuf { sysroot.join(&relative_target_lib_path(sysroot, target_triple)) } +// This function checks if sysroot is found using env::args().next(), and if it +// is not found, uses env::current_exe() to imply sysroot. pub fn get_or_default_sysroot() -> PathBuf { // Follow symlinks. If the resolved path is relative, make it absolute. fn canonicalize(path: PathBuf) -> PathBuf { @@ -123,15 +125,51 @@ pub fn get_or_default_sysroot() -> PathBuf { fix_windows_verbatim_for_gcc(&path) } - match env::current_exe() { - Ok(exe) => { - let mut p = canonicalize(exe); - p.pop(); - p.pop(); - p + // Use env::current_exe() to get the path of the executable following + // symlinks/canonicalizing components. + fn from_current_exe() -> PathBuf { + match env::current_exe() { + Ok(exe) => { + let mut p = canonicalize(exe); + p.pop(); + p.pop(); + p + } + Err(e) => panic!("failed to get current_exe: {}", e), + } + } + + // Use env::args().next() to get the path of the executable without + // following symlinks/canonicalizing any component. This makes the rustc + // binary able to locate Rust libraries in systems using content-addressable + // storage (CAS). + fn from_env_args_next() -> Option { + match env::args_os().next() { + Some(first_arg) => { + let mut p = PathBuf::from(first_arg); + + // Check if sysroot is found using env::args().next() only if the rustc in argv[0] + // is a symlink (see #79253). We might want to change/remove it to conform with + // https://www.gnu.org/prep/standards/standards.html#Finding-Program-Files in the + // future. + if fs::read_link(&p).is_err() { + // Path is not a symbolic link or does not exist. + return None; + } + + p.pop(); + p.pop(); + let mut libdir = PathBuf::from(&p); + libdir.push(find_libdir(&p).as_ref()); + if libdir.exists() { Some(p) } else { None } + } + None => None, } - Err(e) => panic!("failed to get current_exe: {}", e), } + + // Check if sysroot is found using env::args().next(), and if is not found, + // use env::current_exe() to imply sysroot. + from_env_args_next().unwrap_or(from_current_exe()) } // The name of the directory rustc expects libraries to be located. From 5d739180cde7f7350b7a90e8a7542bd9c4cd6783 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 28 Jan 2021 09:47:59 -0500 Subject: [PATCH 08/21] Clone entire `TokenCursor` when collecting tokens Reverts PR #80830 Fixes taiki-e/pin-project#312 We can have an arbitrary number of `None`-delimited group frames pushed on the stack due to proc-macro invocations, which can legally be exited. Attempting to account for this would add a lot of complexity for a tiny performance gain, so let's just use the original strategy. --- compiler/rustc_parse/src/parser/mod.rs | 10 +------- .../auxiliary/nonterminal-recollect-attr.rs | 23 +++++++++++++++++++ .../proc-macro/nonterminal-recollect-attr.rs | 17 ++++++++++++++ 3 files changed, 41 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/proc-macro/auxiliary/nonterminal-recollect-attr.rs create mode 100644 src/test/ui/proc-macro/nonterminal-recollect-attr.rs diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index c575c8219641c..e2af63d1744ec 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -1254,15 +1254,7 @@ impl<'a> Parser<'a> { f: impl FnOnce(&mut Self) -> PResult<'a, (R, TrailingToken)>, ) -> PResult<'a, R> { let start_token = (self.token.clone(), self.token_spacing); - let cursor_snapshot = TokenCursor { - frame: self.token_cursor.frame.clone(), - // We only ever capture tokens within our current frame, - // so we can just use an empty frame stack - stack: vec![], - desugar_doc_comments: self.token_cursor.desugar_doc_comments, - num_next_calls: self.token_cursor.num_next_calls, - append_unglued_token: self.token_cursor.append_unglued_token.clone(), - }; + let cursor_snapshot = self.token_cursor.clone(); let (mut ret, trailing_token) = f(self)?; diff --git a/src/test/ui/proc-macro/auxiliary/nonterminal-recollect-attr.rs b/src/test/ui/proc-macro/auxiliary/nonterminal-recollect-attr.rs new file mode 100644 index 0000000000000..a6903283aa108 --- /dev/null +++ b/src/test/ui/proc-macro/auxiliary/nonterminal-recollect-attr.rs @@ -0,0 +1,23 @@ +// force-host +// no-prefer-dynamic + +#![crate_type = "proc-macro"] +#![feature(proc_macro_quote)] + +extern crate proc_macro; +use proc_macro::{TokenStream, quote}; + +#[proc_macro_attribute] +pub fn first_attr(_: TokenStream, input: TokenStream) -> TokenStream { + let recollected: TokenStream = input.into_iter().collect(); + quote! { + #[second_attr] + $recollected + } +} + +#[proc_macro_attribute] +pub fn second_attr(_: TokenStream, input: TokenStream) -> TokenStream { + let _recollected: TokenStream = input.into_iter().collect(); + TokenStream::new() +} diff --git a/src/test/ui/proc-macro/nonterminal-recollect-attr.rs b/src/test/ui/proc-macro/nonterminal-recollect-attr.rs new file mode 100644 index 0000000000000..5d4649b78c270 --- /dev/null +++ b/src/test/ui/proc-macro/nonterminal-recollect-attr.rs @@ -0,0 +1,17 @@ +// check-pass +// aux-build:nonterminal-recollect-attr.rs + +extern crate nonterminal_recollect_attr; +use nonterminal_recollect_attr::*; + +macro_rules! my_macro { + ($v:ident) => { + #[first_attr] + $v struct Foo { + field: u8 + } + } +} + +my_macro!(pub); +fn main() {} From ada714d9ce80748d6820565135397e46662ace9b Mon Sep 17 00:00:00 2001 From: Kogia-sima Date: Fri, 29 Jan 2021 02:27:20 +0900 Subject: [PATCH 09/21] Optimize udiv_1e19() function --- library/core/src/fmt/num.rs | 55 ++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/library/core/src/fmt/num.rs b/library/core/src/fmt/num.rs index 7a98210995ec7..cdd731fdd4d4e 100644 --- a/library/core/src/fmt/num.rs +++ b/library/core/src/fmt/num.rs @@ -643,25 +643,42 @@ fn fmt_u128(n: u128, is_nonnegative: bool, f: &mut fmt::Formatter<'_>) -> fmt::R } /// Partition of `n` into n > 1e19 and rem <= 1e19 +/// +/// Integer division algorithm is based on the following paper: +/// +/// T. Granlund and P. Montgomery, “Division by Invariant Integers Using Multiplication” +/// in Proc. of the SIGPLAN94 Conference on Programming Language Design and +/// Implementation, 1994, pp. 61–72 +/// fn udiv_1e19(n: u128) -> (u128, u64) { const DIV: u64 = 1e19 as u64; - let high = (n >> 64) as u64; - if high == 0 { - let low = n as u64; - return ((low / DIV) as u128, low % DIV); - } - let sr = 65 - high.leading_zeros(); - let mut q = n << (128 - sr); - let mut r = n >> sr; - let mut carry = 0; - - for _ in 0..sr { - r = (r << 1) | (q >> 127); - q = (q << 1) | carry as u128; - - let s = (DIV as u128).wrapping_sub(r).wrapping_sub(1) as i128 >> 127; - carry = (s & 1) as u64; - r -= (DIV as u128) & s as u128; - } - ((q << 1) | carry as u128, r as u64) + const FACTOR: u128 = 156927543384667019095894735580191660403; + + let quot = if n < 1 << 83 { + ((n >> 19) as u64 / (DIV >> 19)) as u128 + } else { + u128_mulhi(n, FACTOR) >> 62 + }; + + let rem = (n - quot * DIV as u128) as u64; + (quot, rem) +} + +/// Multiply unsigned 128 bit integers, return upper 128 bits of the result +#[inline] +fn u128_mulhi(x: u128, y: u128) -> u128 { + let x_lo = x as u64; + let x_hi = (x >> 64) as u64; + let y_lo = y as u64; + let y_hi = (y >> 64) as u64; + + // handle possibility of overflow + let carry = (x_lo as u128 * y_lo as u128) >> 64; + let m = x_lo as u128 * y_hi as u128 + carry; + let high1 = m >> 64; + + let m_lo = m as u64; + let high2 = (x_hi as u128 * y_lo as u128 + m_lo as u128) >> 64; + + x_hi as u128 * y_hi as u128 + high1 + high2 } From 152f500a7bb0d283134cb55743cd9cb25fa335be Mon Sep 17 00:00:00 2001 From: C Date: Thu, 28 Jan 2021 22:12:03 +0000 Subject: [PATCH 10/21] refactor: removing redundant variables --- library/core/src/iter/adapters/zip.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 7fbbf9481a5a8..fcc55cbb4f940 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -113,13 +113,12 @@ where // of `next_back` does this, otherwise we will break the restriction // on calls to `self.next_back()` after calling `get_unchecked()`. if sz_a != sz_b { - let sz_a = self.a.size(); if a_side_effect && sz_a > self.len { for _ in 0..sz_a - cmp::max(self.len, self.index) { self.a.next_back(); } } - let sz_b = self.b.size(); + if b_side_effect && sz_b > self.len { for _ in 0..sz_b - self.len { self.b.next_back(); From b421cd56d945743defe3b2a32e2901648ac8dd2d Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Thu, 3 Dec 2020 23:40:09 -0500 Subject: [PATCH 11/21] Restrict precision of captures with `capture_disjoint_fields` set - No Derefs in move closure, this will result in value behind a reference getting moved. - No projections are applied to raw pointers, since these require unsafe blocks. We capture them completely. Motivations for these are recorded here: https://hackmd.io/71qq-IOpTNqzMkPpAI1dVg?view --- compiler/rustc_typeck/src/check/upvar.rs | 70 +++++++++++++++++++++--- 1 file changed, 61 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 6b2cba62fa6b7..2252493577ff6 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -419,15 +419,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { base => bug!("Expected upvar, found={:?}", base), }; - // Arrays are captured in entirety, drop Index projections and projections - // after Index projections. - let first_index_projection = - place.projections.split(|proj| ProjectionKind::Index == proj.kind).next(); - let place = Place { - base_ty: place.base_ty, - base: place.base, - projections: first_index_projection.map_or(Vec::new(), |p| p.to_vec()), - }; + let place = restrict_capture_precision(place, capture_info.capture_kind); let min_cap_list = match root_var_min_capture_list.get_mut(&var_hir_id) { None => { @@ -960,6 +952,66 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> { } } +/// Truncate projections so that following rules are obeyed by the captured `place`: +/// +/// - No Derefs in move closure, this will result in value behind a reference getting moved. +/// - No projections are applied to raw pointers, since these require unsafe blocks. We capture +/// them completely. +/// - No Index projections are captured, since arrays are captured completely. +fn restrict_capture_precision<'tcx>( + mut place: Place<'tcx>, + capture_kind: ty::UpvarCapture<'tcx>, +) -> Place<'tcx> { + if place.projections.is_empty() { + // Nothing to do here + return place; + } + + if place.base_ty.is_unsafe_ptr() { + place.projections.truncate(0); + return place; + } + + let mut truncated_length = usize::MAX; + let mut first_deref_projection = usize::MAX; + + for (i, proj) in place.projections.iter().enumerate() { + if proj.ty.is_unsafe_ptr() { + // Don't apply any projections on top of an unsafe ptr + truncated_length = truncated_length.min(i + 1); + break; + } + match proj.kind { + ProjectionKind::Index => { + // Arrays are completely captured, so we drop Index projections + truncated_length = truncated_length.min(i); + break; + } + ProjectionKind::Deref => { + // We only drop Derefs in case of move closures + // There might be an index projection or raw ptr ahead, so we don't stop here. + first_deref_projection = first_deref_projection.min(i); + } + ProjectionKind::Field(..) => {} // ignore + ProjectionKind::Subslice => {} // We never capture this + } + } + + let length = place + .projections + .len() + .min(truncated_length) + // In case of capture `ByValue` we want to not capture derefs + .min(match capture_kind { + ty::UpvarCapture::ByValue(..) => first_deref_projection, + ty::UpvarCapture::ByRef(..) => usize::MAX, + }); + + place.projections.truncate(length); + + place +} + fn construct_place_string(tcx: TyCtxt<'_>, place: &Place<'tcx>) -> String { let variable_name = match place.base { PlaceBase::Upvar(upvar_id) => var_name(tcx, upvar_id.var_path.hir_id).to_string(), From 34880825828260fad0a74621e4a13fe7da9a4a9d Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Wed, 2 Dec 2020 03:03:56 -0500 Subject: [PATCH 12/21] Compute mutability of closure captures When `capture_disjoint_fields` is not enabled, checking if the root variable binding is mutable would suffice. However with the feature enabled, the captured place might be mutable because it dereferences a mutable reference. This PR computes the mutability of each capture after capture analysis in rustc_typeck. We store this in `ty::CapturedPlace` and then use `ty::CapturedPlace::mutability` in mir_build and borrow_check. --- compiler/rustc_middle/src/ty/mod.rs | 8 +++- compiler/rustc_mir/src/borrow_check/mod.rs | 9 +--- compiler/rustc_mir_build/src/build/mod.rs | 11 +---- compiler/rustc_typeck/src/check/upvar.rs | 54 ++++++++++++++++++++-- 4 files changed, 60 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 8e8caa46c3802..ddb78d91759f9 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -661,11 +661,17 @@ pub type RootVariableMinCaptureList<'tcx> = FxIndexMap = Vec>; -/// A `Place` and the corresponding `CaptureInfo`. +/// A composite describing a `Place` that is captured by a closure. #[derive(PartialEq, Clone, Debug, TyEncodable, TyDecodable, TypeFoldable, HashStable)] pub struct CapturedPlace<'tcx> { + /// The `Place` that is captured. pub place: HirPlace<'tcx>, + + /// `CaptureKind` and expression(s) that resulted in such capture of `place`. pub info: CaptureInfo<'tcx>, + + /// Represents if `place` can be mutated or not. + pub mutability: hir::Mutability, } pub fn place_to_string_for_capture(tcx: TyCtxt<'tcx>, place: &HirPlace<'tcx>) -> String { diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index 7c7edfdb5fbaf..1a771157e288a 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -170,17 +170,12 @@ fn do_mir_borrowck<'a, 'tcx>( ty::UpvarCapture::ByValue(_) => false, ty::UpvarCapture::ByRef(..) => true, }; - let mut upvar = Upvar { + Upvar { name: tcx.hir().name(var_hir_id), var_hir_id, by_ref, - mutability: Mutability::Not, - }; - let bm = *tables.pat_binding_modes().get(var_hir_id).expect("missing binding mode"); - if bm == ty::BindByValue(hir::Mutability::Mut) { - upvar.mutability = Mutability::Mut; + mutability: captured_place.mutability, } - upvar }) .collect(); diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 996615995259d..e4891eb5a3c0c 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -851,22 +851,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { _ => bug!("Expected an upvar") }; - let mut mutability = Mutability::Not; + let mutability = captured_place.mutability; // FIXME(project-rfc-2229#8): Store more precise information let mut name = kw::Empty; if let Some(Node::Binding(pat)) = tcx_hir.find(var_id) { if let hir::PatKind::Binding(_, _, ident, _) = pat.kind { name = ident.name; - match hir_typeck_results - .extract_binding_mode(tcx.sess, pat.hir_id, pat.span) - { - Some(ty::BindByValue(hir::Mutability::Mut)) => { - mutability = Mutability::Mut; - } - Some(_) => mutability = Mutability::Not, - _ => {} - } } } diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 2252493577ff6..38330d5a49a94 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -252,8 +252,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let capture = captured_place.info.capture_kind; debug!( - "place={:?} upvar_ty={:?} capture={:?}", - captured_place.place, upvar_ty, capture + "final_upvar_tys: place={:?} upvar_ty={:?} capture={:?}, mutability={:?}", + captured_place.place, upvar_ty, capture, captured_place.mutability, ); match capture { @@ -423,7 +423,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let min_cap_list = match root_var_min_capture_list.get_mut(&var_hir_id) { None => { - let min_cap_list = vec![ty::CapturedPlace { place, info: capture_info }]; + let mutability = self.determine_capture_mutability(&place); + let min_cap_list = + vec![ty::CapturedPlace { place, info: capture_info, mutability }]; root_var_min_capture_list.insert(var_hir_id, min_cap_list); continue; } @@ -486,8 +488,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Only need to insert when we don't have an ancestor in the existing min capture list if !ancestor_found { + let mutability = self.determine_capture_mutability(&place); let captured_place = - ty::CapturedPlace { place: place.clone(), info: updated_capture_info }; + ty::CapturedPlace { place, info: updated_capture_info, mutability }; min_cap_list.push(captured_place); } } @@ -607,6 +610,49 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } } + + /// A captured place is mutable if + /// 1. Projections don't include a Deref of an immut-borrow, **and** + /// 2. PlaceBase is mut or projections include a Deref of a mut-borrow. + fn determine_capture_mutability(&self, place: &Place<'tcx>) -> hir::Mutability { + let var_hir_id = match place.base { + PlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id, + _ => unreachable!(), + }; + + let bm = *self + .typeck_results + .borrow() + .pat_binding_modes() + .get(var_hir_id) + .expect("missing binding mode"); + + let mut is_mutbl = match bm { + ty::BindByValue(mutability) => mutability, + ty::BindByReference(_) => hir::Mutability::Not, + }; + + for pointer_ty in place.deref_tys() { + match pointer_ty.kind() { + // We don't capture derefs of raw ptrs + ty::RawPtr(_) => unreachable!(), + + // Derefencing a mut-ref allows us to mut the Place if we don't deref + // an immut-ref after on top of this. + ty::Ref(.., hir::Mutability::Mut) => is_mutbl = hir::Mutability::Mut, + + // The place isn't mutable once we dereference a immutable reference. + ty::Ref(.., hir::Mutability::Not) => return hir::Mutability::Not, + + // Dereferencing a box doesn't change mutability + ty::Adt(def, ..) if def.is_box() => {} + + unexpected_ty => bug!("deref of unexpected pointer type {:?}", unexpected_ty), + } + } + + is_mutbl + } } struct InferBorrowKind<'a, 'tcx> { From 1373f988fa662aa43a22a9b13300aabe9ce2213b Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Sun, 13 Dec 2020 01:29:20 -0500 Subject: [PATCH 13/21] Test cases for handling mutable references --- .../diagnostics/cant-mutate-imm-borrow.rs | 21 +++++++ .../diagnostics/cant-mutate-imm-borrow.stderr | 31 ++++++++++ .../diagnostics/mut_ref.rs | 40 +++++++++++++ .../diagnostics/mut_ref.stderr | 52 +++++++++++++++++ .../2229_closure_analysis/run_pass/mut_ref.rs | 56 +++++++++++++++++++ .../run_pass/mut_ref.stderr | 11 ++++ .../run_pass/mut_ref_struct_mem.rs | 23 ++++++++ .../run_pass/mut_ref_struct_mem.stderr | 11 ++++ 8 files changed, 245 insertions(+) create mode 100644 src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.stderr diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.rs b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.rs new file mode 100644 index 0000000000000..dd018a0defa1b --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.rs @@ -0,0 +1,21 @@ +// Test that if we deref an immutable borrow to access a Place, +// then we can't mutate the final place. + +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete + +fn main() { + let mut x = (format!(""), format!("X2")); + let mut y = (&x, "Y"); + let z = (&mut y, "Z"); + + // `x.0` is mutable but we access `x` via `z.0.0`, which is an immutable reference and + // therefore can't be mutated. + let mut c = || { + //~^ ERROR: cannot borrow `z.0.0.0` as mutable, as it is behind a `&` reference + z.0.0.0 = format!("X1"); + //~^ ERROR: cannot assign to `z`, as it is not declared as mutable + }; + + c(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.stderr new file mode 100644 index 0000000000000..948e2b731daf0 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.stderr @@ -0,0 +1,31 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/cant-mutate-imm-borrow.rs:4:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error[E0594]: cannot assign to `z`, as it is not declared as mutable + --> $DIR/cant-mutate-imm-borrow.rs:16:9 + | +LL | let z = (&mut y, "Z"); + | - help: consider changing this to be mutable: `mut z` +... +LL | z.0.0.0 = format!("X1"); + | ^^^^^^^ cannot assign + +error[E0596]: cannot borrow `z.0.0.0` as mutable, as it is behind a `&` reference + --> $DIR/cant-mutate-imm-borrow.rs:14:17 + | +LL | let mut c = || { + | ^^ cannot borrow as mutable +LL | +LL | z.0.0.0 = format!("X1"); + | - mutable borrow occurs due to use of `z.0.0.0` in closure + +error: aborting due to 2 previous errors; 1 warning emitted + +Some errors have detailed explanations: E0594, E0596. +For more information about an error, try `rustc --explain E0594`. diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.rs b/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.rs new file mode 100644 index 0000000000000..732c47298242a --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.rs @@ -0,0 +1,40 @@ +// Test that we can't mutate a place if we need to deref an imm-borrow +// to reach it. + +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete + +fn imm_mut_ref() { + let mut x = String::new(); + let y = String::new(); + let mref_x = &mut x; + let ref_mref_x = &mref_x; + + let c = || { + //~^ ERROR: cannot borrow `**ref_mref_x` as mutable, as it is behind a `&` reference + **ref_mref_x = y; + //~^ERROR: cannot assign to `ref_mref_x`, as it is not declared as mutable + }; + + c(); +} + +fn mut_imm_ref() { + let x = String::new(); + let y = String::new(); + let mut ref_x = &x; + let mref_ref_x = &mut ref_x; + + let c = || { + //~^ ERROR: cannot borrow `**mref_ref_x` as mutable, as it is behind a `&` reference + **mref_ref_x = y; + //~^ERROR: cannot assign to `mref_ref_x`, as it is not declared as mutable + }; + + c(); +} + +fn main() { + imm_mut_ref(); + mut_imm_ref(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr new file mode 100644 index 0000000000000..42b3c5090ac66 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr @@ -0,0 +1,52 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/mut_ref.rs:4:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error[E0594]: cannot assign to `ref_mref_x`, as it is not declared as mutable + --> $DIR/mut_ref.rs:15:9 + | +LL | let ref_mref_x = &mref_x; + | ---------- help: consider changing this to be mutable: `mut ref_mref_x` +... +LL | **ref_mref_x = y; + | ^^^^^^^^^^^^ cannot assign + +error[E0596]: cannot borrow `**ref_mref_x` as mutable, as it is behind a `&` reference + --> $DIR/mut_ref.rs:13:13 + | +LL | let ref_mref_x = &mref_x; + | ------- help: consider changing this to be a mutable reference: `&mut mref_x` +LL | +LL | let c = || { + | ^^ `ref_mref_x` is a `&` reference, so the data it refers to cannot be borrowed as mutable +LL | +LL | **ref_mref_x = y; + | ---------- mutable borrow occurs due to use of `**ref_mref_x` in closure + +error[E0594]: cannot assign to `mref_ref_x`, as it is not declared as mutable + --> $DIR/mut_ref.rs:30:9 + | +LL | let mref_ref_x = &mut ref_x; + | ---------- help: consider changing this to be mutable: `mut mref_ref_x` +... +LL | **mref_ref_x = y; + | ^^^^^^^^^^^^ cannot assign + +error[E0596]: cannot borrow `**mref_ref_x` as mutable, as it is behind a `&` reference + --> $DIR/mut_ref.rs:28:13 + | +LL | let c = || { + | ^^ cannot borrow as mutable +LL | +LL | **mref_ref_x = y; + | ---------- mutable borrow occurs due to use of `**mref_ref_x` in closure + +error: aborting due to 4 previous errors; 1 warning emitted + +Some errors have detailed explanations: E0594, E0596. +For more information about an error, try `rustc --explain E0594`. diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref.rs new file mode 100644 index 0000000000000..315622443c3cc --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref.rs @@ -0,0 +1,56 @@ +// run-pass + +// Test that we can mutate a place through a mut-borrow +// that is captured by the closure + +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete + +// Check that we can mutate when one deref is required +fn mut_ref_1() { + let mut x = String::new(); + let rx = &mut x; + + let mut c = || { + *rx = String::new(); + }; + + c(); +} + +// Similar example as mut_ref_1, we don't deref the imm-borrow here, +// and so we are allowed to mutate. +fn mut_ref_2() { + let x = String::new(); + let y = String::new(); + let mut ref_x = &x; + let m_ref_x = &mut ref_x; + + let mut c = || { + *m_ref_x = &y; + }; + + c(); +} + +// Check that we can mutate when multiple derefs of mut-borrows are required to reach +// the target place. +// It works because all derefs are mutable, if either of them was an immutable +// borrow, then we would not be able to deref. +fn mut_mut_ref() { + let mut x = String::new(); + let mut mref_x = &mut x; + let m_mref_x = &mut mref_x; + + let mut c = || { + **m_mref_x = String::new(); + }; + + c(); +} + +fn main() { + mut_ref_1(); + mut_ref_2(); + mut_mut_ref(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref.stderr new file mode 100644 index 0000000000000..4b37a0b405f5e --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref.stderr @@ -0,0 +1,11 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/mut_ref.rs:6:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: 1 warning emitted + diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs new file mode 100644 index 0000000000000..82e723cc82562 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs @@ -0,0 +1,23 @@ +// run-pass + +// Test that we can mutate a place through a mut-borrow +// that is captured by the closure +// +// More specifically we test that the if the mutable reference isn't root variable of a capture +// but rather accessed while acessing the precise capture. + +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete + +fn main() { + let mut t = (10, 10); + + let t1 = (&mut t, 10); + + let mut c = || { + // Mutable because (*t.0) is mutable + t1.0.0 += 10; + }; + + c(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.stderr new file mode 100644 index 0000000000000..418ab29098b2a --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.stderr @@ -0,0 +1,11 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/mut_ref_struct_mem.rs:9:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: 1 warning emitted + From 0897db56098dd8e8355017f4364bc88f1e4f26c0 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Mon, 14 Dec 2020 03:09:17 -0500 Subject: [PATCH 14/21] Test for restricting capture precision --- .../2229_closure_analysis/by_value.rs | 41 +++++ .../2229_closure_analysis/by_value.stderr | 67 ++++++++ .../2229_closure_analysis/move_closure.rs | 72 +++++++++ .../2229_closure_analysis/move_closure.stderr | 147 ++++++++++++++++++ .../run_pass/by_value.rs | 28 ++++ .../run_pass/by_value.stderr | 11 ++ .../run_pass/move_closure.rs | 47 ++++++ .../run_pass/move_closure.stderr | 21 +++ .../run_pass/unsafe_ptr.rs | 47 ++++++ .../run_pass/unsafe_ptr.stderr | 11 ++ .../2229_closure_analysis/unsafe_ptr.rs | 63 ++++++++ .../2229_closure_analysis/unsafe_ptr.stderr | 102 ++++++++++++ 12 files changed, 657 insertions(+) create mode 100644 src/test/ui/closures/2229_closure_analysis/by_value.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/by_value.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/move_closure.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/move_closure.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/by_value.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/by_value.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/unsafe_ptr.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/unsafe_ptr.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/unsafe_ptr.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/unsafe_ptr.stderr diff --git a/src/test/ui/closures/2229_closure_analysis/by_value.rs b/src/test/ui/closures/2229_closure_analysis/by_value.rs new file mode 100644 index 0000000000000..1007fb582e5ed --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/by_value.rs @@ -0,0 +1,41 @@ +// Test that we handle derferences properly when only some of the captures are being moved with +// `capture_disjoint_fields` enabled. + + +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +//~| NOTE: `#[warn(incomplete_features)]` on by default +//~| NOTE: see issue #53488 +#![feature(rustc_attrs)] + +#[derive(Debug, Default)] +struct SomeLargeType; +struct MuchLargerType([SomeLargeType; 32]); + +// Ensure that we don't capture any derefs when moving captures into the closures, +// i.e. only data from the enclosing stack is moved. +fn big_box() { + let s = MuchLargerType(Default::default()); + let b = Box::new(s); + let t = (b, 10); + + let c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + || { + //~^ First Pass analysis includes: + //~| Min Capture analysis includes: + let p = t.0.0; + //~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> ByValue + //~| NOTE: Min Capture t[(0, 0)] -> ByValue + println!("{} {:?}", t.1, p); + //~^ NOTE: Capturing t[(1, 0)] -> ImmBorrow + //~| NOTE: Min Capture t[(1, 0)] -> ImmBorrow + }; + + c(); +} + +fn main() { + big_box(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/by_value.stderr b/src/test/ui/closures/2229_closure_analysis/by_value.stderr new file mode 100644 index 0000000000000..fe04dbef6d8b5 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/by_value.stderr @@ -0,0 +1,67 @@ +error[E0658]: attributes on expressions are experimental + --> $DIR/by_value.rs:22:13 + | +LL | let c = #[rustc_capture_analysis] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #15701 for more information + = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable + +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/by_value.rs:5:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error: First Pass analysis includes: + --> $DIR/by_value.rs:25:5 + | +LL | / || { +LL | | +LL | | +LL | | let p = t.0.0; +... | +LL | | +LL | | }; + | |_____^ + | +note: Capturing t[(0, 0),Deref,(0, 0)] -> ByValue + --> $DIR/by_value.rs:28:17 + | +LL | let p = t.0.0; + | ^^^^^ +note: Capturing t[(1, 0)] -> ImmBorrow + --> $DIR/by_value.rs:31:29 + | +LL | println!("{} {:?}", t.1, p); + | ^^^ + +error: Min Capture analysis includes: + --> $DIR/by_value.rs:25:5 + | +LL | / || { +LL | | +LL | | +LL | | let p = t.0.0; +... | +LL | | +LL | | }; + | |_____^ + | +note: Min Capture t[(0, 0)] -> ByValue + --> $DIR/by_value.rs:28:17 + | +LL | let p = t.0.0; + | ^^^^^ +note: Min Capture t[(1, 0)] -> ImmBorrow + --> $DIR/by_value.rs:31:29 + | +LL | println!("{} {:?}", t.1, p); + | ^^^ + +error: aborting due to 3 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/closures/2229_closure_analysis/move_closure.rs b/src/test/ui/closures/2229_closure_analysis/move_closure.rs new file mode 100644 index 0000000000000..8bdc999ca3c3f --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/move_closure.rs @@ -0,0 +1,72 @@ +// Test that move closures drop derefs with `capture_disjoint_fields` enabled. + +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +//~| NOTE: `#[warn(incomplete_features)]` on by default +//~| NOTE: see issue #53488 +#![feature(rustc_attrs)] + +// Test we truncate derefs properly +fn simple_ref() { + let mut s = 10; + let ref_s = &mut s; + + let mut c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + move || { + //~^ ERROR: First Pass analysis includes: + //~| ERROR: Min Capture analysis includes: + *ref_s += 10; + //~^ NOTE: Capturing ref_s[Deref] -> ByValue + //~| NOTE: Min Capture ref_s[] -> ByValue + }; + c(); +} + +// Test we truncate derefs properly +fn struct_contains_ref_to_another_struct() { + struct S(String); + struct T<'a>(&'a mut S); + + let mut s = S("s".into()); + let t = T(&mut s); + + let mut c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + move || { + //~^ ERROR: First Pass analysis includes: + //~| ERROR: Min Capture analysis includes: + t.0.0 = "new s".into(); + //~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> ByValue + //~| NOTE: Min Capture t[(0, 0)] -> ByValue + }; + + c(); +} + +// Test that we don't reduce precision when there is nothing deref. +fn no_ref() { + struct S(String); + struct T(S); + + let t = T(S("s".into())); + let mut c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + move || { + //~^ ERROR: First Pass analysis includes: + //~| ERROR: Min Capture analysis includes: + t.0.0 = "new S".into(); + //~^ NOTE: Capturing t[(0, 0),(0, 0)] -> ByValue + //~| NOTE: Min Capture t[(0, 0),(0, 0)] -> ByValue + }; + c(); +} + +fn main() { + simple_ref(); + struct_contains_ref_to_another_struct(); + no_ref(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/move_closure.stderr b/src/test/ui/closures/2229_closure_analysis/move_closure.stderr new file mode 100644 index 0000000000000..a745f14598ee2 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/move_closure.stderr @@ -0,0 +1,147 @@ +error[E0658]: attributes on expressions are experimental + --> $DIR/move_closure.rs:14:17 + | +LL | let mut c = #[rustc_capture_analysis] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #15701 for more information + = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable + +error[E0658]: attributes on expressions are experimental + --> $DIR/move_closure.rs:35:17 + | +LL | let mut c = #[rustc_capture_analysis] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #15701 for more information + = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable + +error[E0658]: attributes on expressions are experimental + --> $DIR/move_closure.rs:55:17 + | +LL | let mut c = #[rustc_capture_analysis] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #15701 for more information + = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable + +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/move_closure.rs:3:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error: First Pass analysis includes: + --> $DIR/move_closure.rs:17:5 + | +LL | / move || { +LL | | +LL | | +LL | | *ref_s += 10; +LL | | +LL | | +LL | | }; + | |_____^ + | +note: Capturing ref_s[Deref] -> ByValue + --> $DIR/move_closure.rs:20:9 + | +LL | *ref_s += 10; + | ^^^^^^ + +error: Min Capture analysis includes: + --> $DIR/move_closure.rs:17:5 + | +LL | / move || { +LL | | +LL | | +LL | | *ref_s += 10; +LL | | +LL | | +LL | | }; + | |_____^ + | +note: Min Capture ref_s[] -> ByValue + --> $DIR/move_closure.rs:20:9 + | +LL | *ref_s += 10; + | ^^^^^^ + +error: First Pass analysis includes: + --> $DIR/move_closure.rs:38:5 + | +LL | / move || { +LL | | +LL | | +LL | | t.0.0 = "new s".into(); +LL | | +LL | | +LL | | }; + | |_____^ + | +note: Capturing t[(0, 0),Deref,(0, 0)] -> ByValue + --> $DIR/move_closure.rs:41:9 + | +LL | t.0.0 = "new s".into(); + | ^^^^^ + +error: Min Capture analysis includes: + --> $DIR/move_closure.rs:38:5 + | +LL | / move || { +LL | | +LL | | +LL | | t.0.0 = "new s".into(); +LL | | +LL | | +LL | | }; + | |_____^ + | +note: Min Capture t[(0, 0)] -> ByValue + --> $DIR/move_closure.rs:41:9 + | +LL | t.0.0 = "new s".into(); + | ^^^^^ + +error: First Pass analysis includes: + --> $DIR/move_closure.rs:58:5 + | +LL | / move || { +LL | | +LL | | +LL | | t.0.0 = "new S".into(); +LL | | +LL | | +LL | | }; + | |_____^ + | +note: Capturing t[(0, 0),(0, 0)] -> ByValue + --> $DIR/move_closure.rs:61:9 + | +LL | t.0.0 = "new S".into(); + | ^^^^^ + +error: Min Capture analysis includes: + --> $DIR/move_closure.rs:58:5 + | +LL | / move || { +LL | | +LL | | +LL | | t.0.0 = "new S".into(); +LL | | +LL | | +LL | | }; + | |_____^ + | +note: Min Capture t[(0, 0),(0, 0)] -> ByValue + --> $DIR/move_closure.rs:61:9 + | +LL | t.0.0 = "new S".into(); + | ^^^^^ + +error: aborting due to 9 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/by_value.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/by_value.rs new file mode 100644 index 0000000000000..9a93e6cf1e1ef --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/by_value.rs @@ -0,0 +1,28 @@ +// run-pass + +// Test that ByValue captures compile sucessefully especially when the captures are +// derefenced within the closure. + +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete + +#[derive(Debug, Default)] +struct SomeLargeType; +struct MuchLargerType([SomeLargeType; 32]); + +fn big_box() { + let s = MuchLargerType(Default::default()); + let b = Box::new(s); + let t = (b, 10); + + let c = || { + let p = t.0.0; + println!("{} {:?}", t.1, p); + }; + + c(); +} + +fn main() { + big_box(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/by_value.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/by_value.stderr new file mode 100644 index 0000000000000..98715c6b94365 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/by_value.stderr @@ -0,0 +1,11 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/by_value.rs:6:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: 1 warning emitted + diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs new file mode 100644 index 0000000000000..83ed1c28462d3 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs @@ -0,0 +1,47 @@ +// run-pass + +// Test that move closures compile properly with `capture_disjoint_fields` enabled. + +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete + +fn simple_ref() { + let mut s = 10; + let ref_s = &mut s; + + let mut c = move || { + *ref_s += 10; + }; + c(); +} + +fn struct_contains_ref_to_another_struct() { + struct S(String); + struct T<'a>(&'a mut S); + + let mut s = S("s".into()); + let t = T(&mut s); + + let mut c = move || { + t.0.0 = "new s".into(); + }; + + c(); +} + +fn no_ref() { + struct S(String); + struct T(S); + + let t = T(S("s".into())); + let mut c = move || { + t.0.0 = "new S".into(); + }; + c(); +} + +fn main() { + simple_ref(); + struct_contains_ref_to_another_struct(); + no_ref(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr new file mode 100644 index 0000000000000..724b683bfbf87 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr @@ -0,0 +1,21 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/move_closure.rs:5:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: variable does not need to be mutable + --> $DIR/move_closure.rs:36:9 + | +LL | let mut t = T(S("s".into())); + | ----^ + | | + | help: remove this `mut` + | + = note: `#[warn(unused_mut)]` on by default + +warning: 2 warnings emitted + diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/unsafe_ptr.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/unsafe_ptr.rs new file mode 100644 index 0000000000000..f6e9862b26c11 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/unsafe_ptr.rs @@ -0,0 +1,47 @@ +// run-pass + +// Test that we can use raw ptrs when using `capture_disjoint_fields`. + +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete + +#[derive(Debug)] +struct S { + s: String, + t: String, +} + +struct T(*const S); + +fn unsafe_imm() { + let s = "".into(); + let t = "".into(); + let my_speed: Box = Box::new(S { s, t }); + + let p : *const S = Box::into_raw(my_speed); + let t = T(p); + + let c = || unsafe { + println!("{:?}", (*t.0).s); + }; + + c(); +} + +fn unsafe_mut() { + let s = "".into(); + let t = "".into(); + let mut my_speed: Box = Box::new(S { s, t }); + let p : *mut S = &mut *my_speed; + + let c = || { + let x = unsafe { &mut (*p).s }; + *x = "s".into(); + }; + c(); +} + +fn main() { + unsafe_mut(); + unsafe_imm(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/unsafe_ptr.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/unsafe_ptr.stderr new file mode 100644 index 0000000000000..c64c8b72e8151 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/unsafe_ptr.stderr @@ -0,0 +1,11 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/unsafe_ptr.rs:5:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: 1 warning emitted + diff --git a/src/test/ui/closures/2229_closure_analysis/unsafe_ptr.rs b/src/test/ui/closures/2229_closure_analysis/unsafe_ptr.rs new file mode 100644 index 0000000000000..79d3ecc2d2bed --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/unsafe_ptr.rs @@ -0,0 +1,63 @@ +// Test that we restrict precision of a capture when we access a raw ptr, +// i.e. the capture doesn't deref the raw ptr. + +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +//~| `#[warn(incomplete_features)]` on by default +//~| see issue #53488 +#![feature(rustc_attrs)] + +#[derive(Debug)] +struct S { + s: String, + t: String, +} + +struct T(*const S); + +fn unsafe_imm() { + let s = "".into(); + let t = "".into(); + let my_speed: Box = Box::new(S { s, t }); + + let p : *const S = Box::into_raw(my_speed); + let t = T(p); + + let c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + || unsafe { + //~^ ERROR: First Pass analysis includes: + //~| ERROR: Min Capture analysis includes: + println!("{:?}", (*t.0).s); + //~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow + //~| NOTE: Min Capture t[(0, 0)] -> ImmBorrow + }; + + c(); +} + +fn unsafe_mut() { + let s = "".into(); + let t = "".into(); + let mut my_speed: Box = Box::new(S { s, t }); + let p : *mut S = &mut *my_speed; + + let c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + || { + //~^ ERROR: First Pass analysis includes: + //~| ERROR: Min Capture analysis includes: + let x = unsafe { &mut (*p).s }; + //~^ NOTE: Capturing p[Deref,(0, 0)] -> ImmBorrow + //~| NOTE: Min Capture p[] -> ImmBorrow + *x = "s".into(); + }; + c(); +} + +fn main() { + unsafe_mut(); + unsafe_imm(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/unsafe_ptr.stderr b/src/test/ui/closures/2229_closure_analysis/unsafe_ptr.stderr new file mode 100644 index 0000000000000..4508b2426e8ff --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/unsafe_ptr.stderr @@ -0,0 +1,102 @@ +error[E0658]: attributes on expressions are experimental + --> $DIR/unsafe_ptr.rs:26:13 + | +LL | let c = #[rustc_capture_analysis] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #15701 for more information + = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable + +error[E0658]: attributes on expressions are experimental + --> $DIR/unsafe_ptr.rs:46:13 + | +LL | let c = #[rustc_capture_analysis] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #15701 for more information + = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable + +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/unsafe_ptr.rs:4:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error: First Pass analysis includes: + --> $DIR/unsafe_ptr.rs:29:6 + | +LL | / || unsafe { +LL | | +LL | | +LL | | println!("{:?}", (*t.0).s); +LL | | +LL | | +LL | | }; + | |_____^ + | +note: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow + --> $DIR/unsafe_ptr.rs:32:26 + | +LL | println!("{:?}", (*t.0).s); + | ^^^^^^^^ + +error: Min Capture analysis includes: + --> $DIR/unsafe_ptr.rs:29:6 + | +LL | / || unsafe { +LL | | +LL | | +LL | | println!("{:?}", (*t.0).s); +LL | | +LL | | +LL | | }; + | |_____^ + | +note: Min Capture t[(0, 0)] -> ImmBorrow + --> $DIR/unsafe_ptr.rs:32:26 + | +LL | println!("{:?}", (*t.0).s); + | ^^^^^^^^ + +error: First Pass analysis includes: + --> $DIR/unsafe_ptr.rs:49:5 + | +LL | / || { +LL | | +LL | | +LL | | let x = unsafe { &mut (*p).s }; +... | +LL | | *x = "s".into(); +LL | | }; + | |_____^ + | +note: Capturing p[Deref,(0, 0)] -> ImmBorrow + --> $DIR/unsafe_ptr.rs:52:31 + | +LL | let x = unsafe { &mut (*p).s }; + | ^^^^^^ + +error: Min Capture analysis includes: + --> $DIR/unsafe_ptr.rs:49:5 + | +LL | / || { +LL | | +LL | | +LL | | let x = unsafe { &mut (*p).s }; +... | +LL | | *x = "s".into(); +LL | | }; + | |_____^ + | +note: Min Capture p[] -> ImmBorrow + --> $DIR/unsafe_ptr.rs:52:31 + | +LL | let x = unsafe { &mut (*p).s }; + | ^^^^^^ + +error: aborting due to 6 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0658`. From 604cbdcfddb959cd1d7de2f9afa14a199561a428 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Tue, 15 Dec 2020 23:00:19 -0500 Subject: [PATCH 15/21] Fix unused 'mut' warning for capture's root variable --- compiler/rustc_mir/src/borrow_check/mod.rs | 33 ++++++++++++++++--- .../run_pass/move_closure.rs | 25 +++++++++++--- .../run_pass/move_closure.stderr | 12 +------ .../run_pass/mut_ref_struct_mem.rs | 26 +++++++++++++-- 4 files changed, 75 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index 1a771157e288a..6e1e5c65aea6d 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -1369,13 +1369,38 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { fn propagate_closure_used_mut_upvar(&mut self, operand: &Operand<'tcx>) { let propagate_closure_used_mut_place = |this: &mut Self, place: Place<'tcx>| { - if !place.projection.is_empty() { - if let Some(field) = this.is_upvar_field_projection(place.as_ref()) { + // We have three possiblities here: + // a. We are modifying something through a mut-ref + // b. We are modifying something that is local to our parent + // c. Current body is a nested clsoure, and we are modifying path starting from + // a Place captured by our parent closure. + + // Handle (c), the path being modified is exactly the path captured by our parent + if let Some(field) = this.is_upvar_field_projection(place.as_ref()) { + this.used_mut_upvars.push(field); + return; + } + + for (place_ref, proj) in place.iter_projections().rev() { + // Handle (a) + if proj == ProjectionElem::Deref { + match place_ref.ty(this.body(), this.infcx.tcx).ty.kind() { + // We aren't modifying a variable directly + ty::Ref(_, _, hir::Mutability::Mut) => return, + + _ => {} + } + } + + // Handle (c) + if let Some(field) = this.is_upvar_field_projection(place_ref) { this.used_mut_upvars.push(field); + return; } - } else { - this.used_mut.insert(place.local); } + + // Handle(b) + this.used_mut.insert(place.local); }; // This relies on the current way that by-value diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs index 83ed1c28462d3..4007a5a48aaec 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs @@ -29,19 +29,36 @@ fn struct_contains_ref_to_another_struct() { c(); } -fn no_ref() { - struct S(String); - struct T(S); +#[derive(Debug)] +struct S(String); - let t = T(S("s".into())); +#[derive(Debug)] +struct T(S); + +fn no_ref() { + let mut t = T(S("s".into())); let mut c = move || { t.0.0 = "new S".into(); }; c(); } +fn no_ref_nested() { + let mut t = T(S("s".into())); + let c = || { + println!("{:?}", t.0); + let mut c = move || { + t.0.0 = "new S".into(); + println!("{:?}", t.0.0); + }; + c(); + }; + c(); +} + fn main() { simple_ref(); struct_contains_ref_to_another_struct(); no_ref(); + no_ref_nested(); } diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr index 724b683bfbf87..c1d8ba575d6fd 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr @@ -7,15 +7,5 @@ LL | #![feature(capture_disjoint_fields)] = note: `#[warn(incomplete_features)]` on by default = note: see issue #53488 for more information -warning: variable does not need to be mutable - --> $DIR/move_closure.rs:36:9 - | -LL | let mut t = T(S("s".into())); - | ----^ - | | - | help: remove this `mut` - | - = note: `#[warn(unused_mut)]` on by default - -warning: 2 warnings emitted +warning: 1 warning emitted diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs index 82e723cc82562..2dba923647a2e 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs @@ -2,14 +2,14 @@ // Test that we can mutate a place through a mut-borrow // that is captured by the closure -// + // More specifically we test that the if the mutable reference isn't root variable of a capture // but rather accessed while acessing the precise capture. #![feature(capture_disjoint_fields)] //~^ WARNING: the feature `capture_disjoint_fields` is incomplete -fn main() { +fn mut_tuple() { let mut t = (10, 10); let t1 = (&mut t, 10); @@ -21,3 +21,25 @@ fn main() { c(); } + +fn mut_tuple_nested() { + let mut t = (10, 10); + + let t1 = (&mut t, 10); + + let mut c = || { + let mut c = || { + // Mutable because (*t.0) is mutable + t1.0.0 += 10; + }; + + c(); + }; + + c(); +} + +fn main() { + mut_tuple(); + mut_tuple_nested(); +} From c748f32ee45ed42eaab4d6a62dc64720b5096c68 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Sun, 13 Dec 2020 00:11:15 -0500 Subject: [PATCH 16/21] Fix incorrect use mut diagnostics --- compiler/rustc_middle/src/ty/mod.rs | 9 +++++ .../borrow_check/diagnostics/move_errors.rs | 4 ++- .../diagnostics/mutability_errors.rs | 32 +++++++++++++---- .../src/borrow_check/diagnostics/var_name.rs | 7 ++-- compiler/rustc_mir/src/borrow_check/mod.rs | 30 ++++++---------- compiler/rustc_mir/src/borrow_check/nll.rs | 2 +- .../rustc_mir/src/borrow_check/path_utils.rs | 2 +- .../src/borrow_check/type_check/mod.rs | 8 +++-- .../diagnostics/cant-mutate-imm-borrow.rs | 1 - .../diagnostics/cant-mutate-imm-borrow.stderr | 14 ++------ .../diagnostics/cant-mutate-imm.rs | 35 +++++++++++++++++++ .../diagnostics/cant-mutate-imm.stderr | 30 ++++++++++++++++ .../diagnostics/mut_ref.rs | 2 -- .../diagnostics/mut_ref.stderr | 25 ++----------- 14 files changed, 129 insertions(+), 72 deletions(-) create mode 100644 src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm.stderr diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index ddb78d91759f9..7681041863e9a 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -674,6 +674,15 @@ pub struct CapturedPlace<'tcx> { pub mutability: hir::Mutability, } +impl CapturedPlace<'tcx> { + pub fn get_root_variable(&self) -> hir::HirId { + match self.place.base { + HirPlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id, + base => bug!("Expected upvar, found={:?}", base), + } + } +} + pub fn place_to_string_for_capture(tcx: TyCtxt<'tcx>, place: &HirPlace<'tcx>) -> String { let name = match place.base { HirPlaceBase::Upvar(upvar_id) => tcx.hir().name(upvar_id.var_path.hir_id).to_string(), diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs index 350e0d045fa35..fb7694b7d88e9 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs @@ -345,7 +345,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { }; let upvar = &self.upvars[upvar_field.unwrap().index()]; - let upvar_hir_id = upvar.var_hir_id; + // FIXME(project-rfc-2229#8): Improve borrow-check diagnostics in case of precise + // capture. + let upvar_hir_id = upvar.place.get_root_variable(); let upvar_name = upvar.name; let upvar_span = self.infcx.tcx.hir().span(upvar_hir_id); diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs index 73196c732f5bb..74abe2d35ee74 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs @@ -64,12 +64,29 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { Place::ty_from(local, proj_base, self.body, self.infcx.tcx).ty )); - item_msg = format!("`{}`", access_place_desc.unwrap()); - if self.is_upvar_field_projection(access_place.as_ref()).is_some() { - reason = ", as it is not declared as mutable".to_string(); + let imm_borrow_derefed = self.upvars[upvar_index.index()] + .place + .place + .deref_tys() + .any(|ty| matches!(ty.kind(), ty::Ref(.., hir::Mutability::Not))); + + // If the place is immutable then: + // + // - Either we deref a immutable ref to get to our final place. + // - We don't capture derefs of raw ptrs + // - Or the final place is immut because the root variable of the capture + // isn't marked mut and we should suggest that to the user. + if imm_borrow_derefed { + // If we deref an immutable ref then the suggestion here doesn't help. + return; } else { - let name = self.upvars[upvar_index.index()].name; - reason = format!(", as `{}` is not declared as mutable", name); + item_msg = format!("`{}`", access_place_desc.unwrap()); + if self.is_upvar_field_projection(access_place.as_ref()).is_some() { + reason = ", as it is not declared as mutable".to_string(); + } else { + let name = self.upvars[upvar_index.index()].name; + reason = format!(", as `{}` is not declared as mutable", name); + } } } @@ -259,9 +276,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { Place::ty_from(local, proj_base, self.body, self.infcx.tcx).ty )); + let captured_place = &self.upvars[upvar_index.index()].place; + err.span_label(span, format!("cannot {ACT}", ACT = act)); - let upvar_hir_id = self.upvars[upvar_index.index()].var_hir_id; + let upvar_hir_id = captured_place.get_root_variable(); + if let Some(Node::Binding(pat)) = self.infcx.tcx.hir().find(upvar_hir_id) { if let hir::PatKind::Binding( hir::BindingAnnotation::Unannotated, diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/var_name.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/var_name.rs index a850b85e9bbae..4abc623fc5f37 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/var_name.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/var_name.rs @@ -12,7 +12,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { tcx: TyCtxt<'tcx>, body: &Body<'tcx>, local_names: &IndexVec>, - upvars: &[Upvar], + upvars: &[Upvar<'tcx>], fr: RegionVid, ) -> Option<(Option, Span)> { debug!("get_var_name_and_span_for_region(fr={:?})", fr); @@ -21,6 +21,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { debug!("get_var_name_and_span_for_region: attempting upvar"); self.get_upvar_index_for_region(tcx, fr) .map(|index| { + // FIXME(project-rfc-2229#8): Use place span for diagnostics let (name, span) = self.get_upvar_name_and_span_for_region(tcx, upvars, index); (Some(name), span) }) @@ -59,10 +60,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { crate fn get_upvar_name_and_span_for_region( &self, tcx: TyCtxt<'tcx>, - upvars: &[Upvar], + upvars: &[Upvar<'tcx>], upvar_index: usize, ) -> (Symbol, Span) { - let upvar_hir_id = upvars[upvar_index].var_hir_id; + let upvar_hir_id = upvars[upvar_index].place.get_root_variable(); debug!("get_upvar_name_and_span_for_region: upvar_hir_id={:?}", upvar_hir_id); let upvar_name = tcx.hir().name(upvar_hir_id); diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index 6e1e5c65aea6d..b51cba15d32a7 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -5,11 +5,10 @@ use rustc_data_structures::graph::dominators::Dominators; use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorReported}; use rustc_hir as hir; use rustc_hir::def_id::LocalDefId; -use rustc_hir::{HirId, Node}; +use rustc_hir::Node; use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; -use rustc_middle::hir::place::PlaceBase as HirPlaceBase; use rustc_middle::mir::{ traversal, Body, ClearCrossCrate, Local, Location, Mutability, Operand, Place, PlaceElem, PlaceRef, VarDebugInfoContents, @@ -18,7 +17,7 @@ use rustc_middle::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind use rustc_middle::mir::{Field, ProjectionElem, Promoted, Rvalue, Statement, StatementKind}; use rustc_middle::mir::{InlineAsmOperand, Terminator, TerminatorKind}; use rustc_middle::ty::query::Providers; -use rustc_middle::ty::{self, ParamEnv, RegionVid, TyCtxt}; +use rustc_middle::ty::{self, CapturedPlace, ParamEnv, RegionVid, TyCtxt}; use rustc_session::lint::builtin::{MUTABLE_BORROW_RESERVATION_CONFLICT, UNUSED_MUT}; use rustc_span::{Span, Symbol, DUMMY_SP}; @@ -73,16 +72,15 @@ crate use region_infer::RegionInferenceContext; // FIXME(eddyb) perhaps move this somewhere more centrally. #[derive(Debug)] -crate struct Upvar { +crate struct Upvar<'tcx> { + // FIXME(project-rfc-2229#8): ty::CapturePlace should have a to_string(), or similar + // then this should not be needed. name: Symbol, - // FIXME(project-rfc-2229#8): This should use Place or something similar - var_hir_id: HirId, + place: CapturedPlace<'tcx>, /// If true, the capture is behind a reference. by_ref: bool, - - mutability: Mutability, } const DEREF_PROJECTION: &[PlaceElem<'_>; 1] = &[ProjectionElem::Deref]; @@ -161,21 +159,13 @@ fn do_mir_borrowck<'a, 'tcx>( let upvars: Vec<_> = tables .closure_min_captures_flattened(def.did.to_def_id()) .map(|captured_place| { - let var_hir_id = match captured_place.place.base { - HirPlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id, - _ => bug!("Expected upvar"), - }; + let var_hir_id = captured_place.get_root_variable(); let capture = captured_place.info.capture_kind; let by_ref = match capture { ty::UpvarCapture::ByValue(_) => false, ty::UpvarCapture::ByRef(..) => true, }; - Upvar { - name: tcx.hir().name(var_hir_id), - var_hir_id, - by_ref, - mutability: captured_place.mutability, - } + Upvar { name: tcx.hir().name(var_hir_id), place: captured_place.clone(), by_ref } }) .collect(); @@ -544,7 +534,7 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> { dominators: Dominators, /// Information about upvars not necessarily preserved in types or MIR - upvars: Vec, + upvars: Vec>, /// Names of local (user) variables (extracted from `var_debug_info`). local_names: IndexVec>, @@ -2251,7 +2241,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { place={:?}", upvar, is_local_mutation_allowed, place ); - match (upvar.mutability, is_local_mutation_allowed) { + match (upvar.place.mutability, is_local_mutation_allowed) { ( Mutability::Not, LocalMutationIsAllowed::No diff --git a/compiler/rustc_mir/src/borrow_check/nll.rs b/compiler/rustc_mir/src/borrow_check/nll.rs index 359c5f261a434..a0265b20d127b 100644 --- a/compiler/rustc_mir/src/borrow_check/nll.rs +++ b/compiler/rustc_mir/src/borrow_check/nll.rs @@ -165,7 +165,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( flow_inits: &mut ResultsCursor<'cx, 'tcx, MaybeInitializedPlaces<'cx, 'tcx>>, move_data: &MoveData<'tcx>, borrow_set: &BorrowSet<'tcx>, - upvars: &[Upvar], + upvars: &[Upvar<'tcx>], ) -> NllOutput<'tcx> { let mut all_facts = AllFacts::enabled(infcx.tcx).then_some(AllFacts::default()); diff --git a/compiler/rustc_mir/src/borrow_check/path_utils.rs b/compiler/rustc_mir/src/borrow_check/path_utils.rs index fa3ae2367e08e..80de3b4e363bf 100644 --- a/compiler/rustc_mir/src/borrow_check/path_utils.rs +++ b/compiler/rustc_mir/src/borrow_check/path_utils.rs @@ -143,7 +143,7 @@ pub(super) fn borrow_of_local_data(place: Place<'_>) -> bool { /// of a closure type. pub(crate) fn is_upvar_field_projection( tcx: TyCtxt<'tcx>, - upvars: &[Upvar], + upvars: &[Upvar<'tcx>], place_ref: PlaceRef<'tcx>, body: &Body<'tcx>, ) -> Option { diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index fb9820e853f8f..24bbd2b8c49c1 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -132,7 +132,7 @@ pub(crate) fn type_check<'mir, 'tcx>( flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>, move_data: &MoveData<'tcx>, elements: &Rc, - upvars: &[Upvar], + upvars: &[Upvar<'tcx>], ) -> MirTypeckResults<'tcx> { let implicit_region_bound = infcx.tcx.mk_region(ty::ReVar(universal_regions.fr_fn_body)); let mut constraints = MirTypeckRegionConstraints { @@ -821,7 +821,7 @@ struct BorrowCheckContext<'a, 'tcx> { all_facts: &'a mut Option, borrow_set: &'a BorrowSet<'tcx>, constraints: &'a mut MirTypeckRegionConstraints<'tcx>, - upvars: &'a [Upvar], + upvars: &'a [Upvar<'tcx>], } crate struct MirTypeckResults<'tcx> { @@ -2490,7 +2490,9 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { body, ); let category = if let Some(field) = field { - ConstraintCategory::ClosureUpvar(self.borrowck_context.upvars[field.index()].var_hir_id) + let var_hir_id = self.borrowck_context.upvars[field.index()].place.get_root_variable(); + // FIXME(project-rfc-2229#8): Use Place for better diagnostics + ConstraintCategory::ClosureUpvar(var_hir_id) } else { ConstraintCategory::Boring }; diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.rs b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.rs index dd018a0defa1b..1ea38e260b645 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.rs +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.rs @@ -14,7 +14,6 @@ fn main() { let mut c = || { //~^ ERROR: cannot borrow `z.0.0.0` as mutable, as it is behind a `&` reference z.0.0.0 = format!("X1"); - //~^ ERROR: cannot assign to `z`, as it is not declared as mutable }; c(); diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.stderr index 948e2b731daf0..861bc44b78ded 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.stderr +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.stderr @@ -7,15 +7,6 @@ LL | #![feature(capture_disjoint_fields)] = note: `#[warn(incomplete_features)]` on by default = note: see issue #53488 for more information -error[E0594]: cannot assign to `z`, as it is not declared as mutable - --> $DIR/cant-mutate-imm-borrow.rs:16:9 - | -LL | let z = (&mut y, "Z"); - | - help: consider changing this to be mutable: `mut z` -... -LL | z.0.0.0 = format!("X1"); - | ^^^^^^^ cannot assign - error[E0596]: cannot borrow `z.0.0.0` as mutable, as it is behind a `&` reference --> $DIR/cant-mutate-imm-borrow.rs:14:17 | @@ -25,7 +16,6 @@ LL | LL | z.0.0.0 = format!("X1"); | - mutable borrow occurs due to use of `z.0.0.0` in closure -error: aborting due to 2 previous errors; 1 warning emitted +error: aborting due to previous error; 1 warning emitted -Some errors have detailed explanations: E0594, E0596. -For more information about an error, try `rustc --explain E0594`. +For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm.rs b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm.rs new file mode 100644 index 0000000000000..997ecc7ddddf1 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm.rs @@ -0,0 +1,35 @@ +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete + +// Ensure that diagnostics for mutability error (because the root variable +// isn't mutable) work with `capture_disjoint_fields` enabled. + +fn mut_error_struct() { + let x = (10, 10); + let y = (x, 10); + let z = (y, 10); + + let mut c = || { + z.0.0.0 = 20; + //~^ ERROR: cannot assign to `z`, as it is not declared as mutable + }; + + c(); +} + +fn mut_error_box() { + let x = (10, 10); + let bx = Box::new(x); + + let mut c = || { + bx.0 = 20; + //~^ ERROR: cannot assign to `bx`, as it is not declared as mutable + }; + + c(); +} + +fn main() { + mut_error_struct(); + mut_error_box(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm.stderr new file mode 100644 index 0000000000000..5e15635ac6e1b --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm.stderr @@ -0,0 +1,30 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/cant-mutate-imm.rs:1:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error[E0594]: cannot assign to `z`, as it is not declared as mutable + --> $DIR/cant-mutate-imm.rs:13:9 + | +LL | let z = (y, 10); + | - help: consider changing this to be mutable: `mut z` +... +LL | z.0.0.0 = 20; + | ^^^^^^^^^^^^ cannot assign + +error[E0594]: cannot assign to `bx`, as it is not declared as mutable + --> $DIR/cant-mutate-imm.rs:25:9 + | +LL | let bx = Box::new(x); + | -- help: consider changing this to be mutable: `mut bx` +... +LL | bx.0 = 20; + | ^^^^^^^^^ cannot assign + +error: aborting due to 2 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0594`. diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.rs b/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.rs index 732c47298242a..676fde558dfbc 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.rs +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.rs @@ -13,7 +13,6 @@ fn imm_mut_ref() { let c = || { //~^ ERROR: cannot borrow `**ref_mref_x` as mutable, as it is behind a `&` reference **ref_mref_x = y; - //~^ERROR: cannot assign to `ref_mref_x`, as it is not declared as mutable }; c(); @@ -28,7 +27,6 @@ fn mut_imm_ref() { let c = || { //~^ ERROR: cannot borrow `**mref_ref_x` as mutable, as it is behind a `&` reference **mref_ref_x = y; - //~^ERROR: cannot assign to `mref_ref_x`, as it is not declared as mutable }; c(); diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr index 42b3c5090ac66..8cb2ed2235d55 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr @@ -7,15 +7,6 @@ LL | #![feature(capture_disjoint_fields)] = note: `#[warn(incomplete_features)]` on by default = note: see issue #53488 for more information -error[E0594]: cannot assign to `ref_mref_x`, as it is not declared as mutable - --> $DIR/mut_ref.rs:15:9 - | -LL | let ref_mref_x = &mref_x; - | ---------- help: consider changing this to be mutable: `mut ref_mref_x` -... -LL | **ref_mref_x = y; - | ^^^^^^^^^^^^ cannot assign - error[E0596]: cannot borrow `**ref_mref_x` as mutable, as it is behind a `&` reference --> $DIR/mut_ref.rs:13:13 | @@ -28,17 +19,8 @@ LL | LL | **ref_mref_x = y; | ---------- mutable borrow occurs due to use of `**ref_mref_x` in closure -error[E0594]: cannot assign to `mref_ref_x`, as it is not declared as mutable - --> $DIR/mut_ref.rs:30:9 - | -LL | let mref_ref_x = &mut ref_x; - | ---------- help: consider changing this to be mutable: `mut mref_ref_x` -... -LL | **mref_ref_x = y; - | ^^^^^^^^^^^^ cannot assign - error[E0596]: cannot borrow `**mref_ref_x` as mutable, as it is behind a `&` reference - --> $DIR/mut_ref.rs:28:13 + --> $DIR/mut_ref.rs:27:13 | LL | let c = || { | ^^ cannot borrow as mutable @@ -46,7 +28,6 @@ LL | LL | **mref_ref_x = y; | ---------- mutable borrow occurs due to use of `**mref_ref_x` in closure -error: aborting due to 4 previous errors; 1 warning emitted +error: aborting due to 2 previous errors; 1 warning emitted -Some errors have detailed explanations: E0594, E0596. -For more information about an error, try `rustc --explain E0594`. +For more information about this error, try `rustc --explain E0596`. From ffd53277dc776dcf93bfa34c478bd99da2361515 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Tue, 12 Jan 2021 14:54:12 -0500 Subject: [PATCH 17/21] Add fixme for precise path diagnostics --- compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs | 2 ++ compiler/rustc_mir/src/borrow_check/mod.rs | 10 +++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs index 6d98bf554f1cf..04ea3cbd8b66d 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs @@ -215,6 +215,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { PlaceRef { local, projection: [proj_base @ .., elem] } => { match elem { ProjectionElem::Deref => { + // FIXME(project-rfc_2229#36): print capture precisely here. let upvar_field_projection = self.is_upvar_field_projection(place); if let Some(field) = upvar_field_projection { let var_index = field.index(); @@ -259,6 +260,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ProjectionElem::Field(field, _ty) => { autoderef = true; + // FIXME(project-rfc_2229#36): print capture precisely here. let upvar_field_projection = self.is_upvar_field_projection(place); if let Some(field) = upvar_field_projection { let var_index = field.index(); diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index b51cba15d32a7..c42e271f40e45 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -73,8 +73,7 @@ crate use region_infer::RegionInferenceContext; // FIXME(eddyb) perhaps move this somewhere more centrally. #[derive(Debug)] crate struct Upvar<'tcx> { - // FIXME(project-rfc-2229#8): ty::CapturePlace should have a to_string(), or similar - // then this should not be needed. + // FIXME(project-rfc_2229#36): print capture precisely here. name: Symbol, place: CapturedPlace<'tcx>, @@ -2156,6 +2155,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { place: PlaceRef<'tcx>, is_local_mutation_allowed: LocalMutationIsAllowed, ) -> Result, PlaceRef<'tcx>> { + debug!("is_mutable: place={:?}, is_local...={:?}", place, is_local_mutation_allowed); match place.last_projection() { None => { let local = &self.body.local_decls[place.local]; @@ -2237,9 +2237,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if let Some(field) = upvar_field_projection { let upvar = &self.upvars[field.index()]; debug!( - "upvar.mutability={:?} local_mutation_is_allowed={:?} \ - place={:?}", - upvar, is_local_mutation_allowed, place + "is_mutable: upvar.mutability={:?} local_mutation_is_allowed={:?} \ + place={:?}, place_base={:?}", + upvar, is_local_mutation_allowed, place, place_base ); match (upvar.place.mutability, is_local_mutation_allowed) { ( From fadf03ee1bbe238ff5a46e60550a70470da492af Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Fri, 29 Jan 2021 16:01:27 -0500 Subject: [PATCH 18/21] Fix typos --- compiler/rustc_middle/src/ty/mod.rs | 2 ++ compiler/rustc_mir/src/borrow_check/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 7681041863e9a..babab005edb2b 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -675,6 +675,8 @@ pub struct CapturedPlace<'tcx> { } impl CapturedPlace<'tcx> { + /// Returns the hir-id of the root variable for the captured place. + /// e.g., if `a.b.c` was captured, would return the hir-id for `a`. pub fn get_root_variable(&self) -> hir::HirId { match self.place.base { HirPlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id, diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index c42e271f40e45..5db52db70ac68 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -1358,10 +1358,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { fn propagate_closure_used_mut_upvar(&mut self, operand: &Operand<'tcx>) { let propagate_closure_used_mut_place = |this: &mut Self, place: Place<'tcx>| { - // We have three possiblities here: + // We have three possibilities here: // a. We are modifying something through a mut-ref // b. We are modifying something that is local to our parent - // c. Current body is a nested clsoure, and we are modifying path starting from + // c. Current body is a nested closure, and we are modifying path starting from // a Place captured by our parent closure. // Handle (c), the path being modified is exactly the path captured by our parent From 0f4bab246b341b5d67ba1c01d81c4822f96dd878 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Fri, 29 Jan 2021 16:25:01 -0500 Subject: [PATCH 19/21] Fixme for closure origin when reborrow is implemented --- compiler/rustc_typeck/src/check/upvar.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 38330d5a49a94..f039445bf7780 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -184,10 +184,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let origin = if self.tcx.features().capture_disjoint_fields { origin } else { - // FIXME(project-rfc-2229#26): Once rust-lang#80092 is merged, we should restrict the - // precision of origin as well. Otherwise, this will cause issues when project-rfc-2229#26 - // is fixed as we might see Index projections in the origin, which we can't print because - // we don't store enough information. + // FIXME(project-rfc-2229#31): Once the changes to support reborrowing are + // made, make sure we are selecting and restricting + // the origin correctly. (origin.0, Place { projections: vec![], ..origin.1 }) }; From 56c27360b1f1eb56a12d00586f716aba4b414848 Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Sat, 30 Jan 2021 14:24:00 +0100 Subject: [PATCH 20/21] Replace predecessor with range in collections documentation Fixes #81548. --- library/std/src/collections/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/std/src/collections/mod.rs b/library/std/src/collections/mod.rs index a1aab767eb26f..80a13d52a2a27 100644 --- a/library/std/src/collections/mod.rs +++ b/library/std/src/collections/mod.rs @@ -110,10 +110,10 @@ //! //! For Sets, all operations have the cost of the equivalent Map operation. //! -//! | | get | insert | remove | predecessor | append | -//! |--------------|-----------|-----------|-----------|-------------|--------| -//! | [`HashMap`] | O(1)~ | O(1)~* | O(1)~ | N/A | N/A | -//! | [`BTreeMap`] | O(log(n)) | O(log(n)) | O(log(n)) | O(log(n)) | O(n+m) | +//! | | get | insert | remove | range | append | +//! |--------------|-----------|-----------|-----------|-----------|--------| +//! | [`HashMap`] | O(1)~ | O(1)~* | O(1)~ | N/A | N/A | +//! | [`BTreeMap`] | O(log(n)) | O(log(n)) | O(log(n)) | O(log(n)) | O(n+m) | //! //! # Correct and Efficient Usage of Collections //! From 7e3217845dc5dea331b26efb2bf51b60afae2082 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 28 Jan 2021 22:03:20 +0000 Subject: [PATCH 21/21] Balance sidebar `Deref` cycle check with main content The `Deref` cycle checks added as part of #80653 were "unbalanced" in the sense that the main content code path checks for cycles _before_ descending, while the sidebar checks _after_. Checking _before_ is correct, so this changes the sidebar path to match the main content path. --- src/librustdoc/html/render/mod.rs | 18 +++++++++++------- src/test/rustdoc-ui/deref-generic.rs | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 src/test/rustdoc-ui/deref-generic.rs diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index daaa2c719b0e6..6909ab870db61 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -3510,6 +3510,7 @@ fn render_assoc_items( "deref-methods-{:#}", type_.print(cx.cache()) ))); + debug!("Adding {} to deref id map", type_.print(cx.cache())); cx.deref_id_map .borrow_mut() .insert(type_.def_id_full(cx.cache()).unwrap(), id.clone()); @@ -3626,6 +3627,7 @@ fn render_deref_methods( _ => None, }) .expect("Expected associated type binding"); + debug!("Render deref methods for {:#?}, target {:#?}", impl_.inner_impl().for_, target); let what = AssocItemRender::DerefFor { trait_: deref_type, type_: real_target, deref_mut_: deref_mut }; if let Some(did) = target.def_id_full(cx.cache()) { @@ -4416,6 +4418,15 @@ fn sidebar_deref_methods(cx: &Context<'_>, out: &mut Buffer, impl_: &Impl, v: &V }) { debug!("found target, real_target: {:?} {:?}", target, real_target); + if let Some(did) = target.def_id_full(cx.cache()) { + if let Some(type_did) = impl_.inner_impl().for_.def_id_full(cx.cache()) { + // `impl Deref for S` + if did == type_did { + // Avoid infinite cycles + return; + } + } + } let deref_mut = v .iter() .filter(|i| i.inner_impl().trait_.is_some()) @@ -4464,13 +4475,6 @@ fn sidebar_deref_methods(cx: &Context<'_>, out: &mut Buffer, impl_: &Impl, v: &V .filter(|i| i.inner_impl().trait_.is_some()) .find(|i| i.inner_impl().trait_.def_id_full(cx.cache()) == c.deref_trait_did) { - if let Some(type_did) = impl_.inner_impl().for_.def_id_full(cx.cache()) { - // `impl Deref for S` - if target_did == type_did { - // Avoid infinite cycles - return; - } - } sidebar_deref_methods(cx, out, target_deref_impl, target_impls); } } diff --git a/src/test/rustdoc-ui/deref-generic.rs b/src/test/rustdoc-ui/deref-generic.rs new file mode 100644 index 0000000000000..bc64beb1b939d --- /dev/null +++ b/src/test/rustdoc-ui/deref-generic.rs @@ -0,0 +1,15 @@ +// check-pass +// #81395: Fix ICE when recursing into Deref target only differing in type args + +pub struct Generic(T); + +impl<'a> std::ops::Deref for Generic<&'a mut ()> { + type Target = Generic<&'a ()>; + fn deref(&self) -> &Self::Target { + unimplemented!() + } +} + +impl<'a> Generic<&'a ()> { + pub fn some_method(&self) {} +}