diff options
| -rw-r--r-- | compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs | 23 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/ty/print/pretty.rs | 44 | ||||
| -rw-r--r-- | compiler/rustc_typeck/src/check/method/suggest.rs | 75 | ||||
| -rw-r--r-- | src/test/ui/imports/auxiliary/overlapping_pub_trait_source.rs | 13 | ||||
| -rw-r--r-- | src/test/ui/imports/auxiliary/unnamed_pub_trait_source.rs | 13 | ||||
| -rw-r--r-- | src/test/ui/imports/overlapping_pub_trait.rs | 15 | ||||
| -rw-r--r-- | src/test/ui/imports/overlapping_pub_trait.stderr | 20 | ||||
| -rw-r--r-- | src/test/ui/imports/unnamed_pub_trait.rs | 16 | ||||
| -rw-r--r-- | src/test/ui/imports/unnamed_pub_trait.stderr | 20 |
9 files changed, 212 insertions, 27 deletions
diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 5ba7efc37f8..4e5d21049a0 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -17,7 +17,7 @@ use rustc_session::utils::NativeLibKind; use rustc_session::{Session, StableCrateId}; use rustc_span::hygiene::{ExpnHash, ExpnId}; use rustc_span::source_map::{Span, Spanned}; -use rustc_span::symbol::Symbol; +use rustc_span::symbol::{kw, Symbol}; use rustc_data_structures::sync::Lrc; use smallvec::SmallVec; @@ -295,6 +295,10 @@ pub fn provide(providers: &mut Providers) { use std::collections::vec_deque::VecDeque; let mut visible_parent_map: DefIdMap<DefId> = Default::default(); + // This is a secondary visible_parent_map, storing the DefId of parents that re-export + // the child as `_`. Since we prefer parents that don't do this, merge this map at the + // end, only if we're missing any keys from the former. + let mut fallback_map: DefIdMap<DefId> = Default::default(); // Issue 46112: We want the map to prefer the shortest // paths when reporting the path to an item. Therefore we @@ -317,12 +321,17 @@ pub fn provide(providers: &mut Providers) { bfs_queue.push_back(DefId { krate: cnum, index: CRATE_DEF_INDEX }); } - let mut add_child = |bfs_queue: &mut VecDeque<_>, child: &Export, parent: DefId| { - if !child.vis.is_public() { + let mut add_child = |bfs_queue: &mut VecDeque<_>, export: &Export, parent: DefId| { + if !export.vis.is_public() { return; } - if let Some(child) = child.res.opt_def_id() { + if let Some(child) = export.res.opt_def_id() { + if export.ident.name == kw::Underscore { + fallback_map.insert(child, parent); + return; + } + match visible_parent_map.entry(child) { Entry::Occupied(mut entry) => { // If `child` is defined in crate `cnum`, ensure @@ -345,6 +354,12 @@ pub fn provide(providers: &mut Providers) { } } + // Fill in any missing entries with the (less preferable) path ending in `::_`. + // We still use this path in a diagnostic that suggests importing `::*`. + for (child, parent) in fallback_map { + visible_parent_map.entry(child).or_insert(parent); + } + visible_parent_map }, diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index 3faedf24286..47a9234419c 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -319,6 +319,9 @@ pub trait PrettyPrinter<'tcx>: /// /// `callers` is a chain of visible_parent's leading to `def_id`, /// to support cycle detection during recursion. + /// + /// This method returns false if we can't print the visible path, so + /// `print_def_path` can fall back on the item's real definition path. fn try_print_visible_def_path_recur( mut self, def_id: DefId, @@ -405,19 +408,7 @@ pub trait PrettyPrinter<'tcx>: Some(parent) => parent, None => return Ok((self, false)), }; - if callers.contains(&visible_parent) { - return Ok((self, false)); - } - callers.push(visible_parent); - // HACK(eddyb) this bypasses `path_append`'s prefix printing to avoid - // knowing ahead of time whether the entire path will succeed or not. - // To support printers that do not implement `PrettyPrinter`, a `Vec` or - // linked list on the stack would need to be built, before any printing. - match self.try_print_visible_def_path_recur(visible_parent, callers)? { - (cx, false) => return Ok((cx, false)), - (cx, true) => self = cx, - } - callers.pop(); + let actual_parent = self.tcx().parent(def_id); debug!( "try_print_visible_def_path: visible_parent={:?} actual_parent={:?}", @@ -463,14 +454,21 @@ pub trait PrettyPrinter<'tcx>: // `visible_parent_map`), looking for the specific child we currently have and then // have access to the re-exported name. DefPathData::TypeNs(ref mut name) if Some(visible_parent) != actual_parent => { + // Item might be re-exported several times, but filter for the one + // that's public and whose identifier isn't `_`. let reexport = self .tcx() .item_children(visible_parent) .iter() - .find(|child| child.res.opt_def_id() == Some(def_id)) + .filter(|child| child.res.opt_def_id() == Some(def_id)) + .find(|child| child.vis.is_public() && child.ident.name != kw::Underscore) .map(|child| child.ident.name); - if let Some(reexport) = reexport { - *name = reexport; + + if let Some(new_name) = reexport { + *name = new_name; + } else { + // There is no name that is public and isn't `_`, so bail. + return Ok((self, false)); } } // Re-exported `extern crate` (#43189). @@ -481,6 +479,20 @@ pub trait PrettyPrinter<'tcx>: } debug!("try_print_visible_def_path: data={:?}", data); + if callers.contains(&visible_parent) { + return Ok((self, false)); + } + callers.push(visible_parent); + // HACK(eddyb) this bypasses `path_append`'s prefix printing to avoid + // knowing ahead of time whether the entire path will succeed or not. + // To support printers that do not implement `PrettyPrinter`, a `Vec` or + // linked list on the stack would need to be built, before any printing. + match self.try_print_visible_def_path_recur(visible_parent, callers)? { + (cx, false) => return Ok((cx, false)), + (cx, true) => self = cx, + } + callers.pop(); + Ok((self.path_append(Ok, &DisambiguatedDefPathData { data, disambiguator: 0 })?, true)) } diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs index 1dcc20c29a3..d5a4de86d4d 100644 --- a/compiler/rustc_typeck/src/check/method/suggest.rs +++ b/compiler/rustc_typeck/src/check/method/suggest.rs @@ -12,7 +12,7 @@ use rustc_hir::{ExprKind, Node, QPath}; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_middle::ty::fast_reject::{simplify_type, SimplifyParams, StripReferences}; use rustc_middle::ty::print::with_crate_prefix; -use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{self, DefIdTree, ToPredicate, Ty, TyCtxt, TypeFoldable}; use rustc_span::lev_distance; use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::{source_map, FileName, MultiSpan, Span, Symbol}; @@ -1310,25 +1310,66 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { mut msg: String, candidates: Vec<DefId>, ) { + let parent_map = self.tcx.visible_parent_map(()); + + // Separate out candidates that must be imported with a glob, because they are named `_` + // and cannot be referred with their identifier. + let (candidates, globs): (Vec<_>, Vec<_>) = candidates.into_iter().partition(|trait_did| { + if let Some(parent_did) = parent_map.get(trait_did) { + // If the item is re-exported as `_`, we should suggest a glob-import instead. + if Some(*parent_did) != self.tcx.parent(*trait_did) + && self + .tcx + .item_children(*parent_did) + .iter() + .filter(|child| child.res.opt_def_id() == Some(*trait_did)) + .all(|child| child.ident.name == kw::Underscore) + { + return false; + } + } + + true + }); + let module_did = self.tcx.parent_module(self.body_id); let (span, found_use) = find_use_placement(self.tcx, module_did); if let Some(span) = span { - let path_strings = candidates.iter().map(|did| { + let path_strings = candidates.iter().map(|trait_did| { // Produce an additional newline to separate the new use statement // from the directly following item. let additional_newline = if found_use { "" } else { "\n" }; format!( "use {};\n{}", - with_crate_prefix(|| self.tcx.def_path_str(*did)), + with_crate_prefix(|| self.tcx.def_path_str(*trait_did)), additional_newline ) }); - err.span_suggestions(span, &msg, path_strings, Applicability::MaybeIncorrect); + let glob_path_strings = globs.iter().map(|trait_did| { + let parent_did = parent_map.get(trait_did).unwrap(); + + // Produce an additional newline to separate the new use statement + // from the directly following item. + let additional_newline = if found_use { "" } else { "\n" }; + format!( + "use {}::*; // trait {}\n{}", + with_crate_prefix(|| self.tcx.def_path_str(*parent_did)), + self.tcx.item_name(*trait_did), + additional_newline + ) + }); + + err.span_suggestions( + span, + &msg, + path_strings.chain(glob_path_strings), + Applicability::MaybeIncorrect, + ); } else { - let limit = if candidates.len() == 5 { 5 } else { 4 }; + let limit = if candidates.len() + globs.len() == 5 { 5 } else { 4 }; for (i, trait_did) in candidates.iter().take(limit).enumerate() { - if candidates.len() > 1 { + if candidates.len() + globs.len() > 1 { msg.push_str(&format!( "\ncandidate #{}: `use {};`", i + 1, @@ -1341,8 +1382,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { )); } } + for (i, trait_did) in + globs.iter().take(limit.saturating_sub(candidates.len())).enumerate() + { + let parent_did = parent_map.get(trait_did).unwrap(); + + if candidates.len() + globs.len() > 1 { + msg.push_str(&format!( + "\ncandidate #{}: `use {}::*; // trait {}`", + candidates.len() + i + 1, + with_crate_prefix(|| self.tcx.def_path_str(*parent_did)), + self.tcx.item_name(*trait_did), + )); + } else { + msg.push_str(&format!( + "\n`use {}::*; // trait {}`", + with_crate_prefix(|| self.tcx.def_path_str(*parent_did)), + self.tcx.item_name(*trait_did), + )); + } + } if candidates.len() > limit { - msg.push_str(&format!("\nand {} others", candidates.len() - limit)); + msg.push_str(&format!("\nand {} others", candidates.len() + globs.len() - limit)); } err.note(&msg); } diff --git a/src/test/ui/imports/auxiliary/overlapping_pub_trait_source.rs b/src/test/ui/imports/auxiliary/overlapping_pub_trait_source.rs new file mode 100644 index 00000000000..2a25d60acff --- /dev/null +++ b/src/test/ui/imports/auxiliary/overlapping_pub_trait_source.rs @@ -0,0 +1,13 @@ +/* This crate declares an item as both `prelude::*` and `m::Tr`. + * The compiler should always suggest `m::Tr`. */ + +pub struct S; + +pub mod prelude { + pub use crate::m::Tr as _; +} + +pub mod m { + pub trait Tr { fn method(&self); } + impl Tr for crate::S { fn method(&self) {} } +} diff --git a/src/test/ui/imports/auxiliary/unnamed_pub_trait_source.rs b/src/test/ui/imports/auxiliary/unnamed_pub_trait_source.rs new file mode 100644 index 00000000000..d73c9a795b6 --- /dev/null +++ b/src/test/ui/imports/auxiliary/unnamed_pub_trait_source.rs @@ -0,0 +1,13 @@ +/* This crate declares an item that is unnamed. + * Its only public path is through `prelude::*`. */ + +pub struct S; + +mod m { + pub trait Tr { fn method(&self); } + impl Tr for crate::S { fn method(&self) {} } +} + +pub mod prelude { + pub use crate::m::Tr as _; +} diff --git a/src/test/ui/imports/overlapping_pub_trait.rs b/src/test/ui/imports/overlapping_pub_trait.rs new file mode 100644 index 00000000000..f5f5d4ed380 --- /dev/null +++ b/src/test/ui/imports/overlapping_pub_trait.rs @@ -0,0 +1,15 @@ +// aux-build:overlapping_pub_trait_source.rs + +/* + * This crate declares two public paths, `m::Tr` and `prelude::_`. Make sure we prefer the former. + */ +extern crate overlapping_pub_trait_source; + +fn main() { + //~^ HELP the following trait is implemented but not in scope; perhaps add a `use` for it: + //~| SUGGESTION overlapping_pub_trait_source::m::Tr + use overlapping_pub_trait_source::S; + S.method(); + //~^ ERROR no method named `method` found for struct `S` in the current scope [E0599] + //~| HELP items from traits can only be used if the trait is in scope +} diff --git a/src/test/ui/imports/overlapping_pub_trait.stderr b/src/test/ui/imports/overlapping_pub_trait.stderr new file mode 100644 index 00000000000..d0c845a5e52 --- /dev/null +++ b/src/test/ui/imports/overlapping_pub_trait.stderr @@ -0,0 +1,20 @@ +error[E0599]: no method named `method` found for struct `S` in the current scope + --> $DIR/overlapping_pub_trait.rs:12:7 + | +LL | S.method(); + | ^^^^^^ method not found in `S` + | + ::: $DIR/auxiliary/overlapping_pub_trait_source.rs:11:23 + | +LL | pub trait Tr { fn method(&self); } + | ------ the method is available for `S` here + | + = help: items from traits can only be used if the trait is in scope +help: the following trait is implemented but not in scope; perhaps add a `use` for it: + | +LL | use overlapping_pub_trait_source::m::Tr; + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. diff --git a/src/test/ui/imports/unnamed_pub_trait.rs b/src/test/ui/imports/unnamed_pub_trait.rs new file mode 100644 index 00000000000..b06b1e1d07d --- /dev/null +++ b/src/test/ui/imports/unnamed_pub_trait.rs @@ -0,0 +1,16 @@ +// aux-build:unnamed_pub_trait_source.rs + +/* + * This crate declares an unnameable public path for our item. Make sure we don't suggest + * importing it by name, and instead we suggest importing it by glob. + */ +extern crate unnamed_pub_trait_source; + +fn main() { + //~^ HELP the following trait is implemented but not in scope; perhaps add a `use` for it: + //~| SUGGESTION unnamed_pub_trait_source::prelude::*; // trait Tr + use unnamed_pub_trait_source::S; + S.method(); + //~^ ERROR no method named `method` found for struct `S` in the current scope [E0599] + //~| HELP items from traits can only be used if the trait is in scope +} diff --git a/src/test/ui/imports/unnamed_pub_trait.stderr b/src/test/ui/imports/unnamed_pub_trait.stderr new file mode 100644 index 00000000000..319dfd7e1b2 --- /dev/null +++ b/src/test/ui/imports/unnamed_pub_trait.stderr @@ -0,0 +1,20 @@ +error[E0599]: no method named `method` found for struct `S` in the current scope + --> $DIR/unnamed_pub_trait.rs:13:7 + | +LL | S.method(); + | ^^^^^^ method not found in `S` + | + ::: $DIR/auxiliary/unnamed_pub_trait_source.rs:7:23 + | +LL | pub trait Tr { fn method(&self); } + | ------ the method is available for `S` here + | + = help: items from traits can only be used if the trait is in scope +help: the following trait is implemented but not in scope; perhaps add a `use` for it: + | +LL | use unnamed_pub_trait_source::prelude::*; // trait Tr + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. |