[libcxx-commits] [PATCH] D107671: [libcxx][ranges] Add `ranges::join_view`.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__ranges/join_view.h:340
+  template<class _Range>
+  explicit join_view(_Range&&) -> join_view<views::all_t<_Range>>;
+
----------------
zoecarver wrote:
> ldionne wrote:
> > Please make sure you test the explicit-ness of this deduction guide with something like that:
> > 
> > ```
> > join_view v = some_range; // should not compile
> > join_view v(some_range); // should compile
> > ```
> I can't seem to figure out how to test this. It seems like both compile: https://godbolt.org/z/qefr1vxqz
> 
> I'll think some more and see if I can come up with something. 
@zoecarver: Remember that SFINAE works only on immediate contexts. `requires(T r) { invokesImplicitCTAD(r); }` is invariably true, regardless of what the //body// of `invokesImplicitCTAD` looks like.
I recall you had some macro in one of these recent reviews that was also trying to SFINAE on the body of a function (return-type-SFINAEing on the decltype of a lambda whose body contained the interesting part); again, that doesn't work. SFINAE only looks at the well-formedness of the exact expression being evaluated, not at the-bodies-of-all-the-things-that-expression-might-call.

The other problem with your example is that `X(T)` creates an implicit deduction guide. Change it to `X(int)` or `X(std::type_identity_t<T>)` to fix that part.

I don't think there's any way to test CTAD-ability with SFINAE. With a literal type, you can try something like this https://godbolt.org/z/4GbdT87Ts but right now this works //only// on Clang (not GCC nor MSVC), and it certainly doesn't help with `join_view` specifically. So the "should not compile" test would have to be a `.fail.cpp`, unless I'm mistaken (in which case I smell a blog post in the offing... ;))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107671