[PATCH] D114557: [fir] Add base for runtime builder unittests

[Valentin Clement via Phabricator via llvm-commits]

clementval marked 4 inline comments as done.
clementval added inline comments.


================
Comment at: flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h:62
+template <>
+constexpr TypeBuilderFunc getModel<short int>() {
+  return [](mlir::MLIRContext *context) -> mlir::Type {
----------------
rovka wrote:
> clementval wrote:
> > awarzynski wrote:
> > > rovka wrote:
> > > > (moving the discussion here from [[ https://reviews.llvm.org/D114460 | D114460 ]])
> > > > What I had in mind was either:
> > > > Option 1 - SFINAE:
> > > > 
> > > > ```
> > > > template <typename T>
> > > > static constexpr typenane std::enable_if_t<std::is_integral_v<T>, TypeBuilderFunc> getModel() {
> > > >   return [](mlir::MLIRContext *context) -> mlir::Type {
> > > >     return mlir::IntegerType::get(context, 8 * sizeof(T));
> > > >   };
> > > > }
> > > > // [...] lots of similar templates for is_floating_point, is_pointer etc etc
> > > > ```
> > > > or Option 2 - single template (probably easier to follow and maintain):
> > > > template <typename T>
> > > > static constexpr TypeBuilderFunc getModel() {
> > > >   return [](mlir::MLIRContext *context) -> mlir::Type {
> > > >      if constexpr (std::is_integral_v<T>) {
> > > >         return mlir::IntegerType::get(context, 8 * sizeof(T));
> > > >      } else // [...] branches for std::is_floating_point, is_pointer etc
> > > >   };
> > > > }
> > > +1
> > This can be a nice refactoring. I think this can be done after upstreaming. 
> Ok, SGTM.
Added to the board https://github.com/flang-compiler/f18-llvm-project/issues/1272


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114557