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

Stabilize use_extern_macros #50911

Merged
merged 2 commits into from
Aug 17, 2018
Merged
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
9 changes: 1 addition & 8 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,19 +833,12 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
-> bool {
let allow_shadowing = expansion == Mark::root();
let legacy_imports = self.legacy_macro_imports(&item.attrs);
let mut used = legacy_imports != LegacyMacroImports::default();
let used = legacy_imports != LegacyMacroImports::default();

// `#[macro_use]` is only allowed at the crate root.
if self.current_module.parent.is_some() && used {
span_err!(self.session, item.span, E0468,
"an `extern crate` loading macros must be at the crate root");
} else if !self.use_extern_macros && !used &&
self.cstore.dep_kind_untracked(module.def_id().unwrap().krate)
.macros_only() {
let msg = "proc macro crates and `#[no_link]` crates have no effect without \
`#[macro_use]`";
self.session.span_warn(item.span, msg);
used = true; // Avoid the normal unused extern crate warning
}

let (graph_root, arenas) = (self.graph_root, self.arenas);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ pub fn check_crate(resolver: &mut Resolver, krate: &ast::Crate) {
directive.vis.get() == ty::Visibility::Public ||
directive.span.is_dummy() => {
if let ImportDirectiveSubclass::MacroUse = directive.subclass {
if resolver.use_extern_macros && !directive.span.is_dummy() {
if !directive.span.is_dummy() {
resolver.session.buffer_lint(
lint::builtin::MACRO_USE_EXTERN_CRATE,
directive.id,
Expand Down
58 changes: 2 additions & 56 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ use std::mem::replace;
use rustc_data_structures::sync::Lrc;

use resolve_imports::{ImportDirective, ImportDirectiveSubclass, NameResolution, ImportResolver};
use macros::{InvocationData, LegacyBinding, LegacyScope, MacroBinding};
use macros::{InvocationData, LegacyBinding, MacroBinding};

// NB: This module needs to be declared first so diagnostics are
// registered before they are used.
Expand Down Expand Up @@ -1399,23 +1399,18 @@ pub struct Resolver<'a, 'b: 'a> {
/// crate-local macro expanded `macro_export` referred to by a module-relative path
macro_expanded_macro_export_errors: BTreeSet<(Span, Span)>,

gated_errors: FxHashSet<Span>,
disallowed_shadowing: Vec<&'a LegacyBinding<'a>>,

arenas: &'a ResolverArenas<'a>,
dummy_binding: &'a NameBinding<'a>,
/// true if `#![feature(use_extern_macros)]`
use_extern_macros: bool,

crate_loader: &'a mut CrateLoader<'b>,
macro_names: FxHashSet<Ident>,
macro_prelude: FxHashMap<Name, &'a NameBinding<'a>>,
pub all_macros: FxHashMap<Name, Def>,
lexical_macro_resolutions: Vec<(Ident, &'a Cell<LegacyScope<'a>>)>,
macro_map: FxHashMap<DefId, Lrc<SyntaxExtension>>,
macro_defs: FxHashMap<Mark, DefId>,
local_macro_def_scopes: FxHashMap<NodeId, Module<'a>>,
macro_exports: Vec<Export>, // FIXME: Remove when `use_extern_macros` is stabilized
pub whitelisted_legacy_custom_derives: Vec<Name>,
pub found_unresolved_macro: bool,

Expand Down Expand Up @@ -1657,8 +1652,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
invocations.insert(Mark::root(),
arenas.alloc_invocation_data(InvocationData::root(graph_root)));

let features = session.features_untracked();

let mut macro_defs = FxHashMap();
macro_defs.insert(Mark::root(), root_def_id);

Expand Down Expand Up @@ -1717,7 +1710,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
ambiguity_errors: Vec::new(),
use_injections: Vec::new(),
proc_mac_errors: Vec::new(),
gated_errors: FxHashSet(),
disallowed_shadowing: Vec::new(),
macro_expanded_macro_export_errors: BTreeSet::new(),

Expand All @@ -1729,15 +1721,11 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
vis: ty::Visibility::Public,
}),

use_extern_macros: features.use_extern_macros(),

crate_loader,
macro_names: FxHashSet(),
macro_prelude: FxHashMap(),
all_macros: FxHashMap(),
lexical_macro_resolutions: Vec::new(),
macro_map: FxHashMap(),
macro_exports: Vec::new(),
invocations,
macro_defs,
local_macro_def_scopes: FxHashMap(),
Expand Down Expand Up @@ -1770,9 +1758,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
fn per_ns<F: FnMut(&mut Self, Namespace)>(&mut self, mut f: F) {
f(self, TypeNS);
f(self, ValueNS);
if self.use_extern_macros {
f(self, MacroNS);
}
f(self, MacroNS);
}

fn macro_def(&self, mut ctxt: SyntaxContext) -> DefId {
Expand Down Expand Up @@ -2186,11 +2172,8 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {

fn resolve_item(&mut self, item: &Item) {
let name = item.ident.name;

debug!("(resolving item) resolving {}", name);

self.check_proc_macro_attrs(&item.attrs);

match item.node {
ItemKind::Enum(_, ref generics) |
ItemKind::Ty(_, ref generics) |
Expand Down Expand Up @@ -2218,8 +2201,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
walk_list!(this, visit_param_bound, bounds);

for trait_item in trait_items {
this.check_proc_macro_attrs(&trait_item.attrs);

let type_parameters = HasTypeParameters(&trait_item.generics,
TraitOrImplItemRibKind);
this.with_type_parameter_rib(type_parameters, |this| {
Expand Down Expand Up @@ -2498,7 +2479,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
this.visit_generics(generics);
this.with_current_self_type(self_type, |this| {
for impl_item in impl_items {
this.check_proc_macro_attrs(&impl_item.attrs);
this.resolve_visibility(&impl_item.vis);

// We also need a new scope for the impl item type parameters.
Expand Down Expand Up @@ -4495,10 +4475,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
}

fn report_shadowing_errors(&mut self) {
for (ident, scope) in replace(&mut self.lexical_macro_resolutions, Vec::new()) {
self.resolve_legacy_scope(scope, ident, true);
}

let mut reported_errors = FxHashSet();
for binding in replace(&mut self.disallowed_shadowing, Vec::new()) {
if self.resolve_legacy_scope(&binding.parent, binding.ident, false).is_some() &&
Expand Down Expand Up @@ -4619,36 +4595,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
err.emit();
self.name_already_seen.insert(name, span);
}

fn check_proc_macro_attrs(&mut self, attrs: &[ast::Attribute]) {
if self.use_extern_macros { return; }

for attr in attrs {
if attr.path.segments.len() > 1 {
continue
}
let ident = attr.path.segments[0].ident;
let result = self.resolve_lexical_macro_path_segment(ident,
MacroNS,
false,
false,
true,
attr.path.span);
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 = "use_extern_macros";

feature_err(&self.session.parse_sess, feature,
attr.span, GateIssue::Language, msg)
.span_label(binding.span(), "procedural macro imported here")
.emit();
}
}
}
}
}

fn is_self_type(path: &[Ident], namespace: Namespace) -> bool {
Expand Down
48 changes: 7 additions & 41 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use build_reduced_graph::{BuildReducedGraphVisitor, IsMacroExport};
use resolve_imports::ImportResolver;
use rustc::hir::def_id::{DefId, BUILTIN_MACROS_CRATE, CRATE_DEF_INDEX, DefIndex,
DefIndexAddressSpace};
use rustc::hir::def::{Def, Export, NonMacroAttrKind};
use rustc::hir::def::{Def, NonMacroAttrKind};
use rustc::hir::map::{self, DefCollector};
use rustc::{ty, lint};
use rustc::middle::cstore::CrateStore;
Expand Down Expand Up @@ -524,21 +524,13 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
self.current_module = if module.is_trait() { module.parent.unwrap() } else { module };

// Possibly apply the macro helper hack
if self.use_extern_macros && kind == MacroKind::Bang && path.len() == 1 &&
if kind == MacroKind::Bang && path.len() == 1 &&
path[0].span.ctxt().outer().expn_info().map_or(false, |info| info.local_inner_macros) {
let root = Ident::new(keywords::DollarCrate.name(), path[0].span);
path.insert(0, root);
}

if path.len() > 1 {
if !self.use_extern_macros && self.gated_errors.insert(span) {
let msg = "non-ident macro paths are experimental";
let feature = "use_extern_macros";
emit_feature_err(&self.session.parse_sess, feature, span, GateIssue::Language, msg);
self.found_unresolved_macro = true;
return Err(Determinacy::Determined);
}

let res = self.resolve_path(None, &path, Some(MacroNS), false, span, CrateLint::No);
let def = match res {
PathResult::NonModule(path_res) => match path_res.base_def() {
Expand Down Expand Up @@ -843,7 +835,6 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
record_used: bool)
-> Option<MacroBinding<'a>> {
let ident = ident.modern();
let mut possible_time_travel = None;
let mut relative_depth: u32 = 0;
let mut binding = None;
loop {
Expand All @@ -853,9 +844,6 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
match invocation.expansion.get() {
LegacyScope::Invocation(_) => scope.set(invocation.legacy_scope.get()),
LegacyScope::Empty => {
if possible_time_travel.is_none() {
possible_time_travel = Some(scope);
}
scope = &invocation.legacy_scope;
}
_ => {
Expand All @@ -870,7 +858,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
}
LegacyScope::Binding(potential_binding) => {
if potential_binding.ident == ident {
if (!self.use_extern_macros || record_used) && relative_depth > 0 {
if record_used && relative_depth > 0 {
self.disallowed_shadowing.push(potential_binding);
}
binding = Some(potential_binding);
Expand All @@ -884,21 +872,11 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
let binding = if let Some(binding) = binding {
MacroBinding::Legacy(binding)
} else if let Some(binding) = self.macro_prelude.get(&ident.name).cloned() {
if !self.use_extern_macros {
self.record_use(ident, MacroNS, binding, DUMMY_SP);
}
MacroBinding::Global(binding)
} else {
return None;
};

if !self.use_extern_macros {
if let Some(scope) = possible_time_travel {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIXME: possible_time_travel is now unused

// Check for disallowed shadowing later
self.lexical_macro_resolutions.push((ident, scope));
Copy link
Contributor Author

@petrochenkov petrochenkov Aug 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIXME: lexical_macro_resolutions is now unused

}
}

Some(binding)
}

Expand Down Expand Up @@ -1008,9 +986,6 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
find_best_match_for_name(names, name, None)
// Then check modules.
}).or_else(|| {
if !self.use_extern_macros {
return None;
}
let is_macro = |def| {
if let Def::Macro(_, def_kind) = def {
def_kind == kind
Expand Down Expand Up @@ -1086,19 +1061,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
let def = Def::Macro(def_id, MacroKind::Bang);
self.all_macros.insert(ident.name, def);
if attr::contains_name(&item.attrs, "macro_export") {
if self.use_extern_macros {
let module = self.graph_root;
let vis = ty::Visibility::Public;
self.define(module, ident, MacroNS,
(def, vis, item.span, expansion, IsMacroExport));
} else {
self.macro_exports.push(Export {
ident: ident.modern(),
def: def,
vis: ty::Visibility::Public,
span: item.span,
});
}
let module = self.graph_root;
let vis = ty::Visibility::Public;
self.define(module, ident, MacroNS,
(def, vis, item.span, expansion, IsMacroExport));
} else {
self.unused_macros.insert(def_id);
}
Expand Down
30 changes: 1 addition & 29 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use rustc::lint::builtin::{DUPLICATE_MACRO_EXPORTS, PUB_USE_OF_PRIVATE_EXTERN_CR
use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
use rustc::hir::def::*;
use rustc::session::DiagnosticMessageId;
use rustc::util::nodemap::{FxHashMap, FxHashSet};
use rustc::util::nodemap::FxHashSet;

use syntax::ast::{Ident, Name, NodeId, CRATE_NODE_ID};
use syntax::ext::base::Determinacy::{self, Determined, Undetermined};
Expand Down Expand Up @@ -1142,24 +1142,6 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
*module.globs.borrow_mut() = Vec::new();

let mut reexports = Vec::new();
let mut exported_macro_names = FxHashMap();
if ptr::eq(module, self.graph_root) {
let macro_exports = mem::replace(&mut self.macro_exports, Vec::new());
for export in macro_exports.into_iter().rev() {
if let Some(later_span) = exported_macro_names.insert(export.ident.modern(),
export.span) {
self.session.buffer_lint_with_diagnostic(
DUPLICATE_MACRO_EXPORTS,
CRATE_NODE_ID,
later_span,
&format!("a macro named `{}` has already been exported", export.ident),
BuiltinLintDiagnostics::DuplicatedMacroExports(
export.ident, export.span, later_span));
} else {
reexports.push(export);
}
}
}

for (&(ident, ns), resolution) in module.resolutions.borrow().iter() {
let resolution = &mut *resolution.borrow_mut();
Expand All @@ -1174,16 +1156,6 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
if !def.def_id().is_local() {
self.cstore.export_macros_untracked(def.def_id().krate);
}
if let Def::Macro(..) = def {
if let Some(&span) = exported_macro_names.get(&ident.modern()) {
let msg =
format!("a macro named `{}` has already been exported", ident);
self.session.struct_span_err(span, &msg)
.span_label(span, format!("`{}` already exported", ident))
.span_note(binding.span, "previous macro export here")
.emit();
}
}
reexports.push(Export {
ident: ident.modern(),
def: def,
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@
#![feature(unboxed_closures)]
#![feature(untagged_unions)]
#![feature(unwind_attributes)]
#![feature(use_extern_macros)]
#![cfg_attr(stage0, feature(use_extern_macros))]
#![feature(doc_cfg)]
#![feature(doc_masked)]
#![feature(doc_spotlight)]
Expand Down
12 changes: 2 additions & 10 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1124,9 +1124,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
return attrs;
}

if self.cx.ecfg.use_extern_macros_enabled() {
attr = find_attr_invoc(&mut attrs);
}
attr = find_attr_invoc(&mut attrs);
traits = collect_derives(&mut self.cx, &mut attrs);
attrs
});
Expand All @@ -1147,9 +1145,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
return attrs;
}

if self.cx.ecfg.use_extern_macros_enabled() {
attr = find_attr_invoc(&mut attrs);
}
attr = find_attr_invoc(&mut attrs);
attrs
});

Expand Down Expand Up @@ -1667,10 +1663,6 @@ impl<'feat> ExpansionConfig<'feat> {
fn proc_macro_expr = proc_macro_expr,
fn proc_macro_non_items = proc_macro_non_items,
}

pub fn use_extern_macros_enabled(&self) -> bool {
self.features.map_or(false, |features| features.use_extern_macros())
}
}

// A Marker adds the given mark to the syntax context.
Expand Down
Loading