check earlier for misused crate-level attributes

Author: scrabsha

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 pound_to_opening_bracket: 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

@@ -304,7 +304,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));
@@ -408,15 +408,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, n.item.span());
+                        }
+
+                        attributes.push(Attribute::Unparsed(Box::new(attr)));
+                    };
                 }
             }
         }

compiler/rustc_attr_parsing/src/lib.rs

@@ -79,6 +79,7 @@
 // tidy-alphabetical-start
 #![feature(decl_macro)]
 #![feature(iter_intersperse)]
+#![feature(try_blocks)]
 #![recursion_limit = "256"]
 // tidy-alphabetical-end
 
@@ -99,6 +100,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

@@ -1,14 +1,15 @@
 use std::borrow::Cow;
 
 use rustc_ast::AttrStyle;
-use rustc_errors::DiagArgValue;
+use rustc_errors::{DiagArgValue, StashKey};
 use rustc_feature::Features;
 use rustc_hir::lints::AttributeLintKind;
-use rustc_hir::{MethodKind, Target};
-use rustc_span::sym;
+use rustc_hir::{AttrItem, MethodKind, Target};
+use rustc_span::{BytePos, Span, Symbol, sym};
 
 use crate::AttributeParser;
 use crate::context::{AcceptContext, Stage};
+use crate::errors::{InvalidAttrAtCrateLevel, ItemFollowingInnerAttr};
 use crate::session_diagnostics::InvalidTarget;
 use crate::target_checking::Policy::Allow;
 
@@ -96,6 +97,25 @@ 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| ItemFollowingInnerAttr { span });
+
+            let pound_to_opening_bracket = cx.attr_span.until(cx.inner_span);
+
+            cx.dcx()
+                .create_err(InvalidAttrAtCrateLevel {
+                    span,
+                    pound_to_opening_bracket,
+                    name: sym::repr,
+                    item,
+                })
+                .emit();
+        }
+
         match allowed_targets.is_allowed(target) {
             AllowedResult::Allowed => {}
             AllowedResult::Warn => {
@@ -163,6 +183,87 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
 
         cx.emit_lint(rustc_session::lint::builtin::UNUSED_ATTRIBUTES, kind, attr_span);
     }
+
+    // FIXME: Fix "Cannot determine resolution" error and remove built-in macros
+    // from this check.
+    pub(crate) fn check_invalid_crate_level_attr_item(&self, attr: &AttrItem, inner_span: Span) {
+        // 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,
+                pound_to_opening_bracket: span.until(inner_span),
+                name,
+                item,
+            });
+
+            self.dcx().try_steal_replace_and_emit_err(
+                attr.path.span,
+                StashKey::UndeterminedMacroResolution,
+                err,
+            );
+        }
+    }
+
+    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()
+    }
 }
 
 /// Takes a list of `allowed_targets` for an attribute, and the `target` the attribute was applied to.

compiler/rustc_passes/src/check_attr.rs

@@ -15,8 +15,11 @@ use rustc_attr_parsing::{AttributeParser, Late};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::thin_vec::ThinVec;
 use rustc_data_structures::unord::UnordMap;
-use rustc_errors::{DiagCtxtHandle, IntoDiagArg, MultiSpan, StashKey, msg};
-use rustc_feature::{AttributeDuplicates, AttributeType, BUILTIN_ATTRIBUTE_MAP, BuiltinAttribute};
+use rustc_errors::{DiagCtxtHandle, IntoDiagArg, MultiSpan,  msg};
+ use rustc_feature::{
+     AttributeDuplicates, AttributeType, BUILTIN_ATTRIBUTE_MAP,
+     BuiltinAttribute,
+ };
 use rustc_hir::attrs::diagnostic::Directive;
 use rustc_hir::attrs::{
     AttributeKind, DocAttribute, DocInline, EiiDecl, EiiImpl, EiiImplResolution, InlineAttr,
@@ -45,7 +48,7 @@ use rustc_session::lint::builtin::{
 };
 use rustc_session::parse::feature_err;
 use rustc_span::edition::Edition;
-use rustc_span::{BytePos, DUMMY_SP, Ident, Span, Symbol, sym};
+use rustc_span::{DUMMY_SP, Ident, Span, Symbol, sym};
 use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
 use rustc_trait_selection::infer::{TyCtxtInferExt, ValuePairs};
 use rustc_trait_selection::traits::ObligationCtxt;
@@ -2025,64 +2028,6 @@ fn is_c_like_enum(item: &Item<'_>) -> bool {
     }
 }
 
