incr.comp.: Crate Metadata not corresponding to a DefId is untracked.

[michaelwoerister]

Mentoring update: Mentoring instructions can be found here

---

There are many things in crate metadata that are not covered by dependency tracking (basically everything below this line). Those are things that are unlikely to change in an upstream crate, which is why this has not caused problems yet, but we'll not want to rely on that.

The best way to solve this is probably to create separate "input" DepNodes for these.

cc @nikomatsakis

[nikomatsakis]

Yes, I want to refactor how this stuff is handled. Ultimately, I think that metadata should just be a bunch of (key, value) pairs where the key is some query, and there should be no more "crate store" trait (except for the "connect providers" method).

However, it occurs to me that once we get the red-green system, we could just have a single "metadata" node corresponding to the entire crate, and then let the red/green caching remember what the values of specific metadata queries were.

[nikomatsakis]

After some discussion on IRC, @michaelwoerister and I were thinking that the best immediate way to make progress here is going to involve converting all CrateStore methods into queries. The main sticking point will be resolve, which we will leave 'as is' for now.

To that end, here is a checklist of CrateStore methods. Let's see if we can convert them all to queries. I've checked some off that probably don't make sense at this juncture (or ever).

Remaining

• fn crates(&self) -> Vec<CrateNum>;

Resolve and elsewhere in rustc?

Rename these methods to {name}_untracked in CrateStore, add queries to TyCtxt, convert the compiler to use the query and change resolve to using {name}_untracked

• fn dep_kind(&self, cnum: CrateNum) -> DepKind; - Migrate a slew of metadata methods to queries #44142
• fn visibility(&self, def: DefId) -> ty::Visibility; - Migrate a slew of metadata methods to queries #44142
• fn crate_name(&self, cnum: CrateNum) -> Symbol; - Migrate a slew of metadata methods to queries #44142
• fn item_children(&self, did: DefId) -> Vec<def::Export>; - Migrate a slew of metadata methods to queries #44142
• fn extern_mod_stmt_cnum(&self, emod_id: ast::NodeId) -> Option<CrateNum>; - Migrate a slew of metadata methods to queries #44142

Transitioned

• fn describe_def(&self, def: DefId) -> Option<Def>; -- query for describe_def #41534
• fn def_span(&self, sess: &Session, def: DefId) -> Span; -- query for def_span #41593
• fn stability(&self, def: DefId) -> Option<attr::Stability>; -- Queries for Crate Metadata #41653
• fn deprecation(&self, def: DefId) -> Option<attr::Deprecation>; -- Queries for Crate Metadata #41653
• fn item_attrs(&self, def_id: DefId) -> Vec<ast::Attribute>; -- More Queries for Crate Metadata #41724
• fn fn_arg_names(&self, did: DefId) -> Vec<ast::Name>; -- More Queries for Crate Metadata #41724
• fn impl_polarity(&self, def: DefId) -> hir::ImplPolarity; -- Performance audit, Spring 2017 #41469
• fn impl_parent(&self, impl_def_id: DefId) -> Option<DefId>; -- More Queries for Crate Metadata #41724
• fn trait_of_item(&self, def_id: DefId) -> Option<DefId>; -- More Queries for Crate Metadata #41724
• fn is_const_fn(&self, did: DefId) -> bool; -- Track more crate metadata #42598
• fn is_default_impl(&self, impl_did: DefId) -> bool; -- Translate array drop glue using MIR #41917
• fn is_foreign_item(&self, did: DefId) -> bool; -- More Queries for Crate Metadata #41724
• fn is_staged_api(&self, cnum: CrateNum) -> bool; -- rustc: Add a new -Z force-unstable-if-unmarked flag #41847
• fn is_allocator(&self, cnum: CrateNum) -> bool; -- Track more crate metadata #42598
• fn is_panic_runtime(&self, cnum: CrateNum) -> bool; -- Track more crate metadata #42598
• fn extern_crate(&self, cnum: CrateNum) -> Option<ExternCrate>; -- Track more crate metadata #42598
• fn retrace_path(&self, cnum: CrateNum, path_data: &[DisambiguatedDefPathData]) -> Option<DefId>; -- Remove unused DefPathTable::retrace_path() #43361
• fn item_body_nested_bodies(&self, def: DefId) -> BTreeMap<hir::BodyId, hir::Body>; -- Queryify crate metadata #41611
• fn const_is_rvalue_promotable_to_static(&self, def: DefId) -> bool; -- Queryify crate metadata #41611
• fn is_item_mir_available(&self, def: DefId) -> bool; -- Queryify crate metadata #41611
• fn metadata_filename(&self) -> &str; - Make only rustc_trans depend on rustc_llvm #41565
• fn metadata_section_name(&self, target: &Target) -> &str; - Make only rustc_trans depend on rustc_llvm #41565
• fn is_exported_symbol(&self, def_id: DefId) -> bool; - More Queries for Crate Metadata #41724
• fn dylib_dependency_formats(&self, cnum: CrateNum) -> Vec<(CrateNum, LinkagePreference)>; - Track more crate metadata #42598
• fn original_crate_name(&self, cnum: CrateNum) -> Symbol; - Migrate a slew of metadata methods to queries #44142
• fn crate_hash(&self, cnum: CrateNum) -> Svh; - Migrate a slew of metadata methods to queries #44142
• fn crate_disambiguator(&self, cnum: CrateNum) -> Symbol; - Migrate a slew of metadata methods to queries #44142
• fn plugin_registrar_fn(&self, cnum: CrateNum) -> Option<DefId>; - Migrate a slew of metadata methods to queries #44142
• fn derive_registrar_fn(&self, cnum: CrateNum) -> Option<DefId>; - Migrate a slew of metadata methods to queries #44142
• fn native_libraries(&self, cnum: CrateNum) -> Vec<NativeLibrary>; - Migrate a slew of metadata methods to queries #44142
• fn exported_symbols(&self, cnum: CrateNum) -> Vec<DefId>; - Migrate a slew of metadata methods to queries #44142
• fn impl_defaultness(&self, def: DefId) -> hir::Defaultness; - Migrate a slew of metadata methods to queries #44142
• fn is_compiler_builtins(&self, cnum: CrateNum) -> bool; - Migrate a slew of metadata methods to queries #44142
• fn is_sanitizer_runtime(&self, cnum: CrateNum) -> bool; - Migrate a slew of metadata methods to queries #44142
• fn panic_strategy(&self, cnum: CrateNum) -> PanicStrategy; - Migrate a slew of metadata methods to queries #44142
• fn is_no_builtins(&self, cnum: CrateNum) -> bool; - Migrate a slew of metadata methods to queries #44142
• fn implementations_of_trait(&self, filter: Option<DefId>) -> Vec<DefId>; -Migrate a slew of metadata methods to queries #44142
• fn is_dllimport_foreign_item(&self, def: DefId) -> bool; - Migrate a slew of metadata methods to queries #44142
• fn is_statically_included_foreign_item(&self, def_id: DefId) -> bool; - Migrate a slew of metadata methods to queries #44142
• fn used_libraries(&self) -> Vec<NativeLibrary>; - Migrate a slew of metadata methods to queries #44142
• fn used_link_args(&self) -> Vec<String>; - Migrate a slew of metadata methods to queries #44142
• fn item_body<'a, 'tcx>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>, def: DefId) -> &'tcx hir::Body; - Migrate a slew of metadata methods to queries #44142
• fn visible_parent_map<'a>(&'a self) -> ::std::cell::Ref<'a, DefIdMap<DefId>>; - Migrate a slew of metadata methods to queries #44142
• fn used_crates(&self, prefer: LinkagePreference) -> Vec<(CrateNum, LibSource)>; - Migrate a slew of metadata methods to queries #44142
• fn used_crate_source(&self, cnum: CrateNum) -> CrateSource; - Migrate a slew of metadata methods to queries #44142

