[Sema] Improve const_cast conformance to N4261

[Mordante]

Summary

Allows const_casts of similar non-record types.

[rsmith]

clang/lib/Sema/SemaCast.cpp

1781–1782

This is the bullet that causes class pvalues and non-class prvalues to be treated differently by const_cast to an rvalue reference type, and it still exists unchanged in the latest C++ draft. As far as I can tell, the change here is not in line with the wording for N4261 / CWG issue 330, which Clang does implement.

(Now, I think the change you're suggesting here makes a lot of sense, but we should get agreement in WG21 to fix the language rules first.)

1797–1801

In (for example) const_cast<int&&>(0);, T1 is int, and T2 is "rvalue reference to int". T1 and T2 are not similar types, because they have different types U (for T1, U is int; for T2, U is "rvalue reference to int"). So this doesn't apply.

[rsmith]

For what it's worth, the most recent thoughts from CWG on this question are from 2015 (prior to guaranteed copy elision, which probably changes the answer): http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1965. That direction would permit const_cast to rvalue reference type to materialize a temporary for arrays and classes but not scalar types (which seems just as arbitrary as the current set of rules to me).

[Mordante]

Thanks for the additional information. I agree that the current situation seems a bit arbitrary.
I did some further research to see what GCC allows https://godbolt.org/. I've updated the patch to match that behavior.

[Mordante]

Match the behavior of const_cast to match GCC's behavior. N4261 was proposed as solution to DR330. Since GCC allows the new const_cast behavior retroactively the patch also matches that behavior and no longer restricts the new usage to C++17.

[rsmith]

clang/lib/Sema/SemaCast.cpp

1806–1808

It looks like GCC allowing a cast from T* to T*&& is just a bug in their implementation. Consider:

using T = int*;
T &&r1 = const_cast<T&&>(T{}); // GCC accepts
T &&r2 = const_cast<T&&>(T()); // GCC rejects

... and the same behavior seems to show up for all scalar types: they permit const_cast from T{} but not from T() when T is int, int*, .... This doesn't seem like good behavior to follow. I think we should either implement the current direction of 1965 (that is, only allow classes and arrays here) or leave the current behavior (following the standard as-is) alone.

[Mordante]

clang/lib/Sema/SemaCast.cpp

1806–1808

It looks like GCC allowing a cast from T* to T*&& is just a bug in their implementation. Consider:

using T = int*;
T &&r1 = const_cast<T&&>(T{}); // GCC accepts
T &&r2 = const_cast<T&&>(T()); // GCC rejects

... and the same behavior seems to show up for all scalar types: they permit const_cast from T{} but not from T() when T is int, int*, ....

Interesting I hadn't test with T() only with T{}

This doesn't seem like good behavior to follow.

Agreed.

I think we should either implement the current direction of 1965 (that is, only allow classes and arrays here)

I'll then limit the patch to array types (next to the already allowed record types.

or leave the current behavior (following the standard as-is) alone.

Now I'm somewhat confused, because I thought arrays are allowed in the standard http://eel.is/c++draft/expr.const.cast#3 as introduced by N4261.

[Mordante]

Removes the support for the pointer types.

I adjusted the unit test for void f(), but I think it would be better remove this test now pointers are no longer allowed. Do you agree?

[Mordante]

Friendly ping.

clang/lib/Sema/SemaCast.cpp

 /// and perform it if it is.
 static TryCastResult TryConstCast(Sema &Self, ExprResult &SrcExpr,
                                   QualType DestType, bool CStyle,
                                   unsigned &msg) {
   DestType = Self.Context.getCanonicalType(DestType);
   QualType SrcType = SrcExpr.get()->getType();
   bool NeedToMaterializeTemporary = false;
 
-  if (const ReferenceType *DestTypeTmp =DestType->getAs<ReferenceType>()) {
+  if (const ReferenceType *DestTypeTmp = DestType->getAs<ReferenceType>()) {
     // C++11 5.2.11p4:
     //   if a pointer to T1 can be explicitly converted to the type "pointer to
     //   T2" using a const_cast, then the following conversions can also be
     //   made:
     //    -- an lvalue of type T1 can be explicitly converted to an lvalue of
     //       type T2 using the cast const_cast<T2&>;
     //    -- a glvalue of type T1 can be explicitly converted to an xvalue of
     //       type T2 using the cast const_cast<T2&&>; and
     //    -- if T1 is a class type, a prvalue of type T1 can be explicitly
     //       converted to an xvalue of type T2 using the cast const_cast<T2&&>.
 
     if (isa<LValueReferenceType>(DestTypeTmp) && !SrcExpr.get()->isLValue()) {
       // Cannot const_cast non-lvalue to lvalue reference type. But if this
       // is C-style, static_cast might find a way, so we simply suggest a
       // message and tell the parent to keep searching.
       msg = diag::err_bad_cxx_cast_rvalue;
       return TC_NotApplicable;
     }
 
     if (isa<RValueReferenceType>(DestTypeTmp) && SrcExpr.get()->isRValue()) {
-      if (!SrcType->isRecordType()) {
+      // Materialize the class prvalue so that the const_cast can bind a
+      // reference to it.
+      NeedToMaterializeTemporary = SrcType->isRecordType();
+
+      // C++17 [expr.const.cast]p3
+      // For two similar types T1 and T2, a prvalue of type T1 may be explicitly
+      // converted to the type T2 using a const_cast if, considering the
+      // cv-decompositions of both types, each P1i is the same as P2i for all i.
+      // The result of a const_cast refers to the original entity.
+      //
+      // Prior to C++17 only RecordTypes were allowed.
+      if (!NeedToMaterializeTemporary && !Self.getLangOpts().CPlusPlus17) {
         // Cannot const_cast non-class prvalue to rvalue reference type. But if
         // this is C-style, static_cast can do this.
         msg = diag::err_bad_cxx_cast_rvalue;
         return TC_NotApplicable;
       }
-
-      // Materialize the class prvalue so that the const_cast can bind a
-      // reference to it.
-      NeedToMaterializeTemporary = true;
     }
 
     // It's not completely clear under the standard whether we can
     // const_cast bit-field gl-values.  Doing so would not be
     // intrinsically complicated, but for now, we say no for
     // consistency with other compilers and await the word of the
     // committee.
     if (SrcExpr.get()->refersToBitField()) {

clang/test/SemaCXX/const-cast.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
 struct A {};
 
 // See if aliasing can confuse this baby.
 typedef char c;
 typedef c *cp;
 typedef cp *cpp;
 typedef cpp *cppp;
short *bad_const_cast_test(char const *volatile *const volatile *var)
   (void) const_cast<int *(*)[]>(&ar); // expected-error {{const_cast from 'const int *(*)[100]' to 'int *(*)[]' is not allowed}}
   (void) const_cast<int *(*)[99]>(&ar); // expected-error {{const_cast from 'const int *(*)[100]' to 'int *(*)[99]' is not allowed}}
   (void) const_cast<int *(*)[100]>(&aub); // expected-error {{const_cast from 'const int *(*)[]' to 'int *(*)[100]' is not allowed}}
   f fp1 = 0;
   // Function pointers.
   f fp2 = const_cast<f>(fp1); // expected-error {{const_cast to 'f' (aka 'int (*)(int)'), which is not a reference, pointer-to-object, or pointer-to-data-member}}
   void (A::*mfn)() = 0;
   (void)const_cast<void (A::*)()>(mfn); // expected-error-re {{const_cast to 'void (A::*)(){{( __attribute__\(\(thiscall\)\))?}}', which is not a reference, pointer-to-object, or pointer-to-data-member}}
-  (void)const_cast<int&&>(0); // expected-error {{const_cast from rvalue to reference type 'int &&'}}
+  (void)const_cast<int&&>(0);
+#if __cplusplus < 201703L // N4261
+  // expected-error@-2 {{const_cast from rvalue to reference type 'int &&'}}
+#endif
 #if __cplusplus <= 199711L // C++03 or earlier modes
-  // expected-warning@-2 {{rvalue references are a C++11 extension}}
+  // expected-warning@-5 {{rvalue references are a C++11 extension}}
 #endif
   return **var3;
 }
 
 template <typename T>
 char *PR21845() { return const_cast<char *>((void)T::x); } // expected-error {{const_cast from 'void' to 'char *' is not allowed}}
+
+#if __cplusplus >= 201103L
+namespace N4261 {
+typedef int *A[3];
+typedef const int *const CA[3];
+
+CA &&r = A{};
+A &&r1 = const_cast<A>(CA{}); // expected-error {{const_cast to 'N4261::A' (aka 'int *[3]'), which is not a reference, pointer-to-object, or pointer-to-data-member}}
+
+A &&r2 = const_cast<A &&>(CA{});
+#if __cplusplus < 201703L
+// expected-error@-2 {{const_cast from rvalue to reference type 'N4261::A &&' (aka 'int *(&&)[3]')}}
+#endif
+} // namespace N4261
+#endif