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.

Sign up for GitHub

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

Jump to bottom

Allow use macro imports to shadow global macros #40501

Merged
merged 2 commits into from
Mar 26, 2017
Merged

Allow use macro imports to shadow global macros #40501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64e9af4
Allow declarative macros 2.0 and `use` macro imports to shadow builti…
jseyfried Mar 11, 2017
d64d381
Rename `builtin` => `global`.
jseyfried Mar 16, 2017
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
2 changes: 1 addition & 1 deletion src/librustc_resolve/build_reduced_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)";
Expand Down
44 changes: 24 additions & 20 deletions src/librustc_resolve/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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",
Expand All @@ -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();
}
}

Expand Down Expand Up @@ -3361,22 +3366,21 @@ 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";
let feature = "proc_macro";

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();
}
}
Expand Down
116 changes: 71 additions & 45 deletions src/librustc_resolve/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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) {
Expand All @@ -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))
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand All @@ -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,
}
}
}
Expand Down Expand Up @@ -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;
};
Expand Down Expand Up @@ -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);
},
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 7 additions & 5 deletions src/librustc_resolve/resolve_imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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() {
Expand Down Expand Up @@ -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 =>
Expand All @@ -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() {
Expand Down
Loading