Auto merge of #109935 - michaelwoerister:fix-feed-in-eval-always, r=c…

…jgillot

incr.comp.: Make sure dependencies are recorded when feeding queries during eval-always queries.

This PR makes sure we don't drop dependency edges when feeding queries during an eval-always query.

Background: During eval-always queries, no dependencies are recorded because the system knows to unconditionally re-evaluate them regardless of any actual dependencies. This works fine for these queries themselves but leads to a problem when feeding other queries: When queries are fed, we set up their dependency edges by copying the current set of dependencies of the feeding query. But because this set is empty for eval-always queries, we record no edges at all -- which has the effect that the fed query instances always look "green" to the system, although they should always be "red".

The fix is to explicitly add a dependency on the artificial "always red" dep-node when feeding during eval-always queries.

Fixes #108481
Maybe also fixes issue #88488.

cc `@jyn514`

r? `@cjgillot` or `@oli-obk`

compiler/rustc_passes/src/hir_id_validator.rs

@@ -8,8 +8,6 @@ use rustc_middle::hir::nested_filter;
 use rustc_middle::ty::TyCtxt;
 
 pub fn check_crate(tcx: TyCtxt<'_>) {
-    tcx.dep_graph.assert_ignored();
-
     if tcx.sess.opts.unstable_opts.hir_stats {
         crate::hir_stats::print_hir_stats(tcx);
     }

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -143,16 +143,29 @@ impl<K: DepKind> DepGraph<K> {
         assert_eq!(_green_node_index, DepNodeIndex::SINGLETON_DEPENDENCYLESS_ANON_NODE);
 
         // Instantiate a dependy-less red node only once for anonymous queries.
-        let (_red_node_index, _prev_and_index) = current.intern_node(
+        let (red_node_index, red_node_prev_index_and_color) = current.intern_node(
             profiler,
             &prev_graph,
             DepNode { kind: DepKind::RED, hash: Fingerprint::ZERO.into() },
             smallvec![],
             None,
             false,
         );
-        assert_eq!(_red_node_index, DepNodeIndex::FOREVER_RED_NODE);
-        assert!(matches!(_prev_and_index, None | Some((_, DepNodeColor::Red))));
+        assert_eq!(red_node_index, DepNodeIndex::FOREVER_RED_NODE);
+        match red_node_prev_index_and_color {
+            None => {
+                // This is expected when we have no previous compilation session.
+                assert!(prev_graph_node_count == 0);
+            }
+            Some((prev_red_node_index, DepNodeColor::Red)) => {
+                assert_eq!(prev_red_node_index.as_usize(), red_node_index.as_usize());
+                colors.insert(prev_red_node_index, DepNodeColor::Red);
+            }
+            Some((_, DepNodeColor::Green(_))) => {
+                // There must be a logic error somewhere if we hit this branch.
+                panic!("DepNodeIndex::FOREVER_RED_NODE evaluated to DepNodeColor::Green")
+            }
+        }
 
         DepGraph {
             data: Some(Lrc::new(DepGraphData {
@@ -353,10 +366,8 @@ impl<K: DepKind> DepGraphData<K> {
             }))
         };
 
-        let task_deps_ref = match &task_deps {
-            Some(deps) => TaskDepsRef::Allow(deps),
-            None => TaskDepsRef::Ignore,
-        };
+        let task_deps_ref =
+            task_deps.as_ref().map(TaskDepsRef::Allow).unwrap_or(TaskDepsRef::EvalAlways);
 
         let result = K::with_deps(task_deps_ref, || task(cx, arg));
         let edges = task_deps.map_or_else(|| smallvec![], |lock| lock.into_inner().reads);
@@ -461,6 +472,11 @@ impl<K: DepKind> DepGraph<K> {
             K::read_deps(|task_deps| {
                 let mut task_deps = match task_deps {
                     TaskDepsRef::Allow(deps) => deps.lock(),
+                    TaskDepsRef::EvalAlways => {
+                        // We don't need to record dependencies of eval_always
+                        // queries. They are re-evaluated unconditionally anyway.
+                        return;
+                    }
                     TaskDepsRef::Ignore => return,
                     TaskDepsRef::Forbid => {
                         panic!("Illegal read of: {dep_node_index:?}")
@@ -563,7 +579,10 @@ impl<K: DepKind> DepGraph<K> {
             let mut edges = SmallVec::new();
             K::read_deps(|task_deps| match task_deps {
                 TaskDepsRef::Allow(deps) => edges.extend(deps.lock().reads.iter().copied()),
-                TaskDepsRef::Ignore => {} // During HIR lowering, we have no dependencies.
+                TaskDepsRef::EvalAlways => {
+                    edges.push(DepNodeIndex::FOREVER_RED_NODE);
+                }
+                TaskDepsRef::Ignore => {}
                 TaskDepsRef::Forbid => {
                     panic!("Cannot summarize when dependencies are not recorded.")
                 }
@@ -1356,10 +1375,13 @@ pub enum TaskDepsRef<'a, K: DepKind> {
     /// `TaskDeps`. This is used when executing a 'normal' query
     /// (no `eval_always` modifier)
     Allow(&'a Lock<TaskDeps<K>>),
-    /// New dependencies are ignored. This is used when
-    /// executing an `eval_always` query, since there's no
+    /// This is used when executing an `eval_always` query. We don't
     /// need to track dependencies for a query that's always
-    /// re-executed. This is also used for `dep_graph.with_ignore`
+    /// re-executed -- but we need to know that this is an `eval_always`
+    /// query in order to emit dependencies to `DepNodeIndex::FOREVER_RED_NODE`
+    /// when directly feeding other queries.
+    EvalAlways,
+    /// New dependencies are ignored. This is also used for `dep_graph.with_ignore`.
     Ignore,
     /// Any attempt to add new dependencies will cause a panic.
     /// This is used when decoding a query result from disk,

tests/incremental/issue-108481-feed-eval-always.rs

@@ -0,0 +1,16 @@
+// revisions: cpass1 cpass2
+
+#![crate_type = "rlib"]
+
+use std::fmt::Debug;
+
+// MCVE kindly provided by Nilstrieb at
+// https://github.com/rust-lang/rust/issues/108481#issuecomment-1493080185
+
+#[derive(Debug)]
+pub struct ConstGeneric<const CHUNK_SIZE: usize> {
+    _p: [(); CHUNK_SIZE],
+}
+
+#[cfg(cpass1)]
+impl<const CHUNK_SIZE: usize> ConstGeneric<CHUNK_SIZE> {}