Rollup merge of #57483 - petrochenkov:beta, r=pietroalbini

Author: pietroalbini

src/librustc/hir/def.rs

@@ -248,7 +248,7 @@ impl CtorKind {
 }
 
 impl NonMacroAttrKind {
-    fn descr(self) -> &'static str {
+    pub fn descr(self) -> &'static str {
         match self {
             NonMacroAttrKind::Builtin => "built-in attribute",
             NonMacroAttrKind::Tool => "tool attribute",

src/librustc_resolve/build_reduced_graph.rs

@@ -230,13 +230,18 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                 }
 
                 let subclass = SingleImport {
-                    target: ident,
                     source: source.ident,
-                    result: PerNS {
+                    target: ident,
+                    source_bindings: PerNS {
                         type_ns: Cell::new(Err(Undetermined)),
                         value_ns: Cell::new(Err(Undetermined)),
                         macro_ns: Cell::new(Err(Undetermined)),
                     },
+                    target_bindings: PerNS {
+                        type_ns: Cell::new(None),
+                        value_ns: Cell::new(None),
+                        macro_ns: Cell::new(None),
+                    },
                     type_ns_only,
                 };
                 self.add_import_directive(

src/librustc_resolve/lib.rs

@@ -75,7 +75,7 @@ use syntax_pos::{Span, DUMMY_SP, MultiSpan};
 use errors::{Applicability, DiagnosticBuilder, DiagnosticId};
 
 use std::cell::{Cell, RefCell};
-use std::{cmp, fmt, iter, ptr};
+use std::{cmp, fmt, iter, mem, ptr};
 use std::collections::BTreeSet;
 use std::mem::replace;
 use rustc_data_structures::ptr_key::PtrKey;
@@ -1521,6 +1521,7 @@ pub struct Resolver<'a, 'b: 'a> {
 
     /// FIXME: Refactor things so that this is passed through arguments and not resolver.
     last_import_segment: bool,
+    blacklisted_binding: Option<&'a NameBinding<'a>>,
 
     /// The idents for the primitive types.
     primitive_type_table: PrimitiveTypeTable,
@@ -1871,6 +1872,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             current_self_type: None,
             current_self_item: None,
             last_import_segment: false,
+            blacklisted_binding: None,
 
             primitive_type_table: PrimitiveTypeTable::new(),
 
@@ -2392,11 +2394,27 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                 ast::UseTreeKind::Simple(..) if segments.len() == 1 => &[TypeNS, ValueNS][..],
                 _ => &[TypeNS],
             };
+            let report_error = |this: &Self, ns| {
+                let what = if ns == TypeNS { "type parameters" } else { "local variables" };
+                this.session.span_err(ident.span, &format!("imports cannot refer to {}", what));
+            };
+
             for &ns in nss {
-                if let Some(LexicalScopeBinding::Def(..)) =
-                        self.resolve_ident_in_lexical_scope(ident, ns, None, use_tree.prefix.span) {
-                    let what = if ns == TypeNS { "type parameters" } else { "local variables" };
-                    self.session.span_err(ident.span, &format!("imports cannot refer to {}", what));
+                match self.resolve_ident_in_lexical_scope(ident, ns, None, use_tree.prefix.span) {
+                    Some(LexicalScopeBinding::Def(..)) => {
+                        report_error(self, ns);
+                    }
+                    Some(LexicalScopeBinding::Item(binding)) => {
+                        let orig_blacklisted_binding =
+                            mem::replace(&mut self.blacklisted_binding, Some(binding));
+                        if let Some(LexicalScopeBinding::Def(..)) =
+                                self.resolve_ident_in_lexical_scope(ident, ns, None,
+                                                                    use_tree.prefix.span) {
+                            report_error(self, ns);
+                        }
+                        self.blacklisted_binding = orig_blacklisted_binding;
+                    }
+                    None => {}
                 }
             }
         } else if let ast::UseTreeKind::Nested(use_trees) = &use_tree.kind {
@@ -3858,6 +3876,13 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                         module = Some(ModuleOrUniformRoot::Module(next_module));
                         record_segment_def(self, def);
                     } else if def == Def::ToolMod && i + 1 != path.len() {
+                        if binding.is_import() {
+                            self.session.struct_span_err(
+                                ident.span, "cannot use a tool module through an import"
+                            ).span_note(
+                                binding.span, "the tool module imported here"
+                            ).emit();
+                        }
                         let def = Def::NonMacroAttr(NonMacroAttrKind::Tool);
                         return PathResult::NonModule(PathResolution::new(def));
                     } else if def == Def::Err {

src/librustc_resolve/macros.rs

@@ -499,6 +499,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     .push((path, path_span, kind, parent_scope.clone(), def.ok()));
             }
 
+            self.prohibit_imported_non_macro_attrs(None, def.ok(), path_span);
             def
         } else {
             let binding = self.early_resolve_ident_in_lexical_scope(
@@ -515,7 +516,9 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     .push((path[0].ident, kind, parent_scope.clone(), binding.ok()));
             }
 
-            binding.map(|binding| binding.def_ignoring_ambiguity())
+            let def = binding.map(|binding| binding.def_ignoring_ambiguity());
+            self.prohibit_imported_non_macro_attrs(binding.ok(), def.ok(), path_span);
+            def
         }
     }
 
