[Clang] Implement P2513

[cor3ntin]

Summary

Implement P2513

This change allows initializing an array of unsigned char,
or char from u8 string literals.
This was done both to support legacy code and for compatibility
with C where char8_t will be typedef to unsigned char.

This is backported to C++20 as per WG21 guidance.

[cor3ntin]

Remove unused diagnostic

[aaron.ballman]

Thanks for this! Can you add more details to the patch summary as to what this paper does? Paper numbers help, but don't convey a whole lot of information at a glance if we need to come back to this review for code archeology.

Please also add a test to the correct file in clang/test/CXX/drs/ and regenerate our C++ DR status page.

Are we missing changes for "with an integral conversion [conv.integral] if necessary for the source and destination value." ?

clang/lib/Frontend/InitPreprocessor.cpp

701

Why do we not want to go with 202207L regardless of language mode if this is being applied as a DR?

clang/lib/Sema/SemaInit.cpp

4466–4467

Unintentional indentation changes?

5858–5862

Unrelated change?

[cor3ntin]

In D136449#3874867, @aaron.ballman wrote:

Thanks for this! Can you add more details to the patch summary as to what this paper does? Paper numbers help, but don't convey a whole lot of information at a glance if we need to come back to this review for code archeology.

Please also add a test to the correct file in clang/test/CXX/drs/ and regenerate our C++ DR status page.

I'll check but i don't think we have a number of that, we just decided to backport the paper

Are we missing changes for "with an integral conversion [conv.integral] if necessary for the source and destination value." ?

Nope, that's taken care of afaict, I'm trying to think of a test

[aaron.ballman]

In D136449#3875159, @cor3ntin wrote:

In D136449#3874867, @aaron.ballman wrote:

Thanks for this! Can you add more details to the patch summary as to what this paper does? Paper numbers help, but don't convey a whole lot of information at a glance if we need to come back to this review for code archeology.

Please also add a test to the correct file in clang/test/CXX/drs/ and regenerate our C++ DR status page.

I'll check but i don't think we have a number of that, we just decided to backport the paper

Should we ask CWG for an issue # for it so that it's traceable whether we do/don't implement the DR, or is that overkill?

Are we missing changes for "with an integral conversion [conv.integral] if necessary for the source and destination value." ?

Nope, that's taken care of afaict, I'm trying to think of a test

Ah, a test would be appreciated then!

[cor3ntin]

Improve commit message, check proper integral conversions,
remove ws changes.

[cor3ntin]

clang/lib/Frontend/InitPreprocessor.cpp

701

There is a -fchar8_t that is an extension before C++20 and we are not touching that. It is pre-existing that the feature test macro was defined before c++20 if that flag was set.

[aaron.ballman]

clang/lib/Frontend/InitPreprocessor.cpp

701

Ahhhh, I see now that there's a (small) potential for breakage (thank you for that test case btw) and so you want to leave the extension flag behavior alone. We usually don't do that (usually these sort of extension flags enable the feature as it exists in the language mode enabling support for it) unless there's a good reason to do so. My thinking is:

The potential for breakage is small enough we're not even going to call it out as a potentially disruptive change in the release notes. Presuming there are likely more people using -std=c++20 than there are using -fchar8_t, changing -fchar8_t seems like less risky behavior than changing -std=c++20. So I think it's reasonable for us to enable this functionality for -fchar8_t as well because we're already assuming the larger risk.

WDYT?

[cor3ntin]

Enable the change in -fchar8_t mode as per
suggestion from Aaron.

[aaron.ballman]

LGTM aside from a missing full stop in a comment. Tom's on vacation most of this week, so if he has any concerns, they can be address post-commit unless you want to wait for him (which is also totally fine by me).

clang/lib/Sema/SemaInit.cpp

94

[cor3ntin]

Thanks Aaron!

clang/docs/ReleaseNotes.rst

- Skip rebuilding lambda expressions in arguments of immediate invocations.
   `Issue 50455 <https://github.com/llvm/llvm-project/issues/50455>`_,
   `Issue 54872 <https://github.com/llvm/llvm-project/issues/54872>`_,
   `Issue 54587 <https://github.com/llvm/llvm-project/issues/54587>`_.
 
 C++2b Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 - Support label at end of compound statement (`P2324 <https://wg21.link/p2324r2>`_).
