Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve: allow non-imported items to shadow glob imports #31377

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,11 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {

// These items live in both the type and value namespaces.
ItemStruct(ref struct_def, _) => {
let modifiers = match !struct_def.is_struct() {
true => modifiers | DefModifiers::LINKED_NAMESPACES,
false => modifiers,
};

// Define a name in the type namespace.
let def = Def::Struct(self.ast_map.local_def_id(item.id));
self.define(parent, name, TypeNS, (def, sp, modifiers));
Expand Down Expand Up @@ -434,7 +439,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {

// Variants are always treated as importable to allow them to be glob used.
// All variants are defined in both type and value namespaces as future-proofing.
let modifiers = DefModifiers::PUBLIC | DefModifiers::IMPORTABLE | variant_modifiers;
let modifiers = DefModifiers::PUBLIC | DefModifiers::IMPORTABLE | variant_modifiers |
DefModifiers::LINKED_NAMESPACES;
let def = Def::Variant(item_id, self.ast_map.local_def_id(variant.node.data.id()));

self.define(parent, name, ValueNS, (def, variant.span, modifiers));
Expand Down Expand Up @@ -523,7 +529,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
final_ident);
// Variants are always treated as importable to allow them to be glob used.
// All variants are defined in both type and value namespaces as future-proofing.
let modifiers = DefModifiers::PUBLIC | DefModifiers::IMPORTABLE;
let modifiers = DefModifiers::PUBLIC | DefModifiers::IMPORTABLE |
DefModifiers::LINKED_NAMESPACES;
self.try_define(new_parent, name, TypeNS, (def, DUMMY_SP, modifiers));
self.try_define(new_parent, name, ValueNS, (def, DUMMY_SP, modifiers));
if self.session.cstore.variant_kind(variant_id) == Some(VariantKind::Struct) {
Expand Down Expand Up @@ -577,8 +584,14 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
debug!("(building reduced graph for external crate) building type and value for \
{}",
final_ident);
let ctor_def_id = self.session.cstore.struct_ctor_def_id(def_id);
let modifiers = match ctor_def_id {
Some(_) => modifiers | DefModifiers::LINKED_NAMESPACES,
None => modifiers,
};

self.try_define(new_parent, name, TypeNS, (def, DUMMY_SP, modifiers));
if let Some(ctor_def_id) = self.session.cstore.struct_ctor_def_id(def_id) {
if let Some(ctor_def_id) = ctor_def_id {
let def = Def::Struct(ctor_def_id);
self.try_define(new_parent, name, ValueNS, (def, DUMMY_SP, modifiers));
}
Expand Down
14 changes: 14 additions & 0 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,17 @@ impl<'a> ModuleS<'a> {
// Define the name or return the existing binding if there is a collision.
fn try_define_child(&self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>)
-> Result<(), &'a NameBinding<'a>> {
// If binding is glob imported and is an item that defines both namespaces,
// it will be shadowed by an existing non-imported item in either namespace.
if binding.defined_with(DefModifiers::GLOB_IMPORTED | DefModifiers::LINKED_NAMESPACES) {
for &ns in &[ValueNS, TypeNS] {
match self.children.borrow().get(&(name, ns)).cloned().unwrap_or_default().binding {
Some(binding) if !binding.is_import() => return Ok(()),
_ => {}
}
}
}

let mut children = self.children.borrow_mut();
let resolution = children.entry((name, ns)).or_insert_with(Default::default);

Expand Down Expand Up @@ -989,6 +1000,9 @@ bitflags! {
const PRIVATE_VARIANT = 1 << 2,
const PRELUDE = 1 << 3,
const GLOB_IMPORTED = 1 << 4,
// A binding for a name in a namespace has this flag if the name is defined in both
// namespaces by the same non-import item (i.e. by a tuple struct, unit struct, or variant).
const LINKED_NAMESPACES = 1 << 5,
}
}

Expand Down
9 changes: 5 additions & 4 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ impl ImportDirective {
if self.shadowable == Shadowable::Always {
modifiers = modifiers | DefModifiers::PRELUDE;
}
if binding.defined_with(DefModifiers::LINKED_NAMESPACES) {
modifiers = modifiers | DefModifiers::LINKED_NAMESPACES;
}

NameBinding {
kind: NameBindingKind::Import { binding: binding, id: self.id },
Expand Down Expand Up @@ -136,11 +139,9 @@ impl<'a> NameResolution<'a> {
_ => { self.binding = Some(binding); return Ok(()); }
};

// FIXME #31337: We currently allow items to shadow glob-imported re-exports.
// We currently allow items to shadow glob-imports.
if !old_binding.is_import() && binding.defined_with(DefModifiers::GLOB_IMPORTED) {
if let NameBindingKind::Import { binding, .. } = binding.kind {
if binding.is_import() { return Ok(()); }
}
return Ok(());
}

Err(old_binding)
Expand Down
38 changes: 38 additions & 0 deletions src/test/compile-fail/shadowed-glob-import.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// 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.

mod foo {
pub struct Bar;

pub enum E { V }
pub use self::E::V;

pub fn baz() -> bool {}
pub mod baz { pub fn f() {} }
}

use foo::*;
fn Bar() {}
struct V { x: i32 }
fn baz() {}

fn main() {
// The function `Bar` shadows the imported struct `Bar` in both namespaces
Bar();
let x: Bar = unimplemented!(); //~ ERROR use of undeclared type name `Bar`

// The struct `V` shadows the imported variant `V` in both namespaces
let _ = V { x: 0 };
let _ = V; //~ ERROR `V` is the name of a struct

// The function `baz` only shadows the imported name `baz` in the value namespace
let _: () = baz();
let _ = baz::f();
}
4 changes: 2 additions & 2 deletions src/test/compile-fail/variant-namespacing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@
// aux-build:variant-namespacing.rs

extern crate variant_namespacing;
pub use variant_namespacing::XE::*;
pub use variant_namespacing::XE::{XStruct, XTuple, XUnit};
//~^ ERROR import `XStruct` conflicts with type in this module
//~| ERROR import `XStruct` conflicts with value in this module
//~| ERROR import `XTuple` conflicts with type in this module
//~| ERROR import `XTuple` conflicts with value in this module
//~| ERROR import `XUnit` conflicts with type in this module
//~| ERROR import `XUnit` conflicts with value in this module
pub use E::*;
pub use E::{Struct, Tuple, Unit};
//~^ ERROR import `Struct` conflicts with type in this module
//~| ERROR import `Struct` conflicts with value in this module
//~| ERROR import `Tuple` conflicts with type in this module
Expand Down