From 4334aaacf1da33ece0b13d845bf280aeddcb512b Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 12 Oct 2018 13:47:37 +0200 Subject: [PATCH] Only suggest paths that exist. In order to output a path that could actually be imported (valid and visible), we need to handle re-exports correctly. For example, take `std::os::unix::process::CommandExt`, this trait is actually defined at `std::sys::unix::ext::process::CommandExt` (at time of writing). `std::os::unix` rexports the contents of `std::sys::unix::ext`. `std::sys` is private so the "true" path to `CommandExt` isn't accessible. In this case, the visible parent map will look something like this: (child) -> (parent) `std::sys::unix::ext::process::CommandExt` -> `std::sys::unix::ext::process` `std::sys::unix::ext::process` -> `std::sys::unix::ext` `std::sys::unix::ext` -> `std::os` This is correct, as the visible parent of `std::sys::unix::ext` is in fact `std::os`. When printing the path to `CommandExt` and looking at the current segment that corresponds to `std::sys::unix::ext`, we would normally print `ext` and then go to the parent - resulting in a mangled path like `std::os::ext::process::CommandExt`. Instead, we must detect that there was a re-export and instead print `unix` (which is the name `std::sys::unix::ext` was re-exported as in `std::os`). --- src/librustc/ty/item_path.rs | 74 +++++++++++++++++++++++---- src/test/ui/issues/issue-39175.rs | 25 +++++++++ src/test/ui/issues/issue-39175.stderr | 15 ++++++ 3 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 src/test/ui/issues/issue-39175.rs create mode 100644 src/test/ui/issues/issue-39175.stderr diff --git a/src/librustc/ty/item_path.rs b/src/librustc/ty/item_path.rs index ab081324036..472a2c6d9b7 100644 --- a/src/librustc/ty/item_path.rs +++ b/src/librustc/ty/item_path.rs @@ -10,7 +10,7 @@ use hir::map::DefPathData; use hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; -use ty::{self, Ty, TyCtxt}; +use ty::{self, DefIdTree, Ty, TyCtxt}; use middle::cstore::{ExternCrate, ExternCrateSource}; use syntax::ast; use syntax::symbol::{keywords, LocalInternedString, Symbol}; @@ -219,19 +219,73 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { cur_def_key = self.def_key(parent); } + let visible_parent = visible_parent_map.get(&cur_def).cloned(); + let actual_parent = self.parent(cur_def); + debug!( + "try_push_visible_item_path: visible_parent={:?} actual_parent={:?}", + visible_parent, actual_parent, + ); + let data = cur_def_key.disambiguated_data.data; - let symbol = data.get_opt_name().map(|n| n.as_str()).unwrap_or_else(|| { - if let DefPathData::CrateRoot = data { // reexported `extern crate` (#43189) - self.original_crate_name(cur_def.krate).as_str() - } else { - Symbol::intern("").as_str() - } - }); + let symbol = match data { + // In order to output a path that could actually be imported (valid and visible), + // we need to handle re-exports correctly. + // + // For example, take `std::os::unix::process::CommandExt`, this trait is actually + // defined at `std::sys::unix::ext::process::CommandExt` (at time of writing). + // + // `std::os::unix` rexports the contents of `std::sys::unix::ext`. `std::sys` is + // private so the "true" path to `CommandExt` isn't accessible. + // + // In this case, the `visible_parent_map` will look something like this: + // + // (child) -> (parent) + // `std::sys::unix::ext::process::CommandExt` -> `std::sys::unix::ext::process` + // `std::sys::unix::ext::process` -> `std::sys::unix::ext` + // `std::sys::unix::ext` -> `std::os` + // + // This is correct, as the visible parent of `std::sys::unix::ext` is in fact + // `std::os`. + // + // When printing the path to `CommandExt` and looking at the `cur_def_key` that + // corresponds to `std::sys::unix::ext`, we would normally print `ext` and then go + // to the parent - resulting in a mangled path like + // `std::os::ext::process::CommandExt`. + // + // Instead, we must detect that there was a re-export and instead print `unix` + // (which is the name `std::sys::unix::ext` was re-exported as in `std::os`). To + // do this, we compare the parent of `std::sys::unix::ext` (`std::sys::unix`) with + // the visible parent (`std::os`). If these do not match, then we iterate over + // the children of the visible parent (as was done when computing + // `visible_parent_map`), looking for the specific child we currently have and then + // have access to the re-exported name. + DefPathData::Module(module_name) if visible_parent != actual_parent => { + let mut name: Option = None; + if let Some(visible_parent) = visible_parent { + for child in self.item_children(visible_parent).iter() { + if child.def.def_id() == cur_def { + name = Some(child.ident); + } + } + } + name.map(|n| n.as_str()).unwrap_or(module_name.as_str()) + }, + _ => { + data.get_opt_name().map(|n| n.as_str()).unwrap_or_else(|| { + // Re-exported `extern crate` (#43189). + if let DefPathData::CrateRoot = data { + self.original_crate_name(cur_def.krate).as_str() + } else { + Symbol::intern("").as_str() + } + }) + }, + }; debug!("try_push_visible_item_path: symbol={:?}", symbol); cur_path.push(symbol); - match visible_parent_map.get(&cur_def) { - Some(&def) => cur_def = def, + match visible_parent { + Some(def) => cur_def = def, None => return false, }; } diff --git a/src/test/ui/issues/issue-39175.rs b/src/test/ui/issues/issue-39175.rs new file mode 100644 index 00000000000..efe59c31263 --- /dev/null +++ b/src/test/ui/issues/issue-39175.rs @@ -0,0 +1,25 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// This test ignores some platforms as the particular extension trait used +// to demonstrate the issue is only available on unix. This is fine as +// the fix to suggested paths is not platform-dependent and will apply on +// these platforms also. + +// ignore-windows +// ignore-cloudabi +// ignore-emscripten + +use std::process::Command; +// use std::os::unix::process::CommandExt; + +fn main() { + Command::new("echo").arg("hello").exec(); +} diff --git a/src/test/ui/issues/issue-39175.stderr b/src/test/ui/issues/issue-39175.stderr new file mode 100644 index 00000000000..f5611e2e97b --- /dev/null +++ b/src/test/ui/issues/issue-39175.stderr @@ -0,0 +1,15 @@ +error[E0599]: no method named `exec` found for type `&mut std::process::Command` in the current scope + --> $DIR/issue-39175.rs:24:39 + | +LL | Command::new("echo").arg("hello").exec(); + | ^^^^ + | + = 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 std::os::unix::process::CommandExt; + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`.