WIP: rustdoc-json: Structured attributes

[squash-before-merge] better `#[repr]`

[squash-before-merge] update tests

[squash-before-merge] sufficient now!

Author: aDotInTheVoid

src/librustdoc/clean/types.rs

@@ -746,34 +746,23 @@ impl Item {
         Some(tcx.visibility(def_id))
     }
 
-    fn attributes_without_repr(&self, tcx: TyCtxt<'_>, is_json: bool) -> Vec<String> {
+    /// Get a list of attributes excluding `#[repr]`.
+    ///
+    /// Only used by the HTML output-format.
+    fn attributes_without_repr(&self, tcx: TyCtxt<'_>) -> Vec<String> {
         const ALLOWED_ATTRIBUTES: &[Symbol] =
-            &[sym::export_name, sym::link_section, sym::no_mangle, sym::non_exhaustive];
+            &[sym::export_name, sym::link_section, sym::non_exhaustive];
         self.attrs
             .other_attrs
             .iter()
             .filter_map(|attr| {
-                // NoMangle is special cased, as it appears in HTML output, and we want to show it in source form, not HIR printing.
-                // It is also used by cargo-semver-checks.
-                if matches!(attr, hir::Attribute::Parsed(AttributeKind::NoMangle(..))) {
+                // NoMangle is special cased, as it's currently the only structured
+                // attribute that we want to appear in HTML output.
+                if matches!(attr, hir::Attribute::Parsed(AttributeKind::NoMangle(_))) {
                     Some("#[no_mangle]".to_string())
-                } else if is_json {
-                    match attr {
-                        // rustdoc-json stores this in `Item::deprecation`, so we
-                        // don't want it it `Item::attrs`.
-                        hir::Attribute::Parsed(AttributeKind::Deprecation { .. }) => None,
-                        // We have separate pretty-printing logic for `#[repr(..)]` attributes.
-                        hir::Attribute::Parsed(AttributeKind::Repr(..)) => None,
-                        _ => Some({
-                            let mut s = rustc_hir_pretty::attribute_to_string(&tcx, attr);
-                            assert_eq!(s.pop(), Some('\n'));
-                            s
-                        }),
-                    }
+                } else if !attr.has_any_name(ALLOWED_ATTRIBUTES) {
+                    None
                 } else {
-                    if !attr.has_any_name(ALLOWED_ATTRIBUTES) {
-                        return None;
-                    }
                     Some(
                         rustc_hir_pretty::attribute_to_string(&tcx, attr)
                             .replace("\\\n", "")
@@ -785,18 +774,23 @@ impl Item {
             .collect()
     }
 
-    pub(crate) fn attributes(&self, tcx: TyCtxt<'_>, cache: &Cache, is_json: bool) -> Vec<String> {
-        let mut attrs = self.attributes_without_repr(tcx, is_json);
+    /// Get a list of attributes to display on this item.
+    ///
+    /// Only used by the HTML output-format.
+    pub(crate) fn attributes(&self, tcx: TyCtxt<'_>, cache: &Cache) -> Vec<String> {
+        let mut attrs = self.attributes_without_repr(tcx);
 
-        if let Some(repr_attr) = self.repr(tcx, cache, is_json) {
+        if let Some(repr_attr) = self.repr(tcx, cache) {
             attrs.push(repr_attr);
         }
         attrs
     }
 
     /// Returns a stringified `#[repr(...)]` attribute.
-    pub(crate) fn repr(&self, tcx: TyCtxt<'_>, cache: &Cache, is_json: bool) -> Option<String> {
-        repr_attributes(tcx, cache, self.def_id()?, self.type_(), is_json)
+    ///
+    /// Only used by the HTML output-format.
+    pub(crate) fn repr(&self, tcx: TyCtxt<'_>, cache: &Cache) -> Option<String> {
+        repr_attributes(tcx, cache, self.def_id()?, self.type_())
     }
 
     pub fn is_doc_hidden(&self) -> bool {
@@ -808,12 +802,14 @@ impl Item {
     }
 }
 
+/// Return a string representing the `#[repr]` attribute if present.
+///
+/// Only used by the HTML output-format.
 pub(crate) fn repr_attributes(
     tcx: TyCtxt<'_>,
     cache: &Cache,
     def_id: DefId,
     item_type: ItemType,
-    is_json: bool,
 ) -> Option<String> {
     use rustc_abi::IntegerType;
 
@@ -830,7 +826,6 @@ pub(crate) fn repr_attributes(
         // Render `repr(transparent)` iff the non-1-ZST field is public or at least one
         // field is public in case all fields are 1-ZST fields.
         let render_transparent = cache.document_private
-            || is_json
             || adt
                 .all_fields()
                 .find(|field| {

src/librustdoc/html/render/mod.rs

@@ -1194,7 +1194,7 @@ fn render_assoc_item(
 // a whitespace prefix and newline.
 fn render_attributes_in_pre(it: &clean::Item, prefix: &str, cx: &Context<'_>) -> impl fmt::Display {
     fmt::from_fn(move |f| {
-        for a in it.attributes(cx.tcx(), cx.cache(), false) {
+        for a in it.attributes(cx.tcx(), cx.cache()) {
             writeln!(f, "{prefix}{a}")?;
         }
         Ok(())
@@ -1210,7 +1210,7 @@ fn render_code_attribute(code_attr: CodeAttribute, w: &mut impl fmt::Write) {
 // When an attribute is rendered inside a <code> tag, it is formatted using
 // a div to produce a newline after it.
 fn render_attributes_in_code(w: &mut impl fmt::Write, it: &clean::Item, cx: &Context<'_>) {
-    for attr in it.attributes(cx.tcx(), cx.cache(), false) {
+    for attr in it.attributes(cx.tcx(), cx.cache()) {
         render_code_attribute(CodeAttribute(attr), w);
     }
 }
@@ -1222,7 +1222,7 @@ fn render_repr_attributes_in_code(
     def_id: DefId,
     item_type: ItemType,
 ) {
-    if let Some(repr) = clean::repr_attributes(cx.tcx(), cx.cache(), def_id, item_type, false) {
+    if let Some(repr) = clean::repr_attributes(cx.tcx(), cx.cache(), def_id, item_type) {
         render_code_attribute(CodeAttribute(repr), w);
     }
 }

src/librustdoc/html/render/print_item.rs

@@ -1486,12 +1486,11 @@ impl<'a, 'cx: 'a> ItemUnion<'a, 'cx> {
                     self.cx.cache(),
                     self.def_id,
                     ItemType::Union,
-                    false,
                 ) {
                     writeln!(f, "{repr}")?;
                 };
             } else {
-                for a in self.it.attributes(self.cx.tcx(), self.cx.cache(), false) {
+                for a in self.it.attributes(self.cx.tcx(), self.cx.cache()) {
                     writeln!(f, "{a}")?;
                 }
             }

src/librustdoc/json/conversions.rs

@@ -7,9 +7,11 @@
 use rustc_abi::ExternAbi;
 use rustc_ast::ast;
 use rustc_attr_data_structures::{self as attrs, DeprecatedSince};
+use rustc_hir as hir;
 use rustc_hir::def::CtorKind;
 use rustc_hir::def_id::DefId;
 use rustc_metadata::rendered_const;
+use rustc_middle::ty::TyCtxt;
 use rustc_middle::{bug, ty};
 use rustc_span::{Pos, kw, sym};
 use rustdoc_json_types::*;
@@ -40,7 +42,12 @@ impl JsonRenderer<'_> {
             })
             .collect();
         let docs = item.opt_doc_value();
-        let attrs = item.attributes(self.tcx, &self.cache, true);
+        let attrs = item
+            .attrs
+            .other_attrs
+            .iter()
+            .filter_map(|a| maybe_from_hir_attr(a, item.item_id, self.tcx))
+            .collect();
         let span = item.span(self.tcx);
         let visibility = item.visibility(self.tcx);
         let clean::ItemInner { name, item_id, .. } = *item.inner;
@@ -875,3 +882,104 @@ impl FromClean<ItemType> for ItemKind {
         }
     }
 }
+
+/// Maybe convert a attribue from hir to json.
+///
+/// Returns `None` if the attribute shouldn't be in the output.
+fn maybe_from_hir_attr(
+    attr: &hir::Attribute,
+    item_id: ItemId,
+    tcx: TyCtxt<'_>,
+) -> Option<Attribute> {
+    use attrs::AttributeKind as AK;
+
+    let kind = match attr {
+        hir::Attribute::Parsed(kind) => kind,
+
+        // There are some currently unstrucured attrs that we *do* care about.
+        // As the attribute migration progresses (#131229), this is expected to shrink
+        // and eventually be removed as all attributes gain a strutured representation in
+        // HIR.
+        hir::Attribute::Unparsed(_) => {
+            return Some(if attr.has_name(sym::non_exhaustive) {
+                Attribute::NonExhaustive
+            } else if attr.has_name(sym::automatically_derived) {
+                Attribute::AutomaticallyDerived
+            } else if attr.has_name(sym::export_name) {
+                Attribute::ExportName(
+                    attr.value_str().expect("checked by attr validation").to_string(),
+                )
+            } else if attr.has_name(sym::doc)
+                && attr
+                    .meta_item_list()
+                    .is_some_and(|metas| metas.iter().any(|item| item.has_name(sym::hidden)))
+            {
+                Attribute::DocHidden
+            } else {
+                other_attr(tcx, attr)
+            });
+        }
+    };
+
+    Some(match kind {
+        AK::Deprecation { .. } => return None, // Handled seperatly into Item::deprecation.
+        AK::DocComment { .. } => unreachable!("doc comments stripped out earlier"),
+
+        AK::MustUse { reason, span: _ } => {
+            Attribute::MustUse { reason: reason.map(|s| s.to_string()) }
+        }
+        AK::Repr { .. } => repr_attr(
+            tcx,
+            item_id.as_def_id().expect("all items that could have #[repr] have a DefId"),
+        ),
+        AK::NoMangle(_) => Attribute::NoMangle,
+
+        _ => other_attr(tcx, attr),
+    })
+}
+
+fn other_attr(tcx: TyCtxt<'_>, attr: &hir::Attribute) -> Attribute {
+    let mut s = rustc_hir_pretty::attribute_to_string(&tcx, attr);
+    assert_eq!(s.pop(), Some('\n'));
+    Attribute::Other(s)
+}
+
+fn repr_attr(tcx: TyCtxt<'_>, def_id: DefId) -> Attribute {
+    let repr = tcx.adt_def(def_id).repr();
+
+    let kind = if repr.c() {
+        ReprKind::C
+    } else if repr.transparent() {
+        ReprKind::Transparent
+    } else if repr.simd() {
+        ReprKind::Simd
+    } else {
+        ReprKind::Rust
+    };
+
+    let align = repr.align.map(|a| a.bytes());
+    let packed = repr.pack.map(|p| p.bytes());
+    let int = repr.int.map(format_integer_type);
+
+    Attribute::Repr(AttributeRepr { kind, align, packed, int })
+}
+
+fn format_integer_type(it: rustc_abi::IntegerType) -> String {
+    use rustc_abi::Integer::*;
+    use rustc_abi::IntegerType::*;
+    match it {
+        Pointer(true) => "isize",
+        Pointer(false) => "usize",
+        Fixed(I8, true) => "i8",
+        Fixed(I8, false) => "u8",
+        Fixed(I16, true) => "i16",
+        Fixed(I16, false) => "u16",
+        Fixed(I32, true) => "i32",
+        Fixed(I32, false) => "u32",
+        Fixed(I64, true) => "i64",
+        Fixed(I64, false) => "u64",
+        Fixed(I128, true) => "i128",
+        Fixed(I128, false) => "u128",
+    }
+    .to_owned()
+}

src/rustdoc-json-types/lib.rs

@@ -37,8 +37,8 @@ pub type FxHashMap<K, V> = HashMap<K, V>; // re-export for use in src/librustdoc
 // will instead cause conflicts. See #94591 for more. (This paragraph and the "Latest feature" line
 // are deliberately not in a doc comment, because they need not be in public docs.)
 //
-// Latest feature: Pretty printing of no_mangle attributes changed
-pub const FORMAT_VERSION: u32 = 53;
+// Latest feature: Structured Attributes
+pub const FORMAT_VERSION: u32 = 54;
 
 /// The root of the emitted JSON blob.
 ///
@@ -195,13 +195,70 @@ pub struct Item {
     /// - `#[repr(C)]` and other reprs also appear as themselves,
     ///   though potentially with a different order: e.g. `repr(i8, C)` may become `repr(C, i8)`.
     ///   Multiple repr attributes on the same item may be combined into an equivalent single attr.
-    pub attrs: Vec<String>,
+    pub attrs: Vec<Attribute>,
     /// Information about the item’s deprecation, if present.
     pub deprecation: Option<Deprecation>,
     /// The type-specific fields describing this item.
     pub inner: ItemEnum,
 }
 
+/// An attribute, eg `#[repr(C)]`
+///
+/// This doesn't include:
+/// - `#[doc = "Doc Comment"]` or `/// Doc comment`. These are in [`Item::docs`] instead.
+/// - `#[deprecated]`. These are in [`Item::deprecation`] instead.
+#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum Attribute {
+    /// `#[non_exhaustive]`
+    NonExhaustive,
+
+    /// `#[must_use]`
+    MustUse {
+        reason: Option<String>,
+    },
+
+    /// `#[automatically_derived]`
+    AutomaticallyDerived,
+
+    /// `#[repr]`
+    Repr(AttributeRepr),
+
+    ExportName(String),
+    /// `#[doc(hidden)]`
+    DocHidden,
+    /// `#[no_mangle]`
+    NoMangle,
+
+    /// Something else.
+    ///
+    /// Things here are explicitly *not* covered by the [`FORMAT_VERSION`]
+    /// constant, and may change without bumping the format version. If you rely
+    /// on an attribute here, please open an issue about adding a new variant for
+    /// that attr.
+    Other(String),
+}
+
+#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+pub struct AttributeRepr {
+    pub kind: ReprKind,
+
+    /// Alignment, in bytes.
+    pub align: Option<u64>,
+    pub packed: Option<u64>,
+
+    pub int: Option<String>,
+}
+
+#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum ReprKind {
+    Rust,
+    C,
+    Transparent,
+    Simd,
+}
+
 /// A range of source code.
 #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub struct Span {
@@ -1343,7 +1400,7 @@ pub struct Static {
 
     /// Is the static `unsafe`?
     ///
-    /// This is only true if it's in an `extern` block, and not explicity marked
+    /// This is only true if it's in an `extern` block, and not explicitly marked
     /// as `safe`.
     ///
     /// ```rust

tests/rustdoc-json/attrs/automatically_derived.rs

@@ -9,5 +9,5 @@ impl Default for Manual {
     }
 }
 
-//@ is '$.index[?(@.inner.impl.for.resolved_path.path == "Derive" && @.inner.impl.trait.path == "Default")].attrs' '["#[automatically_derived]"]'
+//@ is '$.index[?(@.inner.impl.for.resolved_path.path == "Derive" && @.inner.impl.trait.path == "Default")].attrs' '["automatically_derived"]'
 //@ is '$.index[?(@.inner.impl.for.resolved_path.path == "Manual" && @.inner.impl.trait.path == "Default")].attrs' '[]'

tests/rustdoc-json/attrs/export_name_2021.rs

@@ -1,6 +1,6 @@
 //@ edition: 2021
 #![no_std]
 
-//@ is "$.index[?(@.name=='example')].attrs" '["#[export_name = \"altered\"]"]'
+//@ is "$.index[?(@.name=='example')].attrs" '[{"export_name": "altered"}]'
 #[export_name = "altered"]
 pub extern "C" fn example() {}

tests/rustdoc-json/attrs/export_name_2024.rs

@@ -2,8 +2,8 @@
 #![no_std]
 
 // The representation of `#[unsafe(export_name = ..)]` in rustdoc in edition 2024
-// is still `#[export_name = ..]` without the `unsafe` attribute wrapper.
+// doesn't mention the `unsafe`.
 
-//@ is "$.index[?(@.name=='example')].attrs" '["#[export_name = \"altered\"]"]'
+//@ is "$.index[?(@.name=='example')].attrs" '[{"export_name": "altered"}]'
 #[unsafe(export_name = "altered")]
 pub extern "C" fn example() {}