refactor target checking, move out of context.rs and rename MaybeWarn to Policy

Author: jdonszelmann

Cargo.lock

@@ -3480,7 +3480,6 @@ dependencies = [
 name = "rustc_attr_parsing"
 version = "0.0.0"
 dependencies = [
- "itertools",
  "rustc_abi",
  "rustc_ast",
  "rustc_ast_pretty",

compiler/rustc_attr_parsing/Cargo.toml

@@ -5,7 +5,6 @@ edition = "2024"
 
 [dependencies]
 # tidy-alphabetical-start
-itertools = "0.12"
 rustc_abi = { path = "../rustc_abi" }
 rustc_ast = { path = "../rustc_ast" }
 rustc_ast_pretty = { path = "../rustc_ast_pretty" }

compiler/rustc_attr_parsing/src/attributes/util.rs

@@ -1,8 +1,12 @@
+use rustc_ast::LitKind;
 use rustc_ast::attr::{AttributeExt, first_attr_value_str_by_name};
 use rustc_feature::is_builtin_attr_name;
 use rustc_hir::RustcVersion;
 use rustc_span::{Symbol, sym};
 
+use crate::context::{AcceptContext, Stage};
+use crate::parser::ArgParser;
+
 /// Parse a rustc version number written inside string literal in an attribute,
 /// like appears in `since = "1.0.0"`. Suffixes like "-dev" and "-nightly" are
 /// not accepted in this position, unlike when parsing CFG_RELEASE.
@@ -56,3 +60,32 @@ pub fn is_doc_alias_attrs_contain_symbol<'tcx, T: AttributeExt + 'tcx>(
     }
     false
 }
+
+/// Parse a single integer.
+///
+/// Used by attributes that take a single integer as argument, such as
+/// `#[link_ordinal]` and `#[rustc_layout_scalar_valid_range_start]`.
+/// `cx` is the context given to the attribute.
+/// `args` is the parser for the attribute arguments.
+pub(crate) fn parse_single_integer<S: Stage>(
+    cx: &mut AcceptContext<'_, '_, S>,
+    args: &ArgParser<'_>,
+) -> Option<u128> {
+    let Some(list) = args.list() else {
+        cx.expected_list(cx.attr_span);
+        return None;
+    };
+    let Some(single) = list.single() else {
+        cx.expected_single_argument(list.span);
+        return None;
+    };
+    let Some(lit) = single.lit() else {
+        cx.expected_integer_literal(single.span());
+        return None;
+    };
+    let LitKind::Int(num, _ty) = lit.kind else {
+        cx.expected_integer_literal(single.span());
+        return None;
+    };
+    Some(num.0)
+}

compiler/rustc_attr_parsing/src/context.rs

@@ -3,16 +3,13 @@ use std::collections::BTreeMap;
 use std::ops::{Deref, DerefMut};
 use std::sync::LazyLock;
 
-use itertools::Itertools;
 use private::Sealed;
-use rustc_ast::{self as ast, LitKind, MetaItemLit, NodeId};
+use rustc_ast::{self as ast, MetaItemLit, NodeId};
 use rustc_errors::{DiagCtxtHandle, Diagnostic};
 use rustc_feature::{AttributeTemplate, Features};
 use rustc_hir::attrs::AttributeKind;
 use rustc_hir::lints::{AttributeLint, AttributeLintKind};
-use rustc_hir::{
-    AttrArgs, AttrItem, AttrPath, Attribute, HashIgnoredAttrId, HirId, MethodKind, Target,
-};
+use rustc_hir::{AttrArgs, AttrItem, AttrPath, Attribute, HashIgnoredAttrId, HirId, Target};
 use rustc_session::Session;
 use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span, Symbol, sym};
 
@@ -64,11 +61,9 @@ use crate::attributes::traits::{
 };
 use crate::attributes::transparency::TransparencyParser;
 use crate::attributes::{AttributeParser as _, Combine, Single, WithoutArgs};
