From a38f903114387601960cd939123d9f7a9033d2d3 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 11 Nov 2018 20:28:56 +0300 Subject: [PATCH] Fix ICEs from imports of items not defined in modules --- src/librustc/hir/def.rs | 14 +++-- src/librustc/middle/dead.rs | 2 +- src/librustc/middle/stability.rs | 6 +- src/librustc_privacy/lib.rs | 8 ++- src/librustc_resolve/macros.rs | 2 +- src/librustc_resolve/resolve_imports.rs | 19 ++++-- .../not-whitelisted.rs | 4 +- .../not-whitelisted.stderr | 20 +++---- .../ui/rust-2018/uniform-paths/macro-rules.rs | 44 ++++++++++++++ .../uniform-paths/macro-rules.stderr | 58 +++++++++++++++++++ .../rust-2018/uniform-paths/prelude-fail.rs | 11 ++++ .../uniform-paths/prelude-fail.stderr | 15 +++++ .../ui/rust-2018/uniform-paths/prelude.rs | 26 +++++++++ 13 files changed, 197 insertions(+), 32 deletions(-) create mode 100644 src/test/ui/rust-2018/uniform-paths/macro-rules.rs create mode 100644 src/test/ui/rust-2018/uniform-paths/macro-rules.stderr create mode 100644 src/test/ui/rust-2018/uniform-paths/prelude-fail.rs create mode 100644 src/test/ui/rust-2018/uniform-paths/prelude-fail.stderr create mode 100644 src/test/ui/rust-2018/uniform-paths/prelude.rs diff --git a/src/librustc/hir/def.rs b/src/librustc/hir/def.rs index 2ca1ff78435..50922ee601d 100644 --- a/src/librustc/hir/def.rs +++ b/src/librustc/hir/def.rs @@ -261,6 +261,12 @@ impl NonMacroAttrKind { impl Def { pub fn def_id(&self) -> DefId { + self.opt_def_id().unwrap_or_else(|| { + bug!("attempted .def_id() on invalid def: {:?}", self) + }) + } + + pub fn opt_def_id(&self) -> Option { match *self { Def::Fn(id) | Def::Mod(id) | Def::Static(id, _) | Def::Variant(id) | Def::VariantCtor(id, ..) | Def::Enum(id) | @@ -268,9 +274,8 @@ impl Def { Def::AssociatedTy(id) | Def::TyParam(id) | Def::Struct(id) | Def::StructCtor(id, ..) | Def::Union(id) | Def::Trait(id) | Def::Method(id) | Def::Const(id) | Def::AssociatedConst(id) | Def::Macro(id, ..) | - Def::Existential(id) | Def::AssociatedExistential(id) | Def::ForeignTy(id) | - Def::SelfCtor(id) => { - id + Def::Existential(id) | Def::AssociatedExistential(id) | Def::ForeignTy(id) => { + Some(id) } Def::Local(..) | @@ -278,10 +283,11 @@ impl Def { Def::Label(..) | Def::PrimTy(..) | Def::SelfTy(..) | + Def::SelfCtor(..) | Def::ToolMod | Def::NonMacroAttr(..) | Def::Err => { - bug!("attempted .def_id() on invalid def: {:?}", self) + None } } } diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs index c5bcfd48cf3..282b5d13e2c 100644 --- a/src/librustc/middle/dead.rs +++ b/src/librustc/middle/dead.rs @@ -80,7 +80,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> { self.check_def_id(def.def_id()); } _ if self.in_pat => (), - Def::PrimTy(..) | Def::SelfTy(..) | + Def::PrimTy(..) | Def::SelfTy(..) | Def::SelfCtor(..) | Def::Local(..) | Def::Upvar(..) => {} Def::Variant(variant_id) | Def::VariantCtor(variant_id, ..) => { if let Some(enum_id) = self.tcx.parent_def_id(variant_id) { diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 5d456481896..543d1053b55 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -781,10 +781,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { fn visit_path(&mut self, path: &'tcx hir::Path, id: hir::HirId) { let id = self.tcx.hir.hir_to_node_id(id); - match path.def { - Def::Local(..) | Def::Upvar(..) | Def::SelfCtor(..) | - Def::PrimTy(..) | Def::SelfTy(..) | Def::Err => {} - _ => self.tcx.check_stability(path.def.def_id(), Some(id), path.span) + if let Some(def_id) = path.def.opt_def_id() { + self.tcx.check_stability(def_id, Some(id), path.span) } intravisit::walk_path(self, path) } diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 4b7bd786672..5f8c7daea6e 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -361,9 +361,11 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> { let def_id = self.tcx.hir.local_def_id(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()) { - if export.vis == ty::Visibility::Public { - self.update(node_id, Some(AccessLevel::Exported)); + if export.vis == ty::Visibility::Public { + if let Some(def_id) = export.def.opt_def_id() { + if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) { + self.update(node_id, Some(AccessLevel::Exported)); + } } } } diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index a795ba0e9d1..600bbbbf5d4 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -227,7 +227,7 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> { let binding = self.arenas.alloc_name_binding(NameBinding { kind: NameBindingKind::Def(Def::Macro(def_id, kind), false), span: DUMMY_SP, - vis: ty::Visibility::Invisible, + vis: ty::Visibility::Public, expansion: Mark::root(), }); if self.builtin_macros.insert(ident.name, binding).is_some() { diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index d98562db3ec..68657fedb42 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -1081,8 +1081,18 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { // this may resolve to either a value or a type, but for documentation // purposes it's good enough to just favor one over the other. self.per_ns(|this, ns| if let Some(binding) = result[ns].get().ok() { + let mut def = binding.def(); + if let Def::Macro(def_id, _) = def { + // `DefId`s from the "built-in macro crate" should not leak from resolve because + // later stages are not ready to deal with them and produce lots of ICEs. Replace + // them with `Def::Err` until some saner scheme is implemented for built-in macros. + if def_id.krate == CrateNum::BuiltinMacros { + this.session.span_err(directive.span, "cannot import a built-in macro"); + def = Def::Err; + } + } let import = this.import_map.entry(directive.id).or_default(); - import[ns] = Some(PathResolution::new(binding.def())); + import[ns] = Some(PathResolution::new(def)); }); debug!("(resolving single import) successfully resolved import"); @@ -1154,9 +1164,10 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { if binding.is_import() || binding.is_macro_def() { let def = binding.def(); if def != Def::Err { - let def_id = def.def_id(); - if !def_id.is_local() && def_id.krate != CrateNum::BuiltinMacros { - self.cstore.export_macros_untracked(def_id.krate); + if let Some(def_id) = def.opt_def_id() { + if !def_id.is_local() && def_id.krate != CrateNum::BuiltinMacros { + self.cstore.export_macros_untracked(def_id.krate); + } } reexports.push(Export { ident: ident.modern(), diff --git a/src/test/ui/rfc-2126-extern-absolute-paths/not-whitelisted.rs b/src/test/ui/rfc-2126-extern-absolute-paths/not-whitelisted.rs index f95961d2a9b..92541afc0c1 100644 --- a/src/test/ui/rfc-2126-extern-absolute-paths/not-whitelisted.rs +++ b/src/test/ui/rfc-2126-extern-absolute-paths/not-whitelisted.rs @@ -13,7 +13,7 @@ // Tests that arbitrary crates (other than `core`, `std` and `meta`) // aren't allowed without `--extern`, even if they're in the sysroot. use alloc; //~ ERROR unresolved import `alloc` -use test; //~ ERROR unresolved import `test` -use proc_macro; //~ ERROR unresolved import `proc_macro` +use test; //~ ERROR cannot import a built-in macro +use proc_macro; // OK, imports the built-in `proc_macro` attribute, but not the `proc_macro` crate. fn main() {} diff --git a/src/test/ui/rfc-2126-extern-absolute-paths/not-whitelisted.stderr b/src/test/ui/rfc-2126-extern-absolute-paths/not-whitelisted.stderr index 0865bd6bea5..e70a359b575 100644 --- a/src/test/ui/rfc-2126-extern-absolute-paths/not-whitelisted.stderr +++ b/src/test/ui/rfc-2126-extern-absolute-paths/not-whitelisted.stderr @@ -1,21 +1,15 @@ +error: cannot import a built-in macro + --> $DIR/not-whitelisted.rs:16:5 + | +LL | use test; //~ ERROR cannot import a built-in macro + | ^^^^ + error[E0432]: unresolved import `alloc` --> $DIR/not-whitelisted.rs:15:5 | LL | use alloc; //~ ERROR unresolved import `alloc` | ^^^^^ no `alloc` external crate -error[E0432]: unresolved import `test` - --> $DIR/not-whitelisted.rs:16:5 - | -LL | use test; //~ ERROR unresolved import `test` - | ^^^^ no `test` external crate - -error[E0432]: unresolved import `proc_macro` - --> $DIR/not-whitelisted.rs:17:5 - | -LL | use proc_macro; //~ ERROR unresolved import `proc_macro` - | ^^^^^^^^^^ no `proc_macro` external crate - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0432`. diff --git a/src/test/ui/rust-2018/uniform-paths/macro-rules.rs b/src/test/ui/rust-2018/uniform-paths/macro-rules.rs new file mode 100644 index 00000000000..1f722228e7d --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/macro-rules.rs @@ -0,0 +1,44 @@ +// edition:2018 + +// For the time being `macro_rules` items are treated as *very* private... + +#![feature(underscore_imports, decl_macro)] + +mod m1 { + macro_rules! legacy_macro { () => () } + + // ... so they can't be imported by themselves, ... + use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported +} + +mod m2 { + macro_rules! legacy_macro { () => () } + + type legacy_macro = u8; + + // ... but don't prevent names from other namespaces from being imported, ... + use legacy_macro as _; // OK +} + +mod m3 { + macro legacy_macro() {} + + fn f() { + macro_rules! legacy_macro { () => () } + + // ... but still create ambiguities with other names in the same namespace. + use legacy_macro as _; //~ ERROR `legacy_macro` is ambiguous + //~| ERROR `legacy_macro` is private, and cannot be re-exported + } +} + +mod exported { + // Exported macros are treated as private as well, + // some better rules need to be figured out later. + #[macro_export] + macro_rules! legacy_macro { () => () } + + use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported +} + +fn main() {} diff --git a/src/test/ui/rust-2018/uniform-paths/macro-rules.stderr b/src/test/ui/rust-2018/uniform-paths/macro-rules.stderr new file mode 100644 index 00000000000..d7bb233dfe9 --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/macro-rules.stderr @@ -0,0 +1,58 @@ +error[E0364]: `legacy_macro` is private, and cannot be re-exported + --> $DIR/macro-rules.rs:11:9 + | +LL | use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported + | ^^^^^^^^^^^^^^^^^ + | +note: consider marking `legacy_macro` as `pub` in the imported module + --> $DIR/macro-rules.rs:11:9 + | +LL | use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported + | ^^^^^^^^^^^^^^^^^ + +error[E0364]: `legacy_macro` is private, and cannot be re-exported + --> $DIR/macro-rules.rs:30:13 + | +LL | use legacy_macro as _; //~ ERROR `legacy_macro` is ambiguous + | ^^^^^^^^^^^^^^^^^ + | +note: consider marking `legacy_macro` as `pub` in the imported module + --> $DIR/macro-rules.rs:30:13 + | +LL | use legacy_macro as _; //~ ERROR `legacy_macro` is ambiguous + | ^^^^^^^^^^^^^^^^^ + +error[E0364]: `legacy_macro` is private, and cannot be re-exported + --> $DIR/macro-rules.rs:41:9 + | +LL | use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported + | ^^^^^^^^^^^^^^^^^ + | +note: consider marking `legacy_macro` as `pub` in the imported module + --> $DIR/macro-rules.rs:41:9 + | +LL | use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported + | ^^^^^^^^^^^^^^^^^ + +error[E0659]: `legacy_macro` is ambiguous (name vs any other name during import resolution) + --> $DIR/macro-rules.rs:30:13 + | +LL | use legacy_macro as _; //~ ERROR `legacy_macro` is ambiguous + | ^^^^^^^^^^^^ ambiguous name + | +note: `legacy_macro` could refer to the macro defined here + --> $DIR/macro-rules.rs:27:9 + | +LL | macro_rules! legacy_macro { () => () } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: `legacy_macro` could also refer to the macro defined here + --> $DIR/macro-rules.rs:24:5 + | +LL | macro legacy_macro() {} + | ^^^^^^^^^^^^^^^^^^^^^^^ + = help: use `self::legacy_macro` to refer to this macro unambiguously + +error: aborting due to 4 previous errors + +Some errors occurred: E0364, E0659. +For more information about an error, try `rustc --explain E0364`. diff --git a/src/test/ui/rust-2018/uniform-paths/prelude-fail.rs b/src/test/ui/rust-2018/uniform-paths/prelude-fail.rs new file mode 100644 index 00000000000..d717884c901 --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/prelude-fail.rs @@ -0,0 +1,11 @@ +// edition:2018 + +// Built-in macro +use env as env_imported; //~ ERROR cannot import a built-in macro + +// Tool attribute +use rustfmt::skip as imported_rustfmt_skip; //~ ERROR unresolved import `rustfmt` + +fn main() { + env_imported!("PATH"); +} diff --git a/src/test/ui/rust-2018/uniform-paths/prelude-fail.stderr b/src/test/ui/rust-2018/uniform-paths/prelude-fail.stderr new file mode 100644 index 00000000000..0ce856c3a31 --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/prelude-fail.stderr @@ -0,0 +1,15 @@ +error: cannot import a built-in macro + --> $DIR/prelude-fail.rs:4:5 + | +LL | use env as env_imported; //~ ERROR cannot import a built-in macro + | ^^^^^^^^^^^^^^^^^^^ + +error[E0432]: unresolved import `rustfmt` + --> $DIR/prelude-fail.rs:7:5 + | +LL | use rustfmt::skip as imported_rustfmt_skip; //~ ERROR unresolved import `rustfmt` + | ^^^^^^^ Not a module `rustfmt` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0432`. diff --git a/src/test/ui/rust-2018/uniform-paths/prelude.rs b/src/test/ui/rust-2018/uniform-paths/prelude.rs new file mode 100644 index 00000000000..ea2dfbb58ce --- /dev/null +++ b/src/test/ui/rust-2018/uniform-paths/prelude.rs @@ -0,0 +1,26 @@ +// compile-pass +// edition:2018 + +// Macro imported with `#[macro_use] extern crate` +use vec as imported_vec; + +// Built-in attribute +use inline as imported_inline; + +// Tool module +use rustfmt as imported_rustfmt; + +// Standard library prelude +use Vec as ImportedVec; + +// Built-in type +use u8 as imported_u8; + +type A = imported_u8; + +#[imported_inline] +#[imported_rustfmt::skip] +fn main() { + imported_vec![0]; + ImportedVec::::new(); +}