Skip to content

Commit 4334aaa

Browse files
committed
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`).
1 parent b8b4150 commit 4334aaa

File tree

3 files changed

+104
-10
lines changed

3 files changed

+104
-10
lines changed

src/librustc/ty/item_path.rs

+64-10
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
use hir::map::DefPathData;
1212
use hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE};
13-
use ty::{self, Ty, TyCtxt};
13+
use ty::{self, DefIdTree, Ty, TyCtxt};
1414
use middle::cstore::{ExternCrate, ExternCrateSource};
1515
use syntax::ast;
1616
use syntax::symbol::{keywords, LocalInternedString, Symbol};
@@ -219,19 +219,73 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
219219
cur_def_key = self.def_key(parent);
220220
}
221221

222+
let visible_parent = visible_parent_map.get(&cur_def).cloned();
223+
let actual_parent = self.parent(cur_def);
224+
debug!(
225+
"try_push_visible_item_path: visible_parent={:?} actual_parent={:?}",
226+
visible_parent, actual_parent,
227+
);
228+
222229
let data = cur_def_key.disambiguated_data.data;
223-
let symbol = data.get_opt_name().map(|n| n.as_str()).unwrap_or_else(|| {
224-
if let DefPathData::CrateRoot = data { // reexported `extern crate` (#43189)
225-
self.original_crate_name(cur_def.krate).as_str()
226-
} else {
227-
Symbol::intern("<unnamed>").as_str()
228-
}
229-
});
230+
let symbol = match data {
231+
// In order to output a path that could actually be imported (valid and visible),
232+
// we need to handle re-exports correctly.
233+
//
234+
// For example, take `std::os::unix::process::CommandExt`, this trait is actually
235+
// defined at `std::sys::unix::ext::process::CommandExt` (at time of writing).
236+
//
237+
// `std::os::unix` rexports the contents of `std::sys::unix::ext`. `std::sys` is
238+
// private so the "true" path to `CommandExt` isn't accessible.
239+
//
240+
// In this case, the `visible_parent_map` will look something like this:
241+
//
242+
// (child) -> (parent)
243+
// `std::sys::unix::ext::process::CommandExt` -> `std::sys::unix::ext::process`
244+
// `std::sys::unix::ext::process` -> `std::sys::unix::ext`
245+
// `std::sys::unix::ext` -> `std::os`
246+
//
247+
// This is correct, as the visible parent of `std::sys::unix::ext` is in fact
248+
// `std::os`.
249+
//
250+
// When printing the path to `CommandExt` and looking at the `cur_def_key` that
251+
// corresponds to `std::sys::unix::ext`, we would normally print `ext` and then go
252+
// to the parent - resulting in a mangled path like
253+
// `std::os::ext::process::CommandExt`.
254+
//
255+
// Instead, we must detect that there was a re-export and instead print `unix`
256+
// (which is the name `std::sys::unix::ext` was re-exported as in `std::os`). To
257+
// do this, we compare the parent of `std::sys::unix::ext` (`std::sys::unix`) with
258+
// the visible parent (`std::os`). If these do not match, then we iterate over
259+
// the children of the visible parent (as was done when computing
260+
// `visible_parent_map`), looking for the specific child we currently have and then
261+
// have access to the re-exported name.
262+
DefPathData::Module(module_name) if visible_parent != actual_parent => {
263+
let mut name: Option<ast::Ident> = None;
264+
if let Some(visible_parent) = visible_parent {
265+
for child in self.item_children(visible_parent).iter() {
266+
if child.def.def_id() == cur_def {
267+
name = Some(child.ident);
268+
}
269+
}
270+
}
271+
name.map(|n| n.as_str()).unwrap_or(module_name.as_str())
272+
},
273+
_ => {
274+
data.get_opt_name().map(|n| n.as_str()).unwrap_or_else(|| {
275+
// Re-exported `extern crate` (#43189).
276+
if let DefPathData::CrateRoot = data {
277+
self.original_crate_name(cur_def.krate).as_str()
278+
} else {
279+
Symbol::intern("<unnamed>").as_str()
280+
}
281+
})
282+
},
283+
};
230284
debug!("try_push_visible_item_path: symbol={:?}", symbol);
231285
cur_path.push(symbol);
232286

233-
match visible_parent_map.get(&cur_def) {
234-
Some(&def) => cur_def = def,
287+
match visible_parent {
288+
Some(def) => cur_def = def,
235289
None => return false,
236290
};
237291
}

src/test/ui/issues/issue-39175.rs

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// This test ignores some platforms as the particular extension trait used
12+
// to demonstrate the issue is only available on unix. This is fine as
13+
// the fix to suggested paths is not platform-dependent and will apply on
14+
// these platforms also.
15+
16+
// ignore-windows
17+
// ignore-cloudabi
18+
// ignore-emscripten
19+
20+
use std::process::Command;
21+
// use std::os::unix::process::CommandExt;
22+
23+
fn main() {
24+
Command::new("echo").arg("hello").exec();
25+
}

src/test/ui/issues/issue-39175.stderr

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error[E0599]: no method named `exec` found for type `&mut std::process::Command` in the current scope
2+
--> $DIR/issue-39175.rs:24:39
3+
|
4+
LL | Command::new("echo").arg("hello").exec();
5+
| ^^^^
6+
|
7+
= help: items from traits can only be used if the trait is in scope
8+
help: the following trait is implemented but not in scope, perhaps add a `use` for it:
9+
|
10+
LL | use std::os::unix::process::CommandExt;
11+
|
12+
13+
error: aborting due to previous error
14+
15+
For more information about this error, try `rustc --explain E0599`.

0 commit comments

Comments
 (0)