user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Auto merge of #40501 - jseyfried:shadow_builtin_macros, r=nrc

Browse source 
Allow `use` macro imports to shadow global macros

Terminology:
 - global scope: builtin macros, macros from the prelude, `#[macro_use]`, or `#![plugin(..)]`.
 - legacy scope: crate-local `macro_rules!`.
 - modern scope: `use` macro imports, `macro` (once implemented).

Today, the legacy scope can shadow the global scope (modulo RFC 1560 expanded shadowing restrictions). However, the modern scope cannot shadow or be shadowed by either the global or legacy scopes, leading to ambiguity errors.

This PR allows the modern scope to shadow the global scope (subject to some restrictions).
More specifically, a name in the global scope is as shadowable as a glob import in the module `self`. In other words, we imagine a special, implicit glob import in each module item:
```rust
mod foo {
    #[lexical_only] // Not accessible via `foo::<name>`, like pre-RFC 1560 `use` imports.
    #[shadowable_by_legacy_scope] // for back-compat
    use <global_macros>::*;
}
```

r? @nrc
This commit is contained in:
bors 2017-03-26 11:45:13 +00:00
commit bcfd5c48b7
5 changed files with 175 additions and 71 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/librustc_resolve/build_reduced_graph.rs
View file

@ -539,7 +539,7 @@ impl<'a> Resolver<'a> {
binding: &'a NameBinding<'a>,
span: Span,
allow_shadowing: bool) {
if self.builtin_macros.insert(name, binding).is_some() && !allow_shadowing {
if self.global_macros.insert(name, binding).is_some() && !allow_shadowing {
let msg = format!("`{}` is already in scope", name);
let note =
"macro-expanded `#[macro_use]`s may not shadow existing macros (see RFC 1560)";

44
src/librustc_resolve/lib.rs
View file

@ -75,7 +75,7 @@ use std::mem::replace;
use std::rc::Rc;
use resolve_imports::{ImportDirective, ImportDirectiveSubclass, NameResolution, ImportResolver};
use macros::{InvocationData, LegacyBinding, LegacyScope};
use macros::{InvocationData, LegacyBinding, LegacyScope, MacroBinding};
// NB: This module needs to be declared first so diagnostics are
// registered before they are used.
@ -1174,7 +1174,7 @@ pub struct Resolver<'a> {
crate_loader: &'a mut CrateLoader,
macro_names: FxHashSet<Name>,
builtin_macros: FxHashMap<Name, &'a NameBinding<'a>>,
global_macros: FxHashMap<Name, &'a NameBinding<'a>>,
lexical_macro_resolutions: Vec<(Name, &'a Cell<LegacyScope<'a>>)>,
macro_map: FxHashMap<DefId, Rc<SyntaxExtension>>,
macro_defs: FxHashMap<Mark, DefId>,
@ -1372,7 +1372,7 @@ impl<'a> Resolver<'a> {
crate_loader: crate_loader,
macro_names: FxHashSet(),
builtin_macros: FxHashMap(),
global_macros: FxHashMap(),
lexical_macro_resolutions: Vec::new(),
macro_map: FxHashMap(),
macro_exports: Vec::new(),
@ -2429,9 +2429,9 @@ impl<'a> Resolver<'a> {
};
}
}
let is_builtin = self.builtin_macros.get(&path[0].name).cloned()
let is_global = self.global_macros.get(&path[0].name).cloned()
.map(|binding| binding.get_macro(self).kind() == MacroKind::Bang).unwrap_or(false);
if primary_ns != MacroNS && (is_builtin || self.macro_names.contains(&path[0].name)) {
if primary_ns != MacroNS && (is_global || self.macro_names.contains(&path[0].name)) {
// Return some dummy definition, it's enough for error reporting.
return Some(
PathResolution::new(Def::Macro(DefId::local(CRATE_DEF_INDEX), MacroKind::Bang))
@ -2566,6 +2566,7 @@ impl<'a> Resolver<'a> {
self.resolve_ident_in_module(module, ident, ns, false, record_used)
} else if opt_ns == Some(MacroNS) {
self.resolve_lexical_macro_path_segment(ident, ns, record_used)
.map(MacroBinding::binding)
} else {
match self.resolve_ident_in_lexical_scope(ident, ns, record_used) {
Some(LexicalScopeBinding::Item(binding)) => Ok(binding),
@ -3223,7 +3224,7 @@ impl<'a> Resolver<'a> {
};
let msg1 = format!("`{}` could refer to the name {} here", name, participle(b1));
let msg2 = format!("`{}` could also refer to the name {} here", name, participle(b2));
let note = if !lexical && b1.is_glob_import() {
let note = if b1.expansion == Mark::root() || !lexical && b1.is_glob_import() {
format!("consider adding an explicit import of `{}` to disambiguate", name)
} else if let Def::Macro(..) = b1.def() {
format!("macro-expanded {} do not shadow",
@ -3243,11 +3244,15 @@ impl<'a> Resolver<'a> {
let msg = format!("`{}` is ambiguous", name);
self.session.add_lint(lint::builtin::LEGACY_IMPORTS, id, span, msg);
} else {
self.session.struct_span_err(span, &format!("`{}` is ambiguous", name))
.span_note(b1.span, &msg1)
.span_note(b2.span, &msg2)
.note(&note)
.emit();
let mut err =
self.session.struct_span_err(span, &format!("`{}` 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();
}
}
@ -3361,14 +3366,13 @@ impl<'a> Resolver<'a> {
if self.proc_macro_enabled { return; }
for attr in attrs {
let name = unwrap_or!(attr.name(), continue);
let maybe_binding = self.builtin_macros.get(&name).cloned().or_else(|| {
let ident = Ident::with_empty_ctxt(name);
self.resolve_lexical_macro_path_segment(ident, MacroNS, None).ok()
});
if let Some(binding) = maybe_binding {
if let SyntaxExtension::AttrProcMacro(..) = *binding.get_macro(self) {
if attr.path.segments.len() > 1 {
continue
}
let ident = attr.path.segments[0].identifier;
let result = self.resolve_lexical_macro_path_segment(ident, MacroNS, None);
if let Ok(binding) = result {
if let SyntaxExtension::AttrProcMacro(..) = *binding.binding().get_macro(self) {
attr::mark_known(attr);
let msg = "attribute procedural macros are experimental";
@ -3376,7 +3380,7 @@ impl<'a> Resolver<'a> {
feature_err(&self.session.parse_sess, feature,
attr.span, GateIssue::Language, msg)
.span_note(binding.span, "procedural macro imported here")
.span_note(binding.span(), "procedural macro imported here")
.emit();
}
}

116
src/librustc_resolve/macros.rs
View file

@ -81,11 +81,29 @@ pub struct LegacyBinding<'a> {
pub span: Span,
}
#[derive(Copy, Clone)]
pub enum MacroBinding<'a> {
Legacy(&'a LegacyBinding<'a>),
Global(&'a NameBinding<'a>),
Modern(&'a NameBinding<'a>),
}
impl<'a> MacroBinding<'a> {
pub fn span(self) -> Span {
match self {
MacroBinding::Legacy(binding) => binding.span,
MacroBinding::Global(binding) | MacroBinding::Modern(binding) => binding.span,
}
}
pub fn binding(self) -> &'a NameBinding<'a> {
match self {
MacroBinding::Global(binding) | MacroBinding::Modern(binding) => binding,
MacroBinding::Legacy(_) => panic!("unexpected MacroBinding::Legacy"),
}
}
}
impl<'a> base::Resolver for Resolver<'a> {
fn next_node_id(&mut self) -> ast::NodeId {
self.session.next_node_id()
@ -171,7 +189,7 @@ impl<'a> base::Resolver for Resolver<'a> {
vis: ty::Visibility::Invisible,
expansion: Mark::root(),
});
self.builtin_macros.insert(ident.name, binding);
self.global_macros.insert(ident.name, binding);
}
fn resolve_imports(&mut self) {
@ -189,7 +207,7 @@ impl<'a> base::Resolver for Resolver<'a> {
attr::mark_known(&attrs[i]);
}
match self.builtin_macros.get(&name).cloned() {
match self.global_macros.get(&name).cloned() {
Some(binding) => match *binding.get_macro(self) {
MultiModifier(..) | MultiDecorator(..) | SyntaxExtension::AttrProcMacro(..) => {
return Some(attrs.remove(i))
@ -221,7 +239,7 @@ impl<'a> base::Resolver for Resolver<'a> {
}
let trait_name = traits[j].segments[0].identifier.name;
let legacy_name = Symbol::intern(&format!("derive_{}", trait_name));
if !self.builtin_macros.contains_key(&legacy_name) {
if !self.global_macros.contains_key(&legacy_name) {
continue
}
let span = traits.remove(j).span;
@ -378,18 +396,18 @@ impl<'a> Resolver<'a> {
}
let name = path[0].name;
let result = match self.resolve_legacy_scope(&invocation.legacy_scope, name, false) {
Some(MacroBinding::Legacy(binding)) => Ok(Def::Macro(binding.def_id, MacroKind::Bang)),
Some(MacroBinding::Modern(binding)) => Ok(binding.def_ignoring_ambiguity()),
None => match self.resolve_lexical_macro_path_segment(path[0], MacroNS, None) {
Ok(binding) => Ok(binding.def_ignoring_ambiguity()),
Err(Determinacy::Undetermined) if !force =>
return Err(Determinacy::Undetermined),
let legacy_resolution = self.resolve_legacy_scope(&invocation.legacy_scope, name, false);
let result = if let Some(MacroBinding::Legacy(binding)) = legacy_resolution {
Ok(Def::Macro(binding.def_id, MacroKind::Bang))
} else {
match self.resolve_lexical_macro_path_segment(path[0], MacroNS, None) {
Ok(binding) => Ok(binding.binding().def_ignoring_ambiguity()),
Err(Determinacy::Undetermined) if !force => return Err(Determinacy::Undetermined),
Err(_) => {
self.found_unresolved_macro = true;
Err(Determinacy::Determined)
}
},
}
};
self.current_module.legacy_macro_resolutions.borrow_mut()
@ -403,42 +421,56 @@ impl<'a> Resolver<'a> {
ident: Ident,
ns: Namespace,
record_used: Option<Span>)
-> Result<&'a NameBinding<'a>, Determinacy> {
let mut module = self.current_module;
let mut potential_expanded_shadower: Option<&NameBinding> = None;
-> Result<MacroBinding<'a>, Determinacy> {
let mut module = Some(self.current_module);
let mut potential_illegal_shadower = Err(Determinacy::Determined);
let determinacy =
if record_used.is_some() { Determinacy::Determined } else { Determinacy::Undetermined };
loop {
// Since expanded macros may not shadow the lexical scope (enforced below),
// we can ignore unresolved invocations (indicated by the penultimate argument).
match self.resolve_ident_in_module(module, ident, ns, true, record_used) {
let result = if let Some(module) = module {
// Since expanded macros may not shadow the lexical scope and
// globs may not shadow global macros (both enforced below),
// we resolve with restricted shadowing (indicated by the penultimate argument).
self.resolve_ident_in_module(module, ident, ns, true, record_used)
.map(MacroBinding::Modern)
} else {
self.global_macros.get(&ident.name).cloned().ok_or(determinacy)
.map(MacroBinding::Global)
};
match result.map(MacroBinding::binding) {
Ok(binding) => {
let span = match record_used {
Some(span) => span,
None => return Ok(binding),
None => return result,
};
match potential_expanded_shadower {
Some(shadower) if shadower.def() != binding.def() => {
if let Ok(MacroBinding::Modern(shadower)) = potential_illegal_shadower {
if shadower.def() != binding.def() {
let name = ident.name;
self.ambiguity_errors.push(AmbiguityError {
span: span, name: name, b1: shadower, b2: binding, lexical: true,
legacy: false,
});
return Ok(shadower);
return potential_illegal_shadower;
}
_ if binding.expansion == Mark::root() => return Ok(binding),
_ => potential_expanded_shadower = Some(binding),
}
if binding.expansion != Mark::root() ||
(binding.is_glob_import() && module.unwrap().def().is_some()) {
potential_illegal_shadower = result;
} else {
return result;
}
},
Err(Determinacy::Undetermined) => return Err(Determinacy::Undetermined),
Err(Determinacy::Determined) => {}
}
match module.kind {
ModuleKind::Block(..) => module = module.parent.unwrap(),
ModuleKind::Def(..) => return match potential_expanded_shadower {
Some(binding) => Ok(binding),
None if record_used.is_some() => Err(Determinacy::Determined),
None => Err(Determinacy::Undetermined),
module = match module {
Some(module) => match module.kind {
ModuleKind::Block(..) => module.parent,
ModuleKind::Def(..) => None,
},
None => return potential_illegal_shadower,
}
}
}
@ -488,11 +520,11 @@ impl<'a> Resolver<'a> {
let binding = if let Some(binding) = binding {
MacroBinding::Legacy(binding)
} else if let Some(binding) = self.builtin_macros.get(&name).cloned() {
} else if let Some(binding) = self.global_macros.get(&name).cloned() {
if !self.use_extern_macros {
self.record_use(Ident::with_empty_ctxt(name), MacroNS, binding, DUMMY_SP);
}
MacroBinding::Modern(binding)
MacroBinding::Global(binding)
} else {
return None;
};
@ -524,21 +556,15 @@ impl<'a> Resolver<'a> {
let legacy_resolution = self.resolve_legacy_scope(legacy_scope, ident.name, true);
let resolution = self.resolve_lexical_macro_path_segment(ident, MacroNS, Some(span));
match (legacy_resolution, resolution) {
(Some(legacy_resolution), Ok(resolution)) => {
let (legacy_span, participle) = match legacy_resolution {
MacroBinding::Modern(binding)
if binding.def() == resolution.def() => continue,
MacroBinding::Modern(binding) => (binding.span, "imported"),
MacroBinding::Legacy(binding) => (binding.span, "defined"),
};
let msg1 = format!("`{}` could refer to the macro {} here", ident, participle);
(Some(MacroBinding::Legacy(legacy_binding)), Ok(MacroBinding::Modern(binding))) => {
let msg1 = format!("`{}` could refer to the macro defined here", ident);
let msg2 = format!("`{}` could also refer to the macro imported here", ident);
self.session.struct_span_err(span, &format!("`{}` is ambiguous", ident))
.span_note(legacy_span, &msg1)
.span_note(resolution.span, &msg2)
.span_note(legacy_binding.span, &msg1)
.span_note(binding.span, &msg2)
.emit();
},
(Some(MacroBinding::Modern(binding)), Err(_)) => {
(Some(MacroBinding::Global(binding)), Ok(MacroBinding::Global(_))) => {
self.record_use(ident, MacroNS, binding, span);
self.err_if_macro_use_proc_macro(ident.name, span, binding);
},
@ -567,11 +593,11 @@ impl<'a> Resolver<'a> {
find_best_match_for_name(self.macro_names.iter(), name, None)
} else {
None
// Then check builtin macros.
// Then check global macros.
}.or_else(|| {
// FIXME: get_macro needs an &mut Resolver, can we do it without cloning?
let builtin_macros = self.builtin_macros.clone();
let names = builtin_macros.iter().filter_map(|(name, binding)| {
let global_macros = self.global_macros.clone();
let names = global_macros.iter().filter_map(|(name, binding)| {
if binding.get_macro(self).kind() == kind {
Some(name)
} else {

12
src/librustc_resolve/resolve_imports.rs
View file

@ -145,7 +145,7 @@ impl<'a> Resolver<'a> {
module: Module<'a>,
ident: Ident,
ns: Namespace,
ignore_unresolved_invocations: bool,
restricted_shadowing: bool,
record_used: Option<Span>)
-> Result<&'a NameBinding<'a>, Determinacy> {
self.populate_module_if_necessary(module);
@ -158,9 +158,8 @@ impl<'a> Resolver<'a> {
if let Some(binding) = resolution.binding {
if let Some(shadowed_glob) = resolution.shadows_glob {
let name = ident.name;
// If we ignore unresolved invocations, we must forbid
// expanded shadowing to avoid time travel.
if ignore_unresolved_invocations &&
// Forbid expanded shadowing to avoid time travel.
if restricted_shadowing &&
binding.expansion != Mark::root() &&
ns != MacroNS && // In MacroNS, `try_define` always forbids this shadowing
binding.def() != shadowed_glob.def() {
@ -215,7 +214,7 @@ impl<'a> Resolver<'a> {
}
let no_unresolved_invocations =
ignore_unresolved_invocations || module.unresolved_invocations.borrow().is_empty();
restricted_shadowing || module.unresolved_invocations.borrow().is_empty();
match resolution.binding {
// In `MacroNS`, expanded bindings do not shadow (enforced in `try_define`).
Some(binding) if no_unresolved_invocations || ns == MacroNS =>
@ -225,6 +224,9 @@ impl<'a> Resolver<'a> {
}
// Check if the globs are determined
if restricted_shadowing && module.def().is_some() {
return Err(Determined);
}
for directive in module.globs.borrow().iter() {
if self.is_accessible(directive.vis.get()) {
if let Some(module) = directive.imported_module.get() {

72
src/test/compile-fail/imports/shadow_builtin_macros.rs Normal file
View file

@ -0,0 +1,72 @@
// Copyright 2017 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.
// aux-build:two_macros.rs
#![feature(use_extern_macros)]
mod foo {
extern crate two_macros;
pub use self::two_macros::m as panic;
}
mod m1 {
use foo::panic; // ok
fn f() { panic!(); }
}
mod m2 {
use foo::*; //~ NOTE `panic` could refer to the name imported here
fn f() { panic!(); } //~ ERROR ambiguous
//~| NOTE `panic` is also a builtin macro
//~| NOTE consider adding an explicit import of `panic` to disambiguate
}
mod m3 {
::two_macros::m!(use foo::panic;); //~ NOTE `panic` could refer to the name imported here
//~| NOTE in this expansion
fn f() { panic!(); } //~ ERROR ambiguous
//~| NOTE `panic` is also a builtin macro
//~| NOTE macro-expanded macro imports do not shadow
}
mod m4 {
macro_rules! panic { () => {} } // ok
panic!();
}
mod m5 {
macro_rules! m { () => {
macro_rules! panic { () => {} } //~ ERROR `panic` is already in scope
//~| NOTE macro-expanded `macro_rules!`s may not shadow existing macros
} }
m!(); //~ NOTE in this expansion
//~| NOTE in this expansion
panic!();
}
#[macro_use(n)] //~ NOTE `n` could also refer to the name imported here
extern crate two_macros;
mod bar {
pub use two_macros::m as n;
}
mod m6 {
use bar::n; // ok
n!();
}
mod m7 {
use bar::*; //~ NOTE `n` could refer to the name imported here
n!(); //~ ERROR ambiguous
//~| NOTE consider adding an explicit import of `n` to disambiguate
}
fn main() {}