Auto merge of #50760 - petrochenkov:legimp, r=nikomatsakis

Turn deprecation lint `legacy_imports` into a hard error

Closes #38260

The lint was introduced in Dec 2016, then made deny-by-default in Jun 2017 when crater run found 0 regressions caused by it.

This lint requires some not entirely trivial amount of import resolution logic that (surprisingly or not) interacts with `feature(use_extern_macros)` (#35896), so it would be desirable to remove it before stabilizing `use_extern_macros`.
In particular, this PR fixes the failing example in #50725 (but not the whole issue, `use std::panic::{self}` still can cause other undesirable errors when `use_extern_macros` is enabled).

src/doc/rustc/src/lints/listing/deny-by-default.md

@@ -91,43 +91,6 @@ The legacy_directory_ownership warning is issued when
 The warning can be fixed by renaming the parent module to "mod.rs" and moving
 it into its own directory if appropriate.
 
-## legacy-imports
-
-This lint detects names that resolve to ambiguous glob imports. Some example
-code that triggers this lint:
-
-```rust,ignore
-pub struct Foo;
-
-mod bar {
-    struct Foo;
-
-    mod baz {
-        use *;
-        use bar::*;
-        fn f(_: Foo) {}
-    }
-}
-```
-
-This will produce:
-
-```text
-error: `Foo` is ambiguous
- --> src/main.rs:9:17
-  |
-7 |         use *;
-  |             - `Foo` could refer to the name imported here
-8 |         use bar::*;
-  |             ------ `Foo` could also refer to the name imported here
-9 |         fn f(_: Foo) {}
-  |                 ^^^
-  |
-  = note: #[deny(legacy_imports)] on by default
-  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-  = note: for more information, see issue #38260 <https://github.com/rust-lang/rust/issues/38260>
-```
-
 
 ## missing-fragment-specifier
 

src/librustc/lint/builtin.rs

@@ -176,12 +176,6 @@ declare_lint! {
      not named `mod.rs`"
 }
 
-declare_lint! {
-    pub LEGACY_IMPORTS,
-    Deny,
-    "detects names that resolve to ambiguous glob imports with RFC 1560"
-}
-
 declare_lint! {
     pub LEGACY_CONSTRUCTOR_VISIBILITY,
     Deny,
@@ -314,7 +308,6 @@ impl LintPass for HardwiredLints {
             SAFE_PACKED_BORROWS,
             PATTERNS_IN_FNS_WITHOUT_BODY,
             LEGACY_DIRECTORY_OWNERSHIP,
-            LEGACY_IMPORTS,
             LEGACY_CONSTRUCTOR_VISIBILITY,
             MISSING_FRAGMENT_SPECIFIER,
             PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES,

src/librustc_lint/lib.rs

@@ -225,11 +225,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
             reference: "issue #37872 <https://github.com/rust-lang/rust/issues/37872>",
             edition: None,
         },
-        FutureIncompatibleInfo {
-            id: LintId::of(LEGACY_IMPORTS),
-            reference: "issue #38260 <https://github.com/rust-lang/rust/issues/38260>",
-            edition: None,
-        },
         FutureIncompatibleInfo {
             id: LintId::of(LEGACY_CONSTRUCTOR_VISIBILITY),
             reference: "issue #39207 <https://github.com/rust-lang/rust/issues/39207>",
@@ -318,6 +313,8 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
         "converted into hard error, see https://github.com/rust-lang/rust/issues/36892");
     store.register_removed("extra_requirement_in_impl",
         "converted into hard error, see https://github.com/rust-lang/rust/issues/37166");
+    store.register_removed("legacy_imports",
+        "converted into hard error, see https://github.com/rust-lang/rust/issues/38260");
     store.register_removed("coerce_never",
         "converted into hard error, see https://github.com/rust-lang/rust/issues/48950");
     store.register_removed("resolve_trait_on_defaulted_unit",

src/librustc_resolve/lib.rs

@@ -1224,12 +1224,10 @@ enum NameBindingKind<'a> {
         binding: &'a NameBinding<'a>,
         directive: &'a ImportDirective<'a>,
         used: Cell<bool>,
-        legacy_self_import: bool,
     },
     Ambiguity {
         b1: &'a NameBinding<'a>,
         b2: &'a NameBinding<'a>,
-        legacy: bool,
     }
 }
 
@@ -1251,15 +1249,13 @@ struct AmbiguityError<'a> {
     lexical: bool,
     b1: &'a NameBinding<'a>,
     b2: &'a NameBinding<'a>,
-    legacy: bool,
 }
 
 impl<'a> NameBinding<'a> {
     fn module(&self) -> Option<Module<'a>> {
         match self.kind {
             NameBindingKind::Module(module) => Some(module),
             NameBindingKind::Import { binding, .. } => binding.module(),
-            NameBindingKind::Ambiguity { legacy: true, b1, .. } => b1.module(),
             _ => None,
         }
     }
