[PATCH] D123576: Support constructing empty function_ref from other callables that can be "empty"

[Christian Sigg via Phabricator via llvm-commits]

csigg added inline comments.


================
Comment at: llvm/include/llvm/ADT/STLFunctionalExtras.h:60
+      // callable is not convertible to bool
+      std::enable_if_t<!std::is_convertible<Callable, bool>::value> * = nullptr,
       // Functor must be callable and return a suitable type.
----------------
Instead of exposing this as an extra c'tor template parameter, would it make sense to split this out?

```
  // Handles construction from an empty callable (for example a default
  // constructed std::function) and ensure that bool conversion to `false` is
  // propagated.
  template <typename Callable,
            std::enable_if_t<std::is_constructible<bool, Callable>::value> * =
                nullptr>
  static Ret get_callback(const Callable &callable) {
    return callable ? callback_fn<std::remove_reference_t<Callable>> : nullptr;
  }
  template <typename Callable> static Ret get_callback(...) {
    return callback_fn<std::remove_reference_t<Callable>>;
  }

public:
  ...
```

and then, on line 66:
```
: callback(get_callback<Callable>(callable)),
```


================
Comment at: llvm/include/llvm/ADT/STLFunctionalExtras.h:76
+      // callable is convertible to bool
+      std::enable_if_t<std::is_convertible<Callable, bool>::value> * = nullptr,
+      // Functor must be callable and return a suitable type.
----------------
I thought this wouldn't apply to std::function because it's only explicitly convertible to bool?

https://stackoverflow.com/questions/66438225/how-should-i-check-whether-a-type-is-contextually-convertible-to-bool


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123576