+- Implemented "char8_t Compatibility and Portability Fix" (`P2513R3 <https://wg21.link/P2513R3>`_).
+  This Change was applied to C++20 as a Defect Report.
 
 CUDA/HIP Language Changes in Clang
 ----------------------------------
 
 Objective-C Language Changes in Clang
 -------------------------------------
 
 OpenCL C Language Changes in Clang

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def err_array_init_wide_string_into_char : Error<
   "initializing char array with wide string literal">;
 def err_array_init_incompat_wide_string_into_wchar : Error<
   "initializing wide char array with incompatible wide string literal">;
 def err_array_init_plain_string_into_char8_t : Error<
   "initializing 'char8_t' array with plain string literal">;
 def note_array_init_plain_string_into_char8_t : Note<
   "add 'u8' prefix to form a 'char8_t' string literal">;
+def err_array_init_utf8_string_into_signed_char : Error<
+  "initialization of signed char array with "
+  "UTF-8 string literal is not permitted">;
 def err_array_init_utf8_string_into_char : Error<
-  "%select{|ISO C++20 does not permit }0initialization of char array with "
-  "UTF-8 string literal%select{ is not permitted by '-fchar8_t'|}0">;
+  "initialization of %select{|signed }0char array with "
+  "UTF-8 string literal is not permitted by %select{'-fchar8_t'|C++20}1">;
 def warn_cxx20_compat_utf8_string : Warning<
   "type of UTF-8 string literal will change from array of const char to "
   "array of const char8_t in C++20">, InGroup<CXX20Compat>, DefaultIgnore;
 def note_cxx20_compat_utf8_string_remove_u8 : Note<
   "remove 'u8' prefix to avoid a change of behavior; "
   "Clang encodes unprefixed narrow string literals as UTF-8">;
 def err_array_init_different_type : Error<
   "cannot initialize array %diff{of type $ with array of type $|"

clang/lib/Frontend/InitPreprocessor.cpp

static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,
   // C++2b features.
   if (LangOpts.CPlusPlus2b) {
     Builder.defineMacro("__cpp_implicit_move", "202011L");
     Builder.defineMacro("__cpp_size_t_suffix", "202011L");
     Builder.defineMacro("__cpp_if_consteval", "202106L");
     Builder.defineMacro("__cpp_multidimensional_subscript", "202110L");
   }
   if (LangOpts.Char8)
-    Builder.defineMacro("__cpp_char8_t", "201811L");
+    Builder.defineMacro("__cpp_char8_t",
+                        LangOpts.CPlusPlus20 ? "202207L" : "201811L");
   Builder.defineMacro("__cpp_impl_destroying_delete", "201806L");
 
   // TS features.
   if (LangOpts.Coroutines)
     Builder.defineMacro("__cpp_coroutines", "201703L");
 }
 
 /// InitializeOpenCLFeatureTestMacros - Define OpenCL macros based on target

clang/lib/Sema/SemaInit.cpp

