From f84967f1ae2a8c56faad2d9d882ae93b805abf70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 12 Jun 2019 14:39:12 +0200 Subject: [PATCH 01/20] Use sharded maps for queries --- src/librustc/ty/query/config.rs | 4 +-- src/librustc/ty/query/on_disk_cache.rs | 6 ++-- src/librustc/ty/query/plumbing.rs | 44 ++++++++++++++------------ 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/librustc/ty/query/config.rs b/src/librustc/ty/query/config.rs index 1cc083ea93c6c..91082c59ba05a 100644 --- a/src/librustc/ty/query/config.rs +++ b/src/librustc/ty/query/config.rs @@ -11,7 +11,7 @@ use crate::util::profiling::ProfileCategory; use std::borrow::Cow; use std::hash::Hash; use std::fmt::Debug; -use rustc_data_structures::sync::Lock; +use rustc_data_structures::sharded::Sharded; use rustc_data_structures::fingerprint::Fingerprint; use crate::ich::StableHashingContext; @@ -34,7 +34,7 @@ pub(crate) trait QueryAccessors<'tcx>: QueryConfig<'tcx> { fn query(key: Self::Key) -> Query<'tcx>; // Don't use this method to access query results, instead use the methods on TyCtxt - fn query_cache<'a>(tcx: TyCtxt<'tcx>) -> &'a Lock>; + fn query_cache<'a>(tcx: TyCtxt<'tcx>) -> &'a Sharded>; fn to_dep_node(tcx: TyCtxt<'tcx>, key: &Self::Key) -> DepNode; diff --git a/src/librustc/ty/query/on_disk_cache.rs b/src/librustc/ty/query/on_disk_cache.rs index 45bc89f5a84ab..2a9f1949d514d 100644 --- a/src/librustc/ty/query/on_disk_cache.rs +++ b/src/librustc/ty/query/on_disk_cache.rs @@ -1063,9 +1063,9 @@ where ::std::any::type_name::()); time_ext(tcx.sess.time_extended(), Some(tcx.sess), desc, || { - let map = Q::query_cache(tcx).borrow(); - assert!(map.active.is_empty()); - for (key, entry) in map.results.iter() { + let shards = Q::query_cache(tcx).lock_shards(); + assert!(shards.iter().all(|shard| shard.active.is_empty())); + for (key, entry) in shards.iter().flat_map(|shard| shard.results.iter()) { if Q::cache_on_disk(tcx, key.clone(), Some(&entry.value)) { let dep_node = SerializedDepNodeIndex::new(entry.index.index()); diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index ce9f67db59232..4dce55f589233 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -17,6 +17,7 @@ use errors::Diagnostic; use errors::FatalError; use rustc_data_structures::fx::{FxHashMap}; use rustc_data_structures::sync::{Lrc, Lock}; +use rustc_data_structures::sharded::Sharded; use rustc_data_structures::thin_vec::ThinVec; #[cfg(not(parallel_compiler))] use rustc_data_structures::cold_path; @@ -90,7 +91,7 @@ macro_rules! profq_query_msg { /// A type representing the responsibility to execute the job in the `job` field. /// This will poison the relevant query if dropped. pub(super) struct JobOwner<'a, 'tcx, Q: QueryDescription<'tcx>> { - cache: &'a Lock>, + cache: &'a Sharded>, key: Q::Key, job: Lrc>, } @@ -107,7 +108,7 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> JobOwner<'a, 'tcx, Q> { pub(super) fn try_get(tcx: TyCtxt<'tcx>, span: Span, key: &Q::Key) -> TryGetJob<'a, 'tcx, Q> { let cache = Q::query_cache(tcx); loop { - let mut lock = cache.borrow_mut(); + let mut lock = cache.get_shard_by_value(key).lock(); if let Some(value) = lock.results.get(key) { profq_msg!(tcx, ProfileQueriesMsg::CacheHit); tcx.sess.profiler(|p| p.record_query_hit(Q::NAME)); @@ -191,7 +192,7 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> JobOwner<'a, 'tcx, Q> { let value = QueryValue::new(result.clone(), dep_node_index); { - let mut lock = cache.borrow_mut(); + let mut lock = cache.get_shard_by_value(&key).lock(); lock.active.remove(&key); lock.results.insert(key, value); } @@ -215,7 +216,8 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> Drop for JobOwner<'a, 'tcx, Q> { #[cold] fn drop(&mut self) { // Poison the query so jobs waiting on it panic - self.cache.borrow_mut().active.insert(self.key.clone(), QueryResult::Poisoned); + let shard = self.cache.get_shard_by_value(&self.key); + shard.lock().active.insert(self.key.clone(), QueryResult::Poisoned); // Also signal the completion of the job, so waiters // will continue execution self.job.signal_complete(); @@ -708,7 +710,7 @@ macro_rules! define_queries_inner { use std::mem; #[cfg(parallel_compiler)] use ty::query::job::QueryResult; - use rustc_data_structures::sync::Lock; + use rustc_data_structures::sharded::Sharded; use crate::{ rustc_data_structures::stable_hasher::HashStable, rustc_data_structures::stable_hasher::StableHasherResult, @@ -740,18 +742,17 @@ macro_rules! define_queries_inner { pub fn collect_active_jobs(&self) -> Vec>> { let mut jobs = Vec::new(); - // We use try_lock here since we are only called from the + // We use try_lock_shards here since we are only called from the // deadlock handler, and this shouldn't be locked. $( - jobs.extend( - self.$name.try_lock().unwrap().active.values().filter_map(|v| - if let QueryResult::Started(ref job) = *v { - Some(job.clone()) - } else { - None - } - ) - ); + let shards = self.$name.try_lock_shards().unwrap(); + jobs.extend(shards.iter().flat_map(|shard| shard.active.values().filter_map(|v| + if let QueryResult::Started(ref job) = *v { + Some(job.clone()) + } else { + None + } + ))); )* jobs @@ -773,26 +774,27 @@ macro_rules! define_queries_inner { fn stats<'tcx, Q: QueryConfig<'tcx>>( name: &'static str, - map: &QueryCache<'tcx, Q> + map: &Sharded>, ) -> QueryStats { + let map = map.lock_shards(); QueryStats { name, #[cfg(debug_assertions)] - cache_hits: map.cache_hits, + cache_hits: map.iter().map(|shard| shard.cache_hits).sum(), #[cfg(not(debug_assertions))] cache_hits: 0, key_size: mem::size_of::(), key_type: type_name::(), value_size: mem::size_of::(), value_type: type_name::(), - entry_count: map.results.len(), + entry_count: map.iter().map(|shard| shard.results.len()).sum(), } } $( queries.push(stats::>( stringify!($name), - &*self.$name.lock() + &self.$name, )); )* @@ -967,7 +969,7 @@ macro_rules! define_queries_inner { } #[inline(always)] - fn query_cache<'a>(tcx: TyCtxt<$tcx>) -> &'a Lock> { + fn query_cache<'a>(tcx: TyCtxt<$tcx>) -> &'a Sharded> { &tcx.queries.$name } @@ -1099,7 +1101,7 @@ macro_rules! define_queries_struct { providers: IndexVec>, fallback_extern_providers: Box>, - $($(#[$attr])* $name: Lock>>,)* + $($(#[$attr])* $name: Sharded>>,)* } }; } From 740f8db85572aef58d0734fc60bc2b54862ebbb0 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 19 Jun 2019 23:15:19 +0300 Subject: [PATCH 02/20] Add FIXME-s that some types should be transparent --- src/libstd/ffi/c_str.rs | 6 ++++++ src/libstd/ffi/os_str.rs | 6 ++++++ src/libstd/path.rs | 12 ++++++++++++ src/libstd/sys_common/os_str_bytes.rs | 6 ++++++ 4 files changed, 30 insertions(+) diff --git a/src/libstd/ffi/c_str.rs b/src/libstd/ffi/c_str.rs index 5c6c43017cf64..f7ad62f4e9a8a 100644 --- a/src/libstd/ffi/c_str.rs +++ b/src/libstd/ffi/c_str.rs @@ -195,6 +195,12 @@ pub struct CString { /// [`from_ptr`]: #method.from_ptr #[derive(Hash)] #[stable(feature = "rust1", since = "1.0.0")] +// FIXME: +// `fn from` in `impl From<&CStr> for Box` current implementation relies +// on `CStr` being layout-compatible with `[u8]`. +// When attribute privacy is implemented, `CStr` should be annotated as `#[repr(transparent)]`. +// Anyway, `CStr` representation and layout are considered implementation detail, are +// not documented and must not be relied upon. pub struct CStr { // FIXME: this should not be represented with a DST slice but rather with // just a raw `c_char` along with some form of marker to make diff --git a/src/libstd/ffi/os_str.rs b/src/libstd/ffi/os_str.rs index c7c5849a00fa0..b57f80c2e002b 100644 --- a/src/libstd/ffi/os_str.rs +++ b/src/libstd/ffi/os_str.rs @@ -97,6 +97,12 @@ pub struct OsString { /// [`String`]: ../string/struct.String.html /// [conversions]: index.html#conversions #[stable(feature = "rust1", since = "1.0.0")] +// FIXME: +// `OsStr::from_inner` current implementation relies +// on `OsStr` being layout-compatible with `Slice`. +// When attribute privacy is implemented, `OsStr` should be annotated as `#[repr(transparent)]`. +// Anyway, `OsStr` representation and layout are considered implementation detail, are +// not documented and must not be relied upon. pub struct OsStr { inner: Slice } diff --git a/src/libstd/path.rs b/src/libstd/path.rs index 126bc3754dabc..9f7a081403da3 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -1123,6 +1123,12 @@ impl FusedIterator for Ancestors<'_> {} /// Which method works best depends on what kind of situation you're in. #[derive(Clone)] #[stable(feature = "rust1", since = "1.0.0")] +// FIXME: +// `PathBuf::as_mut_vec` current implementation relies +// on `PathBuf` being layout-compatible with `Vec`. +// When attribute privacy is implemented, `PathBuf` should be annotated as `#[repr(transparent)]`. +// Anyway, `PathBuf` representation and layout are considered implementation detail, are +// not documented and must not be relied upon. pub struct PathBuf { inner: OsString, } @@ -1745,6 +1751,12 @@ impl AsRef for PathBuf { /// assert_eq!(extension, Some(OsStr::new("txt"))); /// ``` #[stable(feature = "rust1", since = "1.0.0")] +// FIXME: +// `Path::new` current implementation relies +// on `Path` being layout-compatible with `OsStr`. +// When attribute privacy is implemented, `Path` should be annotated as `#[repr(transparent)]`. +// Anyway, `Path` representation and layout are considered implementation detail, are +// not documented and must not be relied upon. pub struct Path { inner: OsStr, } diff --git a/src/libstd/sys_common/os_str_bytes.rs b/src/libstd/sys_common/os_str_bytes.rs index a4961974d89ab..d734f412bf886 100644 --- a/src/libstd/sys_common/os_str_bytes.rs +++ b/src/libstd/sys_common/os_str_bytes.rs @@ -18,6 +18,12 @@ pub(crate) struct Buf { pub inner: Vec } +// FIXME: +// `Buf::as_slice` current implementation relies +// on `Slice` being layout-compatible with `[u8]`. +// When attribute privacy is implemented, `Slice` should be annotated as `#[repr(transparent)]`. +// Anyway, `Slice` representation and layout are considered implementation detail, are +// not documented and must not be relied upon. pub(crate) struct Slice { pub inner: [u8] } From 03e95ae4127db1b0ae465e8b58383744b7184a70 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 10 Aug 2019 13:08:47 +0200 Subject: [PATCH 03/20] Miri shouldn't look at types --- src/librustc_mir/interpret/eval_context.rs | 10 +++++++--- src/librustc_mir/interpret/terminator.rs | 8 +++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 1f23d8c017ccd..6f4227ed34cc4 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -385,15 +385,19 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { local: mir::Local, layout: Option>, ) -> InterpResult<'tcx, TyLayout<'tcx>> { - match frame.locals[local].layout.get() { + // `const_prop` runs into this with an invalid (empty) frame, so we + // have to support that case (mostly by skipping all caching). + match frame.locals.get(local).and_then(|state| state.layout.get()) { None => { let layout = crate::interpret::operand::from_known_layout(layout, || { let local_ty = frame.body.local_decls[local].ty; let local_ty = self.monomorphize_with_substs(local_ty, frame.instance.substs)?; self.layout_of(local_ty) })?; - // Layouts of locals are requested a lot, so we cache them. - frame.locals[local].layout.set(Some(layout)); + if let Some(state) = frame.locals.get(local) { + // Layouts of locals are requested a lot, so we cache them. + state.layout.set(Some(layout)); + } Ok(layout) } Some(layout) => Ok(layout), diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index a0c6fb026dd4b..1d6b48e9da4c4 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -405,9 +405,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } else { let local = mir::RETURN_PLACE; - let ty = self.frame().body.local_decls[local].ty; - if !self.tcx.is_ty_uninhabited_from_any_module(ty) { - throw_unsup!(FunctionRetMismatch(self.tcx.types.never, ty)) + let callee_layout = self.layout_of_local(self.frame(), local, None)?; + if !callee_layout.abi.is_uninhabited() { + throw_unsup!(FunctionRetMismatch( + self.tcx.types.never, callee_layout.ty + )) } } Ok(()) From 62f1e8a7f12c864f97c49faf4bf49940aac266a6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 10 Aug 2019 14:49:11 +0200 Subject: [PATCH 04/20] fix test --- .../consts/uninhabited-const-issue-61744.rs | 4 +- .../uninhabited-const-issue-61744.stderr | 57 +++++++++++++++++-- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/src/test/ui/consts/uninhabited-const-issue-61744.rs b/src/test/ui/consts/uninhabited-const-issue-61744.rs index 21fbbf8cfb5a8..4509ebc6338a8 100644 --- a/src/test/ui/consts/uninhabited-const-issue-61744.rs +++ b/src/test/ui/consts/uninhabited-const-issue-61744.rs @@ -1,11 +1,11 @@ // compile-fail pub const unsafe fn fake_type() -> T { - hint_unreachable() + hint_unreachable() //~ ERROR any use of this value will cause an error } pub const unsafe fn hint_unreachable() -> ! { - fake_type() //~ ERROR any use of this value will cause an error + fake_type() } trait Const { diff --git a/src/test/ui/consts/uninhabited-const-issue-61744.stderr b/src/test/ui/consts/uninhabited-const-issue-61744.stderr index e317bdf103c36..f390676fda6d0 100644 --- a/src/test/ui/consts/uninhabited-const-issue-61744.stderr +++ b/src/test/ui/consts/uninhabited-const-issue-61744.stderr @@ -1,11 +1,60 @@ error: any use of this value will cause an error - --> $DIR/uninhabited-const-issue-61744.rs:8:5 + --> $DIR/uninhabited-const-issue-61744.rs:4:5 | -LL | fake_type() - | ^^^^^^^^^^^ +LL | hint_unreachable() + | ^^^^^^^^^^^^^^^^^^ | | - | tried to call a function with return type T passing return place of type ! + | reached the configured maximum number of stack frames | inside call to `hint_unreachable` at $DIR/uninhabited-const-issue-61744.rs:4:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:12:36 ... LL | const CONSTANT: i32 = unsafe { fake_type() }; From 440a5c810029088649918d738169f8e0bb65bb35 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 10 Aug 2019 16:37:40 +0200 Subject: [PATCH 05/20] rename RUST_CTFE_BACKTRACE to RUSTC_CTFE_BACKTRACE --- src/librustc/mir/interpret/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index f53d2ffb6df54..ef0e205184871 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -217,7 +217,7 @@ fn print_backtrace(backtrace: &mut Backtrace) { impl<'tcx> From> for InterpErrorInfo<'tcx> { fn from(kind: InterpError<'tcx>) -> Self { - let backtrace = match env::var("RUST_CTFE_BACKTRACE") { + let backtrace = match env::var("RUSTC_CTFE_BACKTRACE") { // Matching `RUST_BACKTRACE` -- we treat "0" the same as "not present". Ok(ref val) if val != "0" => { let mut backtrace = Backtrace::new_unresolved(); From 93839c3fb4e3a3c3341595ac8dc2c7f4e39326d2 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 10 Aug 2019 16:31:38 +0000 Subject: [PATCH 06/20] Add an example to show how to insert item to a sorted vec --- src/libcore/slice/mod.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index d5a34ea2bd5a1..29afc43599620 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -1364,6 +1364,20 @@ impl [T] { /// let r = s.binary_search(&1); /// assert!(match r { Ok(1..=4) => true, _ => false, }); /// ``` + /// + /// If you want to insert an item to a sorted vector, while maintaining + /// sort order: + /// + /// ``` + /// let mut s = vec![0, 1, 1, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55]; + /// let num = 42; + /// let idx = match s.binary_search(&num) { + /// Ok(idx) => idx, + /// Err(idx) => idx, + /// }; + /// s.insert(idx, num); + /// assert_eq!(s, [0, 1, 1, 1, 1, 2, 3, 5, 8, 13, 21, 34, 42, 55]); + /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn binary_search(&self, x: &T) -> Result where T: Ord From 30ba4bd8e2da15ce8e2fe3c8fbc339ee67cb3241 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 10 Aug 2019 17:16:58 +0000 Subject: [PATCH 07/20] Use Result::unwrap_or_else instead of matching --- src/libcore/slice/mod.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index 29afc43599620..ce5af13d4ca90 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -1371,10 +1371,7 @@ impl [T] { /// ``` /// let mut s = vec![0, 1, 1, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55]; /// let num = 42; - /// let idx = match s.binary_search(&num) { - /// Ok(idx) => idx, - /// Err(idx) => idx, - /// }; + /// let idx = s.binary_search(&num).unwrap_or_else(|x| x); /// s.insert(idx, num); /// assert_eq!(s, [0, 1, 1, 1, 1, 2, 3, 5, 8, 13, 21, 34, 42, 55]); /// ``` From 6b11a632a7f173d29ab06a35a7a4c506f9bb7725 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sun, 11 Aug 2019 08:25:30 +0300 Subject: [PATCH 08/20] syntax: account for CVarArgs being in the argument list. --- src/libsyntax/parse/parser.rs | 2 +- src/test/ui/c-variadic/variadic-ffi-no-fixed-args.rs | 6 ++++++ src/test/ui/c-variadic/variadic-ffi-no-fixed-args.stderr | 8 ++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/c-variadic/variadic-ffi-no-fixed-args.rs create mode 100644 src/test/ui/c-variadic/variadic-ffi-no-fixed-args.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 30e16592113b6..4d2bd1174ea2e 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -5409,7 +5409,7 @@ impl<'a> Parser<'a> { let args: Vec<_> = args.into_iter().filter_map(|x| x).collect(); - if c_variadic && args.is_empty() { + if c_variadic && args.len() <= 1 { self.span_err(sp, "C-variadic function must be declared with at least one named argument"); } diff --git a/src/test/ui/c-variadic/variadic-ffi-no-fixed-args.rs b/src/test/ui/c-variadic/variadic-ffi-no-fixed-args.rs new file mode 100644 index 0000000000000..e3b642a9d418d --- /dev/null +++ b/src/test/ui/c-variadic/variadic-ffi-no-fixed-args.rs @@ -0,0 +1,6 @@ +extern { + fn foo(...); + //~^ ERROR C-variadic function must be declared with at least one named argument +} + +fn main() {} diff --git a/src/test/ui/c-variadic/variadic-ffi-no-fixed-args.stderr b/src/test/ui/c-variadic/variadic-ffi-no-fixed-args.stderr new file mode 100644 index 0000000000000..cb6060525fc0d --- /dev/null +++ b/src/test/ui/c-variadic/variadic-ffi-no-fixed-args.stderr @@ -0,0 +1,8 @@ +error: C-variadic function must be declared with at least one named argument + --> $DIR/variadic-ffi-no-fixed-args.rs:2:11 + | +LL | fn foo(...); + | ^ + +error: aborting due to previous error + From a8017d6869510407f7f8cad786b81aeda26507f3 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 29 Jul 2019 21:19:50 +0300 Subject: [PATCH 09/20] resolve: Populate external modules in more automatic and lazy way The modules are now populated implicitly on the first access --- src/librustc_resolve/build_reduced_graph.rs | 41 ++++----------------- src/librustc_resolve/diagnostics.rs | 27 +++++++------- src/librustc_resolve/late.rs | 2 +- src/librustc_resolve/late/diagnostics.rs | 14 +++---- src/librustc_resolve/lib.rs | 34 +++++++++-------- src/librustc_resolve/macros.rs | 13 +++++++ src/librustc_resolve/resolve_imports.rs | 34 +++++++++++------ 7 files changed, 80 insertions(+), 85 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 6e5750e752e94..a63e2ec2a96bc 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -157,19 +157,6 @@ impl<'a> Resolver<'a> { self.macro_map.insert(def_id, ext.clone()); Some(ext) } - - /// Ensures that the reduced graph rooted at the given external module - /// is built, building it if it is not. - pub fn populate_module_if_necessary(&mut self, module: Module<'a>) { - if module.populated.get() { return } - let def_id = module.def_id().unwrap(); - for child in self.cstore.item_children_untracked(def_id, self.session) { - let child = child.map_id(|_| panic!("unexpected id")); - BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self } - .build_reduced_graph_for_external_crate_res(module, child); - } - module.populated.set(true) - } } pub struct BuildReducedGraphVisitor<'a, 'b> { @@ -595,7 +582,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.r.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX }) }; - self.r.populate_module_if_necessary(module); if let Some(name) = self.r.session.parse_sess.injected_crate_name.try_get() { if name.as_str() == ident.name.as_str() { self.r.injected_crate = Some(module); @@ -868,7 +854,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } /// Builds the reduced graph for a single item in an external crate. - fn build_reduced_graph_for_external_crate_res( + crate fn build_reduced_graph_for_external_crate_res( &mut self, parent: Module<'a>, child: Export, @@ -922,6 +908,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { span); self.r.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion)); + module.populate_on_access.set(false); for child in self.r.cstore.item_children_untracked(def_id, self.r.session) { let res = child.res.map_id(|_| panic!("unexpected id")); let ns = if let Res::Def(DefKind::AssocTy, _) = res { @@ -935,7 +922,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.r.has_self.insert(res.def_id()); } } - module.populated.set(true); } Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => { self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); @@ -951,19 +937,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } } - fn legacy_import_macro(&mut self, - name: Name, - binding: &'a NameBinding<'a>, - span: Span, - allow_shadowing: bool) { - if self.r.macro_use_prelude.insert(name, binding).is_some() && !allow_shadowing { - let msg = format!("`{}` is already in scope", name); - let note = - "macro-expanded `#[macro_use]`s may not shadow existing macros (see RFC 1560)"; - self.r.session.struct_span_err(span, &msg).note(note).emit(); - } - } - /// Returns `true` if we should consider the underlying `extern crate` to be used. fn process_legacy_macro_imports(&mut self, item: &Item, module: Module<'a>) -> bool { let mut import_all = None; @@ -1021,9 +994,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { if let Some(span) = import_all { let directive = macro_use_directive(self, span); self.r.potentially_unused_imports.push(directive); - module.for_each_child(|ident, ns, binding| if ns == MacroNS { - let imported_binding = self.r.import(binding, directive); - self.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing); + module.for_each_child(&mut self.r, |this, ident, ns, binding| if ns == MacroNS { + let imported_binding = this.import(binding, directive); + this.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing); }); } else { for ident in single_imports.iter().cloned() { @@ -1039,8 +1012,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { let directive = macro_use_directive(self, ident.span); self.r.potentially_unused_imports.push(directive); let imported_binding = self.r.import(binding, directive); - self.legacy_import_macro(ident.name, imported_binding, - ident.span, allow_shadowing); + self.r.legacy_import_macro(ident.name, imported_binding, + ident.span, allow_shadowing); } else { span_err!(self.r.session, ident.span, E0469, "imported macro not found"); } diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index 9e7e56f4a3a26..09046e4043ab7 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -73,10 +73,13 @@ crate fn add_typo_suggestion( false } -crate fn add_module_candidates( - module: Module<'_>, names: &mut Vec, filter_fn: &impl Fn(Res) -> bool +crate fn add_module_candidates<'a>( + resolver: &mut Resolver<'a>, + module: Module<'a>, + names: &mut Vec, + filter_fn: &impl Fn(Res) -> bool, ) { - for (&(ident, _), resolution) in module.resolutions.borrow().iter() { + for (&(ident, _), resolution) in resolver.resolutions(module).borrow().iter() { if let Some(binding) = resolution.borrow().binding { let res = binding.res(); if filter_fn(res) { @@ -391,10 +394,10 @@ impl<'a> Resolver<'a> { Scope::CrateRoot => { let root_ident = Ident::new(kw::PathRoot, ident.span); let root_module = this.resolve_crate_root(root_ident); - add_module_candidates(root_module, &mut suggestions, filter_fn); + add_module_candidates(this, root_module, &mut suggestions, filter_fn); } Scope::Module(module) => { - add_module_candidates(module, &mut suggestions, filter_fn); + add_module_candidates(this, module, &mut suggestions, filter_fn); } Scope::MacroUsePrelude => { suggestions.extend(this.macro_use_prelude.iter().filter_map(|(name, binding)| { @@ -442,7 +445,7 @@ impl<'a> Resolver<'a> { Scope::StdLibPrelude => { if let Some(prelude) = this.prelude { let mut tmp_suggestions = Vec::new(); - add_module_candidates(prelude, &mut tmp_suggestions, filter_fn); + add_module_candidates(this, prelude, &mut tmp_suggestions, filter_fn); suggestions.extend(tmp_suggestions.into_iter().filter(|s| { use_prelude || this.is_builtin_macro(s.res.opt_def_id()) })); @@ -498,11 +501,9 @@ impl<'a> Resolver<'a> { while let Some((in_module, path_segments, in_module_is_extern)) = worklist.pop() { - self.populate_module_if_necessary(in_module); - // We have to visit module children in deterministic order to avoid // instabilities in reported imports (#43552). - in_module.for_each_child_stable(|ident, ns, name_binding| { + in_module.for_each_child_stable(self, |this, ident, ns, name_binding| { // avoid imports entirely if name_binding.is_import() && !name_binding.is_extern_crate() { return; } // avoid non-importable candidates as well @@ -536,7 +537,7 @@ impl<'a> Resolver<'a> { // outside crate private modules => no need to check this) if !in_module_is_extern || name_binding.vis == ty::Visibility::Public { let did = match res { - Res::Def(DefKind::Ctor(..), did) => self.parent(did), + Res::Def(DefKind::Ctor(..), did) => this.parent(did), _ => res.opt_def_id(), }; candidates.push(ImportSuggestion { did, path }); @@ -596,8 +597,6 @@ impl<'a> Resolver<'a> { krate: crate_id, index: CRATE_DEF_INDEX, }); - self.populate_module_if_necessary(&crate_root); - suggestions.extend(self.lookup_import_candidates_from_module( lookup_ident, namespace, crate_root, ident, &filter_fn)); } @@ -794,7 +793,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { /// at the root of the crate instead of the module where it is defined /// ``` pub(crate) fn check_for_module_export_macro( - &self, + &mut self, directive: &'b ImportDirective<'b>, module: ModuleOrUniformRoot<'b>, ident: Ident, @@ -815,7 +814,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { return None; } - let resolutions = crate_module.resolutions.borrow(); + let resolutions = self.r.resolutions(crate_module).borrow(); let resolution = resolutions.get(&(ident, MacroNS))?; let binding = resolution.borrow().binding()?; if let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() { diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 7cb11195ee02b..5df38b8ade8f3 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -1934,7 +1934,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> { let mut traits = module.traits.borrow_mut(); if traits.is_none() { let mut collected_traits = Vec::new(); - module.for_each_child(|name, ns, binding| { + module.for_each_child(&mut self.r, |_, name, ns, binding| { if ns != TypeNS { return } match binding.res() { Res::Def(DefKind::Trait, _) | diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index 68f9c1684d6fb..cc40a41a241d9 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -548,7 +548,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { // Items in scope if let RibKind::ModuleRibKind(module) = rib.kind { // Items from this module - add_module_candidates(module, &mut names, &filter_fn); + add_module_candidates(&mut self.r, module, &mut names, &filter_fn); if let ModuleKind::Block(..) = module.kind { // We can see through blocks @@ -577,7 +577,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { })); if let Some(prelude) = self.r.prelude { - add_module_candidates(prelude, &mut names, &filter_fn); + add_module_candidates(&mut self.r, prelude, &mut names, &filter_fn); } } break; @@ -599,7 +599,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { mod_path, Some(TypeNS), false, span, CrateLint::No ) { if let ModuleOrUniformRoot::Module(module) = module { - add_module_candidates(module, &mut names, &filter_fn); + add_module_candidates(&mut self.r, module, &mut names, &filter_fn); } } } @@ -717,9 +717,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { // abort if the module is already found if result.is_some() { break; } - self.r.populate_module_if_necessary(in_module); - - in_module.for_each_child_stable(|ident, _, name_binding| { + in_module.for_each_child_stable(&mut self.r, |_, ident, _, name_binding| { // abort if the module is already found or if name_binding is private external if result.is_some() || !name_binding.vis.is_visible_locally() { return @@ -750,10 +748,8 @@ impl<'a> LateResolutionVisitor<'a, '_> { fn collect_enum_variants(&mut self, def_id: DefId) -> Option> { self.find_module(def_id).map(|(enum_module, enum_import_suggestion)| { - self.r.populate_module_if_necessary(enum_module); - let mut variants = Vec::new(); - enum_module.for_each_child_stable(|ident, _, name_binding| { + enum_module.for_each_child_stable(&mut self.r, |_, ident, _, name_binding| { if let Res::Def(DefKind::Variant, _) = name_binding.res() { let mut segms = enum_import_suggestion.path.segments.clone(); segms.push(ast::PathSegment::from_ident(ident)); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 9fc3e11505c29..635d184b10a2e 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -408,6 +408,8 @@ impl ModuleKind { } } +type Resolutions<'a> = RefCell>>>; + /// One node in the tree of modules. pub struct ModuleData<'a> { parent: Option>, @@ -416,7 +418,11 @@ pub struct ModuleData<'a> { // The def id of the closest normal module (`mod`) ancestor (including this module). normal_ancestor_id: DefId, - resolutions: RefCell>>>, + // Mapping between names and their (possibly in-progress) resolutions in this module. + // Resolutions in modules from other crates are not populated until accessed. + lazy_resolutions: Resolutions<'a>, + // True if this is a module from other crate that needs to be populated on access. + populate_on_access: Cell, single_segment_macro_resolutions: RefCell, Option<&'a NameBinding<'a>>)>>, multi_segment_macro_resolutions: RefCell, Span, MacroKind, ParentScope<'a>, @@ -434,11 +440,6 @@ pub struct ModuleData<'a> { // Used to memoize the traits in this module for faster searches through all traits in scope. traits: RefCell)]>>>, - // Whether this module is populated. If not populated, any attempt to - // access the children must be preceded with a - // `populate_module_if_necessary` call. - populated: Cell, - /// Span of the module itself. Used for error reporting. span: Span, @@ -457,7 +458,8 @@ impl<'a> ModuleData<'a> { parent, kind, normal_ancestor_id, - resolutions: Default::default(), + lazy_resolutions: Default::default(), + populate_on_access: Cell::new(!normal_ancestor_id.is_local()), single_segment_macro_resolutions: RefCell::new(Vec::new()), multi_segment_macro_resolutions: RefCell::new(Vec::new()), builtin_attrs: RefCell::new(Vec::new()), @@ -466,24 +468,27 @@ impl<'a> ModuleData<'a> { glob_importers: RefCell::new(Vec::new()), globs: RefCell::new(Vec::new()), traits: RefCell::new(None), - populated: Cell::new(normal_ancestor_id.is_local()), span, expansion, } } - fn for_each_child)>(&self, mut f: F) { - for (&(ident, ns), name_resolution) in self.resolutions.borrow().iter() { - name_resolution.borrow().binding.map(|binding| f(ident, ns, binding)); + fn for_each_child(&'a self, resolver: &mut Resolver<'a>, mut f: F) + where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) + { + for (&(ident, ns), name_resolution) in resolver.resolutions(self).borrow().iter() { + name_resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding)); } } - fn for_each_child_stable)>(&self, mut f: F) { - let resolutions = self.resolutions.borrow(); + fn for_each_child_stable(&'a self, resolver: &mut Resolver<'a>, mut f: F) + where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) + { + let resolutions = resolver.resolutions(self).borrow(); let mut resolutions = resolutions.iter().collect::>(); resolutions.sort_by_cached_key(|&(&(ident, ns), _)| (ident.as_str(), ns)); for &(&(ident, ns), &resolution) in resolutions.iter() { - resolution.borrow().binding.map(|binding| f(ident, ns, binding)); + resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding)); } } @@ -2608,7 +2613,6 @@ impl<'a> Resolver<'a> { return None; }; let crate_root = self.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX }); - self.populate_module_if_necessary(&crate_root); Some((crate_root, ty::Visibility::Public, DUMMY_SP, ExpnId::root()) .to_name_binding(self.arenas)) } diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 5af71a0170a7b..efcc14b4cd92e 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -869,4 +869,17 @@ impl<'a> Resolver<'a> { Lrc::new(result) } + + crate fn legacy_import_macro(&mut self, + name: ast::Name, + binding: &'a NameBinding<'a>, + span: Span, + allow_shadowing: bool) { + if self.macro_use_prelude.insert(name, binding).is_some() && !allow_shadowing { + let msg = format!("`{}` is already in scope", name); + let note = + "macro-expanded `#[macro_use]`s may not shadow existing macros (see RFC 1560)"; + self.session.struct_span_err(span, &msg).note(note).emit(); + } + } } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 00e89f0fdae0a..342ca7a34755d 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -5,9 +5,10 @@ use crate::{CrateLint, Module, ModuleOrUniformRoot, PerNS, ScopeSet, ParentScope use crate::Determinacy::{self, *}; use crate::Namespace::{self, TypeNS, MacroNS}; use crate::{NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError}; -use crate::{Resolver, ResolutionError, Segment}; +use crate::{Resolutions, Resolver, ResolutionError, Segment}; use crate::{names_to_string, module_to_string}; use crate::ModuleKind; +use crate::build_reduced_graph::BuildReducedGraphVisitor; use crate::diagnostics::Suggestion; use errors::Applicability; @@ -159,9 +160,22 @@ impl<'a> NameResolution<'a> { } impl<'a> Resolver<'a> { - crate fn resolution(&self, module: Module<'a>, ident: Ident, ns: Namespace) + crate fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> { + if module.populate_on_access.get() { + module.populate_on_access.set(false); + let def_id = module.def_id().expect("unpopulated module without a def-id"); + for child in self.cstore.item_children_untracked(def_id, self.session) { + let child = child.map_id(|_| panic!("unexpected id")); + BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self } + .build_reduced_graph_for_external_crate_res(module, child); + } + } + &module.lazy_resolutions + } + + crate fn resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace) -> &'a RefCell> { - *module.resolutions.borrow_mut().entry((ident.modern(), ns)) + *self.resolutions(module).borrow_mut().entry((ident.modern(), ns)) .or_insert_with(|| self.arenas.alloc_name_resolution()) } @@ -240,8 +254,6 @@ impl<'a> Resolver<'a> { } }; - self.populate_module_if_necessary(module); - let resolution = self.resolution(module, ident, ns) .try_borrow_mut() .map_err(|_| (Determined, Weak::No))?; // This happens when there is a cycle of imports. @@ -1025,7 +1037,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { return if all_ns_failed { let resolutions = match module { - ModuleOrUniformRoot::Module(module) => Some(module.resolutions.borrow()), + ModuleOrUniformRoot::Module(module) => Some(self.r.resolutions(module).borrow()), _ => None, }; let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter()); @@ -1263,8 +1275,6 @@ impl<'a, 'b> ImportResolver<'a, 'b> { } }; - self.r.populate_module_if_necessary(module); - if module.is_trait() { self.r.session.span_err(directive.span, "items in traits are not importable."); return; @@ -1280,7 +1290,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { // Ensure that `resolutions` isn't borrowed during `try_define`, // since it might get updated via a glob cycle. - let bindings = module.resolutions.borrow().iter().filter_map(|(&ident, resolution)| { + let bindings = self.r.resolutions(module).borrow().iter().filter_map(|(&ident, resolution)| { resolution.borrow().binding().map(|binding| (ident, binding)) }).collect::>(); for ((mut ident, ns), binding) in bindings { @@ -1308,7 +1318,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { let mut reexports = Vec::new(); - for (&(ident, ns), resolution) in module.resolutions.borrow().iter() { + for (&(ident, ns), resolution) in self.r.resolutions(module).borrow().iter() { let resolution = &mut *resolution.borrow_mut(); let binding = match resolution.binding { Some(binding) => binding, @@ -1367,8 +1377,8 @@ impl<'a, 'b> ImportResolver<'a, 'b> { Some(ModuleOrUniformRoot::Module(module)) => module, _ => bug!("module should exist"), }; - let resolutions = imported_module.parent.expect("parent should exist") - .resolutions.borrow(); + let parent_module = imported_module.parent.expect("parent should exist"); + let resolutions = self.r.resolutions(parent_module).borrow(); let enum_path_segment_index = directive.module_path.len() - 1; let enum_ident = directive.module_path[enum_path_segment_index].ident; From aee4313544ebb4dc18d793e5e56fdf3656f6f6cf Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 30 Jul 2019 23:07:18 +0300 Subject: [PATCH 10/20] resolve: Populate external traits lazily as well --- src/librustc_resolve/build_reduced_graph.rs | 31 ++++++--------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index a63e2ec2a96bc..8376cf7944702 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -867,7 +867,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { let expansion = ExpnId::root(); // FIXME(jseyfried) intercrate hygiene match res { Res::Def(kind @ DefKind::Mod, def_id) - | Res::Def(kind @ DefKind::Enum, def_id) => { + | Res::Def(kind @ DefKind::Enum, def_id) + | Res::Def(kind @ DefKind::Trait, def_id) => { let module = self.r.new_module(parent, ModuleKind::Def(kind, def_id, ident.name), def_id, @@ -880,6 +881,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { | Res::Def(DefKind::ForeignTy, _) | Res::Def(DefKind::OpaqueTy, _) | Res::Def(DefKind::TraitAlias, _) + | Res::Def(DefKind::AssocTy, _) + | Res::Def(DefKind::AssocOpaqueTy, _) | Res::PrimTy(..) | Res::ToolMod => { self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); @@ -887,6 +890,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { Res::Def(DefKind::Fn, _) | Res::Def(DefKind::Static, _) | Res::Def(DefKind::Const, _) + | Res::Def(DefKind::AssocConst, _) | Res::Def(DefKind::Ctor(CtorOf::Variant, ..), _) => { self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); } @@ -899,28 +903,11 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.r.struct_constructors.insert(struct_def_id, (res, vis)); } } - Res::Def(DefKind::Trait, def_id) => { - let module_kind = ModuleKind::Def(DefKind::Trait, def_id, ident.name); - let module = self.r.new_module(parent, - module_kind, - parent.normal_ancestor_id, - expansion, - span); - self.r.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion)); + Res::Def(DefKind::Method, def_id) => { + self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - module.populate_on_access.set(false); - for child in self.r.cstore.item_children_untracked(def_id, self.r.session) { - let res = child.res.map_id(|_| panic!("unexpected id")); - let ns = if let Res::Def(DefKind::AssocTy, _) = res { - TypeNS - } else { ValueNS }; - self.r.define(module, child.ident, ns, - (res, ty::Visibility::Public, DUMMY_SP, expansion)); - - if self.r.cstore.associated_item_cloned_untracked(child.res.def_id()) - .method_has_self_argument { - self.r.has_self.insert(res.def_id()); - } + if self.r.cstore.associated_item_cloned_untracked(def_id).method_has_self_argument { + self.r.has_self.insert(def_id); } } Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => { From da6fbb18951547da08c2e353838e6b1d47d9b3be Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 6 Aug 2019 23:11:52 +0200 Subject: [PATCH 11/20] add basic lint testing for misuse of mem::zeroed and mem::uninitialized --- src/librustc_lint/builtin.rs | 59 ++++++++++++++++++++ src/librustc_lint/lib.rs | 1 + src/libsyntax_pos/symbol.rs | 3 + src/test/ui/lint/uninitialized-zeroed.rs | 28 ++++++++++ src/test/ui/lint/uninitialized-zeroed.stderr | 44 +++++++++++++++ src/test/ui/panic-uninitialized-zeroed.rs | 2 +- 6 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/lint/uninitialized-zeroed.rs create mode 100644 src/test/ui/lint/uninitialized-zeroed.stderr diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index c9153f285fff7..b4155646c891f 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1862,3 +1862,62 @@ impl EarlyLintPass for IncompleteFeatures { }); } } + +declare_lint! { + pub INVALID_VALUE, + Warn, + "an invalid value is being created (such as a NULL reference)" +} + +declare_lint_pass!(InvalidValue => [INVALID_VALUE]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &hir::Expr) { + + const ZEROED_PATH: &[Symbol] = &[sym::core, sym::mem, sym::zeroed]; + const UININIT_PATH: &[Symbol] = &[sym::core, sym::mem, sym::uninitialized]; + + /// Return `false` only if we are sure this type does *not* + /// allow zero initialization. + fn ty_maybe_allows_zero_init(ty: Ty<'_>) -> bool { + use rustc::ty::TyKind::*; + match ty.sty { + // Primitive types that don't like 0 as a value. + Ref(..) | FnPtr(..) | Never => false, + // Conservative fallback. + _ => true, + } + } + + if let hir::ExprKind::Call(ref path_expr, ref _args) = expr.node { + if let hir::ExprKind::Path(ref qpath) = path_expr.node { + if let Some(def_id) = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id() { + if cx.match_def_path(def_id, &ZEROED_PATH) || + cx.match_def_path(def_id, &UININIT_PATH) + { + // This conjures an instance of a type out of nothing, + // using zeroed or uninitialized memory. + // We are extremely conservative with what we warn about. + let conjured_ty = cx.tables.expr_ty(expr); + + if !ty_maybe_allows_zero_init(conjured_ty) { + cx.span_lint( + INVALID_VALUE, + expr.span, + &format!( + "the type `{}` does not permit {}", + conjured_ty, + if cx.match_def_path(def_id, &ZEROED_PATH) { + "zero-initialization" + } else { + "being left uninitialized" + } + ), + ); + } + } + } + } + } + } +} diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 78bc164ba1a0f..3a540fdf4b91f 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -177,6 +177,7 @@ macro_rules! late_lint_mod_passes { UnreachablePub: UnreachablePub, ExplicitOutlivesRequirements: ExplicitOutlivesRequirements, + InvalidValue: InvalidValue, ]); ) } diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index f7e1b983e5446..2d9556233d15f 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -412,6 +412,7 @@ symbols! { match_beginning_vert, match_default_bindings, may_dangle, + mem, member_constraints, message, meta, @@ -695,6 +696,7 @@ symbols! { underscore_imports, underscore_lifetimes, uniform_paths, + uninitialized, universal_impl_trait, unmarked_api, unreachable_code, @@ -726,6 +728,7 @@ symbols! { windows, windows_subsystem, Yield, + zeroed, } } diff --git a/src/test/ui/lint/uninitialized-zeroed.rs b/src/test/ui/lint/uninitialized-zeroed.rs new file mode 100644 index 0000000000000..40b17651e47b2 --- /dev/null +++ b/src/test/ui/lint/uninitialized-zeroed.rs @@ -0,0 +1,28 @@ +// ignore-tidy-linelength +// This test checks that calling `mem::{uninitialized,zeroed}` with certain types results +// in a lint. + +#![feature(never_type)] +#![allow(deprecated)] +#![deny(invalid_value)] + +use std::mem; + +fn main() { + unsafe { + let _val: ! = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: ! = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + let _val: &'static i32 = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: &'static i32 = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + let _val: fn() = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: fn() = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + // Some types that should work just fine. + let _val: Option<&'static i32> = mem::zeroed(); + let _val: Option = mem::zeroed(); + let _val: bool = mem::zeroed(); + let _val: i32 = mem::zeroed(); + } +} diff --git a/src/test/ui/lint/uninitialized-zeroed.stderr b/src/test/ui/lint/uninitialized-zeroed.stderr new file mode 100644 index 0000000000000..c6a47638d38e5 --- /dev/null +++ b/src/test/ui/lint/uninitialized-zeroed.stderr @@ -0,0 +1,44 @@ +error: the type `!` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:15:23 + | +LL | let _val: ! = mem::zeroed(); + | ^^^^^^^^^^^^^ + | +note: lint level defined here + --> $DIR/uninitialized-zeroed.rs:7:9 + | +LL | #![deny(invalid_value)] + | ^^^^^^^^^^^^^ + +error: the type `!` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:16:23 + | +LL | let _val: ! = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + +error: the type `&'static i32` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:21:34 + | +LL | let _val: &'static i32 = mem::zeroed(); + | ^^^^^^^^^^^^^ + +error: the type `&'static i32` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:22:34 + | +LL | let _val: &'static i32 = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + +error: the type `fn()` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:24:26 + | +LL | let _val: fn() = mem::zeroed(); + | ^^^^^^^^^^^^^ + +error: the type `fn()` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:25:26 + | +LL | let _val: fn() = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors + diff --git a/src/test/ui/panic-uninitialized-zeroed.rs b/src/test/ui/panic-uninitialized-zeroed.rs index 0c97babd51cd4..b0d6629561803 100644 --- a/src/test/ui/panic-uninitialized-zeroed.rs +++ b/src/test/ui/panic-uninitialized-zeroed.rs @@ -4,7 +4,7 @@ // in a runtime panic. #![feature(never_type)] -#![allow(deprecated)] +#![allow(deprecated, invalid_value)] use std::{mem, panic}; From ca1e94b131eec4795f1a8dcacdb12f574e7a4a74 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 08:42:50 +0200 Subject: [PATCH 12/20] warn for more cases --- src/librustc_lint/builtin.rs | 43 +++++- src/test/ui/lint/uninitialized-zeroed.rs | 32 +++- src/test/ui/lint/uninitialized-zeroed.stderr | 145 +++++++++++++++++-- 3 files changed, 204 insertions(+), 16 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index b4155646c891f..c652c196afd0d 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -23,7 +23,7 @@ use rustc::hir::def::{Res, DefKind}; use rustc::hir::def_id::{DefId, LOCAL_CRATE}; -use rustc::ty::{self, Ty, TyCtxt}; +use rustc::ty::{self, Ty, TyCtxt, layout::VariantIdx}; use rustc::{lint, util}; use hir::Node; use util::nodemap::HirIdSet; @@ -1879,11 +1879,40 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { /// Return `false` only if we are sure this type does *not* /// allow zero initialization. - fn ty_maybe_allows_zero_init(ty: Ty<'_>) -> bool { + fn ty_maybe_allows_zero_init<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { use rustc::ty::TyKind::*; match ty.sty { // Primitive types that don't like 0 as a value. Ref(..) | FnPtr(..) | Never => false, + Adt(..) if ty.is_box() => false, + // Recurse for some compound types. + Adt(adt_def, substs) if !adt_def.is_union() => { + match adt_def.variants.len() { + 0 => false, // Uninhabited enum! + 1 => { + // Struct, or enum with exactly one variant. + // Proceed recursively, check all fields. + let variant = &adt_def.variants[VariantIdx::from_u32(0)]; + variant.fields.iter().all(|field| { + ty_maybe_allows_zero_init( + tcx, + field.ty(tcx, substs), + ) + }) + } + _ => true, // Conservative fallback for multi-variant enum. + } + } + Tuple(substs) => { + // Proceed recursively, check all fields. + substs.iter().all(|field| { + ty_maybe_allows_zero_init( + tcx, + field.expect_ty(), + ) + }) + } + // FIXME: Would be nice to also warn for `NonNull`/`NonZero*`. // Conservative fallback. _ => true, } @@ -1900,8 +1929,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { // We are extremely conservative with what we warn about. let conjured_ty = cx.tables.expr_ty(expr); - if !ty_maybe_allows_zero_init(conjured_ty) { - cx.span_lint( + if !ty_maybe_allows_zero_init(cx.tcx, conjured_ty) { + cx.struct_span_lint( INVALID_VALUE, expr.span, &format!( @@ -1913,7 +1942,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { "being left uninitialized" } ), - ); + ) + .note("this means that this code causes undefined behavior \ + when executed") + .help("use `MaybeUninit` instead") + .emit(); } } } diff --git a/src/test/ui/lint/uninitialized-zeroed.rs b/src/test/ui/lint/uninitialized-zeroed.rs index 40b17651e47b2..8f9ca8717bda6 100644 --- a/src/test/ui/lint/uninitialized-zeroed.rs +++ b/src/test/ui/lint/uninitialized-zeroed.rs @@ -6,22 +6,52 @@ #![allow(deprecated)] #![deny(invalid_value)] -use std::mem; +use std::mem::{self, MaybeUninit}; + +enum Void {} + +struct Ref(&'static i32); + +struct Wrap { wrapped: T } + +#[allow(unused)] +fn generic() { + unsafe { + let _val: &'static T = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: &'static T = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + let _val: Wrap<&'static T> = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: Wrap<&'static T> = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + } +} fn main() { unsafe { let _val: ! = mem::zeroed(); //~ ERROR: does not permit zero-initialization let _val: ! = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: (i32, !) = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: (i32, !) = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + let _val: Void = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: Void = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: &'static i32 = mem::zeroed(); //~ ERROR: does not permit zero-initialization let _val: &'static i32 = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: Ref = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: Ref = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: fn() = mem::zeroed(); //~ ERROR: does not permit zero-initialization let _val: fn() = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: Wrap = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: Wrap = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + // Some types that should work just fine. let _val: Option<&'static i32> = mem::zeroed(); let _val: Option = mem::zeroed(); + let _val: MaybeUninit<&'static i32> = mem::zeroed(); let _val: bool = mem::zeroed(); let _val: i32 = mem::zeroed(); } diff --git a/src/test/ui/lint/uninitialized-zeroed.stderr b/src/test/ui/lint/uninitialized-zeroed.stderr index c6a47638d38e5..af54b16bd0b24 100644 --- a/src/test/ui/lint/uninitialized-zeroed.stderr +++ b/src/test/ui/lint/uninitialized-zeroed.stderr @@ -1,44 +1,169 @@ -error: the type `!` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:15:23 +error: the type `&'static T` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:20:32 | -LL | let _val: ! = mem::zeroed(); - | ^^^^^^^^^^^^^ +LL | let _val: &'static T = mem::zeroed(); + | ^^^^^^^^^^^^^ | note: lint level defined here --> $DIR/uninitialized-zeroed.rs:7:9 | LL | #![deny(invalid_value)] | ^^^^^^^^^^^^^ + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `&'static T` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:21:32 + | +LL | let _val: &'static T = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Wrap<&'static T>` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:23:38 + | +LL | let _val: Wrap<&'static T> = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Wrap<&'static T>` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:24:38 + | +LL | let _val: Wrap<&'static T> = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `!` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:30:23 + | +LL | let _val: ! = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead error: the type `!` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:16:23 + --> $DIR/uninitialized-zeroed.rs:31:23 | LL | let _val: ! = mem::uninitialized(); | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `(i32, !)` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:33:30 + | +LL | let _val: (i32, !) = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `(i32, !)` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:34:30 + | +LL | let _val: (i32, !) = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Void` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:36:26 + | +LL | let _val: Void = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Void` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:37:26 + | +LL | let _val: Void = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead error: the type `&'static i32` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:21:34 + --> $DIR/uninitialized-zeroed.rs:39:34 | LL | let _val: &'static i32 = mem::zeroed(); | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead error: the type `&'static i32` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:22:34 + --> $DIR/uninitialized-zeroed.rs:40:34 | LL | let _val: &'static i32 = mem::uninitialized(); | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Ref` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:42:25 + | +LL | let _val: Ref = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Ref` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:43:25 + | +LL | let _val: Ref = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead error: the type `fn()` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:24:26 + --> $DIR/uninitialized-zeroed.rs:45:26 | LL | let _val: fn() = mem::zeroed(); | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead error: the type `fn()` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:25:26 + --> $DIR/uninitialized-zeroed.rs:46:26 | LL | let _val: fn() = mem::uninitialized(); | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Wrap` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:48:32 + | +LL | let _val: Wrap = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Wrap` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:49:32 + | +LL | let _val: Wrap = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead -error: aborting due to 6 previous errors +error: aborting due to 18 previous errors From fbd56131a937ffd2ebaef88c8f9788bd4447d66c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 08:43:42 +0200 Subject: [PATCH 13/20] fix a comment --- src/librustc/ty/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index a10578b0a4390..da7f894e84d0a 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1949,7 +1949,7 @@ pub struct FieldDef { pub struct AdtDef { /// `DefId` of the struct, enum or union item. pub did: DefId, - /// Variants of the ADT. If this is a struct or enum, then there will be a single variant. + /// Variants of the ADT. If this is a struct or union, then there will be a single variant. pub variants: IndexVec, /// Flags of the ADT (e.g. is this a struct? is this non-exhaustive?) flags: AdtFlags, From 8e6fbbec83707fe46c9f406f6ccd0ca75817dd5e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 09:01:53 +0200 Subject: [PATCH 14/20] add tuple_fields convenience method and use it in a few places --- src/librustc/ty/mod.rs | 2 ++ src/librustc/ty/sty.rs | 14 ++++++++++++-- src/librustc/ty/util.rs | 8 ++++---- src/librustc/ty/walk.rs | 4 ++-- src/librustc_lint/builtin.rs | 9 ++------- src/librustc_mir/shim.rs | 2 +- src/librustc_mir/util/elaborate_drops.rs | 4 ++-- 7 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index da7f894e84d0a..f653f0561776c 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2565,6 +2565,8 @@ impl<'tcx> AdtDef { } impl<'tcx> FieldDef { + /// Returns the type of this field. The `subst` is typically obtained + /// via the second field of `TyKind::AdtDef`. pub fn ty(&self, tcx: TyCtxt<'tcx>, subst: SubstsRef<'tcx>) -> Ty<'tcx> { tcx.type_of(self.did).subst(tcx, subst) } diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index 064c374de2b4c..129ea9b5b674a 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -171,6 +171,7 @@ pub enum TyKind<'tcx> { Never, /// A tuple type. For example, `(i32, bool)`. + /// Use `TyS::tuple_fields` to iterate over the field types. Tuple(SubstsRef<'tcx>), /// The projection of an associated type. For example, @@ -1723,8 +1724,8 @@ impl<'tcx> TyS<'tcx> { }) }) } - ty::Tuple(tys) => tys.iter().any(|ty| { - ty.expect_ty().conservative_is_privately_uninhabited(tcx) + ty::Tuple(..) => self.tuple_fields().any(|ty| { + ty.conservative_is_privately_uninhabited(tcx) }), ty::Array(ty, len) => { match len.try_eval_usize(tcx, ParamEnv::empty()) { @@ -2103,6 +2104,15 @@ impl<'tcx> TyS<'tcx> { } } + /// Iterates over tuple fields. + /// Panics when called on anything but a tuple. + pub fn tuple_fields(&self) -> impl DoubleEndedIterator> { + match self.sty { + Tuple(substs) => substs.iter().map(|field| field.expect_ty()), + _ => bug!("tuple_fields called on non-tuple"), + } + } + /// If the type contains variants, returns the valid range of variant indices. /// FIXME This requires the optimized MIR in the case of generators. #[inline] diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index c3ecc04b12e01..96e16efd1300a 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -845,15 +845,15 @@ impl<'tcx> ty::TyS<'tcx> { ty: Ty<'tcx>, ) -> Representability { match ty.sty { - Tuple(ref ts) => { + Tuple(..) => { // Find non representable - fold_repr(ts.iter().map(|ty| { + fold_repr(ty.tuple_fields().map(|ty| { is_type_structurally_recursive( tcx, sp, seen, representable_cache, - ty.expect_ty(), + ty, ) })) } @@ -1095,7 +1095,7 @@ fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx> // state transformation pass ty::Generator(..) => true, - ty::Tuple(ref tys) => tys.iter().map(|k| k.expect_ty()).any(needs_drop), + ty::Tuple(..) => ty.tuple_fields().any(needs_drop), // unions don't have destructors because of the child types, // only if they manually implement `Drop` (handled above). diff --git a/src/librustc/ty/walk.rs b/src/librustc/ty/walk.rs index c74511cf0fdda..8c3110792a8b4 100644 --- a/src/librustc/ty/walk.rs +++ b/src/librustc/ty/walk.rs @@ -119,8 +119,8 @@ fn push_subtypes<'tcx>(stack: &mut TypeWalkerStack<'tcx>, parent_ty: Ty<'tcx>) { ty::GeneratorWitness(ts) => { stack.extend(ts.skip_binder().iter().cloned().rev()); } - ty::Tuple(ts) => { - stack.extend(ts.iter().map(|k| k.expect_ty()).rev()); + ty::Tuple(..) => { + stack.extend(parent_ty.tuple_fields().rev()); } ty::FnDef(_, substs) => { stack.extend(substs.types().rev()); diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index c652c196afd0d..5c2a86f8f3feb 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1903,14 +1903,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { _ => true, // Conservative fallback for multi-variant enum. } } - Tuple(substs) => { + Tuple(..) => { // Proceed recursively, check all fields. - substs.iter().all(|field| { - ty_maybe_allows_zero_init( - tcx, - field.expect_ty(), - ) - }) + ty.tuple_fields().all(|field| ty_maybe_allows_zero_init(tcx, field)) } // FIXME: Would be nice to also warn for `NonNull`/`NonZero*`. // Conservative fallback. diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 42945c79ddf0e..33447eba7492a 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -324,7 +324,7 @@ fn build_clone_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, self_ty: Ty<'tcx>) - substs.upvar_tys(def_id, tcx) ) } - ty::Tuple(tys) => builder.tuple_like_shim(dest, src, tys.iter().map(|k| k.expect_ty())), + ty::Tuple(..) => builder.tuple_like_shim(dest, src, self_ty.tuple_fields()), _ => { bug!("clone shim for `{:?}` which is not `Copy` and is not an aggregate", self_ty) } diff --git a/src/librustc_mir/util/elaborate_drops.rs b/src/librustc_mir/util/elaborate_drops.rs index d17dcaafc04f5..52fd645e38e22 100644 --- a/src/librustc_mir/util/elaborate_drops.rs +++ b/src/librustc_mir/util/elaborate_drops.rs @@ -804,8 +804,8 @@ where let tys : Vec<_> = substs.upvar_tys(def_id, self.tcx()).collect(); self.open_drop_for_tuple(&tys) } - ty::Tuple(tys) => { - let tys: Vec<_> = tys.iter().map(|k| k.expect_ty()).collect(); + ty::Tuple(..) => { + let tys: Vec<_> = ty.tuple_fields().collect(); self.open_drop_for_tuple(&tys) } ty::Adt(def, substs) => { From 4b062a175fa3b9613995eea7d026edbbb3be715e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 10:09:42 +0200 Subject: [PATCH 15/20] note a FIXME --- src/librustc/ty/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index f653f0561776c..b104bbf466a36 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1843,6 +1843,7 @@ pub struct VariantDef { /// Flags of the variant (e.g. is field list non-exhaustive)? flags: VariantFlags, /// Recovered? + // FIXME: Needs proper doc. Recovered whom from what? pub recovered: bool, } From 3972d05fec01cd46c207f6e98aca5ee20102e17a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 11:52:40 +0200 Subject: [PATCH 16/20] proper doc comment for 'recovered' field of variant Curtesy of petrochenkov --- src/librustc/ty/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index b104bbf466a36..9d563e290de96 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1842,8 +1842,8 @@ pub struct VariantDef { pub ctor_kind: CtorKind, /// Flags of the variant (e.g. is field list non-exhaustive)? flags: VariantFlags, - /// Recovered? - // FIXME: Needs proper doc. Recovered whom from what? + /// Variant is obtained as part of recovering from a syntactic error. + /// May be incomplete or bogus. pub recovered: bool, } From c5a63566d6d8d70687410b41b6464bcef3ef89f3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 11:59:23 +0200 Subject: [PATCH 17/20] allow the lint if a few UB-demonstrating doc tests --- src/libcore/mem/maybe_uninit.rs | 3 +++ src/libcore/mem/mod.rs | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libcore/mem/maybe_uninit.rs b/src/libcore/mem/maybe_uninit.rs index 8d064de6f4751..1bbea02e0c7c9 100644 --- a/src/libcore/mem/maybe_uninit.rs +++ b/src/libcore/mem/maybe_uninit.rs @@ -13,6 +13,7 @@ use crate::mem::ManuallyDrop; /// ever gets used to access memory: /// /// ```rust,no_run +/// # #![allow(invalid_value)] /// use std::mem::{self, MaybeUninit}; /// /// let x: &i32 = unsafe { mem::zeroed() }; // undefined behavior! @@ -27,6 +28,7 @@ use crate::mem::ManuallyDrop; /// always be `true` or `false`. Hence, creating an uninitialized `bool` is undefined behavior: /// /// ```rust,no_run +/// # #![allow(invalid_value)] /// use std::mem::{self, MaybeUninit}; /// /// let b: bool = unsafe { mem::uninitialized() }; // undefined behavior! @@ -40,6 +42,7 @@ use crate::mem::ManuallyDrop; /// which otherwise can hold any *fixed* bit pattern: /// /// ```rust,no_run +/// # #![allow(invalid_value)] /// use std::mem::{self, MaybeUninit}; /// /// let x: i32 = unsafe { mem::uninitialized() }; // undefined behavior! diff --git a/src/libcore/mem/mod.rs b/src/libcore/mem/mod.rs index 86dae985fdb00..16cfe0f7683a8 100644 --- a/src/libcore/mem/mod.rs +++ b/src/libcore/mem/mod.rs @@ -445,7 +445,8 @@ pub const fn needs_drop() -> bool { /// /// *Incorrect* usage of this function: initializing a reference with zero. /// -/// ```no_run +/// ```rust,no_run +/// # #![allow(invalid_value)] /// use std::mem; /// /// let _x: &i32 = unsafe { mem::zeroed() }; // Undefined behavior! From 5c77a17d182fd05f06eaf899281b2eda49047e91 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 16:23:11 +0200 Subject: [PATCH 18/20] note down some more future plans --- src/librustc_lint/builtin.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 5c2a86f8f3feb..13ec27aa1ab3f 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1908,6 +1908,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { ty.tuple_fields().all(|field| ty_maybe_allows_zero_init(tcx, field)) } // FIXME: Would be nice to also warn for `NonNull`/`NonZero*`. + // FIXME: *Only for `mem::uninitialized`*, we could also warn for `bool`, + // `char`, and any multivariant enum. // Conservative fallback. _ => true, } From 09307474c28eacf0d971ef95ecab0a2186a18c3b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 11 Aug 2019 12:06:12 +0200 Subject: [PATCH 19/20] update clippy --- src/tools/clippy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/clippy b/src/tools/clippy index b041511b5fcd3..72da1015d6d91 160000 --- a/src/tools/clippy +++ b/src/tools/clippy @@ -1 +1 @@ -Subproject commit b041511b5fcd386c4ae74a30b60a5081f8717fbe +Subproject commit 72da1015d6d918fe1b29170acbf486d30e0c2695 From a087df62111d7805376608e5f1ac2f78cd7a9f3e Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 30 Jul 2019 23:30:54 +0300 Subject: [PATCH 20/20] resolve: Move some code around Avoid using dummy spans for some external items with available spans --- src/librustc_resolve/build_reduced_graph.rs | 58 +++++++++---------- src/librustc_resolve/diagnostics.rs | 34 +++++------ src/librustc_resolve/late.rs | 2 +- src/librustc_resolve/late/diagnostics.rs | 13 ++--- src/librustc_resolve/lib.rs | 58 +++++++++++++------ src/librustc_resolve/resolve_imports.rs | 32 ++-------- ...e-extern-crate-restricted-shadowing.stderr | 8 ++- src/test/ui/issues/issue-27033.stderr | 5 ++ 8 files changed, 109 insertions(+), 101 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 8376cf7944702..dae322e9b72f0 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -865,6 +865,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { // This is only a guess, two equivalent idents may incorrectly get different gensyms here. let ident = ident.gensym_if_underscore(); let expansion = ExpnId::root(); // FIXME(jseyfried) intercrate hygiene + // Record primary definitions. match res { Res::Def(kind @ DefKind::Mod, def_id) | Res::Def(kind @ DefKind::Enum, def_id) @@ -874,9 +875,11 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { def_id, expansion, span); - self.r.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, expansion)); + self.r.define(parent, ident, TypeNS, (module, vis, span, expansion)); } - Res::Def(DefKind::Variant, _) + Res::Def(DefKind::Struct, _) + | Res::Def(DefKind::Union, _) + | Res::Def(DefKind::Variant, _) | Res::Def(DefKind::TyAlias, _) | Res::Def(DefKind::ForeignTy, _) | Res::Def(DefKind::OpaqueTy, _) @@ -884,43 +887,40 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { | Res::Def(DefKind::AssocTy, _) | Res::Def(DefKind::AssocOpaqueTy, _) | Res::PrimTy(..) - | Res::ToolMod => { - self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); - } + | Res::ToolMod => + self.r.define(parent, ident, TypeNS, (res, vis, span, expansion)), Res::Def(DefKind::Fn, _) + | Res::Def(DefKind::Method, _) | Res::Def(DefKind::Static, _) | Res::Def(DefKind::Const, _) | Res::Def(DefKind::AssocConst, _) - | Res::Def(DefKind::Ctor(CtorOf::Variant, ..), _) => { - self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - } - Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => { - self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - - if let Some(struct_def_id) = - self.r.cstore.def_key(def_id).parent - .map(|index| DefId { krate: def_id.krate, index: index }) { - self.r.struct_constructors.insert(struct_def_id, (res, vis)); - } + | Res::Def(DefKind::Ctor(..), _) => + self.r.define(parent, ident, ValueNS, (res, vis, span, expansion)), + Res::Def(DefKind::Macro(..), _) + | Res::NonMacroAttr(..) => + self.r.define(parent, ident, MacroNS, (res, vis, span, expansion)), + Res::Def(DefKind::TyParam, _) | Res::Def(DefKind::ConstParam, _) + | Res::Local(..) | Res::SelfTy(..) | Res::SelfCtor(..) | Res::Err => + bug!("unexpected resolution: {:?}", res) + } + // Record some extra data for better diagnostics. + match res { + Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => { + let field_names = self.r.cstore.struct_field_names_untracked(def_id); + self.insert_field_names(def_id, field_names); } Res::Def(DefKind::Method, def_id) => { - self.r.define(parent, ident, ValueNS, (res, vis, DUMMY_SP, expansion)); - if self.r.cstore.associated_item_cloned_untracked(def_id).method_has_self_argument { self.r.has_self.insert(def_id); } } - Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => { - self.r.define(parent, ident, TypeNS, (res, vis, DUMMY_SP, expansion)); - - // Record field names for error reporting. - let field_names = self.r.cstore.struct_field_names_untracked(def_id); - self.insert_field_names(def_id, field_names); - } - Res::Def(DefKind::Macro(..), _) | Res::NonMacroAttr(..) => { - self.r.define(parent, ident, MacroNS, (res, vis, DUMMY_SP, expansion)); + Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => { + let parent = self.r.cstore.def_key(def_id).parent; + if let Some(struct_def_id) = parent.map(|index| DefId { index, ..def_id }) { + self.r.struct_constructors.insert(struct_def_id, (res, vis)); + } } - _ => bug!("unexpected resolution: {:?}", res) + _ => {} } } @@ -981,7 +981,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { if let Some(span) = import_all { let directive = macro_use_directive(self, span); self.r.potentially_unused_imports.push(directive); - module.for_each_child(&mut self.r, |this, ident, ns, binding| if ns == MacroNS { + self.r.for_each_child(module, |this, ident, ns, binding| if ns == MacroNS { let imported_binding = this.import(binding, directive); this.legacy_import_macro(ident.name, imported_binding, span, allow_shadowing); }); diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index 09046e4043ab7..9ce513801ff6e 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -73,23 +73,23 @@ crate fn add_typo_suggestion( false } -crate fn add_module_candidates<'a>( - resolver: &mut Resolver<'a>, - module: Module<'a>, - names: &mut Vec, - filter_fn: &impl Fn(Res) -> bool, -) { - for (&(ident, _), resolution) in resolver.resolutions(module).borrow().iter() { - if let Some(binding) = resolution.borrow().binding { - let res = binding.res(); - if filter_fn(res) { - names.push(TypoSuggestion::from_res(ident.name, res)); +impl<'a> Resolver<'a> { + crate fn add_module_candidates( + &mut self, + module: Module<'a>, + names: &mut Vec, + filter_fn: &impl Fn(Res) -> bool, + ) { + for (&(ident, _), resolution) in self.resolutions(module).borrow().iter() { + if let Some(binding) = resolution.borrow().binding { + let res = binding.res(); + if filter_fn(res) { + names.push(TypoSuggestion::from_res(ident.name, res)); + } } } } -} -impl<'a> Resolver<'a> { /// Combines an error with provided span and emits it. /// /// This takes the error provided, combines it with the span and any additional spans inside the @@ -394,10 +394,10 @@ impl<'a> Resolver<'a> { Scope::CrateRoot => { let root_ident = Ident::new(kw::PathRoot, ident.span); let root_module = this.resolve_crate_root(root_ident); - add_module_candidates(this, root_module, &mut suggestions, filter_fn); + this.add_module_candidates(root_module, &mut suggestions, filter_fn); } Scope::Module(module) => { - add_module_candidates(this, module, &mut suggestions, filter_fn); + this.add_module_candidates(module, &mut suggestions, filter_fn); } Scope::MacroUsePrelude => { suggestions.extend(this.macro_use_prelude.iter().filter_map(|(name, binding)| { @@ -445,7 +445,7 @@ impl<'a> Resolver<'a> { Scope::StdLibPrelude => { if let Some(prelude) = this.prelude { let mut tmp_suggestions = Vec::new(); - add_module_candidates(this, prelude, &mut tmp_suggestions, filter_fn); + this.add_module_candidates(prelude, &mut tmp_suggestions, filter_fn); suggestions.extend(tmp_suggestions.into_iter().filter(|s| { use_prelude || this.is_builtin_macro(s.res.opt_def_id()) })); @@ -503,7 +503,7 @@ impl<'a> Resolver<'a> { in_module_is_extern)) = worklist.pop() { // We have to visit module children in deterministic order to avoid // instabilities in reported imports (#43552). - in_module.for_each_child_stable(self, |this, ident, ns, name_binding| { + self.for_each_child_stable(in_module, |this, ident, ns, name_binding| { // avoid imports entirely if name_binding.is_import() && !name_binding.is_extern_crate() { return; } // avoid non-importable candidates as well diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 5df38b8ade8f3..30fb9dc5bbbe8 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -1934,7 +1934,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> { let mut traits = module.traits.borrow_mut(); if traits.is_none() { let mut collected_traits = Vec::new(); - module.for_each_child(&mut self.r, |_, name, ns, binding| { + self.r.for_each_child(module, |_, name, ns, binding| { if ns != TypeNS { return } match binding.res() { Res::Def(DefKind::Trait, _) | diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index cc40a41a241d9..cd757c4677f17 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -1,8 +1,7 @@ use crate::{CrateLint, Module, ModuleKind, ModuleOrUniformRoot}; use crate::{PathResult, PathSource, Segment}; use crate::path_names_to_string; -use crate::diagnostics::{add_typo_suggestion, add_module_candidates}; -use crate::diagnostics::{ImportSuggestion, TypoSuggestion}; +use crate::diagnostics::{add_typo_suggestion, ImportSuggestion, TypoSuggestion}; use crate::late::{LateResolutionVisitor, RibKind}; use errors::{Applicability, DiagnosticBuilder, DiagnosticId}; @@ -548,7 +547,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { // Items in scope if let RibKind::ModuleRibKind(module) = rib.kind { // Items from this module - add_module_candidates(&mut self.r, module, &mut names, &filter_fn); + self.r.add_module_candidates(module, &mut names, &filter_fn); if let ModuleKind::Block(..) = module.kind { // We can see through blocks @@ -577,7 +576,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { })); if let Some(prelude) = self.r.prelude { - add_module_candidates(&mut self.r, prelude, &mut names, &filter_fn); + self.r.add_module_candidates(prelude, &mut names, &filter_fn); } } break; @@ -599,7 +598,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { mod_path, Some(TypeNS), false, span, CrateLint::No ) { if let ModuleOrUniformRoot::Module(module) = module { - add_module_candidates(&mut self.r, module, &mut names, &filter_fn); + self.r.add_module_candidates(module, &mut names, &filter_fn); } } } @@ -717,7 +716,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { // abort if the module is already found if result.is_some() { break; } - in_module.for_each_child_stable(&mut self.r, |_, ident, _, name_binding| { + self.r.for_each_child_stable(in_module, |_, ident, _, name_binding| { // abort if the module is already found or if name_binding is private external if result.is_some() || !name_binding.vis.is_visible_locally() { return @@ -749,7 +748,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { fn collect_enum_variants(&mut self, def_id: DefId) -> Option> { self.find_module(def_id).map(|(enum_module, enum_import_suggestion)| { let mut variants = Vec::new(); - enum_module.for_each_child_stable(&mut self.r, |_, ident, _, name_binding| { + self.r.for_each_child_stable(enum_module, |_, ident, _, name_binding| { if let Res::Def(DefKind::Variant, _) = name_binding.res() { let mut segms = enum_import_suggestion.path.segments.clone(); segms.push(ast::PathSegment::from_ident(ident)); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 635d184b10a2e..bb51a19584afb 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -50,6 +50,7 @@ use std::collections::BTreeSet; use rustc_data_structures::ptr_key::PtrKey; use rustc_data_structures::sync::Lrc; +use build_reduced_graph::BuildReducedGraphVisitor; use diagnostics::{Suggestion, ImportSuggestion}; use diagnostics::{find_span_of_binding_until_next_binding, extend_span_to_previous_binding}; use late::{PathSource, Rib, RibKind::*}; @@ -473,25 +474,6 @@ impl<'a> ModuleData<'a> { } } - fn for_each_child(&'a self, resolver: &mut Resolver<'a>, mut f: F) - where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) - { - for (&(ident, ns), name_resolution) in resolver.resolutions(self).borrow().iter() { - name_resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding)); - } - } - - fn for_each_child_stable(&'a self, resolver: &mut Resolver<'a>, mut f: F) - where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) - { - let resolutions = resolver.resolutions(self).borrow(); - let mut resolutions = resolutions.iter().collect::>(); - resolutions.sort_by_cached_key(|&(&(ident, ns), _)| (ident.as_str(), ns)); - for &(&(ident, ns), &resolution) in resolutions.iter() { - resolution.borrow().binding.map(|binding| f(resolver, ident, ns, binding)); - } - } - fn res(&self) -> Option { match self.kind { ModuleKind::Def(kind, def_id, _) => Some(Res::Def(kind, def_id)), @@ -1230,6 +1212,44 @@ impl<'a> Resolver<'a> { self.arenas.alloc_module(module) } + fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> { + if module.populate_on_access.get() { + module.populate_on_access.set(false); + let def_id = module.def_id().expect("unpopulated module without a def-id"); + for child in self.cstore.item_children_untracked(def_id, self.session) { + let child = child.map_id(|_| panic!("unexpected id")); + BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self } + .build_reduced_graph_for_external_crate_res(module, child); + } + } + &module.lazy_resolutions + } + + fn resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace) + -> &'a RefCell> { + *self.resolutions(module).borrow_mut().entry((ident.modern(), ns)) + .or_insert_with(|| self.arenas.alloc_name_resolution()) + } + + fn for_each_child(&mut self, module: Module<'a>, mut f: F) + where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) + { + for (&(ident, ns), name_resolution) in self.resolutions(module).borrow().iter() { + name_resolution.borrow().binding.map(|binding| f(self, ident, ns, binding)); + } + } + + fn for_each_child_stable(&mut self, module: Module<'a>, mut f: F) + where F: FnMut(&mut Resolver<'a>, Ident, Namespace, &'a NameBinding<'a>) + { + let resolutions = self.resolutions(module).borrow(); + let mut resolutions = resolutions.iter().collect::>(); + resolutions.sort_by_cached_key(|&(&(ident, ns), _)| (ident.as_str(), ns)); + for &(&(ident, ns), &resolution) in resolutions.iter() { + resolution.borrow().binding.map(|binding| f(self, ident, ns, binding)); + } + } + fn record_use(&mut self, ident: Ident, ns: Namespace, used_binding: &'a NameBinding<'a>, is_lexical_scope: bool) { if let Some((b2, kind)) = used_binding.ambiguity { diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 342ca7a34755d..fb973821be67a 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -5,10 +5,8 @@ use crate::{CrateLint, Module, ModuleOrUniformRoot, PerNS, ScopeSet, ParentScope use crate::Determinacy::{self, *}; use crate::Namespace::{self, TypeNS, MacroNS}; use crate::{NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError}; -use crate::{Resolutions, Resolver, ResolutionError, Segment}; +use crate::{Resolver, ResolutionError, Segment, ModuleKind}; use crate::{names_to_string, module_to_string}; -use crate::ModuleKind; -use crate::build_reduced_graph::BuildReducedGraphVisitor; use crate::diagnostics::Suggestion; use errors::Applicability; @@ -36,7 +34,7 @@ use syntax_pos::{MultiSpan, Span}; use log::*; -use std::cell::{Cell, RefCell}; +use std::cell::Cell; use std::{mem, ptr}; type Res = def::Res; @@ -160,25 +158,6 @@ impl<'a> NameResolution<'a> { } impl<'a> Resolver<'a> { - crate fn resolutions(&mut self, module: Module<'a>) -> &'a Resolutions<'a> { - if module.populate_on_access.get() { - module.populate_on_access.set(false); - let def_id = module.def_id().expect("unpopulated module without a def-id"); - for child in self.cstore.item_children_untracked(def_id, self.session) { - let child = child.map_id(|_| panic!("unexpected id")); - BuildReducedGraphVisitor { parent_scope: self.dummy_parent_scope(), r: self } - .build_reduced_graph_for_external_crate_res(module, child); - } - } - &module.lazy_resolutions - } - - crate fn resolution(&mut self, module: Module<'a>, ident: Ident, ns: Namespace) - -> &'a RefCell> { - *self.resolutions(module).borrow_mut().entry((ident.modern(), ns)) - .or_insert_with(|| self.arenas.alloc_name_resolution()) - } - crate fn resolve_ident_in_module_unadjusted( &mut self, module: ModuleOrUniformRoot<'a>, @@ -1037,7 +1016,8 @@ impl<'a, 'b> ImportResolver<'a, 'b> { return if all_ns_failed { let resolutions = match module { - ModuleOrUniformRoot::Module(module) => Some(self.r.resolutions(module).borrow()), + ModuleOrUniformRoot::Module(module) => + Some(self.r.resolutions(module).borrow()), _ => None, }; let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter()); @@ -1290,8 +1270,8 @@ impl<'a, 'b> ImportResolver<'a, 'b> { // Ensure that `resolutions` isn't borrowed during `try_define`, // since it might get updated via a glob cycle. - let bindings = self.r.resolutions(module).borrow().iter().filter_map(|(&ident, resolution)| { - resolution.borrow().binding().map(|binding| (ident, binding)) + let bindings = self.r.resolutions(module).borrow().iter().filter_map(|(ident, resolution)| { + resolution.borrow().binding().map(|binding| (*ident, binding)) }).collect::>(); for ((mut ident, ns), binding) in bindings { let scope = match ident.span.reverse_glob_adjust(module.expansion, directive.span) { diff --git a/src/test/ui/imports/extern-prelude-extern-crate-restricted-shadowing.stderr b/src/test/ui/imports/extern-prelude-extern-crate-restricted-shadowing.stderr index e8dfd43b6767c..a0bb557bf27b0 100644 --- a/src/test/ui/imports/extern-prelude-extern-crate-restricted-shadowing.stderr +++ b/src/test/ui/imports/extern-prelude-extern-crate-restricted-shadowing.stderr @@ -13,8 +13,7 @@ error[E0659]: `Vec` is ambiguous (macro-expanded name vs less macro-expanded nam LL | Vec::panic!(); | ^^^ ambiguous name | - = note: `Vec` could refer to a struct from prelude -note: `Vec` could also refer to the crate imported here +note: `Vec` could refer to the crate imported here --> $DIR/extern-prelude-extern-crate-restricted-shadowing.rs:5:9 | LL | extern crate std as Vec; @@ -22,6 +21,11 @@ LL | extern crate std as Vec; ... LL | define_vec!(); | -------------- in this macro invocation +note: `Vec` could also refer to the struct defined here + --> $SRC_DIR/libstd/prelude/v1.rs:LL:COL + | +LL | pub use crate::vec::Vec; + | ^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/issues/issue-27033.stderr b/src/test/ui/issues/issue-27033.stderr index ab95433228037..b11298d4ab90b 100644 --- a/src/test/ui/issues/issue-27033.stderr +++ b/src/test/ui/issues/issue-27033.stderr @@ -3,6 +3,11 @@ error[E0530]: match bindings cannot shadow unit variants | LL | None @ _ => {} | ^^^^ cannot be named the same as a unit variant + | + ::: $SRC_DIR/libstd/prelude/v1.rs:LL:COL + | +LL | pub use crate::option::Option::{self, Some, None}; + | ---- the unit variant `None` is defined here error[E0530]: match bindings cannot shadow constants --> $DIR/issue-27033.rs:7:9