@@ -1269,7 +1265,6 @@ impl<'a> NameBinding<'a> {
             NameBindingKind::Def(def) => def,
             NameBindingKind::Module(module) => module.def().unwrap(),
             NameBindingKind::Import { binding, .. } => binding.def(),
-            NameBindingKind::Ambiguity { legacy: true, b1, .. } => b1.def(),
             NameBindingKind::Ambiguity { .. } => Def::Err,
         }
     }
@@ -1852,27 +1847,20 @@ impl<'a> Resolver<'a> {
     fn record_use(&mut self, ident: Ident, ns: Namespace, binding: &'a NameBinding<'a>, span: Span)
                   -> bool /* true if an error was reported */ {
         match binding.kind {
-            NameBindingKind::Import { directive, binding, ref used, legacy_self_import }
+            NameBindingKind::Import { directive, binding, ref used }
                     if !used.get() => {
                 used.set(true);
                 directive.used.set(true);
-                if legacy_self_import {
-                    self.warn_legacy_self_import(directive);
-                    return false;
-                }
                 self.used_imports.insert((directive.id, ns));
                 self.add_to_glob_map(directive.id, ident);
                 self.record_use(ident, ns, binding, span)
             }
             NameBindingKind::Import { .. } => false,
-            NameBindingKind::Ambiguity { b1, b2, legacy } => {
+            NameBindingKind::Ambiguity { b1, b2 } => {
                 self.ambiguity_errors.push(AmbiguityError {
-                    span: span, name: ident.name, lexical: false, b1: b1, b2: b2, legacy,
+                    span, name: ident.name, lexical: false, b1, b2,
                 });
-                if legacy {
-                    self.record_use(ident, ns, b1, span);
-                }
-                !legacy
+                true
             }
             _ => false
         }
@@ -4128,7 +4116,7 @@ impl<'a> Resolver<'a> {
         self.report_proc_macro_import(krate);
         let mut reported_spans = FxHashSet();
 
-        for &AmbiguityError { span, name, b1, b2, lexical, legacy } in &self.ambiguity_errors {
+        for &AmbiguityError { span, name, b1, b2, lexical } in &self.ambiguity_errors {
             if !reported_spans.insert(span) { continue }
             let participle = |binding: &NameBinding| {
                 if binding.is_import() { "imported" } else { "defined" }
@@ -4144,27 +4132,15 @@ impl<'a> Resolver<'a> {
                 format!("macro-expanded {} do not shadow when used in a macro invocation path",
                         if b1.is_import() { "imports" } else { "items" })
             };
-            if legacy {
-                let id = match b2.kind {
-                    NameBindingKind::Import { directive, .. } => directive.id,
-                    _ => unreachable!(),
-                };
-                let mut span = MultiSpan::from_span(span);
-                span.push_span_label(b1.span, msg1);
-                span.push_span_label(b2.span, msg2);
-                let msg = format!("`{}` is ambiguous", name);
-                self.session.buffer_lint(lint::builtin::LEGACY_IMPORTS, id, span, &msg);
-            } else {
-                let mut err =
-                    struct_span_err!(self.session, span, E0659, "`{}` is ambiguous", name);
-                err.span_note(b1.span, &msg1);
-                match b2.def() {
-                    Def::Macro(..) if b2.span == DUMMY_SP =>
-                        err.note(&format!("`{}` is also a builtin macro", name)),
-                    _ => err.span_note(b2.span, &msg2),
-                };
-                err.note(&note).emit();
-            }
+
+            let mut err = struct_span_err!(self.session, span, E0659, "`{}` is ambiguous", name);
+            err.span_note(b1.span, &msg1);
+            match b2.def() {
+                Def::Macro(..) if b2.span == DUMMY_SP =>
+                    err.note(&format!("`{}` is also a builtin macro", name)),
+                _ => err.span_note(b2.span, &msg2),
+            };
+            err.note(&note).emit();
         }
 
         for &PrivacyError(span, name, binding) in &self.privacy_errors {
@@ -4315,12 +4291,6 @@ impl<'a> Resolver<'a> {
         self.name_already_seen.insert(name, span);
     }
 
-    fn warn_legacy_self_import(&self, directive: &'a ImportDirective<'a>) {
-        let (id, span) = (directive.id, directive.span);
-        let msg = "`self` no longer imports values";
-        self.session.buffer_lint(lint::builtin::LEGACY_IMPORTS, id, span, msg);
-    }
-
     fn check_proc_macro_attrs(&mut self, attrs: &[ast::Attribute]) {
         if self.proc_macro_enabled { return; }
 

src/librustc_resolve/macros.rs

@@ -528,7 +528,6 @@ impl<'a> Resolver<'a> {
                                 b1: shadower,
                                 b2: binding,
                                 lexical: true,
-                                legacy: false,
                             });
                             return potential_illegal_shadower;
                         }

src/librustc_resolve/resolve_imports.rs

@@ -178,7 +178,6 @@ impl<'a> Resolver<'a> {
                             lexical: false,
                             b1: binding,
                             b2: shadowed_glob,
-                            legacy: false,
                         });
                     }
                 }
@@ -350,7 +349,6 @@ impl<'a> Resolver<'a> {
                 binding,
                 directive,
                 used: Cell::new(false),
-                legacy_self_import: false,
             },
             span: directive.span,
             vis,
@@ -399,7 +397,7 @@ impl<'a> Resolver<'a> {
     pub fn ambiguity(&self, b1: &'a NameBinding<'a>, b2: &'a NameBinding<'a>)
                      -> &'a NameBinding<'a> {
         self.arenas.alloc_name_binding(NameBinding {
-            kind: NameBindingKind::Ambiguity { b1: b1, b2: b2, legacy: false },
+            kind: NameBindingKind::Ambiguity { b1, b2 },
             vis: if b1.vis.is_at_least(b2.vis, self) { b1.vis } else { b2.vis },
             span: b1.span,
             expansion: Mark::root(),
@@ -691,7 +689,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                                 binding,
                                 directive,
                                 used: Cell::new(false),
-                                legacy_self_import: false,
                             },
                             vis: directive.vis.get(),
                             span: directive.span,
@@ -751,7 +748,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
         };
 
         let mut all_ns_err = true;
-        let mut legacy_self_import = None;
         self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
             if let Ok(binding) = result[ns].get() {
                 all_ns_err = false;
@@ -760,30 +756,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                         Some(this.dummy_binding);
                 }
             }
-        } else if let Ok(binding) = this.resolve_ident_in_module(module,
-                                                                 ident,
-                                                                 ns,
-                                                                 false,
-                                                                 false,
-                                                                 directive.span) {
-            legacy_self_import = Some(directive);
-            let binding = this.arenas.alloc_name_binding(NameBinding {
-                kind: NameBindingKind::Import {
-                    binding,
-                    directive,
-                    used: Cell::new(false),
-                    legacy_self_import: true,
-                },
-                ..*binding
-            });
-            let _ = this.try_define(directive.parent, ident, ns, binding);
         });
 
         if all_ns_err {
-            if let Some(directive) = legacy_self_import {
-                self.warn_legacy_self_import(directive);
-                return None;
-            }
             let mut all_ns_failed = true;
             self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS {
                 match this.resolve_ident_in_module(module, ident, ns, false, true, span) {
@@ -1050,23 +1025,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                             err.emit();
                     }
                 }
-                NameBindingKind::Ambiguity { b1, b2, .. }
-                        if b1.is_glob_import() && b2.is_glob_import() => {
-                    let (orig_b1, orig_b2) = match (&b1.kind, &b2.kind) {
-                        (&NameBindingKind::Import { binding: b1, .. },
-                         &NameBindingKind::Import { binding: b2, .. }) => (b1, b2),
-                        _ => continue,
-                    };
-                    let (b1, b2) = match (orig_b1.vis, orig_b2.vis) {
-                        (ty::Visibility::Public, ty::Visibility::Public) => continue,
-                        (ty::Visibility::Public, _) => (b1, b2),
-                        (_, ty::Visibility::Public) => (b2, b1),
-                        _ => continue,
-                    };
-                    resolution.binding = Some(self.arenas.alloc_name_binding(NameBinding {
-                        kind: NameBindingKind::Ambiguity { b1: b1, b2: b2, legacy: true }, ..*b1
-                    }));
-                }
                 _ => {}
             }
         }

src/test/compile-fail/issue-38293.rs

@@ -10,23 +10,17 @@
 
 // Test that `fn foo::bar::{self}` only imports `bar` in the type namespace.
 
-#![allow(unused)]
-
 mod foo {
     pub fn f() { }
 }
-use foo::f::{self};
-//~^ ERROR `self` no longer imports values
-//~| WARN hard error
+use foo::f::{self}; //~ ERROR unresolved import `foo::f`
 
 mod bar {
     pub fn baz() {}
     pub mod baz {}
 }
 use bar::baz::{self};
-//~^ ERROR `self` no longer imports values
-//~| WARN hard error
 
 fn main() {
-    baz();
+    baz(); //~ ERROR expected function, found module `baz`
 }

src/test/ui/imports/rfc-1560-warning-cycle.rs

@@ -8,8 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![allow(unused)]
-
 pub struct Foo;
 
 mod bar {
@@ -18,9 +16,7 @@ mod bar {
     mod baz {
         use *;
         use bar::*;
-        fn f(_: Foo) {}
-        //~^ ERROR `Foo` is ambiguous
-        //~| WARN hard error in a future release
+        fn f(_: Foo) {} //~ ERROR `Foo` is ambiguous
     }
 }
 

src/test/ui/imports/rfc-1560-warning-cycle.stderr

@@ -1,16 +1,21 @@
-error: `Foo` is ambiguous
-  --> $DIR/rfc-1560-warning-cycle.rs:21:17
+error[E0659]: `Foo` is ambiguous
+  --> $DIR/rfc-1560-warning-cycle.rs:19:17
    |
-LL |         use *;
-   |             - `Foo` could refer to the name imported here
-LL |         use bar::*;
-   |             ------ `Foo` could also refer to the name imported here
-LL |         fn f(_: Foo) {}
+LL |         fn f(_: Foo) {} //~ ERROR `Foo` is ambiguous
    |                 ^^^
    |
-   = note: #[deny(legacy_imports)] on by default
-   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-   = note: for more information, see issue #38260 <https://github.com/rust-lang/rust/issues/38260>
+note: `Foo` could refer to the name imported here
+  --> $DIR/rfc-1560-warning-cycle.rs:17:13
+   |
+LL |         use *;
+   |             ^
+note: `Foo` could also refer to the name imported here
+  --> $DIR/rfc-1560-warning-cycle.rs:18:13
+   |
+LL |         use bar::*;
+   |             ^^^^^^
+   = note: consider adding an explicit import of `Foo` to disambiguate
 
 error: aborting due to previous error
 
+For more information about this error, try `rustc --explain E0659`.