Skip to content

rustc_metadata: Support non-Option nullable values in metadata tables #107166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 25, 2023
Merged

rustc_metadata: Support non-Option nullable values in metadata tables #107166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
let vis = self.get_visibility(id);
let span = self.get_span(id, sess);
let macro_rules = match kind {
DefKind::Macro(..) => self.root.tables.macro_rules.get(self, id).is_some(),
DefKind::Macro(..) => self.root.tables.is_macro_rules.get(self, id),
_ => false,
};

Expand Down Expand Up @@ -1283,7 +1283,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
fn get_macro(self, id: DefIndex, sess: &Session) -> ast::MacroDef {
match self.def_kind(id) {
DefKind::Macro(_) => {
let macro_rules = self.root.tables.macro_rules.get(self, id).is_some();
let macro_rules = self.root.tables.is_macro_rules.get(self, id);
let body =
self.root.tables.macro_definition.get(self, id).unwrap().decode((self, sess));
ast::MacroDef { macro_rules, body: ast::ptr::P(body) }
Expand Down Expand Up @@ -1595,11 +1595,11 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
}

fn get_attr_flags(self, index: DefIndex) -> AttrFlags {
self.root.tables.attr_flags.get(self, index).unwrap_or(AttrFlags::empty())
self.root.tables.attr_flags.get(self, index)
}

fn get_is_intrinsic(self, index: DefIndex) -> bool {
self.root.tables.is_intrinsic.get(self, index).is_some()
self.root.tables.is_intrinsic.get(self, index)
}
}

Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,7 @@ provide! { tcx, def_id, other, cdata,
deduced_param_attrs => { table }
is_type_alias_impl_trait => {
debug_assert_eq!(tcx.def_kind(def_id), DefKind::OpaqueTy);
cdata
.root
.tables
.is_type_alias_impl_trait
.get(cdata, def_id.index)
.is_some()
cdata.root.tables.is_type_alias_impl_trait.get(cdata, def_id.index)
}
collect_return_position_impl_trait_in_trait_tys => {
Ok(cdata
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
self.lazy(DefPathHashMapRef::BorrowedFromTcx(self.tcx.def_path_hash_to_def_index_map()))
}

fn encode_source_map(&mut self) -> LazyTable<u32, LazyValue<rustc_span::SourceFile>> {
fn encode_source_map(&mut self) -> LazyTable<u32, Option<LazyValue<rustc_span::SourceFile>>> {
let source_map = self.tcx.sess.source_map();
let all_source_files = source_map.files();

Expand Down Expand Up @@ -1130,7 +1130,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
attr_flags |= AttrFlags::IS_DOC_HIDDEN;
}
if !attr_flags.is_empty() {
self.tables.attr_flags.set(def_id.local_def_index, attr_flags);
self.tables.attr_flags.set_nullable(def_id.local_def_index, attr_flags);
}
}

Expand Down Expand Up @@ -1387,7 +1387,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
if impl_item.kind == ty::AssocKind::Fn {
record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id));
if tcx.is_intrinsic(def_id) {
self.tables.is_intrinsic.set(def_id.index, ());
self.tables.is_intrinsic.set_nullable(def_id.index, true);
}
}
}
Expand Down Expand Up @@ -1519,7 +1519,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
hir::ItemKind::Macro(ref macro_def, _) => {
if macro_def.macro_rules {
self.tables.macro_rules.set(def_id.index, ());
self.tables.is_macro_rules.set_nullable(def_id.index, true);
}
record!(self.tables.macro_definition[def_id] <- &*macro_def.body);
}
Expand All @@ -1529,7 +1529,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
hir::ItemKind::OpaqueTy(ref opaque) => {
self.encode_explicit_item_bounds(def_id);
if matches!(opaque.origin, hir::OpaqueTyOrigin::TyAlias) {
self.tables.is_type_alias_impl_trait.set(def_id.index, ());
self.tables.is_type_alias_impl_trait.set_nullable(def_id.index, true);
}
}
hir::ItemKind::Enum(..) => {
Expand Down Expand Up @@ -1636,7 +1636,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
if let hir::ItemKind::Fn(..) = item.kind {
record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id));
if tcx.is_intrinsic(def_id) {
self.tables.is_intrinsic.set(def_id.index, ());
self.tables.is_intrinsic.set_nullable(def_id.index, true);
}
}
if let hir::ItemKind::Impl { .. } = item.kind {
Expand Down Expand Up @@ -2038,7 +2038,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
if let hir::ForeignItemKind::Fn(..) = nitem.kind {
if tcx.is_intrinsic(def_id) {
self.tables.is_intrinsic.set(def_id.index, ());
self.tables.is_intrinsic.set_nullable(def_id.index, true);
}
}
}
Expand Down
39 changes: 22 additions & 17 deletions compiler/rustc_metadata/src/rmeta/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,9 @@ enum LazyState {
Previous(NonZeroUsize),
}

