common_reference doesn't work with some proxy references

[ericniebler]

Consider:

namespace ranges = std::experimental::ranges;

struct MyIntRef {
  MyIntRef(int &);
};

using T = ranges::common_reference_t<int &, MyIntRef>; // doesn't work
using U = ranges::common_type_t<int &, MyIntRef>; // also doesn't work

I haven't decided yet if this is a bug or not. It's a bug. At least, it's a bug for common_reference, not for common_type.

[ Note: As of C++14 (IIRC), std::common_type<int &, MyIntRef>::type is MyIntRef because it doesn't decay types before the ternary conditional test, so this is a regression wrt C++14. However, with C++17, std::common_type first decays types before the ternary conditional test, so we won't be regressing behavior relative to C++17 if we don't accommodate this case.--end note]

Discussion

Stepping through the specification of common_reference with the types int& and MyIntRef:

(3.3.1) — If COMMON_REF(T1, T2) is well-formed and denotes a reference
type then the member typedef type denotes that type

This bullet is meant to handle reference types for which the built-in ternary operator yields another reference type (after some argument munging to avoid some weirdness with the ternary operator that causes premature type decay). The intention is to prevent the next bullet from firing, thereby preventing users from overriding the language rules regarding reference binding via the basic_common_reference customization point.

This rule doesn't fire for int& and MyIntRef because the logical common reference in this case is MyIntRef, which is not a reference type.

(Besides, this bullet should only be considered when T1 and T2 are reference types. We can tighten up the spec by requiring that condition. The issue under discussion doesn't require that change, but we propose it anyway.)

(3.3.2) — Otherwise, if basic_common_reference<UNCVREF(T1), UNCVREF(T2),
XREF(T1), XREF(T2)>::type is well-formed, then the member typedef
type denotes that type.

This bullet only fires when the user has specialized basic_common_reference on their argument types. Not relevant here.

(3.3.3) — Otherwise, if common_type_t<T1, T2> is well-formed, then the
member typedef type denotes that type.

By dispatching to common_type we pick up any user specializations of that type trait. Trouble is, it decays types before looking for a common type. By decaying int& to int, it now becomes impossible to find the common type.

(3.3.4) — Otherwise, there shall be no member type.

Therefore, there is no common reference.

The fix is to, before testing for common_type, first put the two types in a ternary conditional without decaying and see if that results in a well-formed type. Only if it doesn't do we fall back to common_type.

The proposed resolution is expressed in terms of the COND_RES(X, Y) pseudo-macro, which was defined as decltype(declval<bool>() ? declval<X>() : declval<Y>()). This macro is buggy because COND_RES(int, int) comes out as int&& instead of int. That's because the type of declval<int>() is int&&. To make it work, COND_RES(X, Y) needs to be redefined as the somewhat cryptic decltype(declval<bool>() ? declval<X(&)()>()() : declval<Y(&)()>()()). This formulation perfectly preserves the value category of the arguments.

Proposed Resolution

Change [meta.trans.other]/p1 as follows:

 1.
 Let CREF(A) be add_lvalue_reference_t<const remove_reference_t<A>>.
 Let UNCVREF(A) be remove_cv_t<remove_reference_t<A>>. Let XREF(A)
 denote a unary template T such that T<UNCVREF(A)> denotes the same
 type as A. Let COPYCV(FROM, TO) be an alias for type TO with the addition
 of FROM’s top-level cv-qualifiers. [ Example: COPYCV(const int, volatile
 short) is an alias for const volatile short. —end example ] Let RREF_RES(Z)
 be remove_reference_t<Z>&& if Z is a reference type or Z otherwise. Let
 COND_RES(X, Y) be
-decltype(declval<bool>() ? declval<X>() : declval<Y>()).
+decltype(declval<bool>() ? declval<X(&)()>()() : declval<Y(&)()>()()).
 Given types A and B, let X be remove_reference_t<A>, let Y be
 remove_reference_t<B>, and let COMMON_REF(A, B) be:

Change the description of common_reference [meta.trans.other]/p3 as follows:

 3
 For the common_reference trait applied to a parameter pack T of types,
 the member type shall be either defined or not present as follows:
 (3.1) — If sizeof...(T) is zero, there shall be no member type.
 (3.2) — Otherwise, if sizeof...(T) is one, let T1 denote the sole type in the
         pack T. The member typedef type shall denote the same type as T1.
 (3.3) — Otherwise, if sizeof...(T) is two, let T1 and T2 denote the two types
         in the pack T. Then
-(3.3.1) — If COMMON_REF(T1, T2) is well-formed and denotes a reference
+(3.3.1) — If T1 and T2 are reference types and COMMON_REF(T1, T2) is
+          well-formed and denotes a reference
           type then the member typedef type denotes that type.
 (3.3.2) — Otherwise, if basic_common_reference<UNCVREF(T1), UNCVREF(T2),
           XREF(T1), XREF(T2)>::type is well-formed, then the member typedef
           type denotes that type.
+(3.3.x) — Otherwise, if COND_RES(T1, T2) is well-formed, then the
+          member typedef type denotes that type.
 (3.3.3) — Otherwise, if common_type_t<T1, T2> is well-formed, then the
           member typedef type denotes that type.
 (3.3.4) — Otherwise, there shall be no member type.

[ericniebler]

@CaseyCarter for your review.

[CaseyCarter]

("premature type decay" sounds like a serious problem for aging C++ programmers)

declval<X(&)()>()() hurts - there should be a better way to spell "expression E such that decltype((E)) is X." declval<X()>()() is an immediate simplification, but I don't know that it's any more readable. What we really need is template<class T> T val() noexcept; // \expos, but I don't think defining yet another specification tool will help readers of [meta.trans.other].

The PR is fine.