Address PR comments

- Remove `fn_sig()` from Instance.
- Change return value of `AssertMessage::description` to `Cow<>`.
- Add assert to instance `ty()`.
- Generalize uint / int type creation.

Author: celinval

compiler/rustc_smir/src/rustc_smir/context.rs

@@ -5,16 +5,18 @@
 
 use rustc_middle::ty;
 use rustc_middle::ty::print::{with_forced_trimmed_paths, with_no_trimmed_paths};
-use rustc_middle::ty::{GenericPredicates, Instance, ParamEnv, ScalarInt, ValTree};
+use rustc_middle::ty::{
+    GenericPredicates, Instance, ParamEnv, ScalarInt, TypeVisitableExt, ValTree,
+};
 use rustc_span::def_id::LOCAL_CRATE;
 use stable_mir::compiler_interface::Context;
 use stable_mir::mir::alloc::GlobalAlloc;
 use stable_mir::mir::mono::{InstanceDef, StaticDef};
 use stable_mir::mir::Body;
 use stable_mir::target::{MachineInfo, MachineSize};
 use stable_mir::ty::{
-    AdtDef, AdtKind, Allocation, ClosureDef, ClosureKind, Const, FieldDef, FnDef, FnSig,
-    GenericArgs, LineInfo, PolyFnSig, RigidTy, Span, Ty, TyKind, VariantDef,
+    AdtDef, AdtKind, Allocation, ClosureDef, ClosureKind, Const, FieldDef, FnDef, GenericArgs,
+    LineInfo, PolyFnSig, RigidTy, Span, Ty, TyKind, VariantDef,
 };
 use stable_mir::{Crate, CrateItem, DefId, Error, Filename, ItemKind, Symbol};
 use std::cell::RefCell;
