Auto merge of #573 - nathanwhit:fn-refactor, r=jackh726

Refactor and flesh out Fn pointers

Closes #507.

This PR does a few things:
- Renames `Fn` -> `FnPointer`
- Adds ABI to `FnPointer`
- Adds support for unsafe functions to both `FnPointer` and `FnDef`
- Adds a variadic flag to both `FnPointer` and `FnDef` (cc [this comment](#507 (comment)))

Author: bors

chalk-integration/src/lowering.rs

@@ -1210,6 +1210,8 @@ impl LowerFnDefn for FnDefn {
             id: fn_def_id,
             abi: self.abi.lower()?,
             binders,
+            safety: ast_safety_to_chalk_safety(self.safety),
+            variadic: self.variadic,
         })
     }
 }
@@ -1647,6 +1649,9 @@ impl LowerTy for Ty {
             Ty::ForAll {
                 lifetime_names,
                 types,
+                abi,
+                safety,
+                variadic,
             } => {
                 let quantified_env = env.introduce(lifetime_names.iter().map(|id| {
                     chalk_ir::WithKind::new(chalk_ir::VariableKind::Lifetime, id.str.clone())
@@ -1657,9 +1662,12 @@ impl LowerTy for Ty {
                     lowered_tys.push(ty.lower(&quantified_env)?.cast(interner));
                 }
 
-                let function = chalk_ir::Fn {
+                let function = chalk_ir::FnPointer {
                     num_binders: lifetime_names.len(),
                     substitution: Substitution::from_iter(interner, lowered_tys),
+                    abi: abi.lower()?,
+                    safety: ast_safety_to_chalk_safety(*safety),
+                    variadic: *variadic,
                 };
                 Ok(chalk_ir::TyData::Function(function).intern(interner))
             }
@@ -2192,3 +2200,10 @@ fn ast_mutability_to_chalk_mutability(mutability: Mutability) -> chalk_ir::Mutab
         Mutability::Not => chalk_ir::Mutability::Not,
     }
 }
+
+fn ast_safety_to_chalk_safety(safety: Safety) -> chalk_ir::Safety {
+    match safety {
+        Safety::Safe => chalk_ir::Safety::Safe,
+        Safety::Unsafe => chalk_ir::Safety::Unsafe,
+    }
+}

chalk-integration/src/test_macros.rs

@@ -13,12 +13,15 @@ macro_rules! ty {
     };
 
     (function $n:tt $($arg:tt)*) => {
-        chalk_ir::TyData::Function(chalk_ir::Fn {
+        chalk_ir::TyData::Function(chalk_ir::FnPointer {
             num_binders: $n,
             substitution: chalk_ir::Substitution::from_iter(
                 &chalk_integration::interner::ChalkIr,
                 vec![$(arg!($arg)),*] as Vec<chalk_ir::GenericArg<_>>
             ),
+            safety: chalk_ir::Safety::Safe,
+            abi: <chalk_integration::interner::ChalkIr as chalk_ir::interner::Interner>::FnAbi::Rust,
+            variadic: false,
         }).intern(&chalk_integration::interner::ChalkIr)
     };
 

chalk-ir/src/debug.rs

@@ -232,14 +232,21 @@ impl Debug for InferenceVar {
     }
 }
 
-impl<I: Interner> Debug for Fn<I> {
+impl<I: Interner> Debug for FnPointer<I> {
     fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), Error> {
         // FIXME -- we should introduce some names or something here
-        let Fn {
+        let FnPointer {
             num_binders,
             substitution,
+            abi,
+            safety,
+            variadic: _,
         } = self;
-        write!(fmt, "for<{}> {:?}", num_binders, substitution)
+        write!(
+            fmt,
+            "for<{}> {:?} {:?} {:?}",
+            num_binders, safety, abi, substitution
+        )
     }
 }
 

chalk-ir/src/fold/binder_impls.rs

@@ -6,8 +6,8 @@
 use crate::interner::TargetInterner;
 use crate::*;
 
-impl<I: Interner, TI: TargetInterner<I>> Fold<I, TI> for Fn<I> {
-    type Result = Fn<TI>;
+impl<I: Interner, TI: TargetInterner<I>> Fold<I, TI> for FnPointer<I> {
+    type Result = FnPointer<TI>;
     fn fold_with<'i>(
         &self,
         folder: &mut dyn Folder<'i, I, TI>,
@@ -17,13 +17,19 @@ impl<I: Interner, TI: TargetInterner<I>> Fold<I, TI> for Fn<I> {
         I: 'i,
         TI: 'i,
     {
-        let Fn {
+        let FnPointer {
             num_binders,
             substitution,
+            abi,
+            safety,
+            variadic,
         } = self;
-        Ok(Fn {
+        Ok(FnPointer {
             num_binders: *num_binders,
             substitution: substitution.fold_with(folder, outer_binder.shifted_in())?,
+            abi: TI::transfer_abi(*abi),
+            safety: *safety,
+            variadic: *variadic,
         })
     }
 }

chalk-ir/src/fold/boring_impls.rs

@@ -267,6 +267,7 @@ copy_fold!(FloatTy);
 copy_fold!(Scalar);
 copy_fold!(ClausePriority);
 copy_fold!(Mutability);
+copy_fold!(Safety);
 
 #[doc(hidden)]
 #[macro_export]

chalk-ir/src/interner.rs

@@ -633,6 +633,9 @@ pub trait TargetInterner<I: Interner>: Interner {
         &self,
         const_evaluated: &I::InternedConcreteConst,
     ) -> Self::InternedConcreteConst;
+
+    /// Transfer function ABI to the target interner.
+    fn transfer_abi(abi: I::FnAbi) -> Self::FnAbi;
 }
 
 impl<I: Interner> TargetInterner<I> for I {
@@ -662,6 +665,10 @@ impl<I: Interner> TargetInterner<I> for I {
     ) -> Self::InternedConcreteConst {
         const_evaluated.clone()
     }
+
+    fn transfer_abi(abi: I::FnAbi) -> Self::FnAbi {
+        abi
+    }
 }
 
 /// Implemented by types that have an associated interner (which

chalk-ir/src/lib.rs

@@ -193,6 +193,15 @@ pub enum Scalar {
     Float(FloatTy),
 }
 
+/// Whether a function is safe or not.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub enum Safety {
+    /// Safe
+    Safe,
+    /// Unsafe
+    Unsafe,
+}
+
 /// Whether a type is mutable or not.
 #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum Mutability {
@@ -503,7 +512,7 @@ pub enum TyData<I: Interner> {
     /// Note that "higher-ranked" types (starting with `for<>`) are either
     /// function types or dyn types, and do not appear otherwise in Rust
     /// surface syntax.
-    Function(Fn<I>),
+    Function(FnPointer<I>),
 
     /// References the binding at the given depth. The index is a [de
     /// Bruijn index], so it counts back through the in-scope binders.
@@ -860,12 +869,15 @@ impl InferenceVar {
 /// and we use deBruijn indices within `self.ty`
 #[derive(Clone, PartialEq, Eq, Hash, HasInterner)]
 #[allow(missing_docs)]
-pub struct Fn<I: Interner> {
+pub struct FnPointer<I: Interner> {
     pub num_binders: usize,
+    pub abi: I::FnAbi,
+    pub safety: Safety,
+    pub variadic: bool,
     pub substitution: Substitution<I>,
 }
 
-impl<I: Interner> Copy for Fn<I> where I::InternedSubstitution: Copy {}
+impl<I: Interner> Copy for FnPointer<I> where I::InternedSubstitution: Copy {}
 
 /// Constants.
 #[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, HasInterner)]

chalk-ir/src/visit/binder_impls.rs

@@ -4,9 +4,9 @@
 //! The more interesting impls of `Visit` remain in the `visit` module.
 
 use crate::interner::HasInterner;
-use crate::{Binders, Canonical, DebruijnIndex, Fn, Interner, Visit, VisitResult, Visitor};
+use crate::{Binders, Canonical, DebruijnIndex, FnPointer, Interner, Visit, VisitResult, Visitor};
 
-impl<I: Interner> Visit<I> for Fn<I> {
+impl<I: Interner> Visit<I> for FnPointer<I> {
     fn visit_with<'i, R: VisitResult>(
         &self,
         visitor: &mut dyn Visitor<'i, I, Result = R>,

chalk-ir/src/visit/boring_impls.rs

@@ -7,8 +7,8 @@
 use crate::{
     AdtId, AssocTypeId, ClausePriority, ClosureId, Constraints, DebruijnIndex, FloatTy, FnDefId,
     GenericArg, Goals, ImplId, IntTy, Interner, Mutability, OpaqueTyId, PlaceholderIndex,
-    ProgramClause, ProgramClauses, QuantifiedWhereClauses, QuantifierKind, Scalar, Substitution,
-    SuperVisit, TraitId, UintTy, UniverseIndex, Visit, VisitResult, Visitor,
+    ProgramClause, ProgramClauses, QuantifiedWhereClauses, QuantifierKind, Safety, Scalar,
+    Substitution, SuperVisit, TraitId, UintTy, UniverseIndex, Visit, VisitResult, Visitor,
 };
 use std::{marker::PhantomData, sync::Arc};
 
@@ -211,6 +211,7 @@ const_visit!(UintTy);
 const_visit!(IntTy);
 const_visit!(FloatTy);
 const_visit!(Mutability);
+const_visit!(Safety);
 
 #[doc(hidden)]
 #[macro_export]

chalk-parse/src/ast.rs

@@ -74,6 +74,8 @@ pub struct FnDefn {
     pub argument_types: Vec<Ty>,
     pub return_type: Ty,
     pub abi: FnAbi,
+    pub safety: Safety,
+    pub variadic: bool,
 }
 
 #[derive(Clone, PartialEq, Eq, Debug)]
@@ -259,6 +261,9 @@ pub enum Ty {
     ForAll {
         lifetime_names: Vec<Identifier>,
         types: Vec<Box<Ty>>,
+        abi: FnAbi,
+        safety: Safety,
+        variadic: bool,
     },
     Tuple {
         types: Vec<Box<Ty>>,
@@ -327,6 +332,18 @@ pub enum Mutability {
     Not,
 }
 
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+pub enum Safety {
+    Safe,
+    Unsafe,
+}
+
+impl Default for Safety {
+    fn default() -> Self {
+        Self::Safe
+    }
+}
+
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub enum Lifetime {
     Id { name: Identifier },
@@ -450,3 +467,51 @@ pub enum ClosureKind {
     FnMut,
     FnOnce,
 }
+
+#[derive(Clone, Eq, PartialEq, Debug)]
+pub enum FnArg {
+    NonVariadic(Ty),
+    Variadic,
+}
+#[derive(Clone, Eq, PartialEq, Debug)]
+pub enum FnArgs {
+    NonVariadic(Vec<Ty>),
+    Variadic(Vec<Ty>),
+}
+
+impl FnArgs {
+    pub fn is_variadic(&self) -> bool {
+        match self {
+            Self::Variadic(..) => true,
+            _ => false,
+        }
+    }
+
+    pub fn to_tys(self) -> Vec<Ty> {
+        match self {
+            Self::NonVariadic(tys) | Self::Variadic(tys) => tys,
+        }
+    }
+
+    pub fn from_vec(mut args: Vec<FnArg>) -> Result<Self, &'static str> {
+        let mut tys = Vec::with_capacity(args.len());
+        let last = args.pop();
+        for arg in args {
+            match arg {
+                FnArg::NonVariadic(ty) => tys.push(ty),
+                FnArg::Variadic => {
+                    return Err("a variadic argument must be the last parameter in a function");
+                }
+            }
+        }
+
+        Ok(match last {
+            Some(FnArg::NonVariadic(ty)) => {
+                tys.push(ty);
+                FnArgs::NonVariadic(tys)
+            }
+            Some(FnArg::Variadic) => FnArgs::Variadic(tys),
+            None => FnArgs::NonVariadic(tys),
+        })
+    }
+}

chalk-parse/src/parser.lalrpop

@@ -140,26 +140,31 @@ FnReturn: Ty = {
 };
 
 FnDefn: FnDefn = {
-    <abi:FnAbi?> "fn" <n:Id> <p:Angle<VariableKind>>"(" <args:FnArgs> ")"
+    <safety:Safety?> <abi:FnAbi?> "fn" <n:Id> <p:Angle<VariableKind>>"(" <args:FnArgs> ")"
         <ret_ty:FnReturn?> <w:QuantifiedWhereClauses> ";" => FnDefn
     {
         name: n,
         variable_kinds: p,
         where_clauses: w,
-        argument_types: args,
+        variadic: args.is_variadic(),
+        argument_types: args.to_tys(),
         return_type: ret_ty.unwrap_or_else(|| Ty::Tuple { types: Vec::new() }),
         abi: abi.unwrap_or_default(),
+        safety: safety.unwrap_or_default(),
     }
 };
 
 FnAbi: FnAbi = "extern" "\"" <id:Id> "\"" => FnAbi(id.str);
 
-FnArg: Ty = {
-    Id ":" <arg_ty: Ty> => arg_ty
+FnArg: FnArg = {
+    Id ":" "..." => FnArg::Variadic,
+    Id ":" <arg_ty: Ty> => FnArg::NonVariadic(arg_ty),
 };
 
-FnArgs: Vec<Ty> = {
-    <Comma<FnArg>>
+FnArgs: FnArgs = {
+    <Comma<FnArg>> =>? FnArgs::from_vec(<>).map_err(|e| lalrpop_util::ParseError::User {
+        error: e,
+    })
 };
 
 ClosureDefn: ClosureDefn = {
@@ -181,7 +186,7 @@ ClosureSelf: ClosureKind = {
 }
 
 ClosureArgs: Vec<Ty> = {
-    "," <args:FnArgs> => args,
+    "," <args:FnArgs> => args.to_tys(),
 }
 
 TraitDefn: TraitDefn = {
@@ -313,13 +318,32 @@ pub Ty: Ty = {
     TyWithoutId,
 };
 
+Safety: Safety = {
+    "unsafe" => Safety::Unsafe,
+};
+
+FnArgTy: FnArg = {
+    "..." => FnArg::Variadic,
+    <arg_ty: Ty> => FnArg::NonVariadic(arg_ty),
+};
+
+FnArgTys: FnArgs = {
+    <Comma<FnArgTy>> =>? FnArgs::from_vec(<>).map_err(|e| lalrpop_util::ParseError::User {
+        error: e,
+    })
+};
+
 TyWithoutId: Ty = {
-    <l:ForLifetimes?> "fn" "(" <types:Comma<Ty>> ")" <ret_ty:FnReturn?> => Ty::ForAll {
+    <l:ForLifetimes?> <safety:Safety?> <abi:FnAbi?> "fn" "(" <types:FnArgTys> ")" <ret_ty:FnReturn?> => Ty::ForAll {
         lifetime_names: l.unwrap_or_default(),
+        variadic: types.is_variadic(),
         types: types
+                   .to_tys()
                    .into_iter()
                    .chain(std::iter::once(ret_ty.unwrap_or_else(|| Ty::Tuple { types: Vec::new() })))
-                   .map(Box::new).collect()
+                   .map(Box::new).collect(),
+        safety: safety.unwrap_or_default(),
+        abi: abi.unwrap_or_default(),
     },
     <ScalarType> => Ty::Scalar { ty: <> },
     "str" => Ty::Str,