static StringInitFailureKind IsStringInit(Expr *Init, const ArrayType *AT,
   // Otherwise we can only handle string literals.
   StringLiteral *SL = dyn_cast<StringLiteral>(Init);
   if (!SL)
     return SIF_Other;
 
   const QualType ElemTy =
       Context.getCanonicalType(AT->getElementType()).getUnqualifiedType();
 
+  auto IsCharOrUnsignedChar = [](const QualType &T) {
+    const BuiltinType *BT = dyn_cast<BuiltinType>(T.getTypePtr());
+    return BT && BT->isCharType() && BT->getKind() != BuiltinType::SChar;
+  };
+
   switch (SL->getKind()) {
   case StringLiteral::UTF8:
     // char8_t array can be initialized with a UTF-8 string.
-    if (ElemTy->isChar8Type())
+    // - C++20 [dcl.init.string] (DR)
+    //   Additionally, an array of char or unsigned char may be initialized
+    //   by a UTF-8 string literal
+    if (ElemTy->isChar8Type() ||
+        (Context.getLangOpts().CPlusPlus20 &&
+         IsCharOrUnsignedChar(ElemTy.getCanonicalType())))
       return SIF_None;
     [[fallthrough]];
   case StringLiteral::Ordinary:
     // char array can be initialized with a narrow string.
     // Only allow char x[] = "foo";  not char x[] = L"foo";
     if (ElemTy->isCharType())
       return (SL->getKind() == StringLiteral::UTF8 &&
               Context.getLangOpts().Char8)
if (const ArrayType *DestAT = S.Context.getAsArrayType(DestType)) {
 
         // TryStringLiteralInitialization() (in InitializeFrom()) will fail if
         // the element is not an appropriately-typed string literal, in which
         // case we should proceed as in C++11 (below).
         if (Sequence) {
           Sequence.RewrapReferenceInitList(Entity.getType(), InitList);
           return;
         }
         }
       }
   }
 
   // C++11 [dcl.init.list]p3:
   //   - If T is an aggregate, aggregate initialization is performed.
   if ((DestType->isRecordType() && !DestType->isAggregateType()) ||
       (S.getLangOpts().CPlusPlus11 &&
        S.isStdInitializerList(DestType, nullptr))) {
     if (S.getLangOpts().CPlusPlus11) {
if (Initializer) {
         SetFailed(FK_WideStringIntoCharArray);
         return;
       case SIF_IncompatWideStringIntoWideChar:
         SetFailed(FK_IncompatWideStringIntoWideChar);
         return;
       case SIF_PlainStringIntoUTF8Char:
         SetFailed(FK_PlainStringIntoUTF8Char);
         return;
-      case SIF_UTF8StringIntoPlainChar:
+      case SIF_UTF8StringIntoPlainChar: {
         SetFailed(FK_UTF8StringIntoPlainChar);
         return;
+      }
       case SIF_Other:
         break;
       }
     }
 
     // Some kinds of initialization permit an array to be initialized from
     // another array of the same type, and perform elementwise initialization.
     if (Initializer && isa<ConstantArrayType>(DestAT) &&
         S.Context.hasSameUnqualifiedType(Initializer->getType(),
bool InitializationSequence::Diagnose(Sema &S,
   case FK_PlainStringIntoUTF8Char:
     S.Diag(Kind.getLocation(),
            diag::err_array_init_plain_string_into_char8_t);
     S.Diag(Args.front()->getBeginLoc(),
            diag::note_array_init_plain_string_into_char8_t)
         << FixItHint::CreateInsertion(Args.front()->getBeginLoc(), "u8");
     break;
   case FK_UTF8StringIntoPlainChar:
-    S.Diag(Kind.getLocation(),
-           diag::err_array_init_utf8_string_into_char)
-      << S.getLangOpts().CPlusPlus20;
+    S.Diag(Kind.getLocation(), diag::err_array_init_utf8_string_into_char)
+        << DestType->isSignedIntegerType() << S.getLangOpts().CPlusPlus20;
     break;
   case FK_ArrayTypeMismatch:
   case FK_NonConstantArrayInit:
     S.Diag(Kind.getLocation(),
            (Failure == FK_ArrayTypeMismatch
               ? diag::err_array_init_different_type
               : diag::err_array_init_non_constant_array))
       << DestType.getNonReferenceType()

clang/test/Lexer/cxx-features.cpp

 
 // --- C++20 features ---
 
 #if check(aggregate_paren_init, 0, 0, 0, 0, 0, 0)
 // FIXME: 201902 in C++20
 #error "wrong value for __cpp_aggregate_paren_init"
 #endif
 
-#if defined(CHAR8_T) ? check(char8_t, 201811, 201811, 201811, 201811, 201811, 201811) : \
+#if defined(CHAR8_T) ? check(char8_t, 201811, 201811, 201811, 201811, 202207, 202207) : \
     defined(NO_CHAR8_T) ? check(char8_t, 0, 0, 0, 0, 0, 0) : \
-    check(char8_t, 0, 0, 0, 0, 201811, 201811)
+    check(char8_t, 0, 0, 0, 0, 202207, 202207)
 #error "wrong value for __cpp_char8_t"
 #endif
 
 #if check(concepts, 0, 0, 0, 0, 201907, 201907)
 #error "wrong value for __cpp_concepts"
 #endif
 
 #if check(conditional_explicit, 0, 0, 0, 0, 201806, 201806)

clang/test/SemaCXX/char8_t.cpp

 // RUN: %clang_cc1 -fchar8_t -std=c++17 -verify %s
-// RUN: %clang_cc1 -std=c++2a -verify %s
+// RUN: %clang_cc1 -std=c++2a -verify=expected,cxx20 %s
+// RUN: %clang_cc1 -std=c++2a -verify=expected,cxx20 -fno-signed-char %s
+
 
 char8_t a = u8'a';
 char8_t b[] = u8"foo";
 char8_t c = 'a';
 char8_t d[] = "foo"; // expected-error {{initializing 'char8_t' array with plain string literal}} expected-note {{add 'u8' prefix}}
 
 char e = u8'a';
 char f[] = u8"foo";
 #if __cplusplus <= 201703L
 // expected-error@-2 {{initialization of char array with UTF-8 string literal is not permitted by '-fchar8_t'}}
-#else
-// expected-error@-4 {{ISO C++20 does not permit initialization of char array with UTF-8 string literal}}
 #endif
 char g = 'a';
 char h[] = "foo";
 
+#if __cplusplus >= 201902L
+unsigned char i[] = u8"foo";
+unsigned char j[] = { u8"foo" };
+char k[] = u8"foo";
+char l[] = { u8"foo" };
+signed char m[] = u8"foo"; // expected-error {{initialization of char array with UTF-8 string literal is not permitted by C++20}}
+signed char n[] = { u8"foo" }; // expected-error {{cannot initialize an array element of type 'signed char' with an lvalue of type 'const char8_t[4]'}}
+
+const unsigned char* uptr = u8"foo"; // expected-error {{cannot initialize}}
+const signed char* sptr = u8"foo"; // expected-error {{cannot initialize}}
+const char* ptr = u8"foo"; // expected-error {{cannot initialize}}
+
+#endif
+
 void disambig() {
   char8_t (a) = u8'x';
 }
 
 void operator""_a(char);
 void operator""_a(const char*, decltype(sizeof(0)));
 
 void test_udl1() {

 void check_deduction() {
   check<char8_t>(u8'a');
   check<const char8_t(&)[5]>(u8"a\u1000");
 }
 
 static_assert(sizeof(char8_t) == 1);
 static_assert(char8_t(-1) > 0);
 static_assert(u8"\u0080"[0] > 0);
+
+namespace ambiguous {
+
+struct A {
+  char8_t s[10];
+};
+struct B {
+  char s[10];
+};
+
+void f(A); // cxx20-note {{candidate}}
+void f(B); // cxx20-note {{candidate}}
+
+int test() {
+  f({u8"foo"}); // cxx20-error {{call to 'f' is ambiguous}}
+}
+
+}

clang/test/SemaCXX/cxx2a-compat.cpp

 char u8arr[] = u8"hello";
 const char *u8ptr = "wo" u8"rld";
 string u8str = u8"test" u8"test";
 #if __cplusplus <= 201703L
 // expected-warning@-4 {{type of UTF-8 string literal will change}} expected-note@-4 {{remove 'u8' prefix}}
 // expected-warning@-4 {{type of UTF-8 string literal will change}} expected-note@-4 {{remove 'u8' prefix}}
 // expected-warning@-4 {{type of UTF-8 string literal will change}} expected-note@-4 {{remove 'u8' prefix}}
 #else
-// expected-error@-8 {{ISO C++20 does not permit initialization of char array with UTF-8 string literal}}
-// expected-error@-8 {{cannot initialize a variable of type 'const char *' with an lvalue of type 'const char8_t[6]'}}
-// expected-error@-8 {{no viable conversion from 'const char8_t[9]' to 'string'}}
+// expected-error@-7 {{cannot initialize a variable of type 'const char *' with an lvalue of type 'const char8_t[6]'}}
+// expected-error@-7 {{no viable conversion from 'const char8_t[9]' to 'string'}}
 #endif
 
 template<bool b>
 struct C {
   explicit(C)(int);
 };
 #if __cplusplus <= 201703L
 // expected-warning@-3 {{this expression will be parsed as explicit(bool) in C++20}}

clang/www/cxx_status.html

</tr>
     <tr>
       <td>Support for UTF-8 as a portable source file encoding</td>
       <td><a href="https://wg21.link/P2295R6">P2295R6</a></td>
       <td class="full" align="center">Clang 15</td>
     </tr>
     <tr>
       <td><code>char8_t</code> Compatibility and Portability Fix</td>
       <td><a href="https://wg21.link/P2513R3">P2513R3</a></td>
-      <td class="none" align="center">No</td>
+      <td class="unreleased" align="center">Clang 16</td>
     </tr>
     <tr>
       <td>Relax requirements on <code>wchar_t</code> to match existing practices</td>
       <td><a href="https://wg21.link/P2460R2">P2460R2</a></td>
       <td class="full" align="center">Yes</td>
     </tr>
     <tr>
       <td>Explicit lifetime management</td>