[libcxx-commits] [PATCH] D96683: [libcxx] adds concept `std::common_with`

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/include/concepts:195
+  common_reference_with<
+    add_lvalue_reference_t<common_type_t<_Tp, _Up>>,
+    common_reference_t<
----------------
ldionne wrote:
> I see this follows the spec closely, but I'm curious to understand. Here, I would have expected the check to be:
> 
> ```
> common_reference_with<
>     common_type_t<_Tp, _Up> const&,
>     common_reference_t<
>         const _Tp&,
>         const _Up&>>;
> ```
> 
> instead (with `add_lvalue_reference_t` simplified naively for legibility). In other words, why are we looking for a common reference between the non-const common type and the common-ref between the `const`-qualified types? I'm sure that'll highlight a misunderstanding I have about the concept.
> In other words, why are we looking for a common reference between the non-`const` common type and the common-ref between the `const`-qualified types?

This asks the question "does `common_type_t<T, U>&` have a reference in common with `common_reference_t<T const&, U const&>`?" `common_reference_t<T const&, U const&>` should fall out as a reference-to-`const` unless `basic_common_reference` dictates otherwise, so it's essentially checking something to the effect of `common_reference_with<CT&, CR const&>`.

As for why this is the case, it screams proxy types to me (you wouldn't notice that unless you've played with `zip_view`). This is about as far as my knowledge goes though.

@CaseyCarter will probably correct me here, but getting stuff wrong on the Internet is how one learns?


================
Comment at: libcxx/test/std/concepts/lang/common.compile.pass.cpp:19
+constexpr bool CheckCommonWith() noexcept {
+  static_assert(std::common_with<T, U&>);
+  static_assert(std::common_with<T, const U&>);
----------------
ldionne wrote:
> Same comment about testing with `static_assert(std::common_with<T, U&> == std::common_with<T, U>)` as in the other patch, if you think it makes sense.
Yep!


================
Comment at: libcxx/test/std/concepts/lang/common.compile.pass.cpp:286
+
+struct T1 {};
+static_assert(!std::convertible_to<DullCommonType, T1>);
----------------
cjdb wrote:
> miscco wrote:
> > I would greatly prefer if we could give those types names that describe what we are checking rather than numbers where one has to read the source an the standard
> > 
> The interesting information is in all the `CommonTypeX`s. `Tn` really just facilitates the common types.
`T8` and `T9` were respectively renamed to `CommonWithInt` and `CommonWithIntButRefLong`, but I'm still not convinced about `T1` through `T7` (for now!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96683