Auto merge of #154122 - Zalathar:no-anon, r=nnethercote

Remove the `anon` query modifier



Prior experiments:
- https://github.com/rust-lang/rust/pull/152268
- https://github.com/rust-lang/rust/pull/153996

[Zulip thread: *Removing the `anon` query modifier*](https://rust-lang.zulipchat.com/#narrow/channel/582699-t-compiler.2Fquery-system/topic/Removing.20the.20.60anon.60.20query.20modifier/with/580760962)

---

There are currently three queries that use the `anon` modifier:
- `check_representability`
- `check_representability_adt_ty`
- `erase_and_anonymize_regions_ty`

It seems that none of them are using `anon` in an essential way.

According to comments and tests, the *representability* queries mainly care about not being eligible for forcing (despite having a recoverable key type), so that if a cycle does occur then the entire cycle will be on the query stack. Replacing `anon` with a new `no_force` modifier gives a modest perf improvement.

The `erase_and_anonymize_regions_ty` query appears to be using `anon` to reduce the dep-graph overhead of a query that is expected to not call any other queries (and thus have no dependencies). Replacing `anon` with either `no_hash` or nothing appears to give only a very small perf hit on `cargo` benchmarks, which is justified by the fact that it lets us remove a lot of machinery for anonymous queries.

We still need to retain some of the machinery for anonymous *tasks*, because the non-query task `DepKind::TraitSelect` still uses it.

---

I have some ideas for a follow-up that will reduce dep-graph overhead by replacing *all* zero-dependency nodes with a singleton node, but I want to keep that separate in case it causes unexpected issues and needs to be bisected or reverted.

compiler/rustc_hir_analysis/src/check/wfcheck.rs

@@ -998,7 +998,7 @@ fn check_type_defn<'tcx>(
     item: &hir::Item<'tcx>,
     all_sized: bool,
 ) -> Result<(), ErrorGuaranteed> {
-    let _ = tcx.check_representability(item.owner_id.def_id);
+    tcx.ensure_ok().check_representability(item.owner_id.def_id);
     let adt_def = tcx.adt_def(item.owner_id);
 
     enter_wf_checking_ctxt(tcx, item.owner_id.def_id, |wfcx| {

compiler/rustc_macros/src/query.rs

@@ -140,13 +140,13 @@ struct CacheOnDiskIf {
 /// See `rustc_middle::query::modifiers` for documentation of each query modifier.
 struct QueryModifiers {
     // tidy-alphabetical-start
-    anon: Option<Ident>,
     arena_cache: Option<Ident>,
     cache_on_disk_if: Option<CacheOnDiskIf>,
     depth_limit: Option<Ident>,
     desc: Desc,
     eval_always: Option<Ident>,
     feedable: Option<Ident>,
+    no_force: Option<Ident>,
     no_hash: Option<Ident>,
     separate_provide_extern: Option<Ident>,
     // tidy-alphabetical-end
@@ -156,8 +156,8 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
     let mut arena_cache = None;
     let mut cache_on_disk_if = None;
     let mut desc = None;
+    let mut no_force = None;
     let mut no_hash = None;
-    let mut anon = None;
     let mut eval_always = None;
     let mut depth_limit = None;
     let mut separate_provide_extern = None;
@@ -189,10 +189,10 @@ macro_rules! try_insert {
             try_insert!(cache_on_disk_if = CacheOnDiskIf { modifier, block });
         } else if modifier == "arena_cache" {
             try_insert!(arena_cache = modifier);
+        } else if modifier == "no_force" {
+            try_insert!(no_force = modifier);
         } else if modifier == "no_hash" {
             try_insert!(no_hash = modifier);
-        } else if modifier == "anon" {
-            try_insert!(anon = modifier);
         } else if modifier == "eval_always" {
             try_insert!(eval_always = modifier);
         } else if modifier == "depth_limit" {
@@ -212,8 +212,8 @@ macro_rules! try_insert {
         arena_cache,
         cache_on_disk_if,
         desc,
+        no_force,
         no_hash,
-        anon,
         eval_always,
         depth_limit,
         separate_provide_extern,
@@ -243,24 +243,24 @@ fn returns_error_guaranteed(ret_ty: &ReturnType) -> bool {
 fn make_modifiers_stream(query: &Query) -> proc_macro2::TokenStream {
     let QueryModifiers {
         // tidy-alphabetical-start
-        anon,
         arena_cache,
         cache_on_disk_if,
         depth_limit,
         desc: _,
         eval_always,
         feedable,
+        no_force,
         no_hash,
         separate_provide_extern,
         // tidy-alphabetical-end
     } = &query.modifiers;
 
-    let anon = anon.is_some();
     let arena_cache = arena_cache.is_some();
     let cache_on_disk = cache_on_disk_if.is_some();
     let depth_limit = depth_limit.is_some();
     let eval_always = eval_always.is_some();
     let feedable = feedable.is_some();
+    let no_force = no_force.is_some();
     let no_hash = no_hash.is_some();
     let returns_error_guaranteed = returns_error_guaranteed(&query.return_ty);
     let separate_provide_extern = separate_provide_extern.is_some();
@@ -273,12 +273,12 @@ fn make_modifiers_stream(query: &Query) -> proc_macro2::TokenStream {
         query_name_span =>
         // Search for (QMODLIST) to find all occurrences of this query modifier list.
         // tidy-alphabetical-start
-        anon: #anon,
         arena_cache: #arena_cache,
         cache_on_disk: #cache_on_disk,
         depth_limit: #depth_limit,
         eval_always: #eval_always,
         feedable: #feedable,
+        no_force: #no_force,
         no_hash: #no_hash,
         returns_error_guaranteed: #returns_error_guaranteed,
         separate_provide_extern: #separate_provide_extern,
@@ -387,13 +387,15 @@ macro_rules! doc_link {
     }
 
     doc_link!(
+        // tidy-alphabetical-start
         arena_cache,
-        no_hash,
-        anon,
-        eval_always,
         depth_limit,
-        separate_provide_extern,
+        eval_always,
         feedable,
+        no_force,
+        no_hash,
+        separate_provide_extern,
+        // tidy-alphabetical-end
     );
 
     let name = &query.name;
@@ -476,11 +478,6 @@ fn #name(#key_ty) #return_ty
         });
 
         if let Some(feedable) = &modifiers.feedable {
-            assert!(
-                modifiers.anon.is_none(),
-                feedable.span(),
-                "Query {name} cannot be both `feedable` and `anon`."
-            );
             assert!(
                 modifiers.eval_always.is_none(),
                 feedable.span(),

compiler/rustc_middle/src/dep_graph/dep_node.rs

@@ -94,10 +94,14 @@ pub const fn as_usize(&self) -> usize {
 pub struct DepNode {
     pub kind: DepKind,
 
-    /// This is _typically_ a hash of the query key, but sometimes not.
+    /// If `kind` is a query method, then its "key fingerprint" is always a
+    /// stable hash of the query key.
     ///
-    /// For example, `anon` nodes have a fingerprint that is derived from their
+    /// For non-query nodes, the content of this field varies:
-    /// dependencies instead of a key.
+    /// - Some dep kinds always use a dummy `ZERO` fingerprint.
+    /// - Some dep kinds use the stable hash of some relevant key-like value.
+    /// - Some dep kinds use the `with_anon_task` mechanism, and set their key
+    ///   fingerprint to a hash derived from the task's dependencies.
     ///
     /// In some cases the key value can be reconstructed from this fingerprint;
     /// see [`KeyFingerprintStyle`].

compiler/rustc_middle/src/dep_graph/graph.rs

@@ -375,9 +375,8 @@ pub fn with_task<'tcx, A: Debug, R>(
     /// incorrectly marked green.
     ///
     /// FIXME: This could perhaps return a `WithDepNode` to ensure that the
-    /// user of this function actually performs the read; we'll have to see
+    /// user of this function actually performs the read.
-    /// how to make that work with `anon` in `execute_job_incr`, though.
+    fn with_anon_task_inner<'tcx, OP, R>(
-    pub fn with_anon_task_inner<'tcx, OP, R>(
         &self,
         tcx: TyCtxt<'tcx>,
         dep_kind: DepKind,

compiler/rustc_middle/src/queries.rs

@@ -585,16 +585,15 @@
         desc { "checking if `{}` is representable", tcx.def_path_str(key) }
         // We don't want recursive representability calls to be forced with
         // incremental compilation because, if a cycle occurs, we need the
-        // entire cycle to be in memory for diagnostics. This means we can't
+        // entire cycle to be in memory for diagnostics.
-        // use `ensure_ok()` with this query.
+        no_force
-        anon
     }
 
     /// An implementation detail for the `check_representability` query. See that query for more
     /// details, particularly on the modifiers.
     query check_representability_adt_ty(key: Ty<'tcx>) {
         desc { "checking if `{}` is representable", key }
-        anon
+        no_force
     }
 
     /// Set of param indexes for type params that are in the type's representation
@@ -768,14 +767,10 @@
     /// Normally you would just use `tcx.erase_and_anonymize_regions(value)`,
     /// however, which uses this query as a kind of cache.
     query erase_and_anonymize_regions_ty(ty: Ty<'tcx>) -> Ty<'tcx> {
-        // This query is not expected to have input -- as a result, it
-        // is not a good candidates for "replay" because it is essentially a
-        // pure function of its input (and hence the expectation is that
-        // no caller would be green **apart** from just these
-        // queries). Making it anonymous avoids hashing the result, which
-        // may save a bit of time.
-        anon
         desc { "erasing regions from `{}`", ty }
+        // Not hashing the return value appears to give marginally better perf for this query,
+        // which should always be marked green for having no dependencies anyway.
+        no_hash
     }
 
     query wasm_import_module_map(_: CrateNum) -> &'tcx DefIdMap<String> {

compiler/rustc_middle/src/query/modifiers.rs

@@ -7,11 +7,6 @@
 
 // tidy-alphabetical-start
 //
-/// # `anon` query modifier
-///
-/// Generate a dep node based not on the query key, but on the query's dependencies.
-pub(crate) struct anon;
-
 /// # `arena_cache` query modifier
 ///
 /// Query return values must impl `Copy` and be small, but some queries must return values that
@@ -59,6 +54,16 @@
 /// Generate a `feed` method to set the query's value from another query.
 pub(crate) struct feedable;
 
+/// # `no_force` query modifier
+///
+/// Dep nodes of queries with this modifier will never be "forced" when trying
+/// to mark their dependents green, even if their key is recoverable from the
+/// key fingerprint.
+///
+/// Used by some queries with custom cycle-handlers to ensure that if a cycle
+/// occurs, all of the relevant query calls will be on the query stack.
+pub(crate) struct no_force;
+
 /// # `no_hash` query modifier
 ///
 /// Do not hash the query's return value for incremental compilation. If the value needs to be

compiler/rustc_middle/src/query/plumbing.rs

@@ -77,8 +77,6 @@ pub enum EnsureMode {
 pub struct QueryVTable<'tcx, C: QueryCache> {
     pub name: &'static str,
 
-    /// True if this query has the `anon` modifier.
-    pub anon: bool,
     /// True if this query has the `eval_always` modifier.
     pub eval_always: bool,
     /// True if this query has the `depth_limit` modifier.
@@ -283,12 +281,12 @@ macro_rules! define_callbacks {
                 fn $name:ident($($K:tt)*) -> $V:ty
                 {
                     // Search for (QMODLIST) to find all occurrences of this query modifier list.
-                    anon: $anon:literal,
                     arena_cache: $arena_cache:literal,
                     cache_on_disk: $cache_on_disk:literal,
                     depth_limit: $depth_limit:literal,
                     eval_always: $eval_always:literal,
                     feedable: $feedable:literal,
+                    no_force: $no_force:literal,
                     no_hash: $no_hash:literal,
                     returns_error_guaranteed: $returns_error_guaranteed:literal,
                     separate_provide_extern: $separate_provide_extern:literal,

compiler/rustc_middle/src/ty/inhabitedness/mod.rs

@@ -61,7 +61,7 @@ pub(crate) fn provide(providers: &mut Providers) {
 /// requires calling [`InhabitedPredicate::instantiate`]
 fn inhabited_predicate_adt(tcx: TyCtxt<'_>, def_id: DefId) -> InhabitedPredicate<'_> {
     if let Some(def_id) = def_id.as_local() {
-        let _ = tcx.check_representability(def_id);
+        tcx.ensure_ok().check_representability(def_id);
     }
 
     let adt = tcx.adt_def(def_id);

compiler/rustc_query_impl/src/dep_kind_vtables.rs

@@ -96,30 +96,23 @@ pub(crate) fn Metadata<'tcx>() -> DepKindVTable<'tcx> {
 /// Shared implementation of the [`DepKindVTable`] constructor for queries.
 /// Called from macro-generated code for each query.
 pub(crate) fn make_dep_kind_vtable_for_query<'tcx, Q>(
-    is_anon: bool,
     is_cache_on_disk: bool,
     is_eval_always: bool,
+    is_no_force: bool,
 ) -> DepKindVTable<'tcx>
 where
     Q: GetQueryVTable<'tcx>,
 {
-    let key_fingerprint_style = if is_anon {
-        KeyFingerprintStyle::Opaque
-    } else {
-        <Q::Cache as QueryCache>::Key::key_fingerprint_style()
-    };
-
     // A query dep-node can only be forced or promoted if it can recover a key
     // from its key fingerprint.
+    let key_fingerprint_style = <Q::Cache as QueryCache>::Key::key_fingerprint_style();
     let can_recover = key_fingerprint_style.is_maybe_recoverable();
-    if is_anon {
-        assert!(!can_recover);
-    }
 
     DepKindVTable {
         is_eval_always,
         key_fingerprint_style,
-        force_from_dep_node_fn: can_recover.then_some(force_from_dep_node_inner::<Q>),
+        force_from_dep_node_fn: (can_recover && !is_no_force)
+            .then_some(force_from_dep_node_inner::<Q>),
         promote_from_disk_fn: (can_recover && is_cache_on_disk)
             .then_some(promote_from_disk_inner::<Q>),
     }
@@ -133,12 +126,12 @@ macro_rules! define_dep_kind_vtables {
                 fn $name:ident($K:ty) -> $V:ty
                 {
                     // Search for (QMODLIST) to find all occurrences of this query modifier list.
-                    anon: $anon:literal,
                     arena_cache: $arena_cache:literal,
                     cache_on_disk: $cache_on_disk:literal,
                     depth_limit: $depth_limit:literal,
                     eval_always: $eval_always:literal,
                     feedable: $feedable:literal,
+                    no_force: $no_force:literal,
                     no_hash: $no_hash:literal,
                     returns_error_guaranteed: $returns_error_guaranteed:literal,
                     separate_provide_extern: $separate_provide_extern:literal,
@@ -165,9 +158,9 @@ fn $name:ident($K:ty) -> $V:ty
                 $crate::dep_kind_vtables::make_dep_kind_vtable_for_query::<
                     $crate::query_impl::$name::VTableGetter,
                 >(
-                    $anon,
                     $cache_on_disk,
                     $eval_always,
+                    $no_force,
                 )
             ),*
         ];

compiler/rustc_query_impl/src/execution.rs

@@ -424,7 +424,7 @@ fn execute_job_incr<'tcx, C: QueryCache>(
     let dep_graph_data =
         tcx.dep_graph.data().expect("should always be present in incremental mode");
 
-    if !query.anon && !query.eval_always {
+    if !query.eval_always {
         // `to_dep_node` is expensive for some `DepKind`s.
         let dep_node =
             dep_node_opt.get_or_insert_with(|| DepNode::construct(tcx, query.dep_kind, &key));
@@ -451,13 +451,6 @@ fn execute_job_incr<'tcx, C: QueryCache>(
     let prof_timer = tcx.prof.query_provider();
 
     let (result, dep_node_index) = start_query(job_id, query.depth_limit, || {
-        if query.anon {
-            // Call the query provider inside an anon task.
-            return dep_graph_data.with_anon_task_inner(tcx, query.dep_kind, || {
-                (query.invoke_provider_fn)(tcx, key)
-            });
-        }
-
         // `to_dep_node` is expensive for some `DepKind`s.
         let dep_node =
             dep_node_opt.unwrap_or_else(|| DepNode::construct(tcx, query.dep_kind, &key));
@@ -601,9 +594,6 @@ fn check_if_ensure_can_skip_execution<'tcx, C: QueryCache>(
         return EnsureCanSkip { skip_execution: false, dep_node: None };
     }
 
-    // Ensuring an anonymous query makes no sense
-    assert!(!query.anon);
-
     let dep_node = DepNode::construct(tcx, query.dep_kind, &key);
 
     let serialized_dep_node_index = match tcx.dep_graph.try_mark_green(tcx, &dep_node) {
@@ -703,8 +693,6 @@ pub(crate) fn force_query<'tcx, C: QueryCache>(
         return;
     }
 
-    debug_assert!(!query.anon);
-
     ensure_sufficient_stack(|| {
         try_execute_query::<C, true>(query, tcx, DUMMY_SP, key, Some(dep_node))
     });

compiler/rustc_query_impl/src/query_impl.rs

@@ -16,12 +16,12 @@ macro_rules! define_queries {
                 fn $name:ident($K:ty) -> $V:ty
                 {
                     // Search for (QMODLIST) to find all occurrences of this query modifier list.
-                    anon: $anon:literal,
                     arena_cache: $arena_cache:literal,
                     cache_on_disk: $cache_on_disk:literal,
                     depth_limit: $depth_limit:literal,
                     eval_always: $eval_always:literal,
                     feedable: $feedable:literal,
+                    no_force: $no_force:literal,
                     no_hash: $no_hash:literal,
                     returns_error_guaranteed: $returns_error_guaranteed:literal,
                     separate_provide_extern: $separate_provide_extern:literal,
@@ -132,7 +132,6 @@ pub(crate) fn make_query_vtable<'tcx>(incremental: bool)
 
                     QueryVTable {
                         name: stringify!($name),
-                        anon: $anon,
                         eval_always: $eval_always,
                         depth_limit: $depth_limit,
                         feedable: $feedable,

compiler/rustc_ty_utils/src/representability.rs

@@ -19,7 +19,7 @@ fn check_representability(tcx: TyCtxt<'_>, def_id: LocalDefId) {
         DefKind::Struct | DefKind::Union | DefKind::Enum => {
             for variant in tcx.adt_def(def_id).variants() {
                 for field in variant.fields.iter() {
-                    let _ = tcx.check_representability(field.did.expect_local());
+                    tcx.ensure_ok().check_representability(field.did.expect_local());
                 }
             }
         }
@@ -35,7 +35,7 @@ fn check_representability_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) {
         // This one must be a query rather than a vanilla `check_representability_adt_ty` call. See
         // the comment on `check_representability_adt_ty` below for why.
         ty::Adt(..) => {
-            let _ = tcx.check_representability_adt_ty(ty);
+            tcx.ensure_ok().check_representability_adt_ty(ty);
         }
         // FIXME(#11924) allow zero-length arrays?
         ty::Array(ty, _) => {
@@ -69,7 +69,7 @@ fn check_representability_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) {
 fn check_representability_adt_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) {
     let ty::Adt(adt, args) = ty.kind() else { bug!("expected adt") };
     if let Some(def_id) = adt.did().as_local() {
-        let _ = tcx.check_representability(def_id);
+        tcx.ensure_ok().check_representability(def_id);
     }
     // At this point, we know that the item of the ADT type is representable;
     // but the type parameters may cause a cycle with an upstream type

compiler/rustc_ty_utils/src/ty.rs

@@ -117,7 +117,7 @@ fn adt_sizedness_constraint<'tcx>(
     (def_id, sizedness): (DefId, SizedTraitKind),
 ) -> Option<ty::EarlyBinder<'tcx, Ty<'tcx>>> {
     if let Some(def_id) = def_id.as_local() {
-        let _ = tcx.check_representability(def_id);
+        tcx.ensure_ok().check_representability(def_id);
     }
 
     let def = tcx.adt_def(def_id);

src/doc/rustc-dev-guide/src/queries/incremental-compilation-in-detail.md

@@ -418,6 +418,13 @@ deal with all of the above but so far that seemed like more trouble than it woul
 
 ## Query modifiers
 
+> FIXME: Make [`rustc_middle::query::modifiers`] the home for query modifier documentation,
+> and migrate all other useful modifier docs there after verifying that they are still accurate.
+
+[`rustc_middle::query::modifiers`]:
+    https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/query/modifiers/index.html
+
+
 The query system allows for applying [modifiers][mod] to queries.
 These modifiers affect certain aspects of how the system treats the query with
 respect to incremental compilation:
@@ -437,6 +444,9 @@ respect to incremental compilation:
       as an optimization because the system can skip recording dependencies in
       the first place.
 
+ - `no_force` - Never "force" the dep nodes for this query, even if the query's
+   key type is recoverable.
+
  - `no_hash` - Applying `no_hash` to a query tells the system to not compute
    the fingerprint of the query's result.
    This has two consequences:
@@ -475,13 +485,6 @@ respect to incremental compilation:
    For example, it makes no sense to store values from upstream
    crates in the cache because they are already available in the upstream crate's metadata.
 
- - `anon` - This attribute makes the system use "anonymous" dep-nodes for the given query.
-   An anonymous dep-node is not identified by the corresponding query key.
-   Instead, its ID is computed from the IDs of its dependencies.
-   This allows the red-green system to do its change detection even if there is no
-   query key available for a given dep-node -- something which is needed for
-   handling trait selection because it is not based on queries.
-
 [mod]: ../query.html#adding-a-new-kind-of-query