Skip to content

Commit

Permalink
Auto merge of #98450 - lqd:doc-metadata, r=lqd,GuillaumeGomez
Browse files Browse the repository at this point in the history
Remove more attributes from metadata

A lot of the attributes that are currently stored in the metadata aren't used at all. The biggest metadata usage comes from the doc attributes currently but they are needed by rustdoc so we only removed the ones that cannot be used in downstream crates (doc comments on private items).

r? `@ghost`
  • Loading branch information
bors committed Oct 21, 2022
2 parents 657f246 + 41263d2 commit ba9d01b
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 14 deletions.
36 changes: 26 additions & 10 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,20 +296,24 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[

// Lints:
ungated!(
warn, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk
warn, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#),
DuplicatesOk, @only_local: true,
),
ungated!(
allow, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk
allow, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#),
DuplicatesOk, @only_local: true,
),
gated!(
expect, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk,
lint_reasons, experimental!(expect)
),
ungated!(
forbid, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk
forbid, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#),
DuplicatesOk, @only_local: true,
),
ungated!(
deny, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk
deny, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#),
DuplicatesOk, @only_local: true,
),
ungated!(must_use, Normal, template!(Word, NameValueStr: "reason"), FutureWarnFollowing),
gated!(
Expand Down Expand Up @@ -340,7 +344,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
),
ungated!(link_name, Normal, template!(NameValueStr: "name"), FutureWarnPreceding),
ungated!(no_link, Normal, template!(Word), WarnFollowing),
ungated!(repr, Normal, template!(List: "C"), DuplicatesOk),
ungated!(repr, Normal, template!(List: "C"), DuplicatesOk, @only_local: true),
ungated!(export_name, Normal, template!(NameValueStr: "name"), FutureWarnPreceding),
ungated!(link_section, Normal, template!(NameValueStr: "name"), FutureWarnPreceding),
ungated!(no_mangle, Normal, template!(Word), WarnFollowing, @only_local: true),
Expand Down Expand Up @@ -382,7 +386,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
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!(
target_feature, Normal, template!(List: r#"enable = "name""#),
DuplicatesOk, @only_local: true,
),
ungated!(track_caller, Normal, template!(Word), WarnFollowing),
gated!(
no_sanitize, Normal,
Expand Down Expand Up @@ -488,18 +495,24 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
// Internal attributes: Stability, deprecation, and unsafe:
// ==========================================================================

ungated!(feature, CrateLevel, template!(List: "name1, name2, ..."), DuplicatesOk),
ungated!(
feature, CrateLevel,
template!(List: "name1, name2, ..."), DuplicatesOk, @only_local: true,
),
// 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, @only_local: true,
),
ungated!(
unstable, Normal,
template!(List: r#"feature = "name", reason = "...", issue = "N""#), DuplicatesOk,
),
ungated!(rustc_const_unstable, Normal, template!(List: r#"feature = "name""#), DuplicatesOk),
ungated!(rustc_const_stable, Normal, template!(List: r#"feature = "name""#), DuplicatesOk),
ungated!(rustc_safe_intrinsic, Normal, template!(Word), DuplicatesOk),
ungated!(
rustc_const_stable, Normal,
template!(List: r#"feature = "name""#), DuplicatesOk, @only_local: true,
),
ungated!(
rustc_default_body_unstable, Normal,
template!(List: r#"feature = "name", reason = "...", issue = "N""#), DuplicatesOk
Expand All @@ -517,6 +530,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
allow_internal_unsafe, Normal, template!(Word), WarnFollowing,
"allow_internal_unsafe side-steps the unsafe_code lint",
),
ungated!(rustc_safe_intrinsic, Normal, template!(Word), DuplicatesOk),
rustc_attr!(rustc_allowed_through_unstable_modules, Normal, template!(Word), WarnFollowing,
"rustc_allowed_through_unstable_modules special cases accidental stabilizations of stable items \
through unstable paths"),
Expand Down Expand Up @@ -823,6 +837,8 @@ pub fn is_builtin_attr_name(name: Symbol) -> bool {
BUILTIN_ATTRIBUTE_MAP.get(&name).is_some()
}

/// Whether this builtin attribute is only used in the local crate.
/// If so, it is not encoded in the crate metadata.
pub fn is_builtin_only_local(name: Symbol) -> bool {
BUILTIN_ATTRIBUTE_MAP.get(&name).map_or(false, |attr| attr.only_local)
}
Expand Down
45 changes: 41 additions & 4 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::rmeta::def_path_hash_map::DefPathHashMapRef;
use crate::rmeta::table::TableBuilder;
use crate::rmeta::*;

use rustc_ast::Attribute;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
use rustc_data_structures::memmap::{Mmap, MmapMut};
Expand Down Expand Up @@ -764,6 +765,40 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
}

/// Returns whether an attribute needs to be recorded in metadata, that is, if it's usable and
/// useful in downstream crates. Local-only attributes are an obvious example, but some
/// rustdoc-specific attributes can equally be of use while documenting the current crate only.
///
/// Removing these superfluous attributes speeds up compilation by making the metadata smaller.
///
/// Note: the `is_def_id_public` parameter is used to cache whether the given `DefId` has a public
/// visibility: this is a piece of data that can be computed once per defid, and not once per
/// attribute. Some attributes would only be usable downstream if they are public.
#[inline]
fn should_encode_attr(
tcx: TyCtxt<'_>,
attr: &Attribute,
def_id: LocalDefId,
is_def_id_public: &mut Option<bool>,
) -> bool {
if rustc_feature::is_builtin_only_local(attr.name_or_empty()) {
// Attributes marked local-only don't need to be encoded for downstream crates.
false
} else if attr.doc_str().is_some() {
// We keep all public doc comments because they might be "imported" into downstream crates
// if they use `#[doc(inline)]` to copy an item's documentation into their own.
*is_def_id_public.get_or_insert_with(|| {
tcx.privacy_access_levels(()).get_effective_vis(def_id).is_some()
})
} else if attr.has_name(sym::doc) {
// If this is a `doc` attribute, and it's marked `inline` (as in `#[doc(inline)]`), we can
// remove it. It won't be inlinable in downstream crates.
attr.meta_item_list().map(|l| l.iter().any(|l| !l.has_name(sym::inline))).unwrap_or(false)
} else {
true
}
}

fn should_encode_visibility(def_kind: DefKind) -> bool {
match def_kind {
DefKind::Mod
Expand Down Expand Up @@ -1126,12 +1161,14 @@ fn should_encode_trait_impl_trait_tys<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) ->

impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
fn encode_attrs(&mut self, def_id: LocalDefId) {
let mut attrs = self
.tcx
let tcx = self.tcx;
let mut is_public: Option<bool> = None;

let mut attrs = tcx
.hir()
.attrs(self.tcx.hir().local_def_id_to_hir_id(def_id))
.attrs(tcx.hir().local_def_id_to_hir_id(def_id))
.iter()
.filter(|attr| !rustc_feature::is_builtin_only_local(attr.name_or_empty()));
.filter(move |attr| should_encode_attr(tcx, attr, def_id, &mut is_public));

record_array!(self.tables.attributes[def_id.to_def_id()] <- attrs.clone());
if attrs.any(|attr| attr.may_have_doc_links()) {
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/attr-from-macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// aux-build:attr-from-macro.rs
// run-pass

extern crate attr_from_macro;

attr_from_macro::creator! {
struct Foo;
enum Bar;
enum FooBar;
}

fn main() {
// Checking the `repr(u32)` on the enum.
assert_eq!(4, std::mem::size_of::<Bar>());
// Checking the `repr(u16)` on the enum.
assert_eq!(2, std::mem::size_of::<FooBar>());

// Checking the Debug impl on the types.
eprintln!("{:?} {:?} {:?}", Foo, Bar::A, FooBar::A);
}
15 changes: 15 additions & 0 deletions src/test/ui/auxiliary/attr-from-macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#[macro_export]
macro_rules! creator {
(struct $name1:ident; enum $name2:ident; enum $name3:ident;) => {
#[derive(Debug)]
pub struct $name1;

#[derive(Debug)]
#[repr(u32)]
pub enum $name2 { A }

#[derive(Debug)]
#[repr(u16)]
pub enum $name3 { A }
}
}
9 changes: 9 additions & 0 deletions src/test/ui/query-visibility.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// check-pass
// Check that it doesn't panic when `Input` gets its visibility checked.

#![crate_type = "lib"]

pub trait Layer<
/// Hello.
Input,
> {}

0 comments on commit ba9d01b

Please sign in to comment.