@@ -953,36 +956,34 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             // but its `Def` should coincide with a crate passed with `--extern`
             // (otherwise there would be ambiguity) and we can skip feature error in this case.
             'ok: {
-                if !is_import || self.session.features_untracked().uniform_paths {
+                if !is_import || !rust_2015 {
                     break 'ok;
                 }
                 if ns == TypeNS && use_prelude && self.extern_prelude_get(ident, true).is_some() {
                     break 'ok;
                 }
-                if rust_2015 {
-                    let root_ident = Ident::new(keywords::CrateRoot.name(), orig_ident.span);
-                    let root_module = self.resolve_crate_root(root_ident);
-                    if self.resolve_ident_in_module_ext(ModuleOrUniformRoot::Module(root_module),
-                                                        orig_ident, ns, None, false, path_span)
-                                                        .is_ok() {
-                        break 'ok;
-                    }
+                let root_ident = Ident::new(keywords::CrateRoot.name(), orig_ident.span);
+                let root_module = self.resolve_crate_root(root_ident);
+                if self.resolve_ident_in_module_ext(ModuleOrUniformRoot::Module(root_module),
+                                                    orig_ident, ns, None, false, path_span)
+                                                    .is_ok() {
+                    break 'ok;
                 }
 
-                let msg = "imports can only refer to extern crate names \
-                           passed with `--extern` on stable channel";
-                let mut err = feature_err(&self.session.parse_sess, "uniform_paths",
-                                          ident.span, GateIssue::Language, msg);
-
+                let msg = "imports can only refer to extern crate names passed with \
+                           `--extern` in macros originating from 2015 edition";
+                let mut err = self.session.struct_span_err(ident.span, msg);
                 let what = self.binding_description(binding, ident,
                                                     flags.contains(Flags::MISC_FROM_PRELUDE));
                 let note_msg = format!("this import refers to {what}", what = what);
-                if binding.span.is_dummy() {
+                let label_span = if binding.span.is_dummy() {
                     err.note(&note_msg);
+                    ident.span
                 } else {
                     err.span_note(binding.span, &note_msg);
-                    err.span_label(binding.span, "not an extern crate passed with `--extern`");
-                }
+                    binding.span
+                };
+                err.span_label(label_span, "not an extern crate passed with `--extern`");
                 err.emit();
             }
 
@@ -1091,6 +1092,20 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         }
     }
 
