Make trait_environment() take GenericDefId and not ExpressionStoreOwnedId

A `ParamEnv` is fundamentally associated with generic arguments and not with a body. It feels wrong to attach it to `ExpressionStoreOwnedId`.

There is one exception: a defaulted trait method with RPITIT has a different ParamEnv inside its body than for its callers, since inside the body the RPITIT is interpreted as an RPIT equating the internal assoc type. We don't yet handle RPITIT, but even when we'll do I don't feel it's justifies treating *any* body as different.

Author: ChayimFriedman2

crates/hir-def/src/lib.rs

@@ -778,12 +778,12 @@ impl From<EnumVariantId> for DefWithBodyId {
 }
 
 impl DefWithBodyId {
-    pub fn as_generic_def_id(self, db: &dyn DefDatabase) -> Option<GenericDefId> {
+    pub fn generic_def(self, db: &dyn DefDatabase) -> GenericDefId {
         match self {
-            DefWithBodyId::FunctionId(f) => Some(f.into()),
-            DefWithBodyId::StaticId(s) => Some(s.into()),
-            DefWithBodyId::ConstId(c) => Some(c.into()),
-            DefWithBodyId::VariantId(c) => Some(c.lookup(db).parent.into()),
+            DefWithBodyId::FunctionId(f) => f.into(),
+            DefWithBodyId::StaticId(s) => s.into(),
+            DefWithBodyId::ConstId(c) => c.into(),
+            DefWithBodyId::VariantId(c) => c.lookup(db).parent.into(),
         }
     }
 }

crates/hir-ty/src/consteval.rs

@@ -5,8 +5,7 @@ mod tests;
 
 use base_db::Crate;
 use hir_def::{
-    ConstId, EnumVariantId, ExpressionStoreOwnerId, GeneralConstId, GenericDefId, HasModule,
-    StaticId,
+    ConstId, EnumVariantId, GeneralConstId, HasModule, StaticId,
     attrs::AttrFlags,
     expr_store::{Body, ExpressionStore},
     hir::{Expr, ExprId, Literal},
@@ -259,8 +258,11 @@ pub(crate) fn const_eval_discriminant_variant(
     let mir_body = db.monomorphized_mir_body(
         def,
         GenericArgs::empty(interner).store(),
-        ParamEnvAndCrate { param_env: db.trait_environment(def.into()), krate: def.krate(db) }
-            .store(),
+        ParamEnvAndCrate {
+            param_env: db.trait_environment(def.generic_def(db)),
+            krate: def.krate(db),
+        }
+        .store(),
     )?;
     let c = interpret_mir(db, mir_body, false, None)?.0?;
     let c = if is_signed { allocation_as_isize(c) } else { allocation_as_usize(c) as i128 };
@@ -339,12 +341,8 @@ pub(crate) fn const_eval<'db>(
         let body = db.monomorphized_mir_body(
             def.into(),
             subst,
-            ParamEnvAndCrate {
-                param_env: db
-                    .trait_environment(ExpressionStoreOwnerId::from(GenericDefId::from(def))),
-                krate: def.krate(db),
-            }
-            .store(),
+            ParamEnvAndCrate { param_env: db.trait_environment(def.into()), krate: def.krate(db) }
+                .store(),
         )?;
         let c = interpret_mir(db, body, false, trait_env.as_ref().map(|env| env.as_ref()))?.0?;
         Ok(c.store())
@@ -379,12 +377,8 @@ pub(crate) fn const_eval_static<'db>(
         let body = db.monomorphized_mir_body(
             def.into(),
             GenericArgs::empty(interner).store(),
-            ParamEnvAndCrate {
-                param_env: db
-                    .trait_environment(ExpressionStoreOwnerId::from(GenericDefId::from(def))),
-                krate: def.krate(db),
-            }
-            .store(),
+            ParamEnvAndCrate { param_env: db.trait_environment(def.into()), krate: def.krate(db) }
+                .store(),
         )?;
         let c = interpret_mir(db, body, false, None)?.0?;
         Ok(c.store())

crates/hir-ty/src/db.rs

@@ -197,7 +197,7 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug {
 
     #[salsa::invoke(crate::lower::trait_environment)]
     #[salsa::transparent]
-    fn trait_environment<'db>(&'db self, def: ExpressionStoreOwnerId) -> ParamEnv<'db>;
+    fn trait_environment<'db>(&'db self, def: GenericDefId) -> ParamEnv<'db>;
 
     #[salsa::invoke(crate::lower::generic_defaults_with_diagnostics_query)]
     #[salsa::cycle(cycle_result = crate::lower::generic_defaults_with_diagnostics_cycle_result)]

crates/hir-ty/src/diagnostics/expr.rs

@@ -83,7 +83,7 @@ impl BodyValidationDiagnostic {
         let _p = tracing::info_span!("BodyValidationDiagnostic::collect").entered();
         let infer = InferenceResult::of(db, owner);
         let body = Body::of(db, owner);
-        let env = db.trait_environment(owner.into());
+        let env = db.trait_environment(owner.generic_def(db));
         let interner = DbInterner::new_with(db, owner.krate(db));
         let infcx =
             interner.infer_ctxt().build(TypingMode::typeck_for_body(interner, owner.into()));

crates/hir-ty/src/display.rs

@@ -10,8 +10,8 @@ use std::{
 use base_db::{Crate, FxIndexMap};
 use either::Either;
 use hir_def::{
-    ExpressionStoreOwnerId, FindPathConfig, GenericDefId, GenericParamId, HasModule,
-    ItemContainerId, LocalFieldId, Lookup, ModuleDefId, ModuleId, TraitId,
+    FindPathConfig, GenericDefId, GenericParamId, HasModule, ItemContainerId, LocalFieldId, Lookup,
+    ModuleDefId, ModuleId, TraitId,
     expr_store::{ExpressionStore, path::Path},
     find_path::{self, PrefixKind},
     hir::{
@@ -953,9 +953,7 @@ fn render_const_scalar_inner<'db>(
                         s.fields(f.db),
                         f,
                         field_types,
-                        f.db.trait_environment(ExpressionStoreOwnerId::from(GenericDefId::from(
-                            def,
-                        ))),
+                        f.db.trait_environment(def.into()),
                         &layout,
                         args,
                         b,
@@ -987,9 +985,7 @@ fn render_const_scalar_inner<'db>(
                         var_id.fields(f.db),
                         f,
                         field_types,
-                        f.db.trait_environment(ExpressionStoreOwnerId::from(GenericDefId::from(
-                            def,
-                        ))),
+                        f.db.trait_environment(def.into()),
                         var_layout,
                         args,
                         b,

crates/hir-ty/src/infer.rs

@@ -1298,17 +1298,8 @@ impl<'body, 'db> InferenceContext<'body, 'db> {
         store: &'body ExpressionStore,
         resolver: Resolver<'db>,
     ) -> Self {
-        let trait_env = match owner {
-            ExpressionStoreOwnerId::Signature(generic_def_id) => {
-                db.trait_environment(ExpressionStoreOwnerId::from(generic_def_id))
-            }
-            ExpressionStoreOwnerId::Body(def_with_body_id) => {
-                db.trait_environment(ExpressionStoreOwnerId::Body(def_with_body_id))
-            }
-            ExpressionStoreOwnerId::VariantFields(variant_id) => {
-                db.trait_environment(ExpressionStoreOwnerId::VariantFields(variant_id))
-            }
-        };
+        let generic_def = owner.generic_def(db);
+        let trait_env = db.trait_environment(generic_def);
         let table = unify::InferenceTable::new(db, trait_env, resolver.krate(), owner);
         let types = crate::next_solver::default_types(db);
         InferenceContext {
@@ -1325,7 +1316,7 @@ impl<'body, 'db> InferenceContext<'body, 'db> {
             return_coercion: None,
             db,
             owner,
-            generic_def: owner.generic_def(db),
+            generic_def,
             store,
             traits_in_scope: resolver.traits_in_scope(db),
             resolver,

crates/hir-ty/src/layout/tests.rs

@@ -1,7 +1,7 @@
 use base_db::target::TargetData;
 use either::Either;
 use hir_def::{
-    DefWithBodyId, ExpressionStoreOwnerId, GenericDefId, HasModule,
+    DefWithBodyId, HasModule,
     expr_store::Body,
     signatures::{
         EnumSignature, FunctionSignature, StructSignature, TypeAliasSignature, UnionSignature,
@@ -92,13 +92,10 @@ fn eval_goal(
             ),
             Either::Right(ty_id) => db.ty(ty_id.into()).instantiate_identity(),
         };
-        let param_env = db.trait_environment(
-            match adt_or_type_alias_id {
-                Either::Left(adt) => hir_def::GenericDefId::AdtId(adt),
-                Either::Right(ty) => hir_def::GenericDefId::TypeAliasId(ty),
-            }
-            .into(),
-        );
+        let param_env = db.trait_environment(match adt_or_type_alias_id {
+            Either::Left(adt) => hir_def::GenericDefId::AdtId(adt),
+            Either::Right(ty) => hir_def::GenericDefId::TypeAliasId(ty),
+        });
         let krate = match adt_or_type_alias_id {
             Either::Left(it) => it.krate(&db),
             Either::Right(it) => it.krate(&db),
@@ -145,8 +142,7 @@ fn eval_expr(
             .0;
         let infer = InferenceResult::of(&db, DefWithBodyId::from(function_id));
         let goal_ty = infer.type_of_binding[b].clone();
-        let param_env =
-            db.trait_environment(ExpressionStoreOwnerId::from(GenericDefId::from(function_id)));
+        let param_env = db.trait_environment(function_id.into());
         let krate = function_id.krate(&db);
         db.layout_of_ty(goal_ty, ParamEnvAndCrate { param_env, krate }.store())
     })

crates/hir-ty/src/lib.rs

@@ -62,8 +62,7 @@ mod tests;
 use std::{hash::Hash, ops::ControlFlow};
 
 use hir_def::{
-    CallableDefId, ExpressionStoreOwnerId, GenericDefId, LifetimeParamId, TypeAliasId,
-    TypeOrConstParamId, TypeParamId,
+    CallableDefId, GenericDefId, LifetimeParamId, TypeAliasId, TypeOrConstParamId, TypeParamId,
     hir::{BindingId, ExprId, ExprOrPatId, PatId},
     resolver::TypeNs,
     type_ref::{Rawness, TypeRefId},
@@ -511,7 +510,7 @@ pub fn associated_type_shorthand_candidates(
     let mut dedup_map = FxHashSet::default();
     let param_ty = Ty::new_param(interner, param, type_or_const_param_idx(db, param.into()));
     // We use the ParamEnv and not the predicates because the ParamEnv elaborates bounds.
-    let param_env = db.trait_environment(ExpressionStoreOwnerId::from(def));
+    let param_env = db.trait_environment(def);
     for clause in param_env.clauses {
         let ClauseKind::Trait(trait_clause) = clause.kind().skip_binder() else { continue };
         if trait_clause.self_ty() != param_ty {

crates/hir-ty/src/lower.rs

@@ -12,10 +12,10 @@ use std::{cell::OnceCell, iter, mem};
 
 use either::Either;
 use hir_def::{
-    AdtId, AssocItemId, CallableDefId, ConstId, ConstParamId, EnumId, EnumVariantId,
-    ExpressionStoreOwnerId, FunctionId, GeneralConstId, GenericDefId, GenericParamId, HasModule,
-    ImplId, ItemContainerId, LifetimeParamId, LocalFieldId, Lookup, StaticId, StructId, TraitId,
-    TypeAliasId, TypeOrConstParamId, TypeParamId, UnionId, VariantId,
+    AdtId, AssocItemId, CallableDefId, ConstId, ConstParamId, EnumId, EnumVariantId, FunctionId,
+    GeneralConstId, GenericDefId, GenericParamId, HasModule, ImplId, ItemContainerId,
+    LifetimeParamId, LocalFieldId, Lookup, StaticId, StructId, TraitId, TypeAliasId,
+    TypeOrConstParamId, TypeParamId, UnionId, VariantId,
     builtin_type::BuiltinType,
     expr_store::{ExpressionStore, HygieneId, path::Path},
     hir::generics::{
@@ -2148,12 +2148,7 @@ pub(crate) fn param_env_from_predicates<'db>(
     ParamEnv { clauses }
 }
 
-pub(crate) fn trait_environment<'db>(
-    db: &'db dyn HirDatabase,
-    def: ExpressionStoreOwnerId,
-) -> ParamEnv<'db> {
-    let def = def.generic_def(db);
-
+pub(crate) fn trait_environment<'db>(db: &'db dyn HirDatabase, def: GenericDefId) -> ParamEnv<'db> {
     return ParamEnv { clauses: trait_environment_query(db, def).as_ref() };
 
     #[salsa::tracked(returns(ref))]

crates/hir-ty/src/mir/borrowck.rs

@@ -6,7 +6,7 @@
 use std::iter;
 
 use either::Either;
-use hir_def::{DefWithBodyId, ExpressionStoreOwnerId, HasModule};
+use hir_def::{DefWithBodyId, HasModule};
 use la_arena::ArenaMap;
 use rustc_hash::FxHashMap;
 use stdx::never;
@@ -140,7 +140,7 @@ pub fn borrowck_query(
     let _p = tracing::info_span!("borrowck_query").entered();
     let module = def.module(db);
     let interner = DbInterner::new_with(db, module.krate(db));
-    let env = db.trait_environment(ExpressionStoreOwnerId::from(def));
+    let env = db.trait_environment(def.generic_def(db));
     // This calculates opaques defining scope which is a bit costly therefore is put outside `all_mir_bodies()`.
     let typing_mode = TypingMode::borrowck(interner, def.into());
     let res = all_mir_bodies(