From f60d96a4773db747aef75014ef6b41b39ae92372 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 4 Aug 2018 00:25:45 +0300 Subject: [PATCH] Support custom attributes when macro modularization is enabled --- src/librustc_resolve/build_reduced_graph.rs | 4 +- src/librustc_resolve/lib.rs | 7 +- src/librustc_resolve/macros.rs | 90 ++++++++++++------- src/libsyntax/ext/base.rs | 12 ++- src/libsyntax/ext/expand.rs | 22 +++-- src/libsyntax/feature_gate.rs | 47 ++-------- .../stmt_expr_attrs_no_feature.rs | 2 +- src/test/ui/tool-attributes-disabled-2.rs | 6 +- src/test/ui/tool-attributes-disabled-2.stderr | 11 +++ src/test/ui/tool-attributes-misplaced-1.rs | 4 +- .../ui/tool-attributes-misplaced-1.stderr | 8 +- 11 files changed, 108 insertions(+), 105 deletions(-) create mode 100644 src/test/ui/tool-attributes-disabled-2.stderr diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index f79e58f12a81..0be1bf3011e7 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -630,7 +630,9 @@ impl<'a, 'cl> Resolver<'a, 'cl> { pub fn get_macro(&mut self, def: Def) -> Lrc { let def_id = match def { Def::Macro(def_id, ..) => def_id, - Def::NonMacroAttr(..) => return Lrc::new(SyntaxExtension::NonMacroAttr), + Def::NonMacroAttr(attr_kind) => return Lrc::new(SyntaxExtension::NonMacroAttr { + mark_used: attr_kind == NonMacroAttrKind::Tool, + }), _ => panic!("expected `Def::Macro` or `Def::NonMacroAttr`"), }; if let Some(ext) = self.macro_map.get(&def_id) { diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index cc5220de33bb..d96967725f45 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3485,8 +3485,9 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { let binding = if let Some(module) = module { self.resolve_ident_in_module(module, ident, ns, record_used, path_span) } else if opt_ns == Some(MacroNS) { - self.resolve_lexical_macro_path_segment(ident, ns, record_used, path_span) - .map(MacroBinding::binding) + assert!(ns == TypeNS); + self.resolve_lexical_macro_path_segment(ident, ns, record_used, record_used, + false, path_span).map(MacroBinding::binding) } else { let record_used_id = if record_used { crate_lint.node_id().or(Some(CRATE_NODE_ID)) } else { None }; @@ -4549,6 +4550,8 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { 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) { diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index a283992c804e..b0fec11bf581 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -331,18 +331,29 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> { } else if let Def::NonMacroAttr(attr_kind) = def { let is_attr_invoc = if let InvocationKind::Attr { .. } = invoc.kind { true } else { false }; + let path = invoc.path().expect("no path for non-macro attr"); match attr_kind { - NonMacroAttrKind::Tool | NonMacroAttrKind::DeriveHelper if is_attr_invoc => { + NonMacroAttrKind::Tool | NonMacroAttrKind::DeriveHelper | + NonMacroAttrKind::Custom if is_attr_invoc => { if attr_kind == NonMacroAttrKind::Tool && !self.session.features_untracked().tool_attributes { feature_err(&self.session.parse_sess, "tool_attributes", invoc.span(), GateIssue::Language, "tool attributes are unstable").emit(); } - return Ok(Some(Lrc::new(SyntaxExtension::NonMacroAttr))); + if attr_kind == NonMacroAttrKind::Custom && + !self.session.features_untracked().custom_attribute { + let msg = format!("The attribute `{}` is currently unknown to the compiler \ + and may have meaning added to it in the future", path); + feature_err(&self.session.parse_sess, "custom_attribute", invoc.span(), + GateIssue::Language, &msg).emit(); + } + return Ok(Some(Lrc::new(SyntaxExtension::NonMacroAttr { + mark_used: attr_kind == NonMacroAttrKind::Tool, + }))); } _ => { - self.report_non_macro_attr(invoc.path_span(), def); + self.report_non_macro_attr(path.span, def); return Err(Determinacy::Determined); } } @@ -418,43 +429,42 @@ impl<'a, 'cl> Resolver<'a, 'cl> { }; let path = attr.as_ref().unwrap().path.clone(); - let mut determinacy = Determinacy::Determined; - match self.resolve_macro_to_def(scope, &path, MacroKind::Attr, force) { - Ok(def) => return Ok(def), - Err(Determinacy::Undetermined) => determinacy = Determinacy::Undetermined, - Err(Determinacy::Determined) if force => return Err(Determinacy::Determined), - Err(Determinacy::Determined) => {} + let def = self.resolve_macro_to_def(scope, &path, MacroKind::Attr, force); + if let Ok(Def::NonMacroAttr(NonMacroAttrKind::Custom)) = def {} else { + return def; } - // Ok at this point we've determined that the `attr` above doesn't - // actually resolve at this time, so we may want to report an error. - // It could be the case, though, that `attr` won't ever resolve! If - // there's a custom derive that could be used it might declare `attr` as - // a custom attribute accepted by the derive. In this case we don't want - // to report this particular invocation as unresolved, but rather we'd - // want to move on to the next invocation. + // At this point we've found that the `attr` is determinately unresolved and thus can be + // interpreted as a custom attribute. Normally custom attributes are feature gated, but + // it may be a custom attribute whitelisted by a derive macro and they do not require + // a feature gate. // - // This loop here looks through all of the derive annotations in scope - // and tries to resolve them. If they themselves successfully resolve - // *and* the resolve mentions that this attribute's name is a registered - // custom attribute then we return that custom attribute as the resolution result. - let attr_name = match path.segments.len() { - 1 => path.segments[0].ident.name, - _ => return Err(determinacy), - }; + // So here we look through all of the derive annotations in scope and try to resolve them. + // If they themselves successfully resolve *and* one of the resolved derive macros + // whitelists this attribute's name, then this is a registered attribute and we can convert + // it from a "generic custom attrite" into a "known derive helper attribute". + enum ConvertToDeriveHelper { Yes, No, DontKnow } + let mut convert_to_derive_helper = ConvertToDeriveHelper::No; + let attr_name = path.segments[0].ident.name; for path in traits { match self.resolve_macro(scope, path, MacroKind::Derive, force) { Ok(ext) => if let SyntaxExtension::ProcMacroDerive(_, ref inert_attrs, _) = *ext { if inert_attrs.contains(&attr_name) { - return Ok(Def::NonMacroAttr(NonMacroAttrKind::DeriveHelper)); + convert_to_derive_helper = ConvertToDeriveHelper::Yes; + break } }, - Err(Determinacy::Undetermined) => determinacy = Determinacy::Undetermined, + Err(Determinacy::Undetermined) => + convert_to_derive_helper = ConvertToDeriveHelper::DontKnow, Err(Determinacy::Determined) => {} } } - Err(determinacy) + match convert_to_derive_helper { + ConvertToDeriveHelper::Yes => Ok(Def::NonMacroAttr(NonMacroAttrKind::DeriveHelper)), + ConvertToDeriveHelper::No => def, + ConvertToDeriveHelper::DontKnow => Err(Determinacy::determined(force)), + } } fn resolve_macro_to_def(&mut self, scope: Mark, path: &ast::Path, kind: MacroKind, force: bool) @@ -537,10 +547,11 @@ impl<'a, 'cl> Resolver<'a, 'cl> { 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, false, span) { + match self.resolve_lexical_macro_path_segment(path[0], MacroNS, false, force, + kind == MacroKind::Attr, span) { Ok(binding) => Ok(binding.binding().def_ignoring_ambiguity()), - Err(Determinacy::Undetermined) if !force => return Err(Determinacy::Undetermined), - Err(_) => { + Err(Determinacy::Undetermined) => return Err(Determinacy::Undetermined), + Err(Determinacy::Determined) => { self.found_unresolved_macro = true; Err(Determinacy::Determined) } @@ -561,6 +572,8 @@ impl<'a, 'cl> Resolver<'a, 'cl> { mut ident: Ident, ns: Namespace, record_used: bool, + force: bool, + is_attr: bool, path_span: Span) -> Result, Determinacy> { // General principles: @@ -591,6 +604,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { // 3. Builtin attributes (closed, controlled). assert!(ns == TypeNS || ns == MacroNS); + let force = force || record_used; ident = ident.modern(); // Names from inner scope that can't shadow names from outer scopes, e.g. @@ -764,7 +778,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { Err(Determinacy::Determined) => { continue_search!(); } - Err(Determinacy::Undetermined) => return Err(Determinacy::Undetermined), + Err(Determinacy::Undetermined) => return Err(Determinacy::determined(force)), } } @@ -773,7 +787,16 @@ impl<'a, 'cl> Resolver<'a, 'cl> { return Ok(previous_result); } - if record_used { Err(Determinacy::Determined) } else { Err(Determinacy::Undetermined) } + let determinacy = Determinacy::determined(force); + if determinacy == Determinacy::Determined && is_attr { + // For attributes interpret determinate "no solution" as a custom attribute. + let binding = (Def::NonMacroAttr(NonMacroAttrKind::Custom), + ty::Visibility::Public, ident.span, Mark::root()) + .to_name_binding(self.arenas); + Ok(MacroBinding::Global(binding)) + } else { + Err(determinacy) + } } pub fn resolve_legacy_scope(&mut self, @@ -857,7 +880,8 @@ impl<'a, 'cl> Resolver<'a, 'cl> { let span = ident.span; let legacy_scope = &self.invocations[&mark].legacy_scope; let legacy_resolution = self.resolve_legacy_scope(legacy_scope, ident, true); - let resolution = self.resolve_lexical_macro_path_segment(ident, MacroNS, true, span); + let resolution = self.resolve_lexical_macro_path_segment(ident, MacroNS, true, true, + kind == MacroKind::Attr, span); let check_consistency = |this: &Self, binding: MacroBinding| { if let Some(def) = def { diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 1bc5cb93c11a..de391ee4219a 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -589,7 +589,7 @@ impl MacroKind { /// An enum representing the different kinds of syntax extensions. pub enum SyntaxExtension { /// A trivial "extension" that does nothing, only keeps the attribute and marks it as known. - NonMacroAttr, + NonMacroAttr { mark_used: bool }, /// A syntax extension that is attached to an item and creates new items /// based upon it. @@ -670,7 +670,7 @@ impl SyntaxExtension { SyntaxExtension::IdentTT(..) | SyntaxExtension::ProcMacro { .. } => MacroKind::Bang, - SyntaxExtension::NonMacroAttr | + SyntaxExtension::NonMacroAttr { .. } | SyntaxExtension::MultiDecorator(..) | SyntaxExtension::MultiModifier(..) | SyntaxExtension::AttrProcMacro(..) => @@ -700,7 +700,7 @@ impl SyntaxExtension { SyntaxExtension::AttrProcMacro(.., edition) | SyntaxExtension::ProcMacroDerive(.., edition) => edition, // Unstable legacy stuff - SyntaxExtension::NonMacroAttr | + SyntaxExtension::NonMacroAttr { .. } | SyntaxExtension::IdentTT(..) | SyntaxExtension::MultiDecorator(..) | SyntaxExtension::MultiModifier(..) | @@ -739,6 +739,12 @@ pub enum Determinacy { Undetermined, } +impl Determinacy { + pub fn determined(determined: bool) -> Determinacy { + if determined { Determinacy::Determined } else { Determinacy::Undetermined } + } +} + pub struct DummyResolver; impl Resolver for DummyResolver { diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 7148f3c00f2d..72e0abfea8bd 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -244,12 +244,12 @@ impl Invocation { } } - pub fn path_span(&self) -> Span { + pub fn path(&self) -> Option<&Path> { match self.kind { - InvocationKind::Bang { ref mac, .. } => mac.node.path.span, - InvocationKind::Attr { attr: Some(ref attr), .. } => attr.path.span, - InvocationKind::Attr { attr: None, .. } => DUMMY_SP, - InvocationKind::Derive { ref path, .. } => path.span, + InvocationKind::Bang { ref mac, .. } => Some(&mac.node.path), + InvocationKind::Attr { attr: Some(ref attr), .. } => Some(&attr.path), + InvocationKind::Attr { attr: None, .. } => None, + InvocationKind::Derive { ref path, .. } => Some(path), } } } @@ -548,7 +548,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> { _ => unreachable!(), }; - attr::mark_used(&attr); + if let NonMacroAttr { mark_used: false } = *ext {} else { + attr::mark_used(&attr); + } invoc.expansion_data.mark.set_expn_info(ExpnInfo { call_site: attr.span, def_site: None, @@ -560,7 +562,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { }); match *ext { - NonMacroAttr => { + NonMacroAttr { .. } => { attr::mark_known(&attr); let item = item.map_attrs(|mut attrs| { attrs.push(attr); attrs }); Some(invoc.fragment_kind.expect_from_annotatables(iter::once(item))) @@ -810,7 +812,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { } MultiDecorator(..) | MultiModifier(..) | - AttrProcMacro(..) | SyntaxExtension::NonMacroAttr => { + AttrProcMacro(..) | SyntaxExtension::NonMacroAttr { .. } => { self.cx.span_err(path.span, &format!("`{}` can only be used in attributes", path)); self.cx.trace_macros_diag(); @@ -1487,7 +1489,8 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { }; if attr.is_some() || !traits.is_empty() { - if !self.cx.ecfg.macros_in_extern_enabled() { + if !self.cx.ecfg.macros_in_extern_enabled() && + !self.cx.ecfg.custom_attribute_enabled() { if let Some(ref attr) = attr { emit_feature_err(&self.cx.parse_sess, "macros_in_extern", attr.span, GateIssue::Language, explain); @@ -1668,6 +1671,7 @@ impl<'feat> ExpansionConfig<'feat> { fn enable_custom_derive = custom_derive, fn enable_format_args_nl = format_args_nl, fn macros_in_extern_enabled = macros_in_extern, + fn custom_attribute_enabled = custom_attribute, fn proc_macro_mod = proc_macro_mod, fn proc_macro_gen = proc_macro_gen, fn proc_macro_expr = proc_macro_expr, diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 193e560893f3..3f4ee8342565 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -32,7 +32,7 @@ use attr; use codemap::Spanned; use edition::{ALL_EDITIONS, Edition}; use syntax_pos::{Span, DUMMY_SP}; -use errors::{DiagnosticBuilder, Handler, FatalError}; +use errors::{DiagnosticBuilder, Handler}; use visit::{self, FnKind, Visitor}; use parse::ParseSess; use symbol::{keywords, Symbol}; @@ -83,8 +83,10 @@ macro_rules! declare_features { } pub fn use_extern_macros(&self) -> bool { - // The `decl_macro` and `tool_attributes` features imply `use_extern_macros`. - self.use_extern_macros || self.decl_macro || self.tool_attributes + // The `decl_macro`, `tool_attributes` and `custom_attributes` + // features imply `use_extern_macros`. + self.use_extern_macros || self.decl_macro || + self.tool_attributes || self.custom_attribute } } }; @@ -1898,9 +1900,6 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute], } let mut features = Features::new(); - - let mut feature_checker = FeatureChecker::default(); - let mut edition_enabled_features = FxHashMap(); for &(name, .., f_edition, set) in ACTIVE_FEATURES.iter() { @@ -1989,45 +1988,9 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute], } } - feature_checker.check(span_handler); - features } -/// A collector for mutually exclusive and interdependent features and their flag spans. -#[derive(Default)] -struct FeatureChecker { - use_extern_macros: Option, - custom_attribute: Option, -} - -impl FeatureChecker { - // If this method turns out to be a hotspot due to branching, - // the branching can be eliminated by modifying `set!()` to set these spans - // only for the features that need to be checked for mutual exclusion. - fn collect(&mut self, features: &Features, span: Span) { - if features.use_extern_macros() { - // If self.use_extern_macros is None, set to Some(span) - self.use_extern_macros = self.use_extern_macros.or(Some(span)); - } - - if features.custom_attribute { - self.custom_attribute = self.custom_attribute.or(Some(span)); - } - } - - fn check(self, handler: &Handler) { - if let (Some(pm_span), Some(ca_span)) = (self.use_extern_macros, self.custom_attribute) { - handler.struct_span_err(pm_span, "Cannot use `#![feature(use_extern_macros)]` and \ - `#![feature(custom_attribute)] at the same time") - .span_note(ca_span, "`#![feature(custom_attribute)]` declared here") - .emit(); - - FatalError.raise(); - } - } -} - pub fn check_crate(krate: &ast::Crate, sess: &ParseSess, features: &Features, diff --git a/src/test/compile-fail/stmt_expr_attrs_no_feature.rs b/src/test/compile-fail/stmt_expr_attrs_no_feature.rs index d8626dfd39ee..896817bb8583 100644 --- a/src/test/compile-fail/stmt_expr_attrs_no_feature.rs +++ b/src/test/compile-fail/stmt_expr_attrs_no_feature.rs @@ -20,7 +20,7 @@ fn main() { #[attr] fn a() {} - #[attr] + #[attr] //~ ERROR attributes on expressions are experimental { } diff --git a/src/test/ui/tool-attributes-disabled-2.rs b/src/test/ui/tool-attributes-disabled-2.rs index 160dda05b1ed..2d97e160f491 100644 --- a/src/test/ui/tool-attributes-disabled-2.rs +++ b/src/test/ui/tool-attributes-disabled-2.rs @@ -11,9 +11,5 @@ // If macro modularization (`use_extern_macros`) is not enabled, // then tool attributes are treated as custom attributes. -// compile-pass - -#![feature(custom_attribute)] - -#[rustfmt::bar] +#[rustfmt::bar] //~ ERROR attribute `rustfmt::bar` is currently unknown to the compiler fn main() {} diff --git a/src/test/ui/tool-attributes-disabled-2.stderr b/src/test/ui/tool-attributes-disabled-2.stderr new file mode 100644 index 000000000000..b327773dd6ad --- /dev/null +++ b/src/test/ui/tool-attributes-disabled-2.stderr @@ -0,0 +1,11 @@ +error[E0658]: The attribute `rustfmt::bar` is currently unknown to the compiler and may have meaning added to it in the future (see issue #29642) + --> $DIR/tool-attributes-disabled-2.rs:14:1 + | +LL | #[rustfmt::bar] //~ ERROR attribute `rustfmt::bar` is currently unknown to the compiler + | ^^^^^^^^^^^^^^^ + | + = help: add #![feature(custom_attribute)] to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/tool-attributes-misplaced-1.rs b/src/test/ui/tool-attributes-misplaced-1.rs index bdbd80e7a01b..7a6b9ae9943b 100644 --- a/src/test/ui/tool-attributes-misplaced-1.rs +++ b/src/test/ui/tool-attributes-misplaced-1.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(tool_attributes)] +#![feature(tool_attributes, custom_attribute)] type A = rustfmt; //~ ERROR expected type, found tool module `rustfmt` type B = rustfmt::skip; //~ ERROR expected type, found tool attribute `rustfmt::skip` @@ -16,7 +16,7 @@ type B = rustfmt::skip; //~ ERROR expected type, found tool attribute `rustfmt:: #[derive(rustfmt)] //~ ERROR cannot find derive macro `rustfmt` in this scope struct S; -#[rustfmt] //~ ERROR cannot find attribute macro `rustfmt` in this scope +#[rustfmt] // OK, interpreted as a custom attribute fn check() {} #[rustfmt::skip] // OK diff --git a/src/test/ui/tool-attributes-misplaced-1.stderr b/src/test/ui/tool-attributes-misplaced-1.stderr index 5673c7f53052..60188aebce77 100644 --- a/src/test/ui/tool-attributes-misplaced-1.stderr +++ b/src/test/ui/tool-attributes-misplaced-1.stderr @@ -4,12 +4,6 @@ error: cannot find derive macro `rustfmt` in this scope LL | #[derive(rustfmt)] //~ ERROR cannot find derive macro `rustfmt` in this scope | ^^^^^^^ -error: cannot find attribute macro `rustfmt` in this scope - --> $DIR/tool-attributes-misplaced-1.rs:19:3 - | -LL | #[rustfmt] //~ ERROR cannot find attribute macro `rustfmt` in this scope - | ^^^^^^^ - error: cannot find macro `rustfmt!` in this scope --> $DIR/tool-attributes-misplaced-1.rs:25:5 | @@ -40,7 +34,7 @@ error[E0423]: expected value, found tool attribute `rustfmt::skip` LL | rustfmt::skip; //~ ERROR expected value, found tool attribute `rustfmt::skip` | ^^^^^^^^^^^^^ not a value -error: aborting due to 7 previous errors +error: aborting due to 6 previous errors Some errors occurred: E0423, E0573. For more information about an error, try `rustc --explain E0423`.