Auto merge of rust-lang#95562 - lcnr:attr-no-encode, r=davidtwco

don't encode only locally used attrs

Part of rust-lang/compiler-team#505.

We now filter builtin attributes before encoding them in the crate metadata in case they should only be used in the local crate. To prevent accidental misuse `get_attrs` now requires the caller to state which attribute they are interested in. For places where that isn't trivially possible, I've added a method `fn get_attrs_unchecked` which I intend to remove in a followup PR.

After this pull request landed, we can then slowly move all attributes to only be used in the local crate while being certain that we don't accidentally try to access them from extern crates.

cc rust-lang#94963 (comment)

Author: bors

compiler/rustc_attr/src/builtin.rs

@@ -6,6 +6,7 @@ use rustc_hir::def_id::DefId;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_session::config::OptLevel;
+use rustc_span::symbol::sym;
 use rustc_target::spec::abi::Abi;
 use rustc_target::spec::{FramePointer, SanitizerSet, StackProbeType, StackProtector};
 use smallvec::SmallVec;
@@ -329,9 +330,7 @@ pub fn from_fn_attrs<'ll, 'tcx>(
     ) {
         let span = cx
             .tcx
-            .get_attrs(instance.def_id())
-            .iter()
-            .find(|a| a.has_name(rustc_span::sym::target_feature))
+            .get_attr(instance.def_id(), sym::target_feature)
             .map_or_else(|| cx.tcx.def_span(instance.def_id()), |a| a.span);
         let msg = format!(
             "the target features {} must all be either enabled or disabled together",

compiler/rustc_codegen_llvm/src/attributes.rs

@@ -312,11 +312,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
 
             Status::Unstable(gate) if self.tcx.features().enabled(gate) => {
                 let unstable_in_stable = self.ccx.is_const_stable_const_fn()
-                    && !super::rustc_allow_const_fn_unstable(
-                        self.tcx,
-                        self.def_id().to_def_id(),
-                        gate,
-                    );
+                    && !super::rustc_allow_const_fn_unstable(self.tcx, self.def_id(), gate);
                 if unstable_in_stable {
                     emit_unstable_in_stable_error(self.ccx, span, gate);
                 }
@@ -713,7 +709,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
         match &terminator.kind {
             TerminatorKind::Call { func, args, fn_span, from_hir_call, .. } => {
                 let ConstCx { tcx, body, param_env, .. } = *self.ccx;
-                let caller = self.def_id().to_def_id();
+                let caller = self.def_id();
 
                 let fn_ty = func.ty(body, tcx);
 
@@ -797,7 +793,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                             // trait.
                             let callee_trait = tcx.trait_of_item(callee);
                             if callee_trait.is_some()
-                                && tcx.has_attr(caller, sym::default_method_body_is_const)
+                                && tcx.has_attr(caller.to_def_id(), sym::default_method_body_is_const)
                                 && callee_trait == tcx.trait_of_item(caller)
                                 // Can only call methods when it's `<Self as TheTrait>::f`.
                                 && tcx.types.self_param == substs.type_at(0)

compiler/rustc_const_eval/src/transform/check_consts/check.rs

@@ -66,8 +66,12 @@ impl<'mir, 'tcx> ConstCx<'mir, 'tcx> {
     }
 }
 
-pub fn rustc_allow_const_fn_unstable(tcx: TyCtxt<'_>, def_id: DefId, feature_gate: Symbol) -> bool {
-    let attrs = tcx.get_attrs(def_id);
+pub fn rustc_allow_const_fn_unstable(
+    tcx: TyCtxt<'_>,
+    def_id: LocalDefId,
+    feature_gate: Symbol,
+) -> bool {
+    let attrs = tcx.hir().attrs(tcx.hir().local_def_id_to_hir_id(def_id));
     attr::rustc_allow_const_fn_unstable(&tcx.sess, attrs).any(|name| name == feature_gate)
 }
 

compiler/rustc_const_eval/src/transform/check_consts/mod.rs

@@ -1,5 +1,6 @@
 //! Concrete error types for all operations which may be invalid in a certain const context.
 
+use hir::def_id::LocalDefId;
 use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, ErrorGuaranteed};
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
@@ -95,7 +96,7 @@ impl<'tcx> NonConstOp<'tcx> for FnCallIndirect {
 /// A function call where the callee is not marked as `const`.
 #[derive(Debug, Clone, Copy)]
 pub struct FnCallNonConst<'tcx> {
-    pub caller: DefId,
+    pub caller: LocalDefId,
     pub callee: DefId,
     pub substs: SubstsRef<'tcx>,
     pub span: Span,
@@ -117,13 +118,8 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
             match self_ty.kind() {
                 Param(param_ty) => {
                     debug!(?param_ty);
-                    if let Some(generics) = caller
-                        .as_local()
-                        .map(|id| tcx.hir().local_def_id_to_hir_id(id))
-                        .map(|id| tcx.hir().get(id))
-                        .as_ref()
-                        .and_then(|node| node.generics())
-                    {
+                    let caller_hir_id = tcx.hir().local_def_id_to_hir_id(caller);
+                    if let Some(generics) = tcx.hir().get(caller_hir_id).generics() {
                         let constraint = with_no_trimmed_paths!(format!(
                             "~const {}",
                             trait_ref.print_only_trait_path()

compiler/rustc_const_eval/src/transform/check_consts/ops.rs

@@ -147,6 +147,16 @@ pub enum AttributeDuplicates {
     FutureWarnPreceding,
 }
 
+/// A conveniece macro to deal with `$($expr)?`.
+macro_rules! or_default {
+    ($default:expr,) => {
+        $default
+    };
+    ($default:expr, $next:expr) => {
+        $next
+    };
+}
+
 /// A convenience macro for constructing attribute templates.
 /// E.g., `template!(Word, List: "description")` means that the attribute
 /// supports forms `#[attr]` and `#[attr(description)]`.
@@ -168,9 +178,10 @@ macro_rules! template {
 }
 
 macro_rules! ungated {
-    ($attr:ident, $typ:expr, $tpl:expr, $duplicates:expr $(,)?) => {
+    ($attr:ident, $typ:expr, $tpl:expr, $duplicates:expr $(, @only_local: $only_local:expr)? $(,)?) => {
         BuiltinAttribute {
             name: sym::$attr,
+            only_local: or_default!(false, $($only_local)?),
             type_: $typ,
             template: $tpl,
             gate: Ungated,
@@ -180,18 +191,20 @@ macro_rules! ungated {
 }
 
 macro_rules! gated {
-    ($attr:ident, $typ:expr, $tpl:expr, $duplicates:expr, $gate:ident, $msg:expr $(,)?) => {
+    ($attr:ident, $typ:expr, $tpl:expr, $duplicates:expr $(, @only_local: $only_local:expr)?, $gate:ident, $msg:expr $(,)?) => {
         BuiltinAttribute {
             name: sym::$attr,
+            only_local: or_default!(false, $($only_local)?),
             type_: $typ,
             template: $tpl,
             duplicates: $duplicates,
             gate: Gated(Stability::Unstable, sym::$gate, $msg, cfg_fn!($gate)),
         }
     };
-    ($attr:ident, $typ:expr, $tpl:expr, $duplicates:expr, $msg:expr $(,)?) => {
+    ($attr:ident, $typ:expr, $tpl:expr, $duplicates:expr $(, @only_local: $only_local:expr)?, $msg:expr $(,)?) => {
         BuiltinAttribute {
             name: sym::$attr,
+            only_local: or_default!(false, $($only_local)?),
             type_: $typ,
             template: $tpl,
             duplicates: $duplicates,
@@ -201,12 +214,13 @@ macro_rules! gated {
 }
 
 macro_rules! rustc_attr {
-    (TEST, $attr:ident, $typ:expr, $tpl:expr, $duplicate:expr $(,)?) => {
+    (TEST, $attr:ident, $typ:expr, $tpl:expr, $duplicate:expr $(, @only_local: $only_local:expr)? $(,)?) => {
         rustc_attr!(
             $attr,
             $typ,
             $tpl,
             $duplicate,
+            $(@only_local: $only_local,)?
             concat!(
                 "the `#[",
                 stringify!($attr),
@@ -215,9 +229,10 @@ macro_rules! rustc_attr {
             ),
         )
     };
-    ($attr:ident, $typ:expr, $tpl:expr, $duplicates:expr, $msg:expr $(,)?) => {
+    ($attr:ident, $typ:expr, $tpl:expr, $duplicates:expr $(, @only_local: $only_local:expr)?, $msg:expr $(,)?) => {
         BuiltinAttribute {
             name: sym::$attr,
+            only_local: or_default!(false, $($only_local)?),
             type_: $typ,
             template: $tpl,
             duplicates: $duplicates,
@@ -237,6 +252,10 @@ const INTERNAL_UNSTABLE: &str = "this is an internal attribute that will never b
 
 pub struct BuiltinAttribute {
     pub name: Symbol,
+    /// Whether this attribute is only used in the local crate.
+    ///
+    /// If so, it is not encoded in the crate metadata.
+    pub only_local: bool,
     pub type_: AttributeType,
     pub template: AttributeTemplate,
     pub duplicates: AttributeDuplicates,
@@ -295,7 +314,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     ungated!(must_use, Normal, template!(Word, NameValueStr: "reason"), FutureWarnFollowing),
     gated!(
         must_not_suspend, Normal, template!(Word, NameValueStr: "reason"), WarnFollowing,
-        must_not_suspend, experimental!(must_not_suspend)
+        experimental!(must_not_suspend)
     ),
     ungated!(
         deprecated, Normal,
@@ -324,8 +343,8 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     ungated!(repr, Normal, template!(List: "C"), DuplicatesOk),
     ungated!(export_name, Normal, template!(NameValueStr: "name"), FutureWarnPreceding),
     ungated!(link_section, Normal, template!(NameValueStr: "name"), FutureWarnPreceding),
-    ungated!(no_mangle, Normal, template!(Word), WarnFollowing),
-    ungated!(used, Normal, template!(Word, List: "compiler|linker"), WarnFollowing),
+    ungated!(no_mangle, Normal, template!(Word), WarnFollowing, @only_local: true),
+    ungated!(used, Normal, template!(Word, List: "compiler|linker"), WarnFollowing, @only_local: true),
 
     // Limits:
     ungated!(recursion_limit, CrateLevel, template!(NameValueStr: "N"), FutureWarnFollowing),
@@ -358,8 +377,8 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     ungated!(panic_handler, Normal, template!(Word), WarnFollowing), // RFC 2070
 
     // Code generation:
-    ungated!(inline, Normal, template!(Word, List: "always|never"), FutureWarnFollowing),
-    ungated!(cold, Normal, template!(Word), WarnFollowing),
+    ungated!(inline, Normal, template!(Word, List: "always|never"), FutureWarnFollowing, @only_local: true),
+    ungated!(cold, Normal, template!(Word), WarnFollowing, @only_local: true),
     ungated!(no_builtins, CrateLevel, template!(Word), WarnFollowing),
     ungated!(target_feature, Normal, template!(List: r#"enable = "name""#), DuplicatesOk),
     ungated!(track_caller, Normal, template!(Word), WarnFollowing),
@@ -385,7 +404,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     ),
 
     // Linking:
-    gated!(naked, Normal, template!(Word), WarnFollowing, naked_functions, experimental!(naked)),
+    gated!(naked, Normal, template!(Word), WarnFollowing, @only_local: true, naked_functions, experimental!(naked)),
     gated!(
         link_ordinal, Normal, template!(List: "ordinal"), ErrorPreceding, raw_dylib,
         experimental!(link_ordinal)
@@ -394,6 +413,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     // Plugins:
     BuiltinAttribute {
         name: sym::plugin,
+        only_local: false,
         type_: CrateLevel,
         template: template!(List: "name"),
         duplicates: DuplicatesOk,
@@ -475,7 +495,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     ),
     // DuplicatesOk since it has its own validation
     ungated!(
-        stable, Normal, template!(List: r#"feature = "name", since = "version""#), DuplicatesOk
+        stable, Normal, template!(List: r#"feature = "name", since = "version""#), DuplicatesOk,
     ),
     ungated!(
         unstable, Normal,
@@ -546,11 +566,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     // ==========================================================================
 
     gated!(
-        linkage, Normal, template!(NameValueStr: "external|internal|..."), ErrorPreceding,
+        linkage, Normal, template!(NameValueStr: "external|internal|..."), ErrorPreceding, @only_local: true,
         "the `linkage` attribute is experimental and not portable across platforms",
     ),
     rustc_attr!(
-        rustc_std_internal_symbol, Normal, template!(Word), WarnFollowing, INTERNAL_UNSTABLE
+        rustc_std_internal_symbol, Normal, template!(Word), WarnFollowing, @only_local: true, INTERNAL_UNSTABLE
     ),
 
     // ==========================================================================
@@ -633,7 +653,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     // Internal attributes, Misc:
     // ==========================================================================
     gated!(
-        lang, Normal, template!(NameValueStr: "name"), DuplicatesOk, lang_items,
+        lang, Normal, template!(NameValueStr: "name"), DuplicatesOk, @only_local: true, lang_items,
         "language items are subject to change",
     ),
     rustc_attr!(
@@ -642,11 +662,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         "#[rustc_pass_by_value] is used to mark types that must be passed by value instead of reference."
     ),
     rustc_attr!(
-        rustc_coherence_is_core, AttributeType::CrateLevel, template!(Word), ErrorFollowing,
+        rustc_coherence_is_core, AttributeType::CrateLevel, template!(Word), ErrorFollowing, @only_local: true,
         "#![rustc_coherence_is_core] allows inherent methods on builtin types, only intended to be used in `core`."
     ),
     rustc_attr!(
-        rustc_allow_incoherent_impl, AttributeType::Normal, template!(Word), ErrorFollowing,
+        rustc_allow_incoherent_impl, AttributeType::Normal, template!(Word), ErrorFollowing, @only_local: true,
         "#[rustc_allow_incoherent_impl] has to be added to all impl items of an incoherent inherent impl."
     ),
     rustc_attr!(
@@ -656,6 +676,8 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     ),
     BuiltinAttribute {
         name: sym::rustc_diagnostic_item,
+        // FIXME: This can be `true` once we always use `tcx.is_diagnostic_item`.
+        only_local: false,
         type_: Normal,
         template: template!(NameValueStr: "name"),
         duplicates: ErrorFollowing,
@@ -676,7 +698,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         "unboxed_closures are still evolving",
     ),
     rustc_attr!(
-        rustc_inherit_overflow_checks, Normal, template!(Word), WarnFollowing,
+        rustc_inherit_overflow_checks, Normal, template!(Word), WarnFollowing, @only_local: true,
         "the `#[rustc_inherit_overflow_checks]` attribute is just used to control \
         overflow checking behavior of several libcore functions that are inlined \
         across crates and will never be stable",
@@ -778,6 +800,10 @@ pub fn is_builtin_attr_name(name: Symbol) -> bool {
     BUILTIN_ATTRIBUTE_MAP.get(&name).is_some()
 }
 
+pub fn is_builtin_only_local(name: Symbol) -> bool {
+    BUILTIN_ATTRIBUTE_MAP.get(&name).map_or(false, |attr| attr.only_local)
+}
+
 pub static BUILTIN_ATTRIBUTE_MAP: SyncLazy<FxHashMap<Symbol, &BuiltinAttribute>> =
     SyncLazy::new(|| {
         let mut map = FxHashMap::default();

compiler/rustc_feature/src/builtin_attrs.rs

@@ -149,7 +149,8 @@ pub use accepted::ACCEPTED_FEATURES;
 pub use active::{Features, ACTIVE_FEATURES, INCOMPATIBLE_FEATURES};
 pub use builtin_attrs::AttributeDuplicates;
 pub use builtin_attrs::{
-    deprecated_attributes, find_gated_cfg, is_builtin_attr_name, AttributeGate, AttributeTemplate,
-    AttributeType, BuiltinAttribute, GatedCfg, BUILTIN_ATTRIBUTES, BUILTIN_ATTRIBUTE_MAP,
+    deprecated_attributes, find_gated_cfg, is_builtin_attr_name, is_builtin_only_local,
+    AttributeGate, AttributeTemplate, AttributeType, BuiltinAttribute, GatedCfg,
+    BUILTIN_ATTRIBUTES, BUILTIN_ATTRIBUTE_MAP,
 };
 pub use removed::{REMOVED_FEATURES, STABLE_REMOVED_FEATURES};

compiler/rustc_feature/src/lib.rs

@@ -183,10 +183,7 @@ pub struct DirtyCleanVisitor<'tcx> {
 impl<'tcx> DirtyCleanVisitor<'tcx> {
     /// Possibly "deserialize" the attribute into a clean/dirty assertion
     fn assertion_maybe(&mut self, item_id: LocalDefId, attr: &Attribute) -> Option<Assertion> {
-        if !attr.has_name(sym::rustc_clean) {
-            // skip: not rustc_clean/dirty
-            return None;
-        }
+        assert!(attr.has_name(sym::rustc_clean));
         if !check_config(self.tcx, attr) {
             // skip: not the correct `cfg=`
             return None;
@@ -384,7 +381,7 @@ impl<'tcx> DirtyCleanVisitor<'tcx> {
     fn check_item(&mut self, item_id: LocalDefId) {
         let item_span = self.tcx.def_span(item_id.to_def_id());
         let def_path_hash = self.tcx.def_path_hash(item_id.to_def_id());
-        for attr in self.tcx.get_attrs(item_id.to_def_id()).iter() {
+        for attr in self.tcx.get_attrs(item_id.to_def_id(), sym::rustc_clean) {
             let Some(assertion) = self.assertion_maybe(item_id, attr) else {
                 continue;
             };

compiler/rustc_incremental/src/persist/dirty_clean.rs

@@ -258,10 +258,7 @@ impl<'tcx> Queries<'tcx> {
     /// an error.
     fn check_for_rustc_errors_attr(tcx: TyCtxt<'_>) {
         let Some((def_id, _)) = tcx.entry_fn(()) else { return };
-
-        let attrs = &*tcx.get_attrs(def_id);
-        let attrs = attrs.iter().filter(|attr| attr.has_name(sym::rustc_error));
-        for attr in attrs {
+        for attr in tcx.get_attrs(def_id, sym::rustc_error) {
             match attr.meta_item_list() {
                 // Check if there is a `#[rustc_error(delay_span_bug_from_inside_query)]`.
                 Some(list)

compiler/rustc_interface/src/queries.rs

@@ -551,7 +551,7 @@ impl MissingDoc {
             }
         }
 
-        let attrs = cx.tcx.get_attrs(def_id.to_def_id());
+        let attrs = cx.tcx.hir().attrs(cx.tcx.hir().local_def_id_to_hir_id(def_id));
         let has_doc = attrs.iter().any(has_doc);
         if !has_doc {
             cx.struct_span_lint(
@@ -2738,11 +2738,7 @@ impl ClashingExternDeclarations {
                 // bottleneck, this does just fine.
                 (
                     overridden_link_name,
-                    tcx.get_attrs(fi.def_id.to_def_id())
-                        .iter()
-                        .find(|at| at.has_name(sym::link_name))
-                        .unwrap()
-                        .span,
+                    tcx.get_attr(fi.def_id.to_def_id(), sym::link_name).unwrap().span,
                 )
             })
         {