??

• fn encode_metadata<'a, 'tcx>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>, link_meta: &LinkMeta, reachable: &NodeSet) -> EncodedMetadata;
• fn metadata_encoding_version(&self) -> &[u8];
• fn def_key(&self, def: DefId) -> DefKey;
  • def-id is just a canonicalized def-key and def-path, so this is an immutable property or else def-id would be different
• fn def_path(&self, def: DefId) -> hir_map::DefPath;
  • see above
• fn def_path_hash(&self, def: DefId) -> u64;
  • see above
• fn def_path_table(&self, cnum: CrateNum) -> Rc<DefPathTable>;
  • see above
• fn struct_field_names(&self, def: DefId) -> Vec<ast::Name>; -- used in resolve
• fn load_macro(&self, did: DefId, sess: &Session) -> LoadedMacro; -- used in resolve
• fn export_macros(&self, cnum: CrateNum); -- used in resolve
• fn associated_item_cloned(&self, def: DefId) -> ty::AssociatedItem; -- used in resolve
• fn item_generics_cloned(&self, def: DefId) -> ty::Generics;
• fn lang_items(&self, cnum: CrateNum) -> Vec<(DefIndex, usize)>; - deferred to incremental: Hide lang_items behind a query in the TyCtxt #44190
• fn missing_lang_items(&self, cnum: CrateNum) -> Vec<lang_items::LangItem>; - deferred to incremental: Hide lang_items behind a query in the TyCtxt #44190

[nikomatsakis]

cc @cramertj @aochagavia -- you interested in tackling some of these? In some cases, queries actually exist already, and we just have to convert code to use them exclusively (e.g., is_foreign_item)

[nikomatsakis]

Mentoring instructions: The current step in resolving this is to move methods from the CrateStore trait and convert them into queries. The CrateStore trait serves as the "abstract interface" to the metadata -- it is defined in librustc, but implemented in librustc_metadata (exactly once, though there exists a second, dummy impl).

The idea then for a PR is to:

• pick a method in the list
• find uses of it:
  • if it is used in resolve, you can skip it for now and move on to another
• otherwise:
  • add a query to the src/librustc/ty/maps.rs file for this method
  • convert the method into a crate metadata provider
  • convert all calls to the method to use the query (typically by doing ty::queries::QUERY::get(tcx, DUMMY_SP, def_id)

[hackeryarn]

I would love to get started on this. I will try to have an implementation for one of the items up in a couple days to align on direction.

[cramertj]

Yeah, I can take a look at some of these tonight or tomorrow.

[hackeryarn]

@cramertj I was planning on taking a look at the couple of the first ones today or tomorrow morning if you want to start a little further down the list.

Wouldn't want us stepping on each other's toes 😄

[cramertj]

@achernyak Sounds great! Thanks.

[nikomatsakis]

@cramertj @achernyak great! =)

[michaelwoerister]

However, it occurs to me that once we get the red-green system, we could just have a single "metadata" node corresponding to the entire crate, and then let the red/green caching remember what the values of specific metadata queries were.

One downside of this approach, that came up during discussion, is that this would mean that we would have to load and deserialize each value just to find out if it has changed or not. Currently we directly load fingerprints to do this check.

[arielb1]

One downside of this approach, that came up during discussion, is that this would mean that we would have to load and deserialize each value just to find out if it has changed or not. Currently we directly load fingerprints to do this check.

Can't we save fingerprints in the parent crate metadata and copy the expected fingerprints into the child crate IC-info?