Avoid unnecessary MetaItem/Attribute conversions.

`check_builtin_attribute` calls `parse_meta` to convert an `Attribute`
to a `MetaItem`, which it then checks. However, many callers of
`check_builtin_attribute` start with a `MetaItem`, and then convert it
to an `Attribute` by calling `cx.attribute(meta_item)`. This `MetaItem`
to `Attribute` to `MetaItem` conversion is silly.

This commit adds a new function `check_builtin_meta_item`, which can be
called instead from these call sites. `check_builtin_attribute` also now
calls it. The commit also renames `check_meta` as `check_attr` to better
match its arguments.

Author: nnethercote

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -912,7 +912,7 @@ fn validate_generic_param_order(
 
 impl<'a> Visitor<'a> for AstValidator<'a> {
     fn visit_attribute(&mut self, attr: &Attribute) {
-        validate_attr::check_meta(&self.session.parse_sess, attr);
+        validate_attr::check_attr(&self.session.parse_sess, attr);
     }
 
     fn visit_expr(&mut self, expr: &'a Expr) {

compiler/rustc_builtin_macros/src/cfg_accessible.rs

@@ -37,10 +37,10 @@ impl MultiItemModifier for Expander {
         _is_derive_const: bool,
     ) -> ExpandResult<Vec<Annotatable>, Annotatable> {
         let template = AttributeTemplate { list: Some("path"), ..Default::default() };
-        let attr = &ecx.attribute(meta_item.clone());
-        validate_attr::check_builtin_attribute(
+        validate_attr::check_builtin_meta_item(
             &ecx.sess.parse_sess,
-            attr,
+            &meta_item,
+            ast::AttrStyle::Outer,
             sym::cfg_accessible,
             template,
         );

compiler/rustc_builtin_macros/src/derive.rs

@@ -33,14 +33,15 @@ impl MultiItemModifier for Expander {
             ecx.resolver.resolve_derives(ecx.current_expansion.id, ecx.force_mode, &|| {
                 let template =
                     AttributeTemplate { list: Some("Trait1, Trait2, ..."), ..Default::default() };
-                let attr =
-                    attr::mk_attr_outer(&sess.parse_sess.attr_id_generator, meta_item.clone());
-                validate_attr::check_builtin_attribute(
+                validate_attr::check_builtin_meta_item(
                     &sess.parse_sess,
-                    &attr,
+                    &meta_item,
+                    ast::AttrStyle::Outer,
                     sym::derive,
                     template,
                 );
+                let attr =
+                    attr::mk_attr_outer(&sess.parse_sess.attr_id_generator, meta_item.clone());
 
                 let mut resolutions: Vec<_> = attr
                     .meta_item_list()

compiler/rustc_builtin_macros/src/util.rs

@@ -1,4 +1,4 @@
-use rustc_ast::{Attribute, MetaItem};
+use rustc_ast::{AttrStyle, Attribute, MetaItem};
 use rustc_expand::base::{Annotatable, ExtCtxt};
 use rustc_feature::AttributeTemplate;
 use rustc_lint_defs::builtin::DUPLICATE_MACRO_ATTRIBUTES;
@@ -8,8 +8,13 @@ use rustc_span::Symbol;
 pub fn check_builtin_macro_attribute(ecx: &ExtCtxt<'_>, meta_item: &MetaItem, name: Symbol) {
     // All the built-in macro attributes are "words" at the moment.
     let template = AttributeTemplate { word: true, ..Default::default() };
-    let attr = ecx.attribute(meta_item.clone());
-    validate_attr::check_builtin_attribute(&ecx.sess.parse_sess, &attr, name, template);
+    validate_attr::check_builtin_meta_item(
+        &ecx.sess.parse_sess,
+        &meta_item,
+        AttrStyle::Outer,
+        name,
+        template,
+    );
 }
 
 /// Emit a warning if the item is annotated with the given attribute. This is used to diagnose when

compiler/rustc_expand/src/expand.rs

@@ -1644,7 +1644,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
         let mut span: Option<Span> = None;
         while let Some(attr) = attrs.next() {
             rustc_ast_passes::feature_gate::check_attribute(attr, self.cx.sess, features);
-            validate_attr::check_meta(&self.cx.sess.parse_sess, attr);
+            validate_attr::check_attr(&self.cx.sess.parse_sess, attr);
 
             let current_span = if let Some(sp) = span { sp.to(attr.span) } else { attr.span };
             span = Some(current_span);

compiler/rustc_parse/src/validate_attr.rs

@@ -10,9 +10,9 @@ use rustc_errors::{Applicability, FatalError, PResult};
 use rustc_feature::{AttributeTemplate, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP};
 use rustc_session::lint::builtin::ILL_FORMED_ATTRIBUTE_INPUT;
 use rustc_session::parse::ParseSess;
-use rustc_span::{sym, Symbol};
+use rustc_span::{sym, Span, Symbol};
 
-pub fn check_meta(sess: &ParseSess, attr: &Attribute) {
+pub fn check_attr(sess: &ParseSess, attr: &Attribute) {
     if attr.is_doc_comment() {
         return;
     }
@@ -115,25 +115,34 @@ pub fn check_builtin_attribute(
     name: Symbol,
     template: AttributeTemplate,
 ) {
-    // Some special attributes like `cfg` must be checked
-    // before the generic check, so we skip them here.
-    let should_skip = |name| name == sym::cfg;
-
     match parse_meta(sess, attr) {
-        Ok(meta) => {
-            if !should_skip(name) && !is_attr_template_compatible(&template, &meta.kind) {
-                emit_malformed_attribute(sess, attr, name, template);
-            }
-        }
+        Ok(meta) => check_builtin_meta_item(sess, &meta, attr.style, name, template),
         Err(mut err) => {
             err.emit();
         }
     }
 }
 
+pub fn check_builtin_meta_item(
+    sess: &ParseSess,
+    meta: &MetaItem,
+    style: ast::AttrStyle,
+    name: Symbol,
+    template: AttributeTemplate,
+) {
+    // Some special attributes like `cfg` must be checked
+    // before the generic check, so we skip them here.
+    let should_skip = |name| name == sym::cfg;
+
+    if !should_skip(name) && !is_attr_template_compatible(&template, &meta.kind) {
+        emit_malformed_attribute(sess, style, meta.span, name, template);
+    }
+}
+
 fn emit_malformed_attribute(
     sess: &ParseSess,
-    attr: &Attribute,
+    style: ast::AttrStyle,
+    span: Span,
     name: Symbol,
     template: AttributeTemplate,
 ) {
@@ -147,7 +156,7 @@ fn emit_malformed_attribute(
     let mut msg = "attribute must be of the form ".to_owned();
     let mut suggestions = vec![];
     let mut first = true;
-    let inner = if attr.style == ast::AttrStyle::Inner { "!" } else { "" };
+    let inner = if style == ast::AttrStyle::Inner { "!" } else { "" };
     if template.word {
         first = false;
         let code = format!("#{}[{}]", inner, name);
@@ -172,12 +181,12 @@ fn emit_malformed_attribute(
         suggestions.push(code);
     }
     if should_warn(name) {
-        sess.buffer_lint(&ILL_FORMED_ATTRIBUTE_INPUT, attr.span, ast::CRATE_NODE_ID, &msg);
+        sess.buffer_lint(&ILL_FORMED_ATTRIBUTE_INPUT, span, ast::CRATE_NODE_ID, &msg);
     } else {
         sess.span_diagnostic
-            .struct_span_err(attr.span, &error_msg)
+            .struct_span_err(span, &error_msg)
             .span_suggestions(
-                attr.span,
+                span,
                 if suggestions.len() == 1 {
                     "must be of the form"
                 } else {
@@ -196,7 +205,7 @@ pub fn emit_fatal_malformed_builtin_attribute(
     name: Symbol,
 ) -> ! {
     let template = BUILTIN_ATTRIBUTE_MAP.get(&name).expect("builtin attr defined").template;
-    emit_malformed_attribute(sess, attr, name, template);
+    emit_malformed_attribute(sess, attr.style, attr.span, name, template);
     // This is fatal, otherwise it will likely cause a cascade of other errors
     // (and an error here is expected to be very rare).
     FatalError.raise()