From a3126a5013e851354f5c10efe3c166332a0a9b46 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 4 Oct 2019 01:53:20 +0300 Subject: [PATCH] resolve: Introduce a new scope for derive helpers --- src/librustc_resolve/diagnostics.rs | 9 ++++++ src/librustc_resolve/lib.rs | 30 +++++++++++++---- src/librustc_resolve/macros.rs | 22 ++++++++++++- .../ui/proc-macro/derive-helper-shadowing.rs | 4 +-- .../proc-macro/derive-helper-shadowing.stderr | 32 +++++++++++++++---- 5 files changed, 81 insertions(+), 16 deletions(-) diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index 4efbc5a6405..f2858a62156 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -368,6 +368,15 @@ fn early_lookup_typo_candidate( let mut suggestions = Vec::new(); self.visit_scopes(scope_set, parent_scope, ident, |this, scope, use_prelude, _| { match scope { + Scope::DeriveHelpers(expn_id) => { + let res = Res::NonMacroAttr(NonMacroAttrKind::DeriveHelper); + if filter_fn(res) { + suggestions.extend(this.helper_attrs.get(&expn_id) + .into_iter().flatten().map(|ident| { + TypoSuggestion::from_res(ident.name, res) + })); + } + } Scope::DeriveHelpersCompat => { let res = Res::NonMacroAttr(NonMacroAttrKind::DeriveHelper); if filter_fn(res) { diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 2779924d485..0f410c623ba 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -45,7 +45,7 @@ use syntax::source_map::Spanned; use syntax::visit::{self, Visitor}; use syntax_expand::base::SyntaxExtension; -use syntax_pos::hygiene::{MacroKind, ExpnId, Transparency, SyntaxContext}; +use syntax_pos::hygiene::{MacroKind, ExpnId, ExpnKind, Transparency, SyntaxContext}; use syntax_pos::{Span, DUMMY_SP}; use errors::{Applicability, DiagnosticBuilder}; @@ -97,6 +97,7 @@ fn determined(determined: bool) -> Determinacy { /// but not for late resolution yet. #[derive(Clone, Copy)] enum Scope<'a> { + DeriveHelpers(ExpnId), DeriveHelpersCompat, MacroRules(LegacyScope<'a>), CrateRoot, @@ -942,6 +943,8 @@ pub struct Resolver<'a> { /// Legacy scopes *produced* by expanding the macro invocations, /// include all the `macro_rules` items and other invocations generated by them. output_legacy_scopes: FxHashMap>, + /// Helper attributes that are in scope for the given expansion. + helper_attrs: FxHashMap>, /// Avoid duplicated errors for "name already defined". name_already_seen: FxHashMap, @@ -1219,6 +1222,7 @@ pub fn new(session: &'a Session, non_macro_attrs: [non_macro_attr(false), non_macro_attr(true)], invocation_parent_scopes, output_legacy_scopes: Default::default(), + helper_attrs: Default::default(), macro_defs, local_macro_def_scopes: FxHashMap::default(), name_already_seen: FxHashMap::default(), @@ -1467,23 +1471,26 @@ fn visit_scopes( // in prelude, not sure where exactly (creates ambiguities with any other prelude names). let rust_2015 = ident.span.rust_2015(); - let (ns, is_absolute_path) = match scope_set { - ScopeSet::All(ns, _) => (ns, false), - ScopeSet::AbsolutePath(ns) => (ns, true), - ScopeSet::Macro(_) => (MacroNS, false), + let (ns, macro_kind, is_absolute_path) = match scope_set { + ScopeSet::All(ns, _) => (ns, None, false), + ScopeSet::AbsolutePath(ns) => (ns, None, true), + ScopeSet::Macro(macro_kind) => (MacroNS, Some(macro_kind), false), }; // Jump out of trait or enum modules, they do not act as scopes. let module = parent_scope.module.nearest_item_scope(); let mut scope = match ns { _ if is_absolute_path => Scope::CrateRoot, TypeNS | ValueNS => Scope::Module(module), - MacroNS => Scope::DeriveHelpersCompat, + MacroNS => Scope::DeriveHelpers(parent_scope.expansion), }; let mut ident = ident.modern(); let mut use_prelude = !module.no_implicit_prelude; loop { let visit = match scope { + // Derive helpers are not in scope when resolving derives in the same container. + Scope::DeriveHelpers(expn_id) => + !(expn_id == parent_scope.expansion && macro_kind == Some(MacroKind::Derive)), Scope::DeriveHelpersCompat => true, Scope::MacroRules(..) => true, Scope::CrateRoot => true, @@ -1505,6 +1512,17 @@ fn visit_scopes( } scope = match scope { + Scope::DeriveHelpers(expn_id) if expn_id != ExpnId::root() => { + // Derive helpers are not visible to code generated by bang or derive macros. + let expn_data = expn_id.expn_data(); + match expn_data.kind { + ExpnKind::Root | + ExpnKind::Macro(MacroKind::Bang, _) | + ExpnKind::Macro(MacroKind::Derive, _) => Scope::DeriveHelpersCompat, + _ => Scope::DeriveHelpers(expn_data.parent), + } + } + Scope::DeriveHelpers(..) => Scope::DeriveHelpersCompat, Scope::DeriveHelpersCompat => Scope::MacroRules(parent_scope.legacy), Scope::MacroRules(legacy_scope) => match legacy_scope { diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 0d6dd5a7f10..c1d0abece42 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -237,15 +237,23 @@ fn resolve_macro_invocation( // - Derives in the container need to know whether one of them is a built-in `Copy`. // FIXME: Try to avoid repeated resolutions for derives here and in expansion. let mut exts = Vec::new(); + let mut helper_attrs = Vec::new(); for path in derives { exts.push(match self.resolve_macro_path( path, Some(MacroKind::Derive), &parent_scope, true, force ) { - Ok((Some(ext), _)) => ext, + Ok((Some(ext), _)) => { + let span = path.segments.last().unwrap().ident.span.modern(); + helper_attrs.extend( + ext.helper_attrs.iter().map(|name| Ident::new(*name, span)) + ); + ext + } Ok(_) | Err(Determinacy::Determined) => self.dummy_ext(MacroKind::Derive), Err(Determinacy::Undetermined) => return Err(Indeterminate), }) } + self.helper_attrs.insert(invoc_id, helper_attrs); return Ok(InvocationRes::DeriveContainer(exts)); } }; @@ -498,6 +506,18 @@ struct Flags: u8 { Flags::empty(), )); let result = match scope { + Scope::DeriveHelpers(expn_id) => { + if let Some(attr) = this.helper_attrs.get(&expn_id).and_then(|attrs| { + attrs.iter().rfind(|i| ident == **i) + }) { + let binding = (Res::NonMacroAttr(NonMacroAttrKind::DeriveHelper), + ty::Visibility::Public, attr.span, expn_id) + .to_name_binding(this.arenas); + Ok((binding, Flags::empty())) + } else { + Err(Determinacy::Determined) + } + } Scope::DeriveHelpersCompat => { let mut result = Err(Determinacy::Determined); for derive in parent_scope.derives { diff --git a/src/test/ui/proc-macro/derive-helper-shadowing.rs b/src/test/ui/proc-macro/derive-helper-shadowing.rs index 21af4093a03..a8f4eea4cb7 100644 --- a/src/test/ui/proc-macro/derive-helper-shadowing.rs +++ b/src/test/ui/proc-macro/derive-helper-shadowing.rs @@ -1,3 +1,4 @@ +// edition:2018 // aux-build:test-macros.rs #[macro_use] @@ -11,8 +12,7 @@ struct S { // FIXME No ambiguity, attributes in non-macro positions are not resolved properly #[empty_helper] field: [u8; { - // FIXME No ambiguity, derive helpers are not put into scope for non-attributes - use empty_helper; + use empty_helper; //~ ERROR `empty_helper` is ambiguous // FIXME No ambiguity, derive helpers are not put into scope for inner items #[empty_helper] diff --git a/src/test/ui/proc-macro/derive-helper-shadowing.stderr b/src/test/ui/proc-macro/derive-helper-shadowing.stderr index 2ba517ce29e..ca9d6125e4b 100644 --- a/src/test/ui/proc-macro/derive-helper-shadowing.stderr +++ b/src/test/ui/proc-macro/derive-helper-shadowing.stderr @@ -1,21 +1,39 @@ -error[E0659]: `empty_helper` is ambiguous (derive helper attribute vs any other name) - --> $DIR/derive-helper-shadowing.rs:8:3 +error[E0659]: `empty_helper` is ambiguous (name vs any other name during import resolution) + --> $DIR/derive-helper-shadowing.rs:15:13 | -LL | #[empty_helper] - | ^^^^^^^^^^^^ ambiguous name +LL | use empty_helper; + | ^^^^^^^^^^^^ ambiguous name | note: `empty_helper` could refer to the derive helper attribute defined here - --> $DIR/derive-helper-shadowing.rs:9:10 + --> $DIR/derive-helper-shadowing.rs:10:10 | LL | #[derive(Empty)] | ^^^^^ note: `empty_helper` could also refer to the attribute macro imported here - --> $DIR/derive-helper-shadowing.rs:6:5 + --> $DIR/derive-helper-shadowing.rs:7:5 | LL | use test_macros::empty_attr as empty_helper; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: use `crate::empty_helper` to refer to this attribute macro unambiguously -error: aborting due to previous error +error[E0659]: `empty_helper` is ambiguous (derive helper attribute vs any other name) + --> $DIR/derive-helper-shadowing.rs:9:3 + | +LL | #[empty_helper] + | ^^^^^^^^^^^^ ambiguous name + | +note: `empty_helper` could refer to the derive helper attribute defined here + --> $DIR/derive-helper-shadowing.rs:10:10 + | +LL | #[derive(Empty)] + | ^^^^^ +note: `empty_helper` could also refer to the attribute macro imported here + --> $DIR/derive-helper-shadowing.rs:7:5 + | +LL | use test_macros::empty_attr as empty_helper; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: use `crate::empty_helper` to refer to this attribute macro unambiguously + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0659`.