Auto merge of rust-lang#90065 - cjgillot:novalcache, r=Mark-Simulacrum

Do not depend on the stored value when trying to cache on disk.

Having different criteria for loading and saving of query results can lead to saved results that may never be loaded.
Since the on-disk cache is discarded as soon as a compilation error is issued, there should not be any need for an exclusion mecanism based on errors.

As a result, the possibility to condition the storage on the value itself does not appear useful.

Author: bors

compiler/rustc_macros/src/query.rs

@@ -36,7 +36,7 @@ enum QueryModifier {
     Storage(Type),
 
     /// Cache the query to disk if the `Expr` returns true.
-    Cache(Option<(IdentOrWild, IdentOrWild)>, Block),
+    Cache(Option<IdentOrWild>, Block),
 
     /// Custom code to load the query from disk.
     LoadCached(Ident, Ident, Block),
@@ -87,9 +87,7 @@ impl Parse for QueryModifier {
                 let args;
                 parenthesized!(args in input);
                 let tcx = args.parse()?;
-                args.parse::<Token![,]>()?;
-                let value = args.parse()?;
-                Some((tcx, value))
+                Some(tcx)
             } else {
                 None
             };
@@ -197,7 +195,7 @@ struct QueryModifiers {
     storage: Option<Type>,
 
     /// Cache the query to disk if the `Block` returns true.
-    cache: Option<(Option<(IdentOrWild, IdentOrWild)>, Block)>,
+    cache: Option<(Option<IdentOrWild>, Block)>,
 
     /// Custom code to load the query from disk.
     load_cached: Option<(Ident, Ident, Block)>,
@@ -375,14 +373,7 @@ fn add_query_description_impl(
         let tcx = args
             .as_ref()
             .map(|t| {
-                let t = &(t.0).0;
-                quote! { #t }
-            })
-            .unwrap_or_else(|| quote! { _ });
-        let value = args
-            .as_ref()
-            .map(|t| {
-                let t = &(t.1).0;
+                let t = &t.0;
                 quote! { #t }
             })
             .unwrap_or_else(|| quote! { _ });
@@ -394,7 +385,6 @@ fn add_query_description_impl(
             fn cache_on_disk(
                 #tcx: QueryCtxt<'tcx>,
                 #key: &Self::Key,
-                #value: Option<&Self::Value>
             ) -> bool {
                 #expr
             }

compiler/rustc_middle/src/query/mod.rs

@@ -765,10 +765,7 @@ rustc_queries! {
     /// additional requirements that the closure's creator must verify.
     query mir_borrowck(key: LocalDefId) -> &'tcx mir::BorrowCheckResult<'tcx> {
         desc { |tcx| "borrow-checking `{}`", tcx.def_path_str(key.to_def_id()) }
-        cache_on_disk_if(tcx, opt_result) {
-            tcx.is_closure(key.to_def_id())
-                || opt_result.map_or(false, |r| !r.concrete_opaque_types.is_empty())
-        }
+        cache_on_disk_if(tcx) { tcx.is_closure(key.to_def_id()) }
     }
     query mir_borrowck_const_arg(key: (LocalDefId, DefId)) -> &'tcx mir::BorrowCheckResult<'tcx> {
         desc {

compiler/rustc_query_impl/src/on_disk_cache.rs

@@ -1033,7 +1033,7 @@ where
         if res.is_err() {
             return;
         }
-        if Q::cache_on_disk(tcx, &key, Some(value)) {
+        if Q::cache_on_disk(tcx, &key) {
             let dep_node = SerializedDepNodeIndex::new(dep_node.index());
 
             // Record position of the cache entry.

compiler/rustc_query_impl/src/plumbing.rs

@@ -418,7 +418,7 @@ macro_rules! define_queries {
 
                     let key = recover(tcx, dep_node).unwrap_or_else(|| panic!("Failed to recover key for {:?} with hash {}", dep_node, dep_node.hash));
                     let tcx = QueryCtxt::from_tcx(tcx);
-                    if queries::$name::cache_on_disk(tcx, &key, None) {
+                    if queries::$name::cache_on_disk(tcx, &key) {
                         let _ = tcx.$name(key);
                     }
                 }

compiler/rustc_query_system/src/query/config.rs

@@ -27,7 +27,7 @@ pub(crate) struct QueryVtable<CTX: QueryContext, K, V> {
     pub compute: fn(CTX::DepContext, K) -> V,
     pub hash_result: Option<fn(&mut StableHashingContext<'_>, &V) -> Fingerprint>,
     pub handle_cycle_error: fn(CTX, DiagnosticBuilder<'_>) -> V,
-    pub cache_on_disk: fn(CTX, &K, Option<&V>) -> bool,
+    pub cache_on_disk: fn(CTX, &K) -> bool,
     pub try_load_from_disk: fn(CTX, SerializedDepNodeIndex) -> Option<V>,
 }
 
@@ -43,8 +43,8 @@ impl<CTX: QueryContext, K, V> QueryVtable<CTX, K, V> {
         (self.compute)(tcx, key)
     }
 
-    pub(crate) fn cache_on_disk(&self, tcx: CTX, key: &K, value: Option<&V>) -> bool {
-        (self.cache_on_disk)(tcx, key, value)
+    pub(crate) fn cache_on_disk(&self, tcx: CTX, key: &K) -> bool {
+        (self.cache_on_disk)(tcx, key)
     }
 
     pub(crate) fn try_load_from_disk(&self, tcx: CTX, index: SerializedDepNodeIndex) -> Option<V> {
@@ -82,7 +82,7 @@ pub trait QueryDescription<CTX: QueryContext>: QueryAccessors<CTX> {
     fn describe(tcx: CTX, key: Self::Key) -> String;
 
     #[inline]
-    fn cache_on_disk(_: CTX, _: &Self::Key, _: Option<&Self::Value>) -> bool {
+    fn cache_on_disk(_: CTX, _: &Self::Key) -> bool {
         false
     }
 

compiler/rustc_query_system/src/query/plumbing.rs

@@ -512,7 +512,7 @@ where
 
     // First we try to load the result from the on-disk cache.
     // Some things are never cached on disk.
-    if query.cache_on_disk(tcx, key, None) {
+    if query.cache_on_disk(tcx, key) {
         let prof_timer = tcx.dep_context().profiler().incr_cache_loading();
         let result = query.try_load_from_disk(tcx, prev_dep_node_index);
         prof_timer.finish_with_query_invocation_id(dep_node_index.into());