@@ -324,18 +326,10 @@ impl<'tcx> Context for TablesWrapper<'tcx> {
     fn instance_ty(&self, def: InstanceDef) -> stable_mir::ty::Ty {
         let mut tables = self.0.borrow_mut();
         let instance = tables.instances[def];
+        assert!(!instance.has_non_region_param(), "{instance:?} needs further substitution");
         instance.ty(tables.tcx, ParamEnv::reveal_all()).stable(&mut *tables)
     }
 
-    fn instance_sig(&self, def: InstanceDef) -> FnSig {
-        let mut tables = self.0.borrow_mut();
-        let instance = tables.instances[def];
-        let ty = instance.ty(tables.tcx, ParamEnv::reveal_all());
-        let sig = if ty.is_fn() { ty.fn_sig(tables.tcx) } else { instance.args.as_closure().sig() };
-        // Erase late bound regions.
-        tables.tcx.instantiate_bound_regions_with_erased(sig).stable(&mut *tables)
-    }
-
     fn instance_def_id(&self, def: InstanceDef) -> stable_mir::DefId {
         let mut tables = self.0.borrow_mut();
         let def_id = tables.instances[def].def_id();

compiler/stable_mir/src/compiler_interface.rs

@@ -10,9 +10,9 @@ use crate::mir::mono::{Instance, InstanceDef, StaticDef};
 use crate::mir::Body;
 use crate::target::MachineInfo;
 use crate::ty::{
-    AdtDef, AdtKind, Allocation, ClosureDef, ClosureKind, Const, FieldDef, FnDef, FnSig,
-    GenericArgs, GenericPredicates, Generics, ImplDef, ImplTrait, LineInfo, PolyFnSig, RigidTy,
-    Span, TraitDecl, TraitDef, Ty, TyKind, VariantDef,
+    AdtDef, AdtKind, Allocation, ClosureDef, ClosureKind, Const, FieldDef, FnDef, GenericArgs,
+    GenericPredicates, Generics, ImplDef, ImplTrait, LineInfo, PolyFnSig, RigidTy, Span, TraitDecl,
+    TraitDef, Ty, TyKind, VariantDef,
 };
 use crate::{
     mir, Crate, CrateItem, CrateItems, DefId, Error, Filename, ImplTraitDecls, ItemKind, Symbol,
@@ -121,9 +121,6 @@ pub trait Context {
     /// Get the instance type with generic substitutions applied and lifetimes erased.
     fn instance_ty(&self, instance: InstanceDef) -> Ty;
 
-    /// Get the instance signature with .
-    fn instance_sig(&self, def: InstanceDef) -> FnSig;
-
     /// Get the instance.
     fn instance_def_id(&self, instance: InstanceDef) -> DefId;
 

compiler/stable_mir/src/mir/body.rs

@@ -4,6 +4,7 @@ use crate::ty::{
     VariantIdx,
 };
 use crate::{Error, Opaque, Span, Symbol};
+use std::borrow::Cow;
 use std::io;
 /// The SMIR representation of a single function.
 #[derive(Clone, Debug)]
@@ -264,51 +265,63 @@ pub enum AssertMessage {
 }
 
 impl AssertMessage {
-    pub fn description(&self) -> Result<&'static str, Error> {
+    pub fn description(&self) -> Result<Cow<'static, str>, Error> {
         match self {
-            AssertMessage::Overflow(BinOp::Add, _, _) => Ok("attempt to add with overflow"),
-            AssertMessage::Overflow(BinOp::Sub, _, _) => Ok("attempt to subtract with overflow"),
-            AssertMessage::Overflow(BinOp::Mul, _, _) => Ok("attempt to multiply with overflow"),
-            AssertMessage::Overflow(BinOp::Div, _, _) => Ok("attempt to divide with overflow"),
+            AssertMessage::Overflow(BinOp::Add, _, _) => Ok("attempt to add with overflow".into()),
+            AssertMessage::Overflow(BinOp::Sub, _, _) => {
+                Ok("attempt to subtract with overflow".into())
+            }
+            AssertMessage::Overflow(BinOp::Mul, _, _) => {
+                Ok("attempt to multiply with overflow".into())
+            }
+            AssertMessage::Overflow(BinOp::Div, _, _) => {
+                Ok("attempt to divide with overflow".into())
+            }
             AssertMessage::Overflow(BinOp::Rem, _, _) => {
-                Ok("attempt to calculate the remainder with overflow")
+                Ok("attempt to calculate the remainder with overflow".into())
+            }
+            AssertMessage::OverflowNeg(_) => Ok("attempt to negate with overflow".into()),
+            AssertMessage::Overflow(BinOp::Shr, _, _) => {
+                Ok("attempt to shift right with overflow".into())
+            }
+            AssertMessage::Overflow(BinOp::Shl, _, _) => {
+                Ok("attempt to shift left with overflow".into())
             }
-            AssertMessage::OverflowNeg(_) => Ok("attempt to negate with overflow"),
-            AssertMessage::Overflow(BinOp::Shr, _, _) => Ok("attempt to shift right with overflow"),
-            AssertMessage::Overflow(BinOp::Shl, _, _) => Ok("attempt to shift left with overflow"),
             AssertMessage::Overflow(op, _, _) => Err(error!("`{:?}` cannot overflow", op)),
-            AssertMessage::DivisionByZero(_) => Ok("attempt to divide by zero"),
+            AssertMessage::DivisionByZero(_) => Ok("attempt to divide by zero".into()),
             AssertMessage::RemainderByZero(_) => {
-                Ok("attempt to calculate the remainder with a divisor of zero")
+                Ok("attempt to calculate the remainder with a divisor of zero".into())
             }
             AssertMessage::ResumedAfterReturn(CoroutineKind::Coroutine) => {
-                Ok("coroutine resumed after completion")
+                Ok("coroutine resumed after completion".into())
             }
             AssertMessage::ResumedAfterReturn(CoroutineKind::Async(_)) => {
-                Ok("`async fn` resumed after completion")
+                Ok("`async fn` resumed after completion".into())
             }
             AssertMessage::ResumedAfterReturn(CoroutineKind::Gen(_)) => {
-                Ok("`async gen fn` resumed after completion")
+                Ok("`async gen fn` resumed after completion".into())
             }
             AssertMessage::ResumedAfterReturn(CoroutineKind::AsyncGen(_)) => {
-                Ok("`gen fn` should just keep returning `AssertMessage::None` after completion")
+                Ok("`gen fn` should just keep returning `AssertMessage::None` after completion"
+                    .into())
             }
             AssertMessage::ResumedAfterPanic(CoroutineKind::Coroutine) => {
-                Ok("coroutine resumed after panicking")
+                Ok("coroutine resumed after panicking".into())
             }
             AssertMessage::ResumedAfterPanic(CoroutineKind::Async(_)) => {
-                Ok("`async fn` resumed after panicking")
+                Ok("`async fn` resumed after panicking".into())
             }
             AssertMessage::ResumedAfterPanic(CoroutineKind::Gen(_)) => {
-                Ok("`async gen fn` resumed after panicking")
+                Ok("`async gen fn` resumed after panicking".into())
             }
             AssertMessage::ResumedAfterPanic(CoroutineKind::AsyncGen(_)) => {
-                Ok("`gen fn` should just keep returning `AssertMessage::None` after panicking")
+                Ok("`gen fn` should just keep returning `AssertMessage::None` after panicking"
+                    .into())
             }
 
-            AssertMessage::BoundsCheck { .. } => Ok("index out of bounds"),
+            AssertMessage::BoundsCheck { .. } => Ok("index out of bounds".into()),
             AssertMessage::MisalignedPointerDereference { .. } => {
-                Ok("misaligned pointer dereference")
+                Ok("misaligned pointer dereference".into())
             }
         }
     }

compiler/stable_mir/src/mir/mono.rs

@@ -1,6 +1,6 @@
 use crate::crate_def::CrateDef;
 use crate::mir::Body;
-use crate::ty::{Allocation, ClosureDef, ClosureKind, FnDef, FnSig, GenericArgs, IndexedVal, Ty};
+use crate::ty::{Allocation, ClosureDef, ClosureKind, FnDef, GenericArgs, IndexedVal, Ty};
 use crate::{with, CrateItem, DefId, Error, ItemKind, Opaque, Symbol};
 use std::fmt::{Debug, Formatter};
 
@@ -115,11 +115,6 @@ impl Instance {
         })
     }
 
-    /// Get this function signature with all types already instantiated.
-    pub fn fn_sig(&self) -> FnSig {
-        with(|cx| cx.instance_sig(self.def))
-    }
-
     /// Check whether this instance is an empty shim.
     ///
     /// Allow users to check if this shim can be ignored when called directly.

compiler/stable_mir/src/ty.rs

@@ -6,7 +6,6 @@ use super::{
 use crate::crate_def::CrateDef;
 use crate::mir::alloc::{read_target_int, read_target_uint, AllocId};
 use crate::target::MachineInfo;
-use crate::ty::UintTy::U8;
 use crate::{Filename, Opaque};
 use std::fmt::{self, Debug, Display, Formatter};
 use std::ops::Range;
@@ -77,9 +76,14 @@ impl Ty {
         Ty::from_rigid_kind(RigidTy::Bool)
     }
 
-    /// Create a type representing `u8`.
-    pub fn u8_ty() -> Ty {
-        Ty::from_rigid_kind(RigidTy::Uint(U8))
+    /// Create a type representing a signed integer.
+    pub fn signed_ty(inner: IntTy) -> Ty {
+        Ty::from_rigid_kind(RigidTy::Int(inner))
+    }
+
+    /// Create a type representing an unsigned integer.
+    pub fn unsigned_ty(inner: UintTy) -> Ty {
+        Ty::from_rigid_kind(RigidTy::Uint(inner))
     }
 }
 

tests/ui-fulldeps/stable-mir/check_defs.rs

@@ -69,7 +69,7 @@ fn extract_elem_ty(ty: Ty) -> Ty {
 
 /// Check signature and type of `Vec::<u8>::new` and its generic version.
 fn test_vec_new(instance: mir::mono::Instance) {
-    let sig = instance.fn_sig();
+    let sig = instance.ty().kind().fn_sig().unwrap().skip_binder();
     assert_matches!(sig.inputs(), &[]);
     let elem_ty = extract_elem_ty(sig.output());
     assert_matches!(elem_ty.kind(), TyKind::RigidTy(RigidTy::Uint(UintTy::U8)));