owned this note
owned this note
Published
Linked with GitHub
# context
piotr's [original proposal](https://github.com/rust-lang/compiler-team/issues/790) suggested an out-of-band hash of the public interface of a crate which cargo would learn to read and use to trigger ECO (early cutoff) behavior.
this is currently unsound because (among other reasons) there are cases where cargo decides not to recompile, but the downstream crate that hasn't been recompiled refers to items with certain defids that are different in the new rmeta
To see this ICE in action: grab [repro.zip](https://cdn.discordapp.com/attachments/1357381115278659594/1358964475738193980/rdr-repro-panic-items-mismatch.zip?ex=6848278b&is=6846d60b&hm=b726761ae4730230d6a9d17df5a9a2322172843a61f292d5917fb9119ee5a427), and run the following using piotrs rustc on the api-fingerprinting branch:
```bash=
cd outer
cargo +rdr build
# Add a function to inner BEFORE Food enum; this function does not have to be public
cargo +rdr build # again
# ICE
```
at rustweek and the subsequent all-hands we got input from rustc folks that they'd be significantly more invested in an approach that used rmetas as the stable public interface of a crate rather than introducing this out-of-band hash.
this approach has been referred to as "rmetas are header files" (thx tyler/rahul/jakob) ((though really it's probably more clear to describe as "rmetas are c++ module interface files" since those are also generated rather than hand-written)). some reasons this is desirable include:
- lower maintanance burden
- this avoids needing separate codepaths in cargo and rustc and an extra compiler output only used for this mode
- lifting all boats
- solving some issues inherent to rmetas being headers also improves other aspects of rustc. most notably fixing defid stability has the potential to significantly improve the rustc-incremental cache hit rate which makes it desirable to land this change irrespective of other RDR stuff. oli has said he'd be happy to help review/land changes in this direction
- better soundness guarantees
- with a separate hash, we'd have to hope that we correctly took into account the list of things that actually "matter" for stability
- if something is in the rmeta, the downstream crate can access it which means it's part of the interface. otherwise it shouldn't be in there
- "real" build systems (bazel/buck2/nix/etc.) use content hashing rather than mtimes, and we wouldn't easily be able to patch them to only pay attention to separate interface hashes and smuggle through the changed-but-without-interface-changes rmetas
- cam actually got this working using incremental actions in buck2, but that is undesirable for other reasons (like making remote execution worse iiuc)
- if rmetas are a stable representation of the interface, build system support becomes trivial
- late last year cargo grew functionality to rely on the hashes of source files rather than their mtimes: [-Zchecksum-freshness](https://github.com/rust-lang/cargo/pull/14137). This should allow us to benefit from the rmeta-as-header approach without needing additional logic in cargo
- TODO: verify that checksum freshness applies to all inputs, not only sources. and check that ECO does function properly when rmetas don't change
### helpful documentation
- [cargo fingerprinting docs](https://doc.rust-lang.org/nightly/nightly-rustc/cargo/core/compiler/fingerprint/index.html#fingerprints-and-unithashs)
- [dev guide high level overview of what's in an rmeta](https://rustc-dev-guide.rust-lang.org/backend/libs-and-metadata.html#metadata)
## zulip threads
- [original RDR discussion](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Dynamically.20pruning.20jobs.20from.20the.20work.20queue)
- [splitting spans into separate output](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Splitting.20out.20span.20information.20from.20.60.2Ermeta.60.3F/with/510754927)
- [rust-analyzer stable ast-node id's](https://rust-lang.zulipchat.com/#narrow/channel/185405-t-compiler.2Frust-analyzer/topic/Stable-r.20Ast.20IDs/with/519054629) (and [relevant PR](https://github.com/rust-lang/rust-analyzer/pull/19837))
- [stable defids](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/RDR.3A.20Stable.20DefID.2FDisambiguator/with/522856499)
### things that need to cause rebuilds
- obviously any public signature change
- body of a public fn that has generics in the signature
- body of a public fn that returns an existential type (`impl trait`)
### faq:
- do rustc-incremental improvements obviate rdr
- nope, at least today even when you get 100% hit rate for incremental queries, replaying those and re-generating the output rmeta/rlib still takes appreciable time. that is repeated for every crate in the dep-chain to the point where ECO in cargo is a desirable feature
- would stable rmetas let us share artifacts between cargo check and cargo build? (TODO: link issue)
- no:
- fingerprint includes rustc args
- emit=link,metadata with -Znocodegen is different from emit=metadata (TODO: link zulip thread and relevant code in meta-build-systems that uses this trick)
- it's possible clippy could be shared with check, but not build since it needs (for example) encoded MIR which check builds skip according to the dev guide
# things that currently make rmetas not stable
## DefIds
[disambiguators](https://github.com/rust-lang/rust/blob/44f415c1d617ebc7b931a243b7b321ef8a6ca47c/compiler/rustc_hir/src/definitions.rs#L180) make defids unstable because re-ordering (maybe rare) or adding a new item (extremely common) can change disambiguators of following items which in turn changes their defid.
for each `DefKind` there's a corresponding [`DefPathData`](https://github.com/rust-lang/rust/blob/44f415c1d617ebc7b931a243b7b321ef8a6ca47c/compiler/rustc_hir/src/definitions.rs#L278) which in some cases includes a `Symbol` which disambiguates betweens items of the same type. We need to go through each variant and figure out what the best way to make them stable across compilations would be. The current idea (thx taylor for suggesting this) is hashing the token stream used to create the item because we have access to it through the spans at the point where we're assigning defids.
defids are assigned from all over the place, but it seems like most calls funnel down [through the name resolver](https://github.com/rust-lang/rust/blob/44f415c1d617ebc7b931a243b7b321ef8a6ca47c/compiler/rustc_resolve/src/lib.rs#L1347) or elsewhere to [this spot in rustc-middle](https://github.com/rust-lang/rust/blob/44f415c1d617ebc7b931a243b7b321ef8a6ca47c/compiler/rustc_middle/src/ty/context.rs#L2016)
note from julia on 06-11:
right now one easy way to observe defids is with rustdoc-json:
with a `#![no_std]` crate containing just `pub struct Blue;`
```bash
rustdoc +stage1 -Zunstable-options --output-format=json ~/test/test.rs -o /tmp/blah
</tmp/blah/test.json jq '.index | values[] | select(.name=="Blue") | [.crate_id, .id]' -c
# outputs [0,0] (aka 0:0)
# add `pub struct Red;` above `Blue` and rerun:
# outputs [0,40]
```
## Spans
current plan is set them to empty/dummy_sp and re-run the compiler without rdr when you want good diagnostics
shiny far off future: separate spans file output from rustc that's used for diagnostics generation: TODO link to david lattimore's message
## `Zshared-generics`
probably mutually exclusive with rdr for now. that's ok though according to rustc folks and we're fine with kicking the can down the road.
the reason this breaks things is that the set of generics that happen to be instantiated in a parent crate change which ones are instantiated in a child vs reusing existing ones. if we let the set of instantiations become part of the public interface it hugely (not sure if piotr actually tested this, but it makes sense intuitively) pessimizes re-use because that list is quite unstable.
Note that generics are not shared in `check` builds, which means we can land RDR for check-only without worrying too much about shared-generics.
_Jane: I feel like I remember C++ having something similar to this where you pre-instantiate generics in the header file to share them. I wonder if there could be a solution here where we make publicly shared generics become an explicit and intentional part of the API_
_Piotr: what you're thinking of is [`extern template` (explicit template instantiation)](https://en.cppreference.com/w/cpp/language/class_template.html)_
## SVH
currently seems to include hashes of mtimes or straight up file contents?? TODO: figure out why/whether this can be avoided
## source map
??
## more?
to find out what's changing we can perturb a minimal crate and inspect the resulting rmeta
minimal crate:
```rust=
#![no_std]
pub struct Foo;
```
even more minimal crate:
```rust=
#![allow(internal_features)]
#![feature(lang_items)]
#![feature(no_core)]
#![no_std]
#![no_core]
#![no_implicit_prelude]
#[lang = "copy"]
pub trait Copy {}
pub struct Foo;
```
creates about a 1k rmeta. how do we inspect this? can we do better than `vbindiff`? Maybe it's easy to write a structured output view for rmetas and we could use that?
both rahul and piotr have expressed interest in implementing ^, the natural place to start seems to be adding to saethlin's `-Zmeta-stats` to include section contents instead of just counts.
tyler has [these changes](https://github.com/rust-lang/rust/compare/master...tmandry:rust:debuggable-encoding?expand=1) to help debug rustc determinism, but we should be able to repurpose this snippet to get backtraces for which part of emitting an rmeta was different than the previous one.
i've been using `-Zmeta-stats` to see what changed like this:
`echo // >> test.rs && rustc +stage1 --edition=2024 --crate-type=lib --emit=metadata -Zmeta-stats test.rs -o /tmp/test.rmeta && crc32 /tmp/test.rmeta` (this changes the source_map section consistently which we want to avoid)
example output:
```
meta-stats METADATA STATS
meta-stats Section Size
meta-stats ----------------------------------------------------------------
meta-stats lib-features 0 ( 0.0%)
meta-stats stability-implications 0 ( 0.0%)
meta-stats stripped-cfg-items 0 ( 0.0%)
meta-stats diagnostic-items 0 ( 0.0%)
meta-stats native-libs 0 ( 0.0%)
meta-stats foreign-modules 0 ( 0.0%)
meta-stats traits 0 ( 0.0%)
meta-stats impls 0 ( 0.0%)
meta-stats incoherent-impls 0 ( 0.0%)
meta-stats mir 0 ( 0.0%)
meta-stats interpret-alloc-index 0 ( 0.0%)
meta-stats proc-macro-data 0 ( 0.0%)
meta-stats debugger-visualizers 0 ( 0.0%)
meta-stats exportable-items 0 ( 0.0%)
meta-stats exportable-items 0 ( 0.0%)
meta-stats exported-symbols 0 ( 0.0%)
meta-stats target-modifiers 0 ( 0.0%)
meta-stats lang-items 1 ( 0.1%)
meta-stats def-path-table 29 ( 2.1%)
meta-stats preamble 34 ( 2.5%)
meta-stats hygiene 90 ( 6.5%)
meta-stats source-map 123 ( 8.9%)
meta-stats dep 147 (10.7%)
meta-stats def-ids 158 (11.5%)
meta-stats tables 195 (14.2%)
meta-stats def-path-hash-map 258 (18.7%)
meta-stats final 342 (24.8%)
meta-stats ----------------------------------------------------------------
meta-stats Total 1_377 (of which 46.6% are zero bytes)
meta-stats
58f887f5
```
we can do variants of this test such as:
- adding a newline to the top of the file to trigger span changes
- adding a new unit struct to the top of the file to trigger defid reflows
- _Jane: This should still change rmeta if the unit struct is public right? (should it not change if it is private? I assume that would be preferable)_
- adding a println to a function body to see what that changes (like encoded mir?)
## `rmeta` layout
[`with_encode_metadata_header`](https://github.com/rust-lang/rust/blob/996a185ba74755cb39a4fe24151ea65e671d4c0d/compiler/rustc_metadata/src/rmeta/encoder.rs#L2378-L2430) is kind of the entrypoint
layout:
- metadata header
- root position
- rustc version
- ... (misc fields; see `encode_crate_root`)
- `CrateRoot`
## glossary
#### DefId
2 fields: crate id and defindex
#### DefPathData
#### DisambiguatedDefPathData
same as `DefPathData` but with a u32 "disambiguator" that right now is assigned based on order in file.
#### DefIndex
#### DefKey
#### LocalDefId
# Other
### peeps that care about this/who to @ on stuff:
- zed: piotr, julia
- meta: cam, davidbarsky
- compiler team: oli, taylor
- other: ally, jane, rahul