From 235a7a743cc01ab0d5484e02eebe02152b368e99 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 12 Dec 2016 03:35:48 +0000 Subject: [PATCH 1/2] Change `self` in an import list `use foo::{self, ...}` to only import a module or enum `foo`. --- src/librustc_resolve/build_reduced_graph.rs | 8 ++++-- src/librustc_resolve/resolve_imports.rs | 16 ++++++----- src/test/compile-fail/issue-28075.rs | 2 -- src/test/compile-fail/issue-38293.rs | 31 +++++++++++++++++++++ 4 files changed, 45 insertions(+), 12 deletions(-) create mode 100644 src/test/compile-fail/issue-38293.rs diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 1b3791355d2c3..5be21bc62c56c 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -168,6 +168,7 @@ impl<'a> Resolver<'a> { target: binding, source: source, result: self.per_ns(|_, _| Cell::new(Err(Undetermined))), + type_ns_only: false, }; self.add_import_directive( module_path, subclass, view_path.span, item.id, vis, expansion, @@ -195,10 +196,10 @@ impl<'a> Resolver<'a> { for source_item in source_items { let node = source_item.node; - let (module_path, ident, rename) = { + let (module_path, ident, rename, type_ns_only) = { if node.name.name != keywords::SelfValue.name() { let rename = node.rename.unwrap_or(node.name); - (module_path.clone(), node.name, rename) + (module_path.clone(), node.name, rename, false) } else { let ident = *module_path.last().unwrap(); if ident.name == keywords::CrateRoot.name() { @@ -212,13 +213,14 @@ impl<'a> Resolver<'a> { } let module_path = module_path.split_last().unwrap().1; let rename = node.rename.unwrap_or(ident); - (module_path.to_vec(), ident, rename) + (module_path.to_vec(), ident, rename, true) } }; let subclass = SingleImport { target: rename, source: ident, result: self.per_ns(|_, _| Cell::new(Err(Undetermined))), + type_ns_only: type_ns_only, }; let id = source_item.node.id; self.add_import_directive( diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 91197e53d3c82..1702a1441bf8d 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -41,6 +41,7 @@ pub enum ImportDirectiveSubclass<'a> { target: Ident, source: Ident, result: PerNS, Determinacy>>>, + type_ns_only: bool, }, GlobImport { is_prelude: bool, @@ -503,8 +504,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { }; directive.imported_module.set(Some(module)); - let (source, target, result) = match directive.subclass { - SingleImport { source, target, ref result } => (source, target, result), + let (source, target, result, type_ns_only) = match directive.subclass { + SingleImport { source, target, ref result, type_ns_only } => + (source, target, result, type_ns_only), GlobImport { .. } => { self.resolve_glob_import(directive); return true; @@ -513,7 +515,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { }; let mut indeterminate = false; - self.per_ns(|this, ns| { + self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS { if let Err(Undetermined) = result[ns].get() { result[ns].set(this.resolve_ident_in_module(module, source, ns, false, None)); } else { @@ -573,8 +575,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { _ => return None, }; - let (ident, result) = match directive.subclass { - SingleImport { source, ref result, .. } => (source, result), + let (ident, result, type_ns_only) = match directive.subclass { + SingleImport { source, ref result, type_ns_only, .. } => (source, result, type_ns_only), GlobImport { .. } if module.def_id() == directive.parent.def_id() => { // Importing a module into itself is not allowed. return Some("Cannot glob-import a module into itself.".to_string()); @@ -592,7 +594,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { }; let mut all_ns_err = true; - self.per_ns(|this, ns| { + self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS { if let Ok(binding) = result[ns].get() { all_ns_err = false; if this.record_use(ident, ns, binding, directive.span) { @@ -604,7 +606,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { if all_ns_err { let mut all_ns_failed = true; - self.per_ns(|this, ns| { + self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS { match this.resolve_ident_in_module(module, ident, ns, false, Some(span)) { Ok(_) => all_ns_failed = false, _ => {} diff --git a/src/test/compile-fail/issue-28075.rs b/src/test/compile-fail/issue-28075.rs index 3e3d898e3683d..cd73a45641111 100644 --- a/src/test/compile-fail/issue-28075.rs +++ b/src/test/compile-fail/issue-28075.rs @@ -18,7 +18,5 @@ extern crate lint_stability; use lint_stability::{unstable, deprecated}; //~ ERROR use of unstable library feature 'test_feature' -use lint_stability::unstable::{self as u}; //~ ERROR use of unstable library feature 'test_feature' - fn main() { } diff --git a/src/test/compile-fail/issue-38293.rs b/src/test/compile-fail/issue-38293.rs new file mode 100644 index 0000000000000..f97fad7bd7490 --- /dev/null +++ b/src/test/compile-fail/issue-38293.rs @@ -0,0 +1,31 @@ +// Copyright 2016 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that `fn foo::bar::{self}` only imports `bar` in the type namespace. + +mod foo { + pub fn f() { } +} +use foo::f::{self}; +//~^ ERROR unresolved import +//~| NOTE no `f` in `foo` + +mod bar { + pub fn baz() {} + pub mod baz {} +} +use bar::baz::{self}; + +fn main() { + baz(); + //~^ ERROR unresolved name `baz` + //~| NOTE unresolved name + //~| HELP module `baz` cannot be used as an expression +} From d86e487b0b1025c5e8b9b69b318af9ac76dcf693 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 10 Jan 2017 05:15:02 +0000 Subject: [PATCH 2/2] Start warning cycle. --- src/librustc_resolve/lib.rs | 14 +++++++++++++- src/librustc_resolve/resolve_imports.rs | 18 ++++++++++++++++++ src/test/compile-fail/issue-38293.rs | 12 +++++++----- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 39a9194cf5e60..f613025fe3787 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -894,6 +894,7 @@ enum NameBindingKind<'a> { binding: &'a NameBinding<'a>, directive: &'a ImportDirective<'a>, used: Cell, + legacy_self_import: bool, }, Ambiguity { b1: &'a NameBinding<'a>, @@ -1346,8 +1347,13 @@ impl<'a> Resolver<'a> { } match binding.kind { - NameBindingKind::Import { directive, binding, ref used } if !used.get() => { + NameBindingKind::Import { directive, binding, ref used, legacy_self_import } + if !used.get() => { 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) @@ -3110,6 +3116,12 @@ impl<'a> Resolver<'a> { err.emit(); 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".to_string(); + self.session.add_lint(lint::builtin::LEGACY_IMPORTS, id, span, msg); + } } fn is_struct_like(def: Def) -> bool { diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 1702a1441bf8d..65e599ac6c7e8 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -297,6 +297,7 @@ impl<'a> Resolver<'a> { binding: binding, directive: directive, used: Cell::new(false), + legacy_self_import: false, }, span: directive.span, vis: vis, @@ -594,6 +595,7 @@ 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; @@ -602,9 +604,25 @@ 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, None) { + legacy_self_import = Some(directive); + let binding = this.arenas.alloc_name_binding(NameBinding { + kind: NameBindingKind::Import { + binding: binding, + directive: 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, Some(span)) { diff --git a/src/test/compile-fail/issue-38293.rs b/src/test/compile-fail/issue-38293.rs index f97fad7bd7490..bf24621a869fb 100644 --- a/src/test/compile-fail/issue-38293.rs +++ b/src/test/compile-fail/issue-38293.rs @@ -10,22 +10,24 @@ // Test that `fn foo::bar::{self}` only imports `bar` in the type namespace. +#![allow(unused)] +#![deny(legacy_imports)] + mod foo { pub fn f() { } } use foo::f::{self}; -//~^ ERROR unresolved import -//~| NOTE no `f` in `foo` +//~^ ERROR `self` no longer imports values +//~| WARN hard error mod bar { pub fn baz() {} pub mod baz {} } use bar::baz::{self}; +//~^ ERROR `self` no longer imports values +//~| WARN hard error fn main() { baz(); - //~^ ERROR unresolved name `baz` - //~| NOTE unresolved name - //~| HELP module `baz` cannot be used as an expression }