+    fn prohibit_imported_non_macro_attrs(&self, binding: Option<&'a NameBinding<'a>>,
+                                         def: Option<Def>, span: Span) {
+        if let Some(Def::NonMacroAttr(kind)) = def {
+            if kind != NonMacroAttrKind::Tool && binding.map_or(true, |b| b.is_import()) {
+                let msg = format!("cannot use a {} through an import", kind.descr());
+                let mut err = self.session.struct_span_err(span, &msg);
+                if let Some(binding) = binding {
+                    err.span_note(binding.span, &format!("the {} imported here", kind.descr()));
+                }
+                err.emit();
+            }
+        }
+    }
+
     fn suggest_macro_name(&mut self, name: &str, kind: MacroKind,
                           err: &mut DiagnosticBuilder<'a>, span: Span) {
         // First check if this is a locally-defined bang macro.
@@ -1187,17 +1202,21 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             let ident = ident.modern();
             self.macro_names.insert(ident);
             let def = Def::Macro(def_id, MacroKind::Bang);
-            let vis = ty::Visibility::Invisible; // Doesn't matter for legacy bindings
+            let is_macro_export = attr::contains_name(&item.attrs, "macro_export");
+            let vis = if is_macro_export {
+                ty::Visibility::Public
+            } else {
+                ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX))
+            };
             let binding = (def, vis, item.span, expansion).to_name_binding(self.arenas);
             self.set_binding_parent_module(binding, self.current_module);
             let legacy_binding = self.arenas.alloc_legacy_binding(LegacyBinding {
                 parent_legacy_scope: *current_legacy_scope, binding, ident
             });
             *current_legacy_scope = LegacyScope::Binding(legacy_binding);
             self.all_macros.insert(ident.name, def);
