Support custom attributes when macro modularization is enabled

11 files changed, 108 insertions, 105 deletions

diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index f79e58f12a8..0be1bf3011e 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<SyntaxExtension> {
         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 cc5220de33b..d96967725f4 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 a283992c804..b0fec11bf58 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<MacroBinding<'a>, 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 1bc5cb93c11..de391ee4219 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 7148f3c00f2..72e0abfea8b 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 193e560893f..3f4ee834256 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<Span>,
-    custom_attribute: Option<Span>,
-}
-
-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 d8626dfd39e..896817bb858 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 160dda05b1e..2d97e160f49 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 00000000000..b327773dd6a
--- /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 bdbd80e7a01..7a6b9ae9943 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 5673c7f5305..60188aebce7 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`.