Skip to content

Commit

Permalink
Auto merge of #46419 - jseyfried:all_imports_in_metadata, r=nrc
Browse files Browse the repository at this point in the history
Record all imports (`use`, `extern crate`) in the crate metadata

This PR adds non-`pub` `use` and `extern crate` imports in the crate metadata since hygienic macros invoked in other crates may use them. We already include all other non-`pub` items in the crate metadata. This improves import suggestions in some cases.

Fixes #42337.

r? @nrc
  • Loading branch information
bors committed Dec 13, 2017
2 parents dcf3db4 + 1b9d058 commit 6110084
Show file tree
Hide file tree
Showing 15 changed files with 142 additions and 28 deletions.
6 changes: 6 additions & 0 deletions src/librustc/hir/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use syntax::ast;
use syntax::ext::base::MacroKind;
use syntax_pos::Span;
use hir;
use ty;

#[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub enum CtorKind {
Expand Down Expand Up @@ -126,6 +127,11 @@ pub struct Export {
pub def: Def,
/// The span of the target definition.
pub span: Span,
/// The visibility of the export.
/// We include non-`pub` exports for hygienic macros that get used from extern crates.
pub vis: ty::Visibility,
/// True if from a `use` or and `extern crate`.
pub is_import: bool,
}

impl CtorKind {
Expand Down
4 changes: 3 additions & 1 deletion src/librustc/ich/impls_hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,9 @@ for hir::def_id::DefIndex {
impl_stable_hash_for!(struct hir::def::Export {
ident,
def,
span
vis,
span,
is_import
});

impl<'gcx> HashStable<StableHashingContext<'gcx>>
Expand Down
9 changes: 9 additions & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,15 @@ impl Visibility {

self.is_accessible_from(vis_restriction, tree)
}

// Returns true if this item is visible anywhere in the local crate.
pub fn is_visible_locally(self) -> bool {
match self {
Visibility::Public => true,
Visibility::Restricted(def_id) => def_id.is_local(),
Visibility::Invisible => false,
}
}
}

#[derive(Clone, PartialEq, RustcDecodable, RustcEncodable, Copy)]
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,12 +305,12 @@ pub fn provide<'tcx>(providers: &mut Providers<'tcx>) {
let mut add_child = |bfs_queue: &mut VecDeque<_>,
child: &def::Export,
parent: DefId| {
let child = child.def.def_id();

if tcx.visibility(child) != ty::Visibility::Public {
if child.vis != ty::Visibility::Public {
return;
}

let child = child.def.def_id();

match visible_parent_map.entry(child) {
Entry::Occupied(mut entry) => {
// If `child` is defined in crate `cnum`, ensure
Expand Down
23 changes: 19 additions & 4 deletions src/librustc_metadata/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,13 @@ impl<'a, 'tcx> CrateMetadata {
ext.kind()
);
let ident = Ident::with_empty_ctxt(name);
callback(def::Export { ident: ident, def: def, span: DUMMY_SP });
callback(def::Export {
ident: ident,
def: def,
vis: ty::Visibility::Public,
span: DUMMY_SP,
is_import: false,
});
}
}
return
Expand Down Expand Up @@ -676,7 +682,9 @@ impl<'a, 'tcx> CrateMetadata {
callback(def::Export {
def,
ident: Ident::from_str(&self.item_name(child_index)),
vis: self.get_visibility(child_index),
span: self.entry(child_index).span.decode((self, sess)),
is_import: false,
});
}
}
Expand All @@ -693,23 +701,30 @@ impl<'a, 'tcx> CrateMetadata {
if let (Some(def), Some(name)) =
(self.get_def(child_index), def_key.disambiguated_data.data.get_opt_name()) {
let ident = Ident::from_str(&name);
callback(def::Export { def: def, ident: ident, span: span });
let vis = self.get_visibility(child_index);
let is_import = false;
callback(def::Export { def, ident, vis, span, is_import });
// For non-reexport structs and variants add their constructors to children.
// Reexport lists automatically contain constructors when necessary.
match def {
Def::Struct(..) => {
if let Some(ctor_def_id) = self.get_struct_ctor_def_id(child_index) {
let ctor_kind = self.get_ctor_kind(child_index);
let ctor_def = Def::StructCtor(ctor_def_id, ctor_kind);
callback(def::Export { def: ctor_def, ident: ident, span: span });
callback(def::Export {
def: ctor_def,
vis: self.get_visibility(ctor_def_id.index),
ident, span, is_import,
});
}
}
Def::Variant(def_id) => {
// Braced variants, unlike structs, generate unusable names in
// value namespace, they are reserved for possible future use.
let ctor_kind = self.get_ctor_kind(child_index);
let ctor_def = Def::VariantCtor(def_id, ctor_kind);
callback(def::Export { def: ctor_def, ident: ident, span: span });
let vis = self.get_visibility(child_index);
callback(def::Export { def: ctor_def, ident, vis, span, is_import });
}
_ => {}
}
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,9 +506,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {

let data = ModData {
reexports: match tcx.module_exports(def_id) {
Some(ref exports) if *vis == hir::Public => {
self.lazy_seq_from_slice(exports.as_slice())
}
Some(ref exports) => self.lazy_seq_from_slice(exports.as_slice()),
_ => LazySeq::empty(),
},
};
Expand Down
13 changes: 12 additions & 1 deletion src/librustc_privacy/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,9 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
if let Some(exports) = self.tcx.module_exports(def_id) {
for export in exports.iter() {
if let Some(node_id) = self.tcx.hir.as_local_node_id(export.def.def_id()) {
self.update(node_id, Some(AccessLevel::Exported));
if export.vis == ty::Visibility::Public {
self.update(node_id, Some(AccessLevel::Exported));
}
}
}
}
Expand Down Expand Up @@ -365,6 +367,15 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
for id in &module.item_ids {
self.update(id.id, level);
}
let def_id = self.tcx.hir.local_def_id(module_id);
if let Some(exports) = self.tcx.module_exports(def_id) {
for export in exports.iter() {
if let Some(node_id) = self.tcx.hir.as_local_node_id(export.def.def_id()) {
self.update(node_id, level);
}
}
}

if module_id == ast::CRATE_NODE_ID {
break
}
Expand Down
8 changes: 3 additions & 5 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,11 +466,8 @@ impl<'a> Resolver<'a> {

/// Builds the reduced graph for a single item in an external crate.
fn build_reduced_graph_for_external_crate_def(&mut self, parent: Module<'a>, child: Export) {
let ident = child.ident;
let def = child.def;
let Export { ident, def, vis, span, .. } = child;
let def_id = def.def_id();
let vis = self.cstore.visibility_untracked(def_id);
let span = child.span;
let expansion = Mark::root(); // FIXME(jseyfried) intercrate hygiene
match def {
Def::Mod(..) | Def::Enum(..) => {
Expand Down Expand Up @@ -674,7 +671,8 @@ impl<'a> Resolver<'a> {
let ident = Ident::with_empty_ctxt(name);
let result = self.resolve_ident_in_module(module, ident, MacroNS, false, false, span);
if let Ok(binding) = result {
self.macro_exports.push(Export { ident: ident, def: binding.def(), span: span });
let (def, vis) = (binding.def(), binding.vis);
self.macro_exports.push(Export { ident, def, vis, span, is_import: true });
} else {
span_err!(self.session, span, E0470, "reexported macro not found");
}
Expand Down
12 changes: 8 additions & 4 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,11 @@ impl<'a> NameBinding<'a> {

// We sometimes need to treat variants as `pub` for backwards compatibility
fn pseudo_vis(&self) -> ty::Visibility {
if self.is_variant() { ty::Visibility::Public } else { self.vis }
if self.is_variant() && self.def().def_id().is_local() {
ty::Visibility::Public
} else {
self.vis
}
}

fn is_variant(&self) -> bool {
Expand Down Expand Up @@ -3613,9 +3617,9 @@ impl<'a> Resolver<'a> {
self.populate_module_if_necessary(in_module);

in_module.for_each_child_stable(|ident, _, name_binding| {
// abort if the module is already found
if let Some(_) = result {
return ();
// 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
}
if let Some(module) = name_binding.module() {
// form the path
Expand Down
9 changes: 7 additions & 2 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,8 +746,13 @@ impl<'a> Resolver<'a> {
}));
if attr::contains_name(&item.attrs, "macro_export") {
let def = Def::Macro(def_id, MacroKind::Bang);
self.macro_exports
.push(Export { ident: ident.modern(), def: def, span: item.span });
self.macro_exports.push(Export {
ident: ident.modern(),
def: def,
vis: ty::Visibility::Public,
span: item.span,
is_import: false,
});
} else {
self.unused_macros.insert(def_id);
}
Expand Down
11 changes: 8 additions & 3 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,8 +898,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
None => continue,
};

if binding.vis == ty::Visibility::Public &&
(binding.is_import() || binding.is_macro_def()) {
if binding.is_import() || binding.is_macro_def() {
let def = binding.def();
if def != Def::Err {
if !def.def_id().is_local() {
Expand All @@ -915,7 +914,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
.emit();
}
}
reexports.push(Export { ident: ident.modern(), def: def, span: binding.span });
reexports.push(Export {
ident: ident.modern(),
def: def,
span: binding.span,
vis: binding.vis,
is_import: true,
});
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ fn build_module(cx: &DocContext, did: DefId) -> clean::Module {
let mut visited = FxHashSet();
for &item in cx.tcx.item_children(did).iter() {
let def_id = item.def.def_id();
if cx.tcx.visibility(def_id) == ty::Visibility::Public {
if item.vis == ty::Visibility::Public {
if !visited.insert(def_id) { continue }
if let Some(i) = try_inline(cx, item.def, item.ident.name) {
items.extend(i)
Expand Down
4 changes: 3 additions & 1 deletion src/librustdoc/visit_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ impl<'a, 'b, 'tcx> LibEmbargoVisitor<'a, 'b, 'tcx> {
}

for item in self.cx.tcx.item_children(def_id).iter() {
self.visit_item(item.def);
if !item.is_import || item.vis == Visibility::Public {
self.visit_item(item.def);
}
}
}

Expand Down
38 changes: 38 additions & 0 deletions src/test/run-pass/hygiene/auxiliary/xcrate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2017 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(decl_macro)]
#![allow(unused)]

pub use bar::test;

extern crate std as foo;

pub fn f() {}
use f as f2;

mod bar {
pub fn g() {}
use baz::h;

pub macro test() {
use std::mem;
use foo::cell;
::f();
::f2();
g();
h();
::bar::h();
}
}

mod baz {
pub fn h() {}
}
21 changes: 21 additions & 0 deletions src/test/run-pass/hygiene/xcrate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2017 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// ignore-pretty pretty-printing is unhygienic

// aux-build:xcrate.rs

#![feature(decl_macro)]

extern crate xcrate;

fn main() {
xcrate::test!();
}

0 comments on commit 6110084

Please sign in to comment.