check earlier for misused crate-level attributes

Author: scrabsha

compiler/rustc_ast_passes/src/feature_gate.rs

@@ -3,8 +3,8 @@ use rustc_ast::{self as ast, AttrVec, NodeId, PatKind, attr, token};
 use rustc_attr_parsing::AttributeParser;
 use rustc_errors::msg;
 use rustc_feature::{AttributeGate, BUILTIN_ATTRIBUTE_MAP, BuiltinAttribute, Features};
-use rustc_hir::Attribute;
 use rustc_hir::attrs::AttributeKind;
+use rustc_hir::{Attribute, Target};
 use rustc_session::Session;
 use rustc_session::parse::{feature_err, feature_warn};
 use rustc_span::source_map::Spanned;
@@ -657,6 +657,7 @@ fn maybe_stage_features(sess: &Session, features: &Features, krate: &ast::Crate)
             sess,
             &krate.attrs,
             sym::feature,
+            Target::Crate,
             DUMMY_SP,
             krate.id,
             Some(&features),

compiler/rustc_attr_parsing/src/errors.rs

@@ -0,0 +1,26 @@
+use rustc_macros::{Diagnostic, Subdiagnostic};
+use rustc_span::{Span, Symbol};
+
+#[derive(Diagnostic)]
+#[diag("`{$name}` attribute cannot be used at crate level")]
+pub(crate) struct InvalidAttrAtCrateLevel {
+    #[primary_span]
+    pub span: Span,
+    #[suggestion(
+        "perhaps you meant to use an outer attribute",
+        code = "",
+        applicability = "machine-applicable",
+        style = "verbose"
+    )]
+    pub sugg_span: Option<Span>,
+    pub name: Symbol,
+    #[subdiagnostic]
+    pub item: Option<ItemFollowingInnerAttr>,
+}
+
+#[derive(Clone, Copy, Subdiagnostic)]
+#[label("the inner attribute doesn't annotate this item")]
+pub(crate) struct ItemFollowingInnerAttr {
+    #[primary_span]
+    pub span: Span,
+}

compiler/rustc_attr_parsing/src/interface.rs

@@ -3,17 +3,18 @@ use std::convert::identity;
 use rustc_ast as ast;
 use rustc_ast::token::DocFragmentKind;
 use rustc_ast::{AttrItemKind, AttrStyle, NodeId, Safety};
-use rustc_errors::DiagCtxtHandle;
+use rustc_errors::{DiagCtxtHandle, StashKey};
 use rustc_feature::{AttributeTemplate, Features};
 use rustc_hir::attrs::AttributeKind;
 use rustc_hir::lints::AttributeLintKind;
 use rustc_hir::{AttrArgs, AttrItem, AttrPath, Attribute, HashIgnoredAttrId, Target};
 use rustc_session::Session;
 use rustc_session::lint::{BuiltinLintDiag, LintId};
-use rustc_span::{DUMMY_SP, Span, Symbol, sym};
+use rustc_span::{BytePos, DUMMY_SP, Span, Symbol, sym};
 
 use crate::context::{AcceptContext, FinalizeContext, FinalizeFn, SharedContext, Stage};
 use crate::early_parsed::{EARLY_PARSED_ATTRIBUTES, EarlyParsedState};
+use crate::errors::{InvalidAttrAtCrateLevel, ItemFollowingInnerAttr};
 use crate::parser::{ArgParser, PathParser, RefPathParser};
 use crate::session_diagnostics::ParsedDescription;
 use crate::{Early, Late, OmitDoc, ShouldEmit};
