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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 25 12:40:43 PDT 2022


dblaikie added a comment.

In D123576#3469352 <https://reviews.llvm.org/D123576#3469352>, @dexonsmith wrote:

> In D123576#3467855 <https://reviews.llvm.org/D123576#3467855>, @dblaikie wrote:
>
>> I worry this is overly general (it's not obvious that any functor with boolean conversion has the semantics this patch relies on) & inconsistent with the standard library (`std::function` for instance doesn't provide this (or a narrower version of it) functionality - so we'll still have the bugs for any API that takes a `function_ref` and passes it to `std::function`)
>
> Seems nice to make it safer than the STL. Would you be more comfortable if it were special-cased to known functions that behave this way (std::function and llvm::unique_function)?

Would certainly be more comfortable (guess we could have a static_assert that fails if the functor is boolean convertible but not in the list - suggesting they be added to the list (& if someone comes up with a functor that can't be added to the list because its boolean testability doesn't have the desired semantics, we can keep some explicit allow-list of "convertible-to-function_ref without boolean testing despite being boolean testable for some other semantics")
 For your other concern, I'm wondering if we could also make it symmetric (add `operator std::function()` that adds this check somehow) or if that would lead to ambiguities.

>> I don't know that there's /no/ useful direction here, but I have my reservations.
>>
>> (@rsmith as the original author of `function_ref`)

Wouldn't mind hearing from @rsmith still, but wouldn't block on that if this is restricted to only known-good cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123576



More information about the llvm-commits mailing list