-// FIXME: Fix "Cannot determine resolution" error and remove built-in macros
-// from this check.
-fn check_invalid_crate_level_attr(tcx: TyCtxt<'_>, attrs: &[Attribute]) {
-    // 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];
-
-    for attr in attrs {
-        // FIXME(jdonszelmann): all attrs should be combined here cleaning this up some day.
-        let (span, name) = if let Some(a) =
-            ATTRS_TO_CHECK.iter().find(|attr_to_check| attr.has_name(**attr_to_check))
-        {
-            (attr.span(), *a)
-        } else if let Attribute::Parsed(AttributeKind::Repr {
-            reprs: _,
-            first_span: first_attr_span,
-        }) = attr
-        {
-            (*first_attr_span, sym::repr)
-        } else {
-            continue;
-        };
-
-        let item = tcx
-            .hir_free_items()
-            .map(|id| tcx.hir_item(id))
-            .find(|item| !item.span.is_dummy()) // Skip prelude `use`s
-            .map(|item| errors::ItemFollowingInnerAttr {
-                span: if let Some(ident) = item.kind.ident() { ident.span } else { item.span },
-                kind: tcx.def_descr(item.owner_id.to_def_id()),
-            });
-        let err = tcx.dcx().create_err(errors::InvalidAttrAtCrateLevel {
-            span,
-            sugg_span: tcx
-                .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))),
-            name,
-            item,
-        });
-
-        if let Attribute::Unparsed(p) = attr {
-            tcx.dcx().try_steal_replace_and_emit_err(
-                p.path.span,
-                StashKey::UndeterminedMacroResolution,
-                err,
-            );
-        } else {
-            err.emit();
-        }
-    }
-}
-
 fn check_non_exported_macro_for_invalid_attrs(tcx: TyCtxt<'_>, item: &Item<'_>) {
     let attrs = tcx.hir_attrs(item.hir_id());
 
@@ -2098,7 +2043,6 @@ fn check_mod_attrs(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
     tcx.hir_visit_item_likes_in_module(module_def_id, check_attr_visitor);
     if module_def_id.to_local_def_id().is_top_level_module() {
         check_attr_visitor.check_attributes(CRATE_HIR_ID, DUMMY_SP, Target::Mod, None);
-        check_invalid_crate_level_attr(tcx, tcx.hir_krate_attrs());
     }
     if check_attr_visitor.abort.get() {
         tcx.dcx().abort_if_errors()

compiler/rustc_passes/src/errors.rs

@@ -3,8 +3,7 @@ use std::path::{Path, PathBuf};
 
 use rustc_errors::codes::*;
 use rustc_errors::{
-    Applicability, Diag, DiagCtxtHandle, DiagSymbolList, Diagnostic, EmissionGuarantee, Level,
-    MultiSpan, msg,
+    Diag, DiagCtxtHandle, DiagSymbolList, Diagnostic, EmissionGuarantee, Level, MultiSpan, msg,
 };
 use rustc_hir::Target;
 use rustc_macros::{Diagnostic, Subdiagnostic};
@@ -447,44 +446,6 @@ pub(crate) struct LangItemOnIncorrectTarget {
     pub actual_target: Target,
 }
 
-pub(crate) struct InvalidAttrAtCrateLevel {
-    pub span: Span,
-    pub sugg_span: Option<Span>,
-    pub name: Symbol,
-    pub item: Option<ItemFollowingInnerAttr>,
-}
-
-#[derive(Clone, Copy)]
-pub(crate) struct ItemFollowingInnerAttr {
-    pub span: Span,
-    pub kind: &'static str,
-}
-
-impl<G: EmissionGuarantee> Diagnostic<'_, G> for InvalidAttrAtCrateLevel {
-    #[track_caller]
-    fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
-        let mut diag =
-            Diag::new(dcx, level, msg!("`{$name}` attribute cannot be used at crate level"));
-        diag.span(self.span);
-        diag.arg("name", self.name);
-        // Only emit an error with a suggestion if we can create a string out
-        // of the attribute span
-        if let Some(span) = self.sugg_span {
-            diag.span_suggestion_verbose(
-                span,
-                msg!("perhaps you meant to use an outer attribute"),
-                String::new(),
-                Applicability::MachineApplicable,
-            );
-        }
-        if let Some(item) = self.item {
-            diag.arg("kind", item.kind);
-            diag.span_label(item.span, msg!("the inner attribute doesn't annotate this {$kind}"));
-        }
-        diag
-    }
-}
-
 #[derive(Diagnostic)]
 #[diag("duplicate diagnostic item in crate `{$crate_name}`: `{$name}`")]
 pub(crate) struct DuplicateDiagnosticItemInCrate {

tests/ui/attributes/malformed-reprs.stderr

@@ -6,27 +6,27 @@ LL | #![repr]
    |
    = note: for more information, visit <https://doc.rust-lang.org/reference/type-layout.html#representations>
 
-error[E0589]: invalid `repr(align)` attribute: not a power of two
-  --> $DIR/malformed-reprs.rs:9:14
-   |
-LL | #[repr(align(0))]
-   |              ^
-
 error: `repr` attribute cannot be used at crate level
   --> $DIR/malformed-reprs.rs:4:1
    |
 LL | #![repr]
    | ^^^^^^^^
 ...
 LL | enum Foo {}
-   |      --- the inner attribute doesn't annotate this enum
+   | ----------- the inner attribute doesn't annotate this item
    |
 help: perhaps you meant to use an outer attribute
    |
 LL - #![repr]
 LL + #[repr]
    |
 
+error[E0589]: invalid `repr(align)` attribute: not a power of two
+  --> $DIR/malformed-reprs.rs:9:14
+   |
+LL | #[repr(align(0))]
+   |              ^
+
 error[E0084]: unsupported representation for zero-variant enum
   --> $DIR/malformed-reprs.rs:9:1
    |