-use crate::context::MaybeWarn::{Allow, Error, Warn};
 use crate::parser::{ArgParser, MetaItemParser, PathParser};
-use crate::session_diagnostics::{
-    AttributeParseError, AttributeParseErrorReason, InvalidTarget, UnknownMetaItem,
-};
+use crate::session_diagnostics::{AttributeParseError, AttributeParseErrorReason, UnknownMetaItem};
+use crate::target_checking::AllowedTargets;
 
 type GroupType<S> = LazyLock<GroupTypeInner<S>>;
 
@@ -651,69 +646,11 @@ impl ShouldEmit {
     }
 }
 
-#[derive(Debug)]
-pub(crate) enum AllowedTargets {
-    AllowList(&'static [MaybeWarn]),
-    AllowListWarnRest(&'static [MaybeWarn]),
-}
-
-pub(crate) enum AllowedResult {
-    Allowed,
-    Warn,
-    Error,
-}
-
-impl AllowedTargets {
-    pub(crate) fn is_allowed(&self, target: Target) -> AllowedResult {
-        match self {
-            AllowedTargets::AllowList(list) => {
-                if list.contains(&Allow(target)) {
-                    AllowedResult::Allowed
-                } else if list.contains(&Warn(target)) {
-                    AllowedResult::Warn
-                } else {
-                    AllowedResult::Error
-                }
-            }
-            AllowedTargets::AllowListWarnRest(list) => {
-                if list.contains(&Allow(target)) {
-                    AllowedResult::Allowed
-                } else if list.contains(&Error(target)) {
-                    AllowedResult::Error
-                } else {
-                    AllowedResult::Warn
-                }
-            }
-        }
-    }
-
-    pub(crate) fn allowed_targets(&self) -> Vec<Target> {
-        match self {
-            AllowedTargets::AllowList(list) => list,
-            AllowedTargets::AllowListWarnRest(list) => list,
-        }
-        .iter()
-        .filter_map(|target| match target {
-            Allow(target) => Some(*target),
-            Warn(_) => None,
-            Error(_) => None,
-        })
-        .collect()
-    }
-}
-
-#[derive(Debug, Eq, PartialEq)]
-pub(crate) enum MaybeWarn {
-    Allow(Target),
-    Warn(Target),
-    Error(Target),
-}
-
 /// Context created once, for example as part of the ast lowering
 /// context, through which all attributes can be lowered.
 pub struct AttributeParser<'sess, S: Stage = Late> {
     pub(crate) tools: Vec<Symbol>,
-    features: Option<&'sess Features>,
+    pub(crate) features: Option<&'sess Features>,
     sess: &'sess Session,
     stage: S,
 
@@ -900,9 +837,9 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
                     let parser = MetaItemParser::from_attr(n, self.dcx());
                     let path = parser.path();
                     let args = parser.args();
-                    let parts = path.segments().map(|i| i.name).collect::<Vec<_>>();
+                    let path_parts = path.segments().map(|i| i.name).collect::<Vec<_>>();
 
-                    if let Some(accepts) = S::parsers().accepters.get(parts.as_slice()) {
+                    if let Some(accepts) = S::parsers().accepters.get(path_parts.as_slice()) {
                         for accept in accepts {
                             let mut cx: AcceptContext<'_, 'sess, S> = AcceptContext {
                                 shared: SharedContext {
@@ -919,44 +856,14 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
                             (accept.accept_fn)(&mut cx, args);
 
                             if self.stage.should_emit().should_emit() {
-                                match accept.allowed_targets.is_allowed(target) {
-                                    AllowedResult::Allowed => {}
-                                    AllowedResult::Warn => {
-                                        let allowed_targets =
-                                            accept.allowed_targets.allowed_targets();
-                                        let (applied, only) = allowed_targets_applied(
-                                            allowed_targets,
-                                            target,
-                                            self.features,
-                                        );
-                                        emit_lint(AttributeLint {
-                                            id: target_id,
-                                            span: attr.span,
-                                            kind: AttributeLintKind::InvalidTarget {
-                                                name: parts[0],
-                                                target,
-                                                only: if only { "only " } else { "" },
-                                                applied,
-                                            },
-                                        });
-                                    }
-                                    AllowedResult::Error => {
-                                        let allowed_targets =
-                                            accept.allowed_targets.allowed_targets();
-                                        let (applied, only) = allowed_targets_applied(
-                                            allowed_targets,
-                                            target,
-                                            self.features,
-                                        );
-                                        self.dcx().emit_err(InvalidTarget {
-                                            span: attr.span,
-                                            name: parts[0],
-                                            target: target.plural_name(),
-                                            only: if only { "only " } else { "" },
-                                            applied,
-                                        });
-                                    }
-                                }
+                                self.check_target(
+                                    path.get_attribute_path(),
+                                    attr.span,
+                                    &accept.allowed_targets,
+                                    target,
+                                    target_id,
+                                    &mut emit_lint,
+                                );
                             }
                         }
                     } else {
@@ -1044,158 +951,3 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
         }
     }
 }
-
-/// Takes a list of `allowed_targets` for an attribute, and the `target` the attribute was applied to.
-/// Does some heuristic-based filtering to remove uninteresting targets, and formats the targets into a string
-pub(crate) fn allowed_targets_applied(
-    mut allowed_targets: Vec<Target>,
-    target: Target,
-    features: Option<&Features>,
-) -> (String, bool) {
-    // Remove unstable targets from `allowed_targets` if their features are not enabled
-    if let Some(features) = features {
-        if !features.fn_delegation() {
-            allowed_targets.retain(|t| !matches!(t, Target::Delegation { .. }));
-        }
-        if !features.stmt_expr_attributes() {
-            allowed_targets.retain(|t| !matches!(t, Target::Expression | Target::Statement));
-        }
-    }
-
-    // We define groups of "similar" targets.
-    // If at least two of the targets are allowed, and the `target` is not in the group,
-    // we collapse the entire group to a single entry to simplify the target list
-    const FUNCTION_LIKE: &[Target] = &[
-        Target::Fn,
-        Target::Closure,
-        Target::ForeignFn,
-        Target::Method(MethodKind::Inherent),
-        Target::Method(MethodKind::Trait { body: false }),
-        Target::Method(MethodKind::Trait { body: true }),
-        Target::Method(MethodKind::TraitImpl),
-    ];
-    const METHOD_LIKE: &[Target] = &[
-        Target::Method(MethodKind::Inherent),
-        Target::Method(MethodKind::Trait { body: false }),
-        Target::Method(MethodKind::Trait { body: true }),
-        Target::Method(MethodKind::TraitImpl),
-    ];
-    const IMPL_LIKE: &[Target] =
-        &[Target::Impl { of_trait: false }, Target::Impl { of_trait: true }];
-    const ADT_LIKE: &[Target] = &[Target::Struct, Target::Enum];
-
-    let mut added_fake_targets = Vec::new();
-    filter_targets(
-        &mut allowed_targets,
-        FUNCTION_LIKE,
-        "functions",
-        target,
-        &mut added_fake_targets,
-    );
-    filter_targets(&mut allowed_targets, METHOD_LIKE, "methods", target, &mut added_fake_targets);
-    filter_targets(&mut allowed_targets, IMPL_LIKE, "impl blocks", target, &mut added_fake_targets);
-    filter_targets(&mut allowed_targets, ADT_LIKE, "data types", target, &mut added_fake_targets);
-
-    // If there is now only 1 target left, show that as the only possible target
-    (
-        added_fake_targets
-            .iter()
-            .copied()
-            .chain(allowed_targets.iter().map(|t| t.plural_name()))
-            .join(", "),
-        allowed_targets.len() + added_fake_targets.len() == 1,
-    )
-}
-
-fn filter_targets(
-    allowed_targets: &mut Vec<Target>,
-    target_group: &'static [Target],
-    target_group_name: &'static str,
-    target: Target,
-    added_fake_targets: &mut Vec<&'static str>,
-) {
-    if target_group.contains(&target) {
-        return;
-    }
-    if allowed_targets.iter().filter(|at| target_group.contains(at)).count() < 2 {
-        return;
-    }
-    allowed_targets.retain(|t| !target_group.contains(t));
-    added_fake_targets.push(target_group_name);
-}
-
-/// This is the list of all targets to which a attribute can be applied
-/// This is used for:
-/// - `rustc_dummy`, which can be applied to all targets
-/// - Attributes that are not parted to the new target system yet can use this list as a placeholder
-pub(crate) const ALL_TARGETS: &'static [MaybeWarn] = &[
-    Allow(Target::ExternCrate),
-    Allow(Target::Use),
-    Allow(Target::Static),
-    Allow(Target::Const),
-    Allow(Target::Fn),
-    Allow(Target::Closure),
-    Allow(Target::Mod),
-    Allow(Target::ForeignMod),
-    Allow(Target::GlobalAsm),
-    Allow(Target::TyAlias),
-    Allow(Target::Enum),
-    Allow(Target::Variant),
-    Allow(Target::Struct),
-    Allow(Target::Field),
-    Allow(Target::Union),
-    Allow(Target::Trait),
-    Allow(Target::TraitAlias),
-    Allow(Target::Impl { of_trait: false }),
-    Allow(Target::Impl { of_trait: true }),
-    Allow(Target::Expression),
-    Allow(Target::Statement),
-    Allow(Target::Arm),
-    Allow(Target::AssocConst),
-    Allow(Target::Method(MethodKind::Inherent)),
-    Allow(Target::Method(MethodKind::Trait { body: false })),
-    Allow(Target::Method(MethodKind::Trait { body: true })),
-    Allow(Target::Method(MethodKind::TraitImpl)),
-    Allow(Target::AssocTy),
-    Allow(Target::ForeignFn),
-    Allow(Target::ForeignStatic),
-    Allow(Target::ForeignTy),
-    Allow(Target::MacroDef),
-    Allow(Target::Param),
-    Allow(Target::PatField),
-    Allow(Target::ExprField),
-    Allow(Target::WherePredicate),
-    Allow(Target::MacroCall),
-    Allow(Target::Crate),
-    Allow(Target::Delegation { mac: false }),
-    Allow(Target::Delegation { mac: true }),
-];
-
-/// Parse a single integer.
-///
-/// Used by attributes that take a single integer as argument, such as
-/// `#[link_ordinal]` and `#[rustc_layout_scalar_valid_range_start]`.
-/// `cx` is the context given to the attribute.
-/// `args` is the parser for the attribute arguments.
-pub(crate) fn parse_single_integer<S: Stage>(
-    cx: &mut AcceptContext<'_, '_, S>,
-    args: &ArgParser<'_>,
-) -> Option<u128> {
-    let Some(list) = args.list() else {
-        cx.expected_list(cx.attr_span);
-        return None;
-    };
-    let Some(single) = list.single() else {
-        cx.expected_single_argument(list.span);
-        return None;
-    };
-    let Some(lit) = single.lit() else {
-        cx.expected_integer_literal(single.span());
-        return None;
-    };
-    let LitKind::Int(num, _ty) = lit.kind else {
-        cx.expected_integer_literal(single.span());
-        return None;
-    };
-    Some(num.0)
-}

compiler/rustc_attr_parsing/src/lib.rs

@@ -89,6 +89,7 @@ pub(crate) mod context;
 mod lints;
 pub mod parser;
 mod session_diagnostics;
+mod target_checking;
 
 pub use attributes::cfg::{CFG_TEMPLATE, EvalConfigResult, eval_config_entry, parse_cfg_attr};
 pub use attributes::cfg_old::*;