type SyntaxContextTable = LazyTable<u32, LazyValue<SyntaxContextData>>;
type ExpnDataTable = LazyTable<ExpnIndex, LazyValue<ExpnData>>;
type ExpnHashTable = LazyTable<ExpnIndex, LazyValue<ExpnHash>>;
type SyntaxContextTable = LazyTable<u32, Option<LazyValue<SyntaxContextData>>>;
type ExpnDataTable = LazyTable<ExpnIndex, Option<LazyValue<ExpnData>>>;
type ExpnHashTable = LazyTable<ExpnIndex, Option<LazyValue<ExpnHash>>>;

#[derive(MetadataEncodable, MetadataDecodable)]
pub(crate) struct ProcMacroData {
Expand Down Expand Up @@ -253,7 +253,7 @@ pub(crate) struct CrateRoot {

def_path_hash_map: LazyValue<DefPathHashMapRef<'static>>,

source_map: LazyTable<u32, LazyValue<rustc_span::SourceFile>>,
source_map: LazyTable<u32, Option<LazyValue<rustc_span::SourceFile>>>,

compiler_builtins: bool,
needs_allocator: bool,
Expand Down Expand Up @@ -315,31 +315,43 @@ pub(crate) struct IncoherentImpls {

/// Define `LazyTables` and `TableBuilders` at the same time.
macro_rules! define_tables {
($($name:ident: Table<$IDX:ty, $T:ty>),+ $(,)?) => {
(
- nullable: $($name1:ident: Table<$IDX1:ty, $T1:ty>,)+
- optional: $($name2:ident: Table<$IDX2:ty, $T2:ty>,)+
) => {
#[derive(MetadataEncodable, MetadataDecodable)]
pub(crate) struct LazyTables {
$($name: LazyTable<$IDX, $T>),+
$($name1: LazyTable<$IDX1, $T1>,)+
$($name2: LazyTable<$IDX2, Option<$T2>>,)+
}

#[derive(Default)]
struct TableBuilders {
$($name: TableBuilder<$IDX, $T>),+
$($name1: TableBuilder<$IDX1, $T1>,)+
$($name2: TableBuilder<$IDX2, Option<$T2>>,)+
}

impl TableBuilders {
fn encode(&self, buf: &mut FileEncoder) -> LazyTables {
LazyTables {
$($name: self.$name.encode(buf)),+
$($name1: self.$name1.encode(buf),)+
$($name2: self.$name2.encode(buf),)+
}
}
}
}
}

define_tables! {
- nullable:
is_intrinsic: Table<DefIndex, bool>,
is_macro_rules: Table<DefIndex, bool>,
is_type_alias_impl_trait: Table<DefIndex, bool>,
attr_flags: Table<DefIndex, AttrFlags>,

- optional:
attributes: Table<DefIndex, LazyArray<ast::Attribute>>,
children: Table<DefIndex, LazyArray<DefIndex>>,

opt_def_kind: Table<DefIndex, DefKind>,
visibility: Table<DefIndex, LazyValue<ty::Visibility<DefIndex>>>,
def_span: Table<DefIndex, LazyValue<Span>>,
Expand Down Expand Up @@ -370,7 +382,6 @@ define_tables! {
impl_parent: Table<DefIndex, RawDefId>,
impl_polarity: Table<DefIndex, ty::ImplPolarity>,
constness: Table<DefIndex, hir::Constness>,
is_intrinsic: Table<DefIndex, ()>,
impl_defaultness: Table<DefIndex, hir::Defaultness>,
// FIXME(eddyb) perhaps compute this on the fly if cheap enough?
coerce_unsized_info: Table<DefIndex, LazyValue<ty::adjustment::CoerceUnsizedInfo>>,
Expand All @@ -380,7 +391,6 @@ define_tables! {
fn_arg_names: Table<DefIndex, LazyArray<Ident>>,
generator_kind: Table<DefIndex, LazyValue<hir::GeneratorKind>>,
trait_def: Table<DefIndex, LazyValue<ty::TraitDef>>,

trait_item_def_id: Table<DefIndex, RawDefId>,
inherent_impls: Table<DefIndex, LazyArray<DefIndex>>,
expn_that_defined: Table<DefIndex, LazyValue<ExpnId>>,
Expand All @@ -395,18 +405,12 @@ define_tables! {
def_path_hashes: Table<DefIndex, DefPathHash>,
proc_macro_quoted_spans: Table<usize, LazyValue<Span>>,
generator_diagnostic_data: Table<DefIndex, LazyValue<GeneratorDiagnosticData<'static>>>,
attr_flags: Table<DefIndex, AttrFlags>,
variant_data: Table<DefIndex, LazyValue<VariantData>>,
assoc_container: Table<DefIndex, ty::AssocItemContainer>,
// Slot is full when macro is macro_rules.
macro_rules: Table<DefIndex, ()>,
macro_definition: Table<DefIndex, LazyValue<ast::DelimArgs>>,
proc_macro: Table<DefIndex, MacroKind>,
module_reexports: Table<DefIndex, LazyArray<ModChild>>,
deduced_param_attrs: Table<DefIndex, LazyArray<DeducedParamAttrs>>,
// Slot is full when opaque is TAIT.
is_type_alias_impl_trait: Table<DefIndex, ()>,

trait_impl_trait_tys: Table<DefIndex, LazyValue<FxHashMap<DefId, Ty<'static>>>>,
}

Expand All @@ -419,6 +423,7 @@ struct VariantData {
}

bitflags::bitflags! {
#[derive(Default)]
pub struct AttrFlags: u8 {
const MAY_HAVE_DOC_LINKS = 1 << 0;
const IS_DOC_HIDDEN = 1 << 1;
Expand Down
72 changes: 30 additions & 42 deletions compiler/rustc_metadata/src/rmeta/table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use std::num::NonZeroUsize;
/// but this has no impact on safety.
pub(super) trait FixedSizeEncoding: Default {
/// This should be `[u8; BYTE_LEN]`;
/// Cannot use an associated `const BYTE_LEN: usize` instead due to const eval limitations.
type ByteArray;

fn from_bytes(b: &Self::ByteArray) -> Self;
Expand Down Expand Up @@ -199,31 +200,31 @@ impl FixedSizeEncoding for Option<RawDefId> {
}
}

impl FixedSizeEncoding for Option<AttrFlags> {
impl FixedSizeEncoding for AttrFlags {
type ByteArray = [u8; 1];

#[inline]
fn from_bytes(b: &[u8; 1]) -> Self {
(b[0] != 0).then(|| AttrFlags::from_bits_truncate(b[0]))
AttrFlags::from_bits_truncate(b[0])
}

#[inline]
fn write_to_bytes(self, b: &mut [u8; 1]) {
b[0] = self.map_or(0, |flags| flags.bits())
b[0] = self.bits();
}
}

impl FixedSizeEncoding for Option<()> {
impl FixedSizeEncoding for bool {
type ByteArray = [u8; 1];

#[inline]
fn from_bytes(b: &[u8; 1]) -> Self {
(b[0] != 0).then(|| ())
b[0] != 0
}

#[inline]
fn write_to_bytes(self, b: &mut [u8; 1]) {
b[0] = self.is_some() as u8
b[0] = self as u8
}
}

Expand Down Expand Up @@ -273,44 +274,38 @@ impl<T> FixedSizeEncoding for Option<LazyArray<T>> {
}

/// Helper for constructing a table's serialization (also see `Table`).
pub(super) struct TableBuilder<I: Idx, T>
where
Option<T>: FixedSizeEncoding,
{
blocks: IndexVec<I, <Option<T> as FixedSizeEncoding>::ByteArray>,
pub(super) struct TableBuilder<I: Idx, T: FixedSizeEncoding> {
blocks: IndexVec<I, T::ByteArray>,
_marker: PhantomData<T>,
}

impl<I: Idx, T> Default for TableBuilder<I, T>
where
Option<T>: FixedSizeEncoding,
{
impl<I: Idx, T: FixedSizeEncoding> Default for TableBuilder<I, T> {
fn default() -> Self {
TableBuilder { blocks: Default::default(), _marker: PhantomData }
}
}

impl<I: Idx, T> TableBuilder<I, T>
impl<I: Idx, const N: usize, T> TableBuilder<I, Option<T>>
where
Option<T>: FixedSizeEncoding,
Option<T>: FixedSizeEncoding<ByteArray = [u8; N]>,
{
pub(crate) fn set<const N: usize>(&mut self, i: I, value: T)
where
Option<T>: FixedSizeEncoding<ByteArray = [u8; N]>,
{
pub(crate) fn set(&mut self, i: I, value: T) {
self.set_nullable(i, Some(value))
}
}

impl<I: Idx, const N: usize, T: FixedSizeEncoding<ByteArray = [u8; N]>> TableBuilder<I, T> {
pub(crate) fn set_nullable(&mut self, i: I, value: T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have a convenience wrapper around set_nullable for TableBuilder<I, bool> to just set it to true. Invoking set_nullable(val, false) is never done and likely a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Together with #107166 (comment) I'll try to move the true/false conditions to inside the set methods.
So it will look approximately like this

fn set(value) {
    if value.is_default() {
        return;
    }
    // possibly resize the buffer and set the non-default value
}

and false/None/etc arguments will be accepted but result in doing nothing.

The "reset to None" semantic is never currently used for metadata tables, but if it will be needed it will require a separate method like fn clear in this case.

// FIXME(eddyb) investigate more compact encodings for sparse tables.
// On the PR @michaelwoerister mentioned:
// > Space requirements could perhaps be optimized by using the HAMT `popcnt`
// > trick (i.e. divide things into buckets of 32 or 64 items and then
// > store bit-masks of which item in each bucket is actually serialized).
self.blocks.ensure_contains_elem(i, || [0; N]);
Some(value).write_to_bytes(&mut self.blocks[i]);
value.write_to_bytes(&mut self.blocks[i]);
}

pub(crate) fn encode<const N: usize>(&self, buf: &mut FileEncoder) -> LazyTable<I, T>
where
Option<T>: FixedSizeEncoding<ByteArray = [u8; N]>,
{
pub(crate) fn encode(&self, buf: &mut FileEncoder) -> LazyTable<I, T> {
let pos = buf.position();
for block in &self.blocks {
buf.emit_raw_bytes(block);
Expand All @@ -323,34 +318,27 @@ where
}
}

impl<I: Idx, T: ParameterizedOverTcx> LazyTable<I, T>
impl<I: Idx, const N: usize, T: FixedSizeEncoding<ByteArray = [u8; N]> + ParameterizedOverTcx>
LazyTable<I, T>
where
Option<T>: FixedSizeEncoding,
for<'tcx> T::Value<'tcx>: FixedSizeEncoding<ByteArray = [u8; N]>,
{
/// Given the metadata, extract out the value at a particular index (if any).
#[inline(never)]
pub(super) fn get<'a, 'tcx, M: Metadata<'a, 'tcx>, const N: usize>(
&self,
metadata: M,
i: I,
) -> Option<T::Value<'tcx>>
where
Option<T::Value<'tcx>>: FixedSizeEncoding<ByteArray = [u8; N]>,
{
pub(super) fn get<'a, 'tcx, M: Metadata<'a, 'tcx>>(&self, metadata: M, i: I) -> T::Value<'tcx> {
debug!("LazyTable::lookup: index={:?} len={:?}", i, self.encoded_size);

let start = self.position.get();
let bytes = &metadata.blob()[start..start + self.encoded_size];
let (bytes, []) = bytes.as_chunks::<N>() else { panic!() };
let bytes = bytes.get(i.index())?;
FixedSizeEncoding::from_bytes(bytes)
match bytes.get(i.index()) {
Some(bytes) => FixedSizeEncoding::from_bytes(bytes),
None => FixedSizeEncoding::from_bytes(&[0; N]),
}
}

/// Size of the table in entries, including possible gaps.
pub(super) fn size<const N: usize>(&self) -> usize
where
for<'tcx> Option<T::Value<'tcx>>: FixedSizeEncoding<ByteArray = [u8; N]>,
{
pub(super) fn size(&self) -> usize {
self.encoded_size / N
}
}