@@ -51,6 +52,7 @@ impl<'sess> AttributeParser<'sess, Early> {
         sess: &'sess Session,
         attrs: &[ast::Attribute],
         sym: Symbol,
+        target: Target,
         target_span: Span,
         target_node_id: NodeId,
         features: Option<&'sess Features>,
@@ -59,6 +61,7 @@ impl<'sess> AttributeParser<'sess, Early> {
             sess,
             attrs,
             sym,
+            target,
             target_span,
             target_node_id,
             features,
@@ -72,6 +75,7 @@ impl<'sess> AttributeParser<'sess, Early> {
         sess: &'sess Session,
         attrs: &[ast::Attribute],
         sym: Symbol,
+        target: Target,
         target_span: Span,
         target_node_id: NodeId,
         features: Option<&'sess Features>,
@@ -81,7 +85,7 @@ impl<'sess> AttributeParser<'sess, Early> {
             sess,
             attrs,
             Some(sym),
-            Target::Crate, // Does not matter, we're not going to emit errors anyways
+            target,
             target_span,
             target_node_id,
             features,
@@ -302,7 +306,7 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
                         kind: DocFragmentKind::Sugared(*comment_kind),
                         span: attr_span,
                         comment: *symbol,
-                    }))
+                    }));
                 }
                 ast::AttrKind::Normal(n) => {
                     attr_paths.push(PathParser(&n.item.path));
@@ -405,15 +409,23 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
                         //     "attribute {path} wasn't parsed and isn't a know tool attribute",
                         // );
 
-                        attributes.push(Attribute::Unparsed(Box::new(AttrItem {
+                        let attr = AttrItem {
                             path: attr_path.clone(),
                             args: self
                                 .lower_attr_args(n.item.args.unparsed_ref().unwrap(), lower_span),
                             id: HashIgnoredAttrId { attr_id: attr.id },
                             style: attr.style,
                             span: attr_span,
-                        })));
-                    }
+                        };
+
+                        if !matches!(self.stage.should_emit(), ShouldEmit::Nothing)
+                            && target == Target::Crate
+                        {
+                            self.check_invalid_crate_level_attr_item(&attr);
+                        }
+
+                        attributes.push(Attribute::Unparsed(Box::new(attr)));
+                    };
                 }
             }
         }
@@ -477,4 +489,96 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
             }
         }
     }
+
+    // FIXME: Fix "Cannot determine resolution" error and remove built-in macros
+    // from this check.
+    fn check_invalid_crate_level_attr_item(&self, attr: &AttrItem) {
+        // Check for builtin attributes at the crate level
+        // which were unsuccessfully resolved due to cannot determine
+        // resolution for the attribute macro error.
+        const ATTRS_TO_CHECK: &[Symbol] =
+            &[sym::derive, sym::test, sym::test_case, sym::global_allocator, sym::bench];
+
+        // FIXME(jdonszelmann): all attrs should be combined here cleaning this up some day.
+        if let Some(name) = ATTRS_TO_CHECK.iter().find(|attr_to_check| matches!(attr.path.segments.as_ref(), [segment] if segment == *attr_to_check)) {
+            let span = attr.span;
+            let name = *name;
+
+            let item = self.first_line_of_next_item(span).map(|span| ItemFollowingInnerAttr { span });
+
+            let err = self.dcx().create_err(InvalidAttrAtCrateLevel {
+                span,
+                sugg_span: self
+                    .sess
+                    .source_map()
+                    .span_to_snippet(attr.span)
+                    .ok()
+                    .filter(|src| src.starts_with("#!["))
+                    .map(|_| {
+                        span
+                            .with_lo(span.lo() + BytePos(1))
+                            .with_hi(span.lo() + BytePos(2))
+                    }),
+                name,
+                item,
+            });
+
+            self.dcx().try_steal_replace_and_emit_err(
+                attr.path.span,
+                StashKey::UndeterminedMacroResolution,
+                err,
+            );
+        }
+    }
+
+    pub(crate) fn first_line_of_next_item(&self, span: Span) -> Option<Span> {
+        // We can't exactly call `tcx.hir_free_items()` here because it's too early and querying
+        // this would create a circular dependency. Instead, we resort to getting the original
+        // source code that follows `span` and find the next item from here.
+
+        self.sess()
+            .source_map()
+            .span_to_source(span, |content, _, span_end| {
+                let mut source = &content[span_end..];
+                let initial_source_len = source.len();
+                let span = try {
+                    loop {
+                        let first = source.chars().next()?;
+
+                        if first.is_whitespace() {
+                            let split_idx = source.find(|c: char| !c.is_whitespace())?;
+                            source = &source[split_idx..];
+                        } else if source.starts_with("//") {
+                            let line_idx = source.find('\n')?;
+                            source = &source[line_idx + '\n'.len_utf8()..];
+                        } else if source.starts_with("/*") {
+                            // FIXME: support nested comments.
+                            let close_idx = source.find("*/")?;
+                            source = &source[close_idx + "*/".len()..];
+                        } else if first == '#' {
+                            // FIXME: properly find the end of the attributes in order to accurately
+                            // skip them. This version just consumes the source code until the next
+                            // `]`.
+                            let close_idx = source.find(']')?;
+                            source = &source[close_idx + ']'.len_utf8()..];
+                        } else {
+                            let lo = span_end + initial_source_len - source.len();
+                            let last_line = source.split('\n').next().map(|s| s.trim_end())?;
+
+                            let hi = lo + last_line.len();
+                            let lo = BytePos(lo as u32);
+                            let hi = BytePos(hi as u32);
+                            let next_item_span = Span::new(lo, hi, span.ctxt(), None);
+
+                            break next_item_span;
+                        }
+                    }
+                };
+                
+
+                Ok(span)
+            })
+            .ok()
+            .flatten()
+    }
 }

