Skip to content

Commit

Permalink
Auto merge of #38313 - jseyfried:self_imports, r=nrc
Browse files Browse the repository at this point in the history
resolve: clean up the semantics of `self` in an import list

Change `self` in an import list `use foo::bar::{self, ...};` to import `bar` only in the type namespace. Today, `bar` is imported in every namespace in which `foo::bar` is defined.

This is a [breaking-change], see #38293 (comment) for examples of code that would break.

Fixes #38293.
r? @nrc
  • Loading branch information
bors committed Jan 11, 2017
2 parents 513d942 + d86e487 commit b27c709
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 13 deletions.
8 changes: 5 additions & 3 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand All @@ -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(
Expand Down
14 changes: 13 additions & 1 deletion src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,7 @@ enum NameBindingKind<'a> {
binding: &'a NameBinding<'a>,
directive: &'a ImportDirective<'a>,
used: Cell<bool>,
legacy_self_import: bool,
},
Ambiguity {
b1: &'a NameBinding<'a>,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -3112,6 +3118,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 {
Expand Down
34 changes: 27 additions & 7 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub enum ImportDirectiveSubclass<'a> {
target: Ident,
source: Ident,
result: PerNS<Cell<Result<&'a NameBinding<'a>, Determinacy>>>,
type_ns_only: bool,
},
GlobImport {
is_prelude: bool,
Expand Down Expand Up @@ -296,6 +297,7 @@ impl<'a> Resolver<'a> {
binding: binding,
directive: directive,
used: Cell::new(false),
legacy_self_import: false,
},
span: directive.span,
vis: vis,
Expand Down Expand Up @@ -503,8 +505,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;
Expand All @@ -513,7 +516,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 {
Expand Down Expand Up @@ -573,8 +576,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());
Expand All @@ -592,19 +595,36 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
};

let mut all_ns_err = true;
self.per_ns(|this, ns| {
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;
if this.record_use(ident, ns, binding, directive.span) {
this.resolution(module, ident, ns).borrow_mut().binding =
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| {
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,
_ => {}
Expand Down
2 changes: 0 additions & 2 deletions src/test/compile-fail/issue-28075.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
}
33 changes: 33 additions & 0 deletions src/test/compile-fail/issue-38293.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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.

#![allow(unused)]
#![deny(legacy_imports)]

mod foo {
pub fn f() { }
}
use foo::f::{self};
//~^ 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();
}

0 comments on commit b27c709

Please sign in to comment.