-            if attr::contains_name(&item.attrs, "macro_export") {
+            if is_macro_export {
                 let module = self.graph_root;
-                let vis = ty::Visibility::Public;
                 self.define(module, ident, MacroNS,
                             (def, vis, item.span, expansion, IsMacroExport));
             } else {

src/librustc_resolve/resolve_imports.rs

@@ -42,9 +42,10 @@ use std::{mem, ptr};
 #[derive(Clone, Debug)]
 pub enum ImportDirectiveSubclass<'a> {
     SingleImport {
-        target: Ident,
         source: Ident,
-        result: PerNS<Cell<Result<&'a NameBinding<'a>, Determinacy>>>,
+        target: Ident,
+        source_bindings: PerNS<Cell<Result<&'a NameBinding<'a>, Determinacy>>>,
+        target_bindings: PerNS<Cell<Option<&'a NameBinding<'a>>>>,
         type_ns_only: bool,
     },
     GlobImport {
@@ -227,14 +228,30 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
         }
 
         let check_usable = |this: &mut Self, binding: &'a NameBinding<'a>| {
+            if let Some(blacklisted_binding) = this.blacklisted_binding {
+                if ptr::eq(binding, blacklisted_binding) {
+                    return Err((Determined, Weak::No));
+                }
+            }
             // `extern crate` are always usable for backwards compatibility, see issue #37020,
             // remove this together with `PUB_USE_OF_PRIVATE_EXTERN_CRATE`.
             let usable = this.is_accessible(binding.vis) || binding.is_extern_crate();
             if usable { Ok(binding) } else { Err((Determined, Weak::No)) }
         };
 
         if record_used {
-            return resolution.binding.ok_or((Determined, Weak::No)).and_then(|binding| {
+            return resolution.binding.and_then(|binding| {
+                // If the primary binding is blacklisted, search further and return the shadowed
+                // glob binding if it exists. What we really want here is having two separate
+                // scopes in a module - one for non-globs and one for globs, but until that's done
+                // use this hack to avoid inconsistent resolution ICEs during import validation.
+                if let Some(blacklisted_binding) = self.blacklisted_binding {
+                    if ptr::eq(binding, blacklisted_binding) {
+                        return resolution.shadowed_glob;
+                    }
+                }
+                Some(binding)
+            }).ok_or((Determined, Weak::No)).and_then(|binding| {
                 if self.last_import_segment && check_usable(self, binding).is_err() {
                     Err((Determined, Weak::No))
                 } else {
@@ -646,10 +663,10 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
             if let Some((span, err, note)) = self.finalize_import(import) {
                 errors = true;
 
-                if let SingleImport { source, ref result, .. } = import.subclass {
+                if let SingleImport { source, ref source_bindings, .. } = import.subclass {
                     if source.name == "self" {
                         // Silence `unresolved import` error if E0429 is already emitted
-                        if let Err(Determined) = result.value_ns.get() {
+                        if let Err(Determined) = source_bindings.value_ns.get() {
                             continue;
                         }
                     }
@@ -769,9 +786,11 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
         };
 
         directive.imported_module.set(Some(module));
-        let (source, target, result, type_ns_only) = match directive.subclass {
-            SingleImport { source, target, ref result, type_ns_only } =>
-                (source, target, result, type_ns_only),
+        let (source, target, source_bindings, target_bindings, type_ns_only) =
+                match directive.subclass {
+            SingleImport { source, target, ref source_bindings,
+                           ref target_bindings, type_ns_only } =>
+                (source, target, source_bindings, target_bindings, type_ns_only),
             GlobImport { .. } => {
                 self.resolve_glob_import(directive);
                 return true;
@@ -781,7 +800,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
 
         let mut indeterminate = false;
         self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
-            if let Err(Undetermined) = result[ns].get() {
+            if let Err(Undetermined) = source_bindings[ns].get() {
                 // For better failure detection, pretend that the import will
                 // not define any names while resolving its module path.
                 let orig_vis = directive.vis.replace(ty::Visibility::Invisible);
@@ -790,13 +809,13 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
                 );
                 directive.vis.set(orig_vis);
 
-                result[ns].set(binding);
+                source_bindings[ns].set(binding);
             } else {
                 return
             };
 
             let parent = directive.parent_scope.module;
-            match result[ns].get() {
+            match source_bindings[ns].get() {
                 Err(Undetermined) => indeterminate = true,
                 Err(Determined) => {
                     this.update_resolution(parent, target, ns, |_, resolution| {
@@ -814,6 +833,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
                 }
                 Ok(binding) => {
                     let imported_binding = this.import(binding, directive);
+                    target_bindings[ns].set(Some(imported_binding));
                     let conflict = this.try_define(parent, target, ns, imported_binding);
                     if let Err(old_binding) = conflict {
                         this.report_conflict(parent, target, ns, imported_binding, old_binding);
@@ -885,8 +905,9 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
             PathResult::Indeterminate | PathResult::NonModule(..) => unreachable!(),
         };
 
-        let (ident, result, type_ns_only) = match directive.subclass {
-            SingleImport { source, ref result, type_ns_only, .. } => (source, result, type_ns_only),
+        let (ident, source_bindings, target_bindings, type_ns_only) = match directive.subclass {
+            SingleImport { source, ref source_bindings, ref target_bindings, type_ns_only, .. } =>
+                (source, source_bindings, target_bindings, type_ns_only),
             GlobImport { is_prelude, ref max_vis } => {
                 if directive.module_path.len() <= 1 {
                     // HACK(eddyb) `lint_if_path_starts_with_module` needs at least
@@ -925,17 +946,20 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
         let mut all_ns_err = true;
         self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
             let orig_vis = directive.vis.replace(ty::Visibility::Invisible);
+            let orig_blacklisted_binding =
+                mem::replace(&mut this.blacklisted_binding, target_bindings[ns].get());
             let orig_last_import_segment = mem::replace(&mut this.last_import_segment, true);
             let binding = this.resolve_ident_in_module(
                 module, ident, ns, Some(&directive.parent_scope), true, directive.span
             );
             this.last_import_segment = orig_last_import_segment;
+            this.blacklisted_binding = orig_blacklisted_binding;
             directive.vis.set(orig_vis);
 
             match binding {
                 Ok(binding) => {
                     // Consistency checks, analogous to `finalize_current_module_macro_resolutions`.
-                    let initial_def = result[ns].get().map(|initial_binding| {
+                    let initial_def = source_bindings[ns].get().map(|initial_binding| {
                         all_ns_err = false;
                         this.record_use(ident, ns, initial_binding,
                                         directive.module_path.is_empty());
@@ -1040,7 +1064,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
         let mut reexport_error = None;
         let mut any_successful_reexport = false;
         self.per_ns(|this, ns| {
-            if let Ok(binding) = result[ns].get() {
+            if let Ok(binding) = source_bindings[ns].get() {
                 let vis = directive.vis.get();
                 if !binding.pseudo_vis().is_at_least(vis, &*this) {
                     reexport_error = Some((ns, binding));
@@ -1084,7 +1108,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
             let mut full_path = directive.module_path.clone();
             full_path.push(Segment::from_ident(ident));
             self.per_ns(|this, ns| {
-                if let Ok(binding) = result[ns].get() {
+                if let Ok(binding) = source_bindings[ns].get() {
                     this.lint_if_path_starts_with_module(
                         directive.crate_lint(),
                         &full_path,
@@ -1098,7 +1122,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
         // Record what this import resolves to for later uses in documentation,
         // 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() {
+        self.per_ns(|this, ns| if let Some(binding) = source_bindings[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