Merge #981

981: Remove FnSig from FnDef type r=matklad a=flodiebold

It doesn't need to be in there since it's just information from the def. Another
step towards aligning Ty with Chalk's representation.

Co-authored-by: Florian Diebold <flodiebold@gmail.com>

crates/ra_hir/src/db.rs

@@ -12,7 +12,7 @@ use crate::{
     macros::MacroExpansion,
     module_tree::ModuleTree,
     nameres::{ItemMap, lower::{LoweredModule, ImportSourceMap}},
-    ty::{InferenceResult, Ty, method_resolution::CrateImplBlocks, TypableDef},
+    ty::{InferenceResult, Ty, method_resolution::CrateImplBlocks, TypableDef, CallableDef, FnSig},
     adt::{StructData, EnumData},
     impl_block::{ModuleImplBlocks, ImplSourceMap},
     generics::{GenericParams, GenericDef},
@@ -105,6 +105,9 @@ pub trait HirDatabase: PersistentHirDatabase {
     #[salsa::invoke(crate::ty::type_for_field)]
     fn type_for_field(&self, field: StructField) -> Ty;
 
+    #[salsa::invoke(crate::ty::callable_item_sig)]
+    fn callable_item_signature(&self, def: CallableDef) -> FnSig;
+
     #[salsa::invoke(crate::expr::body_with_source_map_query)]
     fn body_with_source_map(
         &self,

crates/ra_hir/src/ty.rs

@@ -16,7 +16,7 @@ use std::{fmt, mem};
 
 use crate::{Name, AdtDef, type_ref::Mutability, db::HirDatabase};
 
-pub(crate) use lower::{TypableDef, CallableDef, type_for_def, type_for_field};
+pub(crate) use lower::{TypableDef, CallableDef, type_for_def, type_for_field, callable_item_sig};
 pub(crate) use infer::{infer, InferenceResult, InferTy};
 use display::{HirDisplay, HirFormatter};
 
@@ -77,8 +77,6 @@ pub enum Ty {
     FnDef {
         /// The definition of the function / constructor.
         def: CallableDef,
-        /// Parameters and return type
-        sig: Arc<FnSig>,
         /// Substitutions for the generic parameters of the type
         substs: Substs,
     },
@@ -91,7 +89,7 @@ pub enum Ty {
     /// fn foo() -> i32 { 1 }
     /// let bar: fn() -> i32 = foo;
     /// ```
-    FnPtr(Arc<FnSig>),
+    FnPtr(FnSig),
 
     /// The never type `!`.
     Never,
@@ -128,13 +126,44 @@ impl Substs {
     pub fn empty() -> Substs {
         Substs(Arc::new([]))
     }
+
+    pub fn walk_mut(&mut self, f: &mut impl FnMut(&mut Ty)) {
+        // Without an Arc::make_mut_slice, we can't avoid the clone here:
+        let mut v: Vec<_> = self.0.iter().cloned().collect();
+        for t in &mut v {
+            t.walk_mut(f);
+        }
+        self.0 = v.into();
+    }
 }
 
 /// A function signature.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct FnSig {
-    input: Vec<Ty>,
-    output: Ty,
+    params_and_return: Arc<[Ty]>,
+}
+
+impl FnSig {
+    pub fn from_params_and_return(mut params: Vec<Ty>, ret: Ty) -> FnSig {
+        params.push(ret);
+        FnSig { params_and_return: params.into() }
+    }
+    pub fn params(&self) -> &[Ty] {
+        &self.params_and_return[0..self.params_and_return.len() - 1]
+    }
+
+    pub fn ret(&self) -> &Ty {
+        &self.params_and_return[self.params_and_return.len() - 1]
+    }
+
+    pub fn walk_mut(&mut self, f: &mut impl FnMut(&mut Ty)) {
+        // Without an Arc::make_mut_slice, we can't avoid the clone here:
+        let mut v: Vec<_> = self.params_and_return.iter().cloned().collect();
+        for t in &mut v {
+            t.walk_mut(f);
+        }
+        self.params_and_return = v.into();
+    }
 }
 
 impl Ty {
@@ -153,16 +182,12 @@ impl Ty {
                 }
             }
             Ty::FnPtr(sig) => {
-                for input in &sig.input {
+                for input in sig.params() {
                     input.walk(f);
                 }
-                sig.output.walk(f);
+                sig.ret().walk(f);
             }
-            Ty::FnDef { substs, sig, .. } => {
-                for input in &sig.input {
-                    input.walk(f);
-                }
-                sig.output.walk(f);
+            Ty::FnDef { substs, .. } => {
                 for t in substs.0.iter() {
                     t.walk(f);
                 }
@@ -199,32 +224,13 @@ impl Ty {
                 *ts = v.into();
             }
             Ty::FnPtr(sig) => {
-                let sig_mut = Arc::make_mut(sig);
-                for input in &mut sig_mut.input {
-                    input.walk_mut(f);
-                }
-                sig_mut.output.walk_mut(f);
+                sig.walk_mut(f);
             }
-            Ty::FnDef { substs, sig, .. } => {
-                let sig_mut = Arc::make_mut(sig);
-                for input in &mut sig_mut.input {
-                    input.walk_mut(f);
-                }
-                sig_mut.output.walk_mut(f);
-                // Without an Arc::make_mut_slice, we can't avoid the clone here:
-                let mut v: Vec<_> = substs.0.iter().cloned().collect();
-                for t in &mut v {
-                    t.walk_mut(f);
-                }
-                substs.0 = v.into();
+            Ty::FnDef { substs, .. } => {
+                substs.walk_mut(f);
             }
             Ty::Adt { substs, .. } => {
-                // Without an Arc::make_mut_slice, we can't avoid the clone here:
-                let mut v: Vec<_> = substs.0.iter().cloned().collect();
-                for t in &mut v {
-                    t.walk_mut(f);
-                }
-                substs.0 = v.into();
+                substs.walk_mut(f);
             }
             Ty::Bool
             | Ty::Char
@@ -262,7 +268,7 @@ impl Ty {
     pub fn apply_substs(self, substs: Substs) -> Ty {
         match self {
             Ty::Adt { def_id, .. } => Ty::Adt { def_id, substs },
-            Ty::FnDef { def, sig, .. } => Ty::FnDef { def, sig, substs },
+            Ty::FnDef { def, .. } => Ty::FnDef { def, substs },
             _ => self,
         }
     }
@@ -304,8 +310,8 @@ impl HirDisplay for Ty {
         match self {
             Ty::Bool => write!(f, "bool")?,
             Ty::Char => write!(f, "char")?,
-            Ty::Int(t) => write!(f, "{}", t.ty_to_string())?,
-            Ty::Float(t) => write!(f, "{}", t.ty_to_string())?,
+            Ty::Int(t) => write!(f, "{}", t)?,
+            Ty::Float(t) => write!(f, "{}", t)?,
             Ty::Str => write!(f, "str")?,
             Ty::Slice(t) | Ty::Array(t) => {
                 write!(f, "[{}]", t.display(f.db))?;
@@ -328,10 +334,11 @@ impl HirDisplay for Ty {
             }
             Ty::FnPtr(sig) => {
                 write!(f, "fn(")?;
-                f.write_joined(&sig.input, ", ")?;
-                write!(f, ") -> {}", sig.output.display(f.db))?;
+                f.write_joined(sig.params(), ", ")?;
+                write!(f, ") -> {}", sig.ret().display(f.db))?;
             }
-            Ty::FnDef { def, substs, sig, .. } => {
+            Ty::FnDef { def, substs, .. } => {
+                let sig = f.db.callable_item_signature(*def);
                 let name = match def {
                     CallableDef::Function(ff) => ff.name(f.db),
                     CallableDef::Struct(s) => s.name(f.db).unwrap_or_else(Name::missing),
@@ -347,8 +354,8 @@ impl HirDisplay for Ty {
                     write!(f, ">")?;
                 }
                 write!(f, "(")?;
-                f.write_joined(&sig.input, ", ")?;
-                write!(f, ") -> {}", sig.output.display(f.db))?;
+                f.write_joined(sig.params(), ", ")?;
+                write!(f, ") -> {}", sig.ret().display(f.db))?;
             }
             Ty::Adt { def_id, substs, .. } => {
                 let name = match def_id {

crates/ra_hir/src/ty/infer.rs

@@ -724,11 +724,12 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
             Expr::Call { callee, args } => {
                 let callee_ty = self.infer_expr(*callee, &Expectation::none());
                 let (param_tys, ret_ty) = match &callee_ty {
-                    Ty::FnPtr(sig) => (sig.input.clone(), sig.output.clone()),
-                    Ty::FnDef { substs, sig, .. } => {
-                        let ret_ty = sig.output.clone().subst(&substs);
+                    Ty::FnPtr(sig) => (sig.params().to_vec(), sig.ret().clone()),
+                    Ty::FnDef { substs, def, .. } => {
+                        let sig = self.db.callable_item_signature(*def);
+                        let ret_ty = sig.ret().clone().subst(&substs);
                         let param_tys =
-                            sig.input.iter().map(|ty| ty.clone().subst(&substs)).collect();
+                            sig.params().iter().map(|ty| ty.clone().subst(&substs)).collect();
                         (param_tys, ret_ty)
                     }
                     _ => {
@@ -762,19 +763,21 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
                 let method_ty = self.insert_type_vars(method_ty);
                 let (expected_receiver_ty, param_tys, ret_ty) = match &method_ty {
                     Ty::FnPtr(sig) => {
-                        if !sig.input.is_empty() {
-                            (sig.input[0].clone(), sig.input[1..].to_vec(), sig.output.clone())
+                        if !sig.params().is_empty() {
+                            (sig.params()[0].clone(), sig.params()[1..].to_vec(), sig.ret().clone())
                         } else {
-                            (Ty::Unknown, Vec::new(), sig.output.clone())
+                            (Ty::Unknown, Vec::new(), sig.ret().clone())
                         }
                     }
-                    Ty::FnDef { substs, sig, .. } => {
-                        let ret_ty = sig.output.clone().subst(&substs);
+                    Ty::FnDef { substs, def, .. } => {
+                        let sig = self.db.callable_item_signature(*def);
+                        let ret_ty = sig.ret().clone().subst(&substs);
 
-                        if !sig.input.is_empty() {
-                            let mut arg_iter = sig.input.iter().map(|ty| ty.clone().subst(&substs));
-                            let receiver_ty = arg_iter.next().unwrap();
-                            (receiver_ty, arg_iter.collect(), ret_ty)
+                        if !sig.params().is_empty() {
+                            let mut params_iter =
+                                sig.params().iter().map(|ty| ty.clone().subst(&substs));
+                            let receiver_ty = params_iter.next().unwrap();
+                            (receiver_ty, params_iter.collect(), ret_ty)
                         } else {
                             (Ty::Unknown, Vec::new(), ret_ty)
                         }

crates/ra_hir/src/ty/lower.rs

@@ -51,12 +51,10 @@ impl Ty {
             }
             TypeRef::Placeholder => Ty::Unknown,
             TypeRef::Fn(params) => {
-                let mut inner_tys =
+                let inner_tys =
                     params.iter().map(|tr| Ty::from_hir(db, resolver, tr)).collect::<Vec<_>>();
-                let return_ty =
-                    inner_tys.pop().expect("TypeRef::Fn should always have at least return type");
-                let sig = FnSig { input: inner_tys, output: return_ty };
-                Ty::FnPtr(Arc::new(sig))
+                let sig = FnSig { params_and_return: inner_tys.into() };
+                Ty::FnPtr(sig)
             }
             TypeRef::Error => Ty::Unknown,
         }
@@ -214,6 +212,15 @@ pub(crate) fn type_for_def(db: &impl HirDatabase, def: TypableDef, ns: Namespace
     }
 }
 
+/// Build the signature of a callable item (function, struct or enum variant).
+pub(crate) fn callable_item_sig(db: &impl HirDatabase, def: CallableDef) -> FnSig {
+    match def {
+        CallableDef::Function(f) => fn_sig_for_fn(db, f),
+        CallableDef::Struct(s) => fn_sig_for_struct_constructor(db, s),
+        CallableDef::EnumVariant(e) => fn_sig_for_enum_variant_constructor(db, e),
+    }
+}
+
 /// Build the type of a specific field of a struct or enum variant.
 pub(crate) fn type_for_field(db: &impl HirDatabase, field: StructField) -> Ty {
     let parent_def = field.parent_def(db);
@@ -226,18 +233,21 @@ pub(crate) fn type_for_field(db: &impl HirDatabase, field: StructField) -> Ty {
     Ty::from_hir(db, &resolver, type_ref)
 }
 
+fn fn_sig_for_fn(db: &impl HirDatabase, def: Function) -> FnSig {
+    let signature = def.signature(db);
+    let resolver = def.resolver(db);
+    let params =
+        signature.params().iter().map(|tr| Ty::from_hir(db, &resolver, tr)).collect::<Vec<_>>();
+    let ret = Ty::from_hir(db, &resolver, signature.ret_type());
+    FnSig::from_params_and_return(params, ret)
+}
+
 /// Build the declared type of a function. This should not need to look at the
 /// function body.
 fn type_for_fn(db: &impl HirDatabase, def: Function) -> Ty {
-    let signature = def.signature(db);
-    let resolver = def.resolver(db);
     let generics = def.generic_params(db);
-    let input =
-        signature.params().iter().map(|tr| Ty::from_hir(db, &resolver, tr)).collect::<Vec<_>>();
-    let output = Ty::from_hir(db, &resolver, signature.ret_type());
-    let sig = Arc::new(FnSig { input, output });
     let substs = make_substs(&generics);
-    Ty::FnDef { def: def.into(), sig, substs }
+    Ty::FnDef { def: def.into(), substs }
 }
 
 /// Build the declared type of a const.
@@ -256,42 +266,58 @@ fn type_for_static(db: &impl HirDatabase, def: Static) -> Ty {
     Ty::from_hir(db, &resolver, signature.type_ref())
 }
 
-/// Build the type of a tuple struct constructor.
-fn type_for_struct_constructor(db: &impl HirDatabase, def: Struct) -> Ty {
+fn fn_sig_for_struct_constructor(db: &impl HirDatabase, def: Struct) -> FnSig {
     let var_data = def.variant_data(db);
     let fields = match var_data.fields() {
         Some(fields) => fields,
-        None => return type_for_struct(db, def), // Unit struct
+        None => panic!("fn_sig_for_struct_constructor called on unit struct"),
     };
     let resolver = def.resolver(db);
-    let generics = def.generic_params(db);
-    let input = fields
+    let params = fields
         .iter()
         .map(|(_, field)| Ty::from_hir(db, &resolver, &field.type_ref))
         .collect::<Vec<_>>();
-    let output = type_for_struct(db, def);
-    let sig = Arc::new(FnSig { input, output });
+    let ret = type_for_struct(db, def);
+    FnSig::from_params_and_return(params, ret)
+}
+
+/// Build the type of a tuple struct constructor.
+fn type_for_struct_constructor(db: &impl HirDatabase, def: Struct) -> Ty {
+    let var_data = def.variant_data(db);
+    if var_data.fields().is_none() {
+        return type_for_struct(db, def); // Unit struct
+    }
+    let generics = def.generic_params(db);
     let substs = make_substs(&generics);
-    Ty::FnDef { def: def.into(), sig, substs }
+    Ty::FnDef { def: def.into(), substs }
+}
+
+fn fn_sig_for_enum_variant_constructor(db: &impl HirDatabase, def: EnumVariant) -> FnSig {
+    let var_data = def.variant_data(db);
+    let fields = match var_data.fields() {
+        Some(fields) => fields,
+        None => panic!("fn_sig_for_enum_variant_constructor called for unit variant"),
+    };
+    let resolver = def.parent_enum(db).resolver(db);
+    let params = fields
+        .iter()
+        .map(|(_, field)| Ty::from_hir(db, &resolver, &field.type_ref))
+        .collect::<Vec<_>>();
+    let generics = def.parent_enum(db).generic_params(db);
+    let substs = make_substs(&generics);
+    let ret = type_for_enum(db, def.parent_enum(db)).subst(&substs);
+    FnSig::from_params_and_return(params, ret)
 }
 
 /// Build the type of a tuple enum variant constructor.
 fn type_for_enum_variant_constructor(db: &impl HirDatabase, def: EnumVariant) -> Ty {
     let var_data = def.variant_data(db);
-    let fields = match var_data.fields() {
-        Some(fields) => fields,
-        None => return type_for_enum(db, def.parent_enum(db)), // Unit variant
-    };
-    let resolver = def.parent_enum(db).resolver(db);
+    if var_data.fields().is_none() {
+        return type_for_enum(db, def.parent_enum(db)); // Unit variant
+    }
     let generics = def.parent_enum(db).generic_params(db);
-    let input = fields
-        .iter()
-        .map(|(_, field)| Ty::from_hir(db, &resolver, &field.type_ref))
-        .collect::<Vec<_>>();
     let substs = make_substs(&generics);
-    let output = type_for_enum(db, def.parent_enum(db)).subst(&substs);
-    let sig = Arc::new(FnSig { input, output });
-    Ty::FnDef { def: def.into(), sig, substs }
+    Ty::FnDef { def: def.into(), substs }
 }
 
 fn make_substs(generics: &GenericParams) -> Substs {

crates/ra_hir/src/ty/primitive.rs

@@ -10,14 +10,6 @@ pub enum UncertainIntTy {
 }
 
 impl UncertainIntTy {
-    pub fn ty_to_string(&self) -> &'static str {
-        match *self {
-            UncertainIntTy::Unknown => "{integer}",
-            UncertainIntTy::Signed(ty) => ty.ty_to_string(),
-            UncertainIntTy::Unsigned(ty) => ty.ty_to_string(),
-        }
-    }
-
     pub fn from_name(name: &Name) -> Option<UncertainIntTy> {
         if let Some(ty) = IntTy::from_name(name) {
             Some(UncertainIntTy::Signed(ty))
@@ -29,6 +21,16 @@ impl UncertainIntTy {
     }
 }
 
+impl fmt::Display for UncertainIntTy {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            UncertainIntTy::Unknown => write!(f, "{{integer}}"),
+            UncertainIntTy::Signed(ty) => write!(f, "{}", ty),
+            UncertainIntTy::Unsigned(ty) => write!(f, "{}", ty),
+        }
+    }
+}
+
 #[derive(Debug, Clone, Eq, PartialEq, Hash, Copy)]
 pub enum UncertainFloatTy {
     Unknown,
@@ -36,13 +38,6 @@ pub enum UncertainFloatTy {
 }
 
 impl UncertainFloatTy {
-    pub fn ty_to_string(&self) -> &'static str {
-        match *self {
-            UncertainFloatTy::Unknown => "{float}",
-            UncertainFloatTy::Known(ty) => ty.ty_to_string(),
-        }
-    }
-
     pub fn from_name(name: &Name) -> Option<UncertainFloatTy> {
         if let Some(ty) = FloatTy::from_name(name) {
             Some(UncertainFloatTy::Known(ty))
@@ -52,6 +47,15 @@ impl UncertainFloatTy {
     }
 }
 
+impl fmt::Display for UncertainFloatTy {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            UncertainFloatTy::Unknown => write!(f, "{{float}}"),
+            UncertainFloatTy::Known(ty) => write!(f, "{}", ty),
+        }
+    }
+}
+
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Copy)]
 pub enum IntTy {
     Isize,
@@ -70,22 +74,19 @@ impl fmt::Debug for IntTy {
 
 impl fmt::Display for IntTy {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", self.ty_to_string())
-    }
-}
-
-impl IntTy {
-    pub fn ty_to_string(&self) -> &'static str {
-        match *self {
+        let s = match *self {
             IntTy::Isize => "isize",
             IntTy::I8 => "i8",
             IntTy::I16 => "i16",
             IntTy::I32 => "i32",
             IntTy::I64 => "i64",
             IntTy::I128 => "i128",
-        }
+        };
+        write!(f, "{}", s)
     }
+}
 
+impl IntTy {
     pub fn from_name(name: &Name) -> Option<IntTy> {
         match name.as_known_name()? {
             KnownName::Isize => Some(IntTy::Isize),
@@ -109,18 +110,21 @@ pub enum UintTy {
     U128,
 }
 
-impl UintTy {
-    pub fn ty_to_string(&self) -> &'static str {
-        match *self {
+impl fmt::Display for UintTy {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        let s = match *self {
             UintTy::Usize => "usize",
             UintTy::U8 => "u8",
             UintTy::U16 => "u16",
             UintTy::U32 => "u32",
             UintTy::U64 => "u64",
             UintTy::U128 => "u128",
-        }
+        };
+        write!(f, "{}", s)
     }
+}
 
+impl UintTy {
     pub fn from_name(name: &Name) -> Option<UintTy> {
         match name.as_known_name()? {
             KnownName::Usize => Some(UintTy::Usize),
@@ -140,12 +144,6 @@ impl fmt::Debug for UintTy {
     }
 }
 
-impl fmt::Display for UintTy {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", self.ty_to_string())
-    }
-}
-
 #[derive(Clone, PartialEq, Eq, Hash, Copy, PartialOrd, Ord)]
 pub enum FloatTy {
     F32,