[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