Skip to content

Commit a38f903

Browse files
petrochenkovpetrochenkov
committed
Fix ICEs from imports of items not defined in modules
1 parent cfd7629 commit a38f903

File tree

13 files changed

+197
-32
lines changed

13 files changed

+197
-32
lines changed

‎src/librustc/hir/def.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -261,27 +261,33 @@ impl NonMacroAttrKind {
261261

262262
impl Def {
263263
pub fn def_id(&self) -> DefId {
264+
self.opt_def_id().unwrap_or_else(|| {
265+
bug!("attempted .def_id() on invalid def: {:?}", self)
266+
})
267+
}
268+
269+
pub fn opt_def_id(&self) -> Option<DefId> {
264270
match *self {
265271
Def::Fn(id) | Def::Mod(id) | Def::Static(id, _) |
266272
Def::Variant(id) | Def::VariantCtor(id, ..) | Def::Enum(id) |
267273
Def::TyAlias(id) | Def::TraitAlias(id) |
268274
Def::AssociatedTy(id) | Def::TyParam(id) | Def::Struct(id) | Def::StructCtor(id, ..) |
269275
Def::Union(id) | Def::Trait(id) | Def::Method(id) | Def::Const(id) |
270276
Def::AssociatedConst(id) | Def::Macro(id, ..) |
271-
Def::Existential(id) | Def::AssociatedExistential(id) | Def::ForeignTy(id) |
272-
Def::SelfCtor(id) => {
273-
id
277+
Def::Existential(id) | Def::AssociatedExistential(id) | Def::ForeignTy(id) => {
278+
Some(id)
274279
}
275280

276281
Def::Local(..) |
277282
Def::Upvar(..) |
278283
Def::Label(..) |
279284
Def::PrimTy(..) |
280285
Def::SelfTy(..) |
286+
Def::SelfCtor(..) |
281287
Def::ToolMod |
282288
Def::NonMacroAttr(..) |
283289
Def::Err => {
284-
bug!("attempted .def_id() on invalid def: {:?}", self)
290+
None
285291
}
286292
}
287293
}

‎src/librustc/middle/dead.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
8080
self.check_def_id(def.def_id());
8181
}
8282
_ if self.in_pat => (),
83-
Def::PrimTy(..) | Def::SelfTy(..) |
83+
Def::PrimTy(..) | Def::SelfTy(..) | Def::SelfCtor(..) |
8484
Def::Local(..) | Def::Upvar(..) => {}
8585
Def::Variant(variant_id) | Def::VariantCtor(variant_id, ..) => {
8686
if let Some(enum_id) = self.tcx.parent_def_id(variant_id) {

‎src/librustc/middle/stability.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -781,10 +781,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> {
781781

782782
fn visit_path(&mut self, path: &'tcx hir::Path, id: hir::HirId) {
783783
let id = self.tcx.hir.hir_to_node_id(id);
784-
match path.def {
785-
Def::Local(..) | Def::Upvar(..) | Def::SelfCtor(..) |
786-
Def::PrimTy(..) | Def::SelfTy(..) | Def::Err => {}
787-
_ => self.tcx.check_stability(path.def.def_id(), Some(id), path.span)
784+
if let Some(def_id) = path.def.opt_def_id() {
785+
self.tcx.check_stability(def_id, Some(id), path.span)
788786
}
789787
intravisit::walk_path(self, path)
790788
}

‎src/librustc_privacy/lib.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -361,9 +361,11 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
361361
let def_id = self.tcx.hir.local_def_id(id);
362362
if let Some(exports) = self.tcx.module_exports(def_id) {
363363
for export in exports.iter() {
364-
if let Some(node_id) = self.tcx.hir.as_local_node_id(export.def.def_id()) {
365-
if export.vis == ty::Visibility::Public {
366-
self.update(node_id, Some(AccessLevel::Exported));
364+
if export.vis == ty::Visibility::Public {
365+
if let Some(def_id) = export.def.opt_def_id() {
366+
if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) {
367+
self.update(node_id, Some(AccessLevel::Exported));
368+
}
367369
}
368370
}
369371
}

‎src/librustc_resolve/macros.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
227227
let binding = self.arenas.alloc_name_binding(NameBinding {
228228
kind: NameBindingKind::Def(Def::Macro(def_id, kind), false),
229229
span: DUMMY_SP,
230-
vis: ty::Visibility::Invisible,
230+
vis: ty::Visibility::Public,
231231
expansion: Mark::root(),
232232
});
233233
if self.builtin_macros.insert(ident.name, binding).is_some() {

‎src/librustc_resolve/resolve_imports.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,8 +1081,18 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
10811081
// this may resolve to either a value or a type, but for documentation
10821082
// purposes it's good enough to just favor one over the other.
10831083
self.per_ns(|this, ns| if let Some(binding) = result[ns].get().ok() {
1084+
let mut def = binding.def();
1085+
if let Def::Macro(def_id, _) = def {
1086+
// `DefId`s from the "built-in macro crate" should not leak from resolve because
1087+
// later stages are not ready to deal with them and produce lots of ICEs. Replace
1088+
// them with `Def::Err` until some saner scheme is implemented for built-in macros.
1089+
if def_id.krate == CrateNum::BuiltinMacros {
1090+
this.session.span_err(directive.span, "cannot import a built-in macro");
1091+
def = Def::Err;
1092+
}
1093+
}
10841094
let import = this.import_map.entry(directive.id).or_default();
1085-
import[ns] = Some(PathResolution::new(binding.def()));
1095+
import[ns] = Some(PathResolution::new(def));
10861096
});
10871097

10881098
debug!("(resolving single import) successfully resolved import");
@@ -1154,9 +1164,10 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
11541164
if binding.is_import() || binding.is_macro_def() {
11551165
let def = binding.def();
11561166
if def != Def::Err {
1157-
let def_id = def.def_id();
1158-
if !def_id.is_local() && def_id.krate != CrateNum::BuiltinMacros {
1159-
self.cstore.export_macros_untracked(def_id.krate);
1167+
if let Some(def_id) = def.opt_def_id() {
1168+
if !def_id.is_local() && def_id.krate != CrateNum::BuiltinMacros {
1169+
self.cstore.export_macros_untracked(def_id.krate);
1170+
}
11601171
}
11611172
reexports.push(Export {
11621173
ident: ident.modern(),

‎src/test/ui/rfc-2126-extern-absolute-paths/not-whitelisted.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
// Tests that arbitrary crates (other than `core`, `std` and `meta`)
1414
// aren't allowed without `--extern`, even if they're in the sysroot.
1515
use alloc; //~ ERROR unresolved import `alloc`
16-
use test; //~ ERROR unresolved import `test`
17-
use proc_macro; //~ ERROR unresolved import `proc_macro`
16+
use test; //~ ERROR cannot import a built-in macro
17+
use proc_macro; // OK, imports the built-in `proc_macro` attribute, but not the `proc_macro` crate.
1818

1919
fn main() {}

‎src/test/ui/rfc-2126-extern-absolute-paths/not-whitelisted.stderr

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,15 @@
1+
error: cannot import a built-in macro
2+
--> $DIR/not-whitelisted.rs:16:5
3+
|
4+
LL | use test; //~ ERROR cannot import a built-in macro
5+
| ^^^^
6+
17
error[E0432]: unresolved import `alloc`
28
--> $DIR/not-whitelisted.rs:15:5
39
|
410
LL | use alloc; //~ ERROR unresolved import `alloc`
511
| ^^^^^ no `alloc` external crate
612

7-
error[E0432]: unresolved import `test`
8-
--> $DIR/not-whitelisted.rs:16:5
9-
|
10-
LL | use test; //~ ERROR unresolved import `test`
11-
| ^^^^ no `test` external crate
12-
13-
error[E0432]: unresolved import `proc_macro`
14-
--> $DIR/not-whitelisted.rs:17:5
15-
|
16-
LL | use proc_macro; //~ ERROR unresolved import `proc_macro`
17-
| ^^^^^^^^^^ no `proc_macro` external crate
18-
19-
error: aborting due to 3 previous errors
13+
error: aborting due to 2 previous errors
2014

2115
For more information about this error, try `rustc --explain E0432`.

‎src/test/ui/rust-2018/uniform-paths/macro-rules.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// edition:2018
2+
3+
// For the time being `macro_rules` items are treated as *very* private...
4+
5+
#![feature(underscore_imports, decl_macro)]
6+
7+
mod m1 {
8+
macro_rules! legacy_macro { () => () }
9+
10+
// ... so they can't be imported by themselves, ...
11+
use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported
12+
}
13+
14+
mod m2 {
15+
macro_rules! legacy_macro { () => () }
16+
17+
type legacy_macro = u8;
18+
19+
// ... but don't prevent names from other namespaces from being imported, ...
20+
use legacy_macro as _; // OK
21+
}
22+
23+
mod m3 {
24+
macro legacy_macro() {}
25+
26+
fn f() {
27+
macro_rules! legacy_macro { () => () }
28+
29+
// ... but still create ambiguities with other names in the same namespace.
30+
use legacy_macro as _; //~ ERROR `legacy_macro` is ambiguous
31+
//~| ERROR `legacy_macro` is private, and cannot be re-exported
32+
}
33+
}
34+
35+
mod exported {
36+
// Exported macros are treated as private as well,
37+
// some better rules need to be figured out later.
38+
#[macro_export]
39+
macro_rules! legacy_macro { () => () }
40+
41+
use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported
42+
}
43+
44+
fn main() {}

‎src/test/ui/rust-2018/uniform-paths/macro-rules.stderr

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
error[E0364]: `legacy_macro` is private, and cannot be re-exported
2+
--> $DIR/macro-rules.rs:11:9
3+
|
4+
LL | use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported
5+
| ^^^^^^^^^^^^^^^^^
6+
|
7+
note: consider marking `legacy_macro` as `pub` in the imported module
8+
--> $DIR/macro-rules.rs:11:9
9+
|
10+
LL | use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported
11+
| ^^^^^^^^^^^^^^^^^
12+
13+
error[E0364]: `legacy_macro` is private, and cannot be re-exported
14+
--> $DIR/macro-rules.rs:30:13
15+
|
16+
LL | use legacy_macro as _; //~ ERROR `legacy_macro` is ambiguous
17+
| ^^^^^^^^^^^^^^^^^
18+
|
19+
note: consider marking `legacy_macro` as `pub` in the imported module
20+
--> $DIR/macro-rules.rs:30:13
21+
|
22+
LL | use legacy_macro as _; //~ ERROR `legacy_macro` is ambiguous
23+
| ^^^^^^^^^^^^^^^^^
24+
25+
error[E0364]: `legacy_macro` is private, and cannot be re-exported
26+
--> $DIR/macro-rules.rs:41:9
27+
|
28+
LL | use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported
29+
| ^^^^^^^^^^^^^^^^^
30+
|
31+
note: consider marking `legacy_macro` as `pub` in the imported module
32+
--> $DIR/macro-rules.rs:41:9
33+
|
34+
LL | use legacy_macro as _; //~ ERROR `legacy_macro` is private, and cannot be re-exported
35+
| ^^^^^^^^^^^^^^^^^
36+
37+
error[E0659]: `legacy_macro` is ambiguous (name vs any other name during import resolution)
38+
--> $DIR/macro-rules.rs:30:13
39+
|
40+
LL | use legacy_macro as _; //~ ERROR `legacy_macro` is ambiguous
41+
| ^^^^^^^^^^^^ ambiguous name
42+
|
43+
note: `legacy_macro` could refer to the macro defined here
44+
--> $DIR/macro-rules.rs:27:9
45+
|
46+
LL | macro_rules! legacy_macro { () => () }
47+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
48+
note: `legacy_macro` could also refer to the macro defined here
49+
--> $DIR/macro-rules.rs:24:5
50+
|
51+
LL | macro legacy_macro() {}
52+
| ^^^^^^^^^^^^^^^^^^^^^^^
53+
= help: use `self::legacy_macro` to refer to this macro unambiguously
54+
55+
error: aborting due to 4 previous errors
56+
57+
Some errors occurred: E0364, E0659.
58+
For more information about an error, try `rustc --explain E0364`.

0 commit comments

Comments
 (0)