[libcxx-commits] [PATCH] D128983: [libcxx][ranges] implement `std::ranges::set_difference`

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const accepted this revision.
var-const added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/include/__algorithm/make_projected.h:50
+decltype(auto) __make_projected_comp(_Comp& __comp, _Proj1& __proj1, _Proj2& __proj2) {
+  if constexpr (same_as<_Proj1, identity> && same_as<_Proj2, identity> && !is_member_pointer_v<decay_t<_Comp>>) {
+    // Avoid creating the lambda and just use the pristine comparator -- for certain algorithms, this would enable
----------------
huixie90 wrote:
> var-const wrote:
> > Question: why is this check necessary (`is_member_pointer`)?
> if the caller is passing a member function pointer as comparator and two `identity`s as projections, without this check, if would just `return __comp`, which is a member function pointer. And the returned member function pointer is used by the `__impl` function and the `__impl` function simply invoke `operator()` as `__comp(*first, *second)`. This would fail because member function pointer don't support this syntax.
> 
> I suspect the overload above needs the same treatment. For the algorithms you've implemented, do you have any tests that pass a member function pointer as comparator and `std::identity` as projection?
Thanks a lot for fixing the existing overload!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128983