adds `__reference_constructs_from_temporary`

[cjdb]

Summary

This is information that the compiler already has, and should be exposed
so that the library doesn't need to reimplement the exact same
functionality.

[cjdb]

I learnt last night that this one is incredibly wrong and needs revising.

[ldionne]

Interesting, I actually wasn't even aware of that C++23 feature in the library. FWIW, libc++ will be more than happy to use that builtin instead of trying to figure it out inside the library (if that's even possible)! We'll have to check whether GCC implements it, but hopefully it has something similar. So this definitely LGTM from the libc++ side of things, assuming the Clang folks are happy with the implementation.

[cjdb]

In D135341#3841126, @ldionne wrote:

Interesting, I actually wasn't even aware of that C++23 feature in the library. FWIW, libc++ will be more than happy to use that builtin instead of trying to figure it out inside the library (if that's even possible)! We'll have to check whether GCC implements it, but hopefully it has something similar. So this definitely LGTM from the libc++ side of things, assuming the Clang folks are happy with the implementation.

It's possible to easily implement reference_constructs_from_temporary in the library, but the converts one is far more tricky because __reference_binds_to_temporary only cares about construction. I don't know if GCC implements it (I thought it did, but I can't find the commit in a trivial Google search), but I did learn that libstdc++ has a guard for this name.

[cjdb]

Gets __reference_constructs_from_temporary correct. Still WIP for the latter.

[cjdb]

discards work on __reference_converts_from_temporary for now. This feature isn't as trivial to implement as __reference_constructs_from_temporary, so it's deserving of its own commit. The two features are used exclusively, so it's not like adding one without the other will lead to an incomplete standard type.

[erichkeane]

clang/include/clang/Basic/DiagnosticCommonKinds.td

397 ↗

(On Diff #466318)

I don't think we should diagnose this for individual identifiers, I don't think this adds much value, and the identifier is already reserved for implementation use. This implies we are going to diagnose ALL future reserved things, which we have no intention of doing.

[cjdb]

clang/include/clang/Basic/DiagnosticCommonKinds.td

397 ↗

(On Diff #466318)

The motivation for this error is to ensure that standard library implementations don't take this name (__remove_cvref and __is_null_pointer had to get names that weren't a 1:1 mapping to their library counterparts as a result).

[royjacobson]

clang/docs/LanguageExtensions.rst

1623–1627

We should deprecate this, as __reference_constructs_from_temporary is implemented now. There's a list of deprecated builtins in SemaExprCXX.cpp:DiagnoseBuiltinDeprecation so I think it should be pretty easy to add a warning for users that this is a non-standard attribute.

[royjacobson]

clang/docs/LanguageExtensions.rst

1623–1627

Though for the CI's sake maybe it's better to wait until after libcxx stop using it.

[ldionne]

I've been looking at implementing reference_constructs_from_temporary & friends and this would be sweet to have. Is this blocked on something specific?

clang/include/clang/Basic/DiagnosticCommonKinds.td

397 ↗

(On Diff #466318)

Oh, just ask and we can rename ours! What do you want us to rename?

[cjdb]

In D135341#4111673, @ldionne wrote:

I've been looking at implementing reference_constructs_from_temporary & friends and this would be sweet to have. Is this blocked on something specific?

This trait should be ready to go, pending an LGTM. __reference_converts_from_temporary is a different story, and someone else might need to do this one if you need it in a timely fashion (but I'll be happy to review it).

clang/include/clang/Basic/DiagnosticCommonKinds.td

397 ↗

(On Diff #466318)

Much appreciated! I think __remove_cvref had a libc++ conflict and __is_null_pointer had a libstdc++ conflict, so it's not specific to one library. I figured that it would be easier for backwards-compatibility reasons to just rename them to something slightly more awkward, so that previous versions of the stdlib don't spontaneously break for more recent compilers. This is more of an issue when you're dealing with package managers like apt, where some things depend on libc++-v$OLD, but there aren't any restrictions on Clang's version (for example, I use nightly Clang on Ubuntu, but apt notes that the Discord app has an older libc++ dependency).

After having let this sit for a few months, I don't really have strong opinions on reserving __std_trait anymore, so I'll touch this up in the morning.

[ldionne]

In D135341#4111884, @cjdb wrote:

In D135341#4111673, @ldionne wrote:

I've been looking at implementing reference_constructs_from_temporary & friends and this would be sweet to have. Is this blocked on something specific?

This trait should be ready to go, pending an LGTM. __reference_converts_from_temporary is a different story, and someone else might need to do this one if you need it in a timely fashion (but I'll be happy to review it).

Sounds good! This naively looks good to me, but it would be nice if a Clang regular could take a look!

clang/include/clang/Basic/DiagnosticCommonKinds.td

397 ↗

(On Diff #466318)

I just checked and I think we define __remove_cvref_t, but not __remove_cvref (that probably changed in the last year). In the future, don't hesitate to ping us if we are using a name that Clang would like to use, we should almost always be able to quickly get out of your way!

[erichkeane]

On the fence about the diagnostic at all, but definitely should not be doing string magic to make it quoted. Otherwise this is a LGTM.

clang/lib/Sema/SemaExprCXX.cpp

5715

This is my only problem with this... we shouldn't have to do this magic to get this to work. The diagnostic should have the quote in it, OR we should just pass an identifier (which, IIRC, ensures we wrap it in quotes anyway).

[cjdb]

In D135341#4126101, @erichkeane wrote:

On the fence about the diagnostic at all, but definitely should not be doing string magic to make it quoted. Otherwise this is a LGTM.

Whoops, missed this comment. I'll be addressing your feedback and merging later this week.

[cjdb]

responds to feedback:

• removes diagnostic
• adds documentation

[erichkeane]

Is the changes to cxx_status.html still relevant/current? Also, did we ever figure out what GCC did for a builtin here? Do they have one yet? If not, have we encouraged them to implement this one? If so, DID they implement this one?

[cjdb]

In D135341#4643712, @erichkeane wrote:

Is the changes to cxx_status.html still relevant/current?

Yeah.

Also, did we ever figure out what GCC did for a builtin here? Do they have one yet? If not, have we encouraged them to implement this one? If so, DID they implement this one?

They have implemented both, it seems :)

See https://godbolt.org/z/G8eeMfsej

[cjdb]

Oh, I shouldn't have so much green in the status, whoops!

[cjdb]

Corrects cxx_status.html, which accidentally duplicated a lot of the file

[cjdb]

I somehow made cxx_status.html worse in the previous commit, not better

[cjdb]

Sorry, I had read @erichkeane's LGTM and taken that as actually approved. Would you like me to revert? I have reverted on principle.

[cor3ntin]

clang/lib/Sema/SemaExprCXX.cpp

33

Superfluous change

5513

BuiltinRemoveReference is nice here as I don't think we have a way to remove all cv and ref in one go at the ASTContext/QualType level.
However BuiltinAddPointer does nothing for us over Context.getPointerType(), so I'd rather we used that.

5515

Could you put the implementation of BTT_IsConvertibleTo in its own function?

[erichkeane]

All of @cor3ntin 's suggestions are really good ones, I'm ok with this once he is.

[cjdb]

• responds to @cor3ntin's feedback
• updates commit message to include [clang] tag and also indicate this is a revert

clang/lib/Sema/SemaExprCXX.cpp

33

Nice catch!

[h-vetinari]

clang/www/cxx_status.html

297

[cor3ntin]

clang/lib/Sema/SemaExprCXX.cpp

5515

I was thinking something like

QualType TPtr = S.Context.getPointerType(S.BuiltinRemoveReference(T, UnaryTransformType::RemoveCVRef, {}));
QualType UPtr = S.Context.getPointerType(S.BuiltinRemoveReference(U, UnaryTransformType::RemoveCVRef, {}));
return IsConvertibleTo(S, UPtr, TPtr);

and then EvaluateBinaryTypeTrait can use this new IsConvertibleTo (which we can extract from EvaluateBinaryTypeTrait)

[cjdb]

clang/lib/Sema/SemaExprCXX.cpp

5515

That sounds like an orthogonal change. How about I revert to what was previously there and do that as a second PR? It'll also need a location parameter.

[cor3ntin]

clang/lib/Sema/SemaExprCXX.cpp

5515

I don't insist. Please keep the getPointerType changes though.

It is pretty unusual to evaluate type traits in terms of the implementation of other type traits, because all of these things do different Sema (some of it is wasteful) and
could add undesired diagnostics in the future.

But if you prefer that can be done later!

[cjdb]

partially reverts change and fixes typo

[cor3ntin]

LGTM

clang/docs/LanguageExtensions.rst

* ``__is_unsigned`` (C++, Embarcadero):
   Returns false for enumeration types. Note, before Clang 13, returned true for
   enumeration types if the underlying type was unsigned.
 * ``__is_void`` (C++, Embarcadero)
 * ``__is_volatile`` (C++, Embarcadero)
 * ``__reference_binds_to_temporary(T, U)`` (Clang):  Determines whether a
   reference of type ``T`` bound to an expression of type ``U`` would bind to a
   materialized temporary object. If ``T`` is not a reference type the result
   is false. Note this trait will also return false when the initialization of
   ``T`` from ``U`` is ill-formed.
+  Deprecated, use ``__reference_constructs_from_temporary``.
+* ``__reference_constructs_from_temporary(T, U)`` (C++)
+  Returns true if a reference ``T`` can be constructed from a temporary of type
+  a non-cv-qualified ``U``.
 * ``__underlying_type`` (C++, GNU, Microsoft)
 
 In addition, the following expression traits are supported:
 
 * ``__is_lvalue_expr(e)`` (Embarcadero):
   Returns true if ``e`` is an lvalue expression.
   Deprecated, use ``__is_lvalue_reference(decltype((e)))`` instead.
 * ``__is_rvalue_expr(e)`` (Embarcadero):

clang/include/clang/Basic/TokenKinds.def

 TYPE_TRAIT_1(__is_trivially_equality_comparable, IsTriviallyEqualityComparable, KEYCXX)
 TYPE_TRAIT_1(__is_bounded_array, IsBoundedArray, KEYCXX)
 TYPE_TRAIT_1(__is_unbounded_array, IsUnboundedArray, KEYCXX)
 TYPE_TRAIT_1(__is_nullptr, IsNullPointer, KEYCXX)
 TYPE_TRAIT_1(__is_scoped_enum, IsScopedEnum, KEYCXX)
 TYPE_TRAIT_1(__is_referenceable, IsReferenceable, KEYCXX)
 TYPE_TRAIT_1(__can_pass_in_regs, CanPassInRegs, KEYCXX)
 TYPE_TRAIT_2(__reference_binds_to_temporary, ReferenceBindsToTemporary, KEYCXX)
+TYPE_TRAIT_2(__reference_constructs_from_temporary, ReferenceConstructsFromTemporary, KEYCXX)
 
 // Embarcadero Expression Traits
 EXPRESSION_TRAIT(__is_lvalue_expr, IsLValueExpr, KEYCXX)
 EXPRESSION_TRAIT(__is_rvalue_expr, IsRValueExpr, KEYCXX)
 
 // Embarcadero Unary Type Traits
 TYPE_TRAIT_1(__is_arithmetic, IsArithmetic, KEYCXX)
 TYPE_TRAIT_1(__is_floating_point, IsFloatingPoint, KEYCXX)

clang/lib/Lex/PPMacroExpansion.cpp

EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, false,
           if (II->getName().startswith("__builtin_") ||
               II->getName().startswith("__is_") ||
               II->getName().startswith("__has_"))
             return true;
           return llvm::StringSwitch<bool>(II->getName())
               .Case("__array_rank", true)
               .Case("__array_extent", true)
               .Case("__reference_binds_to_temporary", true)
+              .Case("__reference_constructs_from_temporary", true)
 #define TRANSFORM_TYPE_TRAIT_DEF(_, Trait) .Case("__" #Trait, true)
 #include "clang/Basic/TransformTypeTraits.def"
               .Default(false);
         } else {
           return llvm::StringSwitch<bool>(II->getName())
               // Report builtin templates as being builtins.
               .Case("__make_integer_seq", getLangOpts().CPlusPlus)
               .Case("__type_pack_element", getLangOpts().CPlusPlus)

clang/lib/Parse/ParseDeclCXX.cpp

#include "clang/Basic/TransformTypeTraits.def"
           tok::kw___is_trivially_equality_comparable,
           tok::kw___is_trivially_assignable,
           tok::kw___is_trivially_constructible,
           tok::kw___is_trivially_copyable,
           tok::kw___is_unbounded_array,
           tok::kw___is_union,
           tok::kw___is_unsigned,
           tok::kw___is_void,
-          tok::kw___is_volatile))
+          tok::kw___is_volatile,
+          tok::kw___reference_binds_to_temporary,
+          tok::kw___reference_constructs_from_temporary))
     // GNU libstdc++ 4.2 and libc++ use certain intrinsic names as the
     // name of struct templates, but some are keywords in GCC >= 4.3
     // and Clang. Therefore, when we see the token sequence "struct
     // X", make X into a normal identifier rather than a keyword, to
     // allow libstdc++ 4.2 and libc++ to work properly.
     TryKeywordIdentFallback(true);
 
   struct PreserveAtomicIdentifierInfoRAII {

clang/lib/Parse/ParseExpr.cpp

#define REVERTIBLE_TYPE_TRAIT(Name)                         \
           REVERTIBLE_TYPE_TRAIT(__is_trivially_assignable);
           REVERTIBLE_TYPE_TRAIT(__is_trivially_constructible);
           REVERTIBLE_TYPE_TRAIT(__is_trivially_copyable);
           REVERTIBLE_TYPE_TRAIT(__is_unbounded_array);
           REVERTIBLE_TYPE_TRAIT(__is_union);
           REVERTIBLE_TYPE_TRAIT(__is_unsigned);
           REVERTIBLE_TYPE_TRAIT(__is_void);
           REVERTIBLE_TYPE_TRAIT(__is_volatile);
+          REVERTIBLE_TYPE_TRAIT(__reference_binds_to_temporary);
+          REVERTIBLE_TYPE_TRAIT(__reference_constructs_from_temporary);
 #define TRANSFORM_TYPE_TRAIT_DEF(_, Trait)                                     \
   REVERTIBLE_TYPE_TRAIT(RTT_JOIN(__, Trait));
 #include "clang/Basic/TransformTypeTraits.def"
 #undef REVERTIBLE_TYPE_TRAIT
 #undef RTT_JOIN
         }
 
         // If we find that this is in fact the name of a type trait,

clang/lib/Sema/SemaExprCXX.cpp

 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/AlignedAllocation.h"
 #include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Basic/TypeTraits.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
static bool EvaluateBooleanTypeTrait(Sema &S, TypeTrait Kind,
                                      SourceLocation RParenLoc,
                                      bool IsDependent) {
   if (IsDependent)
     return false;
 
   if (Kind <= UTT_Last)
     return EvaluateUnaryTypeTrait(S, Kind, KWLoc, Args[0]->getType());
 
-  // Evaluate BTT_ReferenceBindsToTemporary alongside the IsConstructible
-  // traits to avoid duplication.
-  if (Kind <= BTT_Last && Kind != BTT_ReferenceBindsToTemporary)
+  // Evaluate ReferenceBindsToTemporary and ReferenceConstructsFromTemporary
+  // alongside the IsConstructible traits to avoid duplication.
+  if (Kind <= BTT_Last && Kind != BTT_ReferenceBindsToTemporary && Kind != BTT_ReferenceConstructsFromTemporary)
     return EvaluateBinaryTypeTrait(S, Kind, Args[0]->getType(),
                                    Args[1]->getType(), RParenLoc);
 
   switch (Kind) {
   case clang::BTT_ReferenceBindsToTemporary:
+  case clang::BTT_ReferenceConstructsFromTemporary:
   case clang::TT_IsConstructible:
   case clang::TT_IsNothrowConstructible:
   case clang::TT_IsTriviallyConstructible: {
     // C++11 [meta.unary.prop]:
     //   is_trivially_constructible is defined as:
     //
     //     is_constructible<T, Args...>::value is true and the variable
     //     definition for is_constructible, as defined below, is known to call
case clang::TT_IsTriviallyConstructible: {
 
     ExprResult Result = Init.Perform(S, To, InitKind, ArgExprs);
     if (Result.isInvalid() || SFINAE.hasErrorOccurred())
       return false;
 
     if (Kind == clang::TT_IsConstructible)
       return true;
 
-    if (Kind == clang::BTT_ReferenceBindsToTemporary) {
+    if (Kind == clang::BTT_ReferenceBindsToTemporary || Kind == clang::BTT_ReferenceConstructsFromTemporary) {
       if (!T->isReferenceType())
         return false;
 
-      return !Init.isDirectReferenceBinding();
+      if (!Init.isDirectReferenceBinding())
+        return true;
+
+      if (Kind == clang::BTT_ReferenceBindsToTemporary)
+        return false;
+
+      QualType U = Args[1]->getType();
+      if (U->isReferenceType())
+        return false;
+
+      QualType TPtr = S.BuiltinAddPointer(S.BuiltinRemoveReference(T, UnaryTransformType::RemoveCVRef, {}), {});
+      QualType UPtr = S.BuiltinAddPointer(S.BuiltinRemoveReference(U, UnaryTransformType::RemoveCVRef, {}), {});
+      return EvaluateBinaryTypeTrait(S, TypeTrait::BTT_IsConvertibleTo, UPtr, TPtr, RParenLoc);
     }
 
     if (Kind == clang::TT_IsNothrowConstructible)
       return S.canThrow(Result.get()) == CT_Cannot;
 
     if (Kind == clang::TT_IsTriviallyConstructible) {
       // Under Objective-C ARC and Weak, if the destination has non-trivial
       // Objective-C lifetime, this is a non-trivial construction.
case BTT_TypeCompatible: {
     // GCC ignores cv-qualifiers on arrays for this builtin.
     Qualifiers LhsQuals, RhsQuals;
     QualType Lhs = Self.getASTContext().getUnqualifiedArrayType(LhsT, LhsQuals);
     QualType Rhs = Self.getASTContext().getUnqualifiedArrayType(RhsT, RhsQuals);
     return Self.Context.typesAreCompatible(Lhs, Rhs);
   }
   case BTT_IsConvertible:
   case BTT_IsConvertibleTo: {
     // C++0x [meta.rel]p4:
     //   Given the following function prototype:
     //
     //     template <class T>
     //       typename add_rvalue_reference<T>::type create();
     //
     //   the predicate condition for a template specialization
     //   is_convertible<From, To> shall be satisfied if and only if
     //   the return expression in the following code would be

clang/test/SemaCXX/type-traits.cpp

void reference_binds_to_temporary_checks() {
   { int arr[T((__reference_binds_to_temporary(int const &, LongRef)))]; }
 
   // Test that it doesn't accept non-reference types as input.
   { int arr[F((__reference_binds_to_temporary(int, long)))]; }
 
   { int arr[T((__reference_binds_to_temporary(const int &, long)))]; }
 }
 
+void reference_constructs_from_temporary_checks() {
+  static_assert(!__reference_constructs_from_temporary(int &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int const &, int &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int const &), "");
+  static_assert(!__reference_constructs_from_temporary(int const &, int &&), "");
+
+  static_assert(!__reference_constructs_from_temporary(int &, long &), ""); // doesn't construct
+
+  static_assert(__reference_constructs_from_temporary(int const &, long &), "");
+  static_assert(__reference_constructs_from_temporary(int const &, long &&), "");
+  static_assert(__reference_constructs_from_temporary(int &&, long &), "");
+
+  using LRef = ConvertsToRef<int, int &>;
+  using RRef = ConvertsToRef<int, int &&>;
+  using CLRef = ConvertsToRef<int, const int &>;
+  using LongRef = ConvertsToRef<long, long &>;
+  static_assert(__is_constructible(int &, LRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &, LRef), "");
+
+  static_assert(__is_constructible(int &&, RRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, RRef), "");
+
+  static_assert(__is_constructible(int const &, CLRef), "");
+  static_assert(!__reference_constructs_from_temporary(int &&, CLRef), "");
+
+  static_assert(__is_constructible(int const &, LongRef), "");
+  static_assert(__reference_constructs_from_temporary(int const &, LongRef), "");
+
+  // Test that it doesn't accept non-reference types as input.
+  static_assert(!__reference_constructs_from_temporary(int, long), "");
+
+  static_assert(__reference_constructs_from_temporary(const int &, long), "");
+
+  // Additional checks
+  static_assert(__reference_constructs_from_temporary(POD const&, Derives), "");
+  static_assert(__reference_constructs_from_temporary(int&&, int), "");
+  static_assert(__reference_constructs_from_temporary(const int&, int), "");
+  static_assert(!__reference_constructs_from_temporary(int&&, int&&), "");
+  static_assert(!__reference_constructs_from_temporary(const int&, int&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long&&), "");
+  static_assert(__reference_constructs_from_temporary(int&&, long), "");
+}
+
 void array_rank() {
   int t01[T(__array_rank(IntAr) == 1)];
   int t02[T(__array_rank(ConstIntArAr) == 2)];
 }
 
 void array_extent() {
   int t01[T(__array_extent(IntAr, 0) == 10)];
   int t02[T(__array_extent(ConstIntArAr, 0) == 4)];

clang/www/cxx_status.html

<tr>
       <td><a href="https://wg21.link/P0533R9">P0533R9</a></td>
       <td class="none" align="center">No</td>
     </tr>
     <tr>
       <td>Type trait to determine if a reference binds to a temporary</td>
       <td><a href="https://wg21.link/P2255R2">P2255R2</a></td>
       <td class="partial" align="center">
         <details><summary>Partial</summary>
-          Clang provides a <tt>__reference_binds_to_temporary</tt> type trait
-          builtin, with which the library facility can be partially implemented.
-          Both <tt>__reference_constructs_from_temporary</tt> and
-          <tt>__reference_converts_from_temporary</tt> builtins should be
+          Clang provides <tt>__reference_constructs_from_temporary</tt> type
+          trait builtin, with which <tt>std::reference_constructs_from_temporary</tt>
+          implemented. <tt>__reference_converts_from_temporary</tt> needs to be
           provided, following the normal cross-vendor convention to implement
           traits requiring compiler support directly.
         </details></td>
       </td>
     </tr>
     <!-- July 2022 papers -->
     <tr>
       <td>The Equality Operator You Are Looking For</td>