[PATCH] D78059: [llvm][STLExtras] Add various type_trait utilities currently present in MLIR

[River Riddle via Phabricator via llvm-commits]

rriddle marked an inline comment as done.
rriddle added inline comments.


================
Comment at: llvm/include/llvm/ADT/STLExtras.h:109-145
+/// This class provides various trait information about a callable object.
+///   * To access the number of arguments: Traits::num_args
+///   * To access the type of an argument: Traits::arg_t<i>
+///   * To access the type of the result:  Traits::result_t
+template <typename T, bool isClass = std::is_class<T>::value>
+struct function_traits : public function_traits<decltype(&T::operator())> {};
+
----------------
dblaikie wrote:
> Use of this device makes me a bit uncomfortable - such a thing would be incompatible with overloading, default arguments, etc. 
> 
> A general idea that's gone around inside Google but I can't find a good reference for it externally, is to only depend on the "call only interface" (ie: never take the address of a function you don't own - only call it with whatever types it was intended to have and treat it as though it returns the type it's documented to return). This ensures maximum flexibility for the function vendor to refactor it - add default arguments or overload the function as part of migrating/improving/refactoring the API, etc.
> 
> Is there any chance the existing uses of the mlir-equivalent of this trait could reasonably avoid it & instead rely on the call-only contract when interacting with functions/functors?
Thanks for the comment David. In all of the usages in MLIR, we aren't necessarily concerned about overloading/default arguments/etc. The general usage is that there is an API that takes a callback(generally expected to be a lambda), and then switches depending on the type of the function argument. Removing the ability to do that would greatly uglify a large amount of code. Is there a way to do that without doing the above? I'm open to any suggestions you might have. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78059/new/

https://reviews.llvm.org/D78059