From dd0a766e06fc553a0321fb04eb51910bfd2c7097 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 11 Aug 2018 14:33:43 +0300 Subject: [PATCH] Prohibit using macro-expanded `macro_export` macros through module-relative paths --- src/librustc_resolve/build_reduced_graph.rs | 1 - src/librustc_resolve/lib.rs | 16 ++++++--- src/librustc_resolve/macros.rs | 2 -- src/librustc_resolve/resolve_imports.rs | 20 +++++++++-- .../proc-macro/proc-macro-attributes.rs | 2 +- .../local-modularized-tricky-fail-3.rs | 32 +++++++++++++++++ .../local-modularized-tricky-fail-3.stderr | 36 +++++++++++++++++++ 7 files changed, 98 insertions(+), 11 deletions(-) create mode 100644 src/test/ui/imports/local-modularized-tricky-fail-3.rs create mode 100644 src/test/ui/imports/local-modularized-tricky-fail-3.stderr diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 0be1bf3011e78..bf87b00c14969 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -789,7 +789,6 @@ impl<'a, 'b, 'cl> BuildReducedGraphVisitor<'a, 'b, 'cl> { fn visit_invoc(&mut self, id: ast::NodeId) -> &'b InvocationData<'b> { let mark = id.placeholder_to_mark(); self.resolver.current_module.unresolved_invocations.borrow_mut().insert(mark); - self.resolver.unresolved_invocations_macro_export.insert(mark); let invocation = self.resolver.invocations[&mark]; invocation.module.set(self.resolver.current_module); invocation.legacy_scope.set(self.legacy_scope); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 8ff4483492906..e31e2cc1dff59 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1385,6 +1385,8 @@ pub struct Resolver<'a, 'b: 'a> { use_injections: Vec>, /// `use` injections for proc macros wrongly imported with #[macro_use] proc_mac_errors: Vec, + /// crate-local macro expanded `macro_export` referred to by a module-relative path + macro_expanded_macro_export_errors: BTreeSet<(Span, Span)>, gated_errors: FxHashSet, disallowed_shadowing: Vec<&'a LegacyBinding<'a>>, @@ -1432,9 +1434,6 @@ pub struct Resolver<'a, 'b: 'a> { /// Only supposed to be used by rustdoc, otherwise should be false. pub ignore_extern_prelude_feature: bool, - - /// Macro invocations in the whole crate that can expand into a `#[macro_export] macro_rules`. - unresolved_invocations_macro_export: FxHashSet, } /// Nothing really interesting here, it just provides memory for the rest of the crate. @@ -1706,6 +1705,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { proc_mac_errors: Vec::new(), gated_errors: FxHashSet(), disallowed_shadowing: Vec::new(), + macro_expanded_macro_export_errors: BTreeSet::new(), arenas, dummy_binding: arenas.alloc_name_binding(NameBinding { @@ -1737,7 +1737,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { current_type_ascription: Vec::new(), injected_crate: None, ignore_extern_prelude_feature: false, - unresolved_invocations_macro_export: FxHashSet(), } } @@ -4126,6 +4125,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { ns: Namespace, module: Module<'a>, found_traits: &mut Vec) { + assert!(ns == TypeNS || ns == ValueNS); let mut traits = module.traits.borrow_mut(); if traits.is_none() { let mut collected_traits = Vec::new(); @@ -4371,6 +4371,14 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { self.report_proc_macro_import(krate); let mut reported_spans = FxHashSet(); + for &(span_use, span_def) in &self.macro_expanded_macro_export_errors { + let msg = "macro-expanded `macro_export` macros from the current crate \ + cannot be referred to by absolute paths"; + self.session.struct_span_err(span_use, msg) + .span_note(span_def, "the macro is defined here") + .emit(); + } + for &AmbiguityError { span, name, b1, b2, lexical } in &self.ambiguity_errors { if !reported_spans.insert(span) { continue } let participle = |binding: &NameBinding| { diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index f111a44efe05b..fe9d3c7eb9982 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -196,9 +196,7 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> { self.current_module = invocation.module.get(); self.current_module.unresolved_invocations.borrow_mut().remove(&mark); - self.unresolved_invocations_macro_export.remove(&mark); self.current_module.unresolved_invocations.borrow_mut().extend(derives); - self.unresolved_invocations_macro_export.extend(derives); for &derive in derives { self.invocations.insert(derive, invocation); } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index a3a9b938bbd6f..715292bc11622 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -146,6 +146,14 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> { .try_borrow_mut() .map_err(|_| Determined)?; // This happens when there is a cycle of imports + if let Some(binding) = resolution.binding { + if !restricted_shadowing && binding.expansion != Mark::root() { + if let NameBindingKind::Def(_, true) = binding.kind { + self.macro_expanded_macro_export_errors.insert((path_span, binding.span)); + } + } + } + if record_used { if let Some(binding) = resolution.binding { if let Some(shadowed_glob) = resolution.shadowed_glob { @@ -211,9 +219,15 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> { // if it cannot be shadowed by some new item/import expanded from a macro. // This happens either if there are no unexpanded macros, or expanded names cannot // shadow globs (that happens in macro namespace or with restricted shadowing). - let unexpanded_macros = !module.unresolved_invocations.borrow().is_empty() || - (ns == MacroNS && ptr::eq(module, self.graph_root) && - !self.unresolved_invocations_macro_export.is_empty()); + // + // Additionally, any macro in any module can plant names in the root module if it creates + // `macro_export` macros, so the root module effectively has unresolved invocations if any + // module has unresolved invocations. + // However, it causes resolution/expansion to stuck too often (#53144), so, to make + // progress, we have to ignore those potential unresolved invocations from other modules + // and prohibit access to macro-expanded `macro_export` macros instead (unless restricted + // shadowing is enabled, see `macro_expanded_macro_export_errors`). + let unexpanded_macros = !module.unresolved_invocations.borrow().is_empty(); if let Some(binding) = resolution.binding { if !unexpanded_macros || ns == MacroNS || restricted_shadowing { return check_usable(self, binding); diff --git a/src/test/compile-fail-fulldeps/proc-macro/proc-macro-attributes.rs b/src/test/compile-fail-fulldeps/proc-macro/proc-macro-attributes.rs index 153e4dd05717a..8d53699c0640e 100644 --- a/src/test/compile-fail-fulldeps/proc-macro/proc-macro-attributes.rs +++ b/src/test/compile-fail-fulldeps/proc-macro/proc-macro-attributes.rs @@ -21,7 +21,7 @@ extern crate derive_b; #[C] //~ ERROR: The attribute `C` is currently unknown to the compiler #[B(D)] #[B(E = "foo")] -#[B arbitrary tokens] //~ expected one of `(` or `=`, found `arbitrary` +#[B arbitrary tokens] //~ ERROR arbitrary tokens in non-macro attributes are unstable struct B; fn main() {} diff --git a/src/test/ui/imports/local-modularized-tricky-fail-3.rs b/src/test/ui/imports/local-modularized-tricky-fail-3.rs new file mode 100644 index 0000000000000..ab1f312e161e8 --- /dev/null +++ b/src/test/ui/imports/local-modularized-tricky-fail-3.rs @@ -0,0 +1,32 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Crate-local macro expanded `macro_export` macros cannot be accessed with module-relative paths. + +#![feature(use_extern_macros)] + +macro_rules! define_exported { () => { + #[macro_export] + macro_rules! exported { + () => () + } +}} + +define_exported!(); + +mod m { + use exported; + //~^ ERROR macro-expanded `macro_export` macros from the current crate cannot +} + +fn main() { + ::exported!(); + //~^ ERROR macro-expanded `macro_export` macros from the current crate cannot +} diff --git a/src/test/ui/imports/local-modularized-tricky-fail-3.stderr b/src/test/ui/imports/local-modularized-tricky-fail-3.stderr new file mode 100644 index 0000000000000..6da52842d83d7 --- /dev/null +++ b/src/test/ui/imports/local-modularized-tricky-fail-3.stderr @@ -0,0 +1,36 @@ +error: macro-expanded `macro_export` macros from the current crate cannot be referred to by absolute paths + --> $DIR/local-modularized-tricky-fail-3.rs:25:9 + | +LL | use exported; + | ^^^^^^^^ + | +note: the macro is defined here + --> $DIR/local-modularized-tricky-fail-3.rs:17:5 + | +LL | / macro_rules! exported { +LL | | () => () +LL | | } + | |_____^ +... +LL | define_exported!(); + | ------------------- in this macro invocation + +error: macro-expanded `macro_export` macros from the current crate cannot be referred to by absolute paths + --> $DIR/local-modularized-tricky-fail-3.rs:30:5 + | +LL | ::exported!(); + | ^^^^^^^^^^ + | +note: the macro is defined here + --> $DIR/local-modularized-tricky-fail-3.rs:17:5 + | +LL | / macro_rules! exported { +LL | | () => () +LL | | } + | |_____^ +... +LL | define_exported!(); + | ------------------- in this macro invocation + +error: aborting due to 2 previous errors +