compiler/rustc_attr_parsing/src/lib.rs

@@ -80,6 +80,7 @@
 #![cfg_attr(bootstrap, feature(if_let_guard))]
 #![feature(decl_macro)]
 #![feature(iter_intersperse)]
+#![feature(try_blocks)]
 #![recursion_limit = "256"]
 // tidy-alphabetical-end
 
@@ -100,6 +101,7 @@ mod interface;
 pub mod parser;
 
 mod early_parsed;
+mod errors;
 mod safety;
 mod session_diagnostics;
 mod target_checking;

compiler/rustc_attr_parsing/src/target_checking.rs

@@ -5,7 +5,7 @@ use rustc_errors::DiagArgValue;
 use rustc_feature::Features;
 use rustc_hir::lints::AttributeLintKind;
 use rustc_hir::{MethodKind, Target};
-use rustc_span::sym;
+use rustc_span::{BytePos, sym};
 
 use crate::AttributeParser;
 use crate::context::{AcceptContext, Stage};
@@ -96,6 +96,34 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
             return;
         }
 
+        if matches!(cx.attr_path.segments.as_ref(), [sym::repr]) && target == Target::Crate {
+            // The allowed targets of `repr` depend on its arguments. They can't be checked using
+            // the `AttributeParser` code.
+            let span = cx.attr_span;
+            let item = cx
+                .cx
+                .first_line_of_next_item(span)
+                .map(|span| crate::errors::ItemFollowingInnerAttr { span });
+
+            let sugg_span = cx
+                .cx
+                .sess
+                .source_map()
+                .span_to_snippet(span)
+                .ok()
+                .filter(|src| src.starts_with("#!["))
+                .map(|_| span.with_lo(span.lo() + BytePos(1)).with_hi(span.lo() + BytePos(2)));
+
+            cx.dcx()
+                .create_err(crate::errors::InvalidAttrAtCrateLevel {
+                    span,
+                    sugg_span,
+                    name: sym::repr,
+                    item,
+                })
+                .emit();
+        }
+
         match allowed_targets.is_allowed(target) {
             AllowedResult::Allowed => {}
             AllowedResult::Warn => {

compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

@@ -188,8 +188,8 @@ use rustc_ast::{
 };
 use rustc_attr_parsing::AttributeParser;
 use rustc_expand::base::{Annotatable, ExtCtxt};
-use rustc_hir::Attribute;
 use rustc_hir::attrs::{AttributeKind, ReprPacked};
+use rustc_hir::{Attribute, Target};
 use rustc_span::{DUMMY_SP, Ident, Span, Symbol, kw, sym};
 use thin_vec::{ThinVec, thin_vec};
 use ty::{Bounds, Path, Ref, Self_, Ty};
@@ -493,7 +493,7 @@ impl<'a> TraitDef<'a> {
         match item {
             Annotatable::Item(item) => {
                 let is_packed = matches!(
-                    AttributeParser::parse_limited(cx.sess, &item.attrs, sym::repr, item.span, item.id, None),
+                    AttributeParser::parse_limited(cx.sess, &item.attrs, sym::repr, Target::from_ast_item(item), item.span, item.id, None),
                     Some(Attribute::Parsed(AttributeKind::Repr { reprs, .. })) if reprs.iter().any(|(x, _)| matches!(x, ReprPacked(..)))
                 );
 

compiler/rustc_builtin_macros/src/proc_macro_harness.rs

@@ -8,6 +8,7 @@ use rustc_errors::DiagCtxtHandle;
 use rustc_expand::base::{ExtCtxt, ResolverExpand};
 use rustc_expand::expand::{AstFragment, ExpansionConfig};
 use rustc_feature::Features;
+use rustc_hir::Target;
 use rustc_hir::attrs::AttributeKind;
 use rustc_session::Session;
 use rustc_span::hygiene::AstPass;
@@ -109,6 +110,7 @@ impl<'a> CollectProcMacros<'a> {
             self.session,
             slice::from_ref(attr),
             sym::proc_macro_derive,
+            Target::from_ast_item(item),
             item.span,
             item.node_id(),
             None,

compiler/rustc_builtin_macros/src/test.rs

@@ -9,8 +9,8 @@ use rustc_attr_parsing::AttributeParser;
 use rustc_data_structures::assert_matches;
 use rustc_errors::{Applicability, Diag, Level};
 use rustc_expand::base::*;
-use rustc_hir::Attribute;
 use rustc_hir::attrs::AttributeKind;
+use rustc_hir::{Attribute, Target};
 use rustc_span::{ErrorGuaranteed, Ident, RemapPathScopeComponents, Span, Symbol, sym};
 use thin_vec::{ThinVec, thin_vec};
 use tracing::debug;
@@ -485,6 +485,7 @@ fn should_panic(cx: &ExtCtxt<'_>, i: &ast::Item) -> ShouldPanic {
             cx.sess,
             &i.attrs,
             sym::should_panic,
+            Target::from_ast_item(i),
             i.span,
             i.node_id(),
             None,

compiler/rustc_builtin_macros/src/test_harness.rs

@@ -12,6 +12,7 @@ use rustc_attr_parsing::AttributeParser;
 use rustc_expand::base::{ExtCtxt, ResolverExpand};
 use rustc_expand::expand::{AstFragment, ExpansionConfig};
 use rustc_feature::Features;
+use rustc_hir::Target;
 use rustc_hir::attrs::AttributeKind;
 use rustc_session::Session;
 use rustc_session::lint::builtin::UNNAMEABLE_TEST_ITEMS;
@@ -392,6 +393,7 @@ fn get_test_runner(sess: &Session, features: &Features, krate: &ast::Crate) -> O
         sess,
         &krate.attrs,
         sym::test_runner,
+        Target::Crate,
         krate.spans.inner_span,
         krate.id,
         Some(features),

compiler/rustc_expand/src/config.rs

@@ -54,6 +54,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute], crate_name: Symbol) -
             sess,
             krate_attrs,
             sym::feature,
+            Target::Crate,
             DUMMY_SP,
             DUMMY_NODE_ID,
             Some(&features),