Avoid u8"" literals in tests, their type changes in C++20

[massberg]

Summary

Just specify the encoded bytes instead.

[ilya-biryukov]

Are there any uses of u8 string literals with formatted_raw_ostream outside tests? What was the effect on the final behavior here?

I just wanted to make sure we are not doing the wrong thing here. If the code used to compile and it now changes behavior, we may run into runtime failures in real code.
If that's the case, we could potentially consider adding overloads for u8 string literals to either support it or with =delete to make sure the code using them will be rewritten.

[massberg]

In D141392#4040037, @ilya-biryukov wrote:

Are there any uses of u8 string literals with formatted_raw_ostream outside tests? What was the effect on the final behavior here?

I just wanted to make sure we are not doing the wrong thing here. If the code used to compile and it now changes behavior, we may run into runtime failures in real code.
If that's the case, we could potentially consider adding overloads for u8 string literals to either support it or with =delete to make sure the code using them will be rewritten.

u8"" literals (respectively char8_t*) used with ostreams changed behavior in C++20 in an unexpected way, see e.g.
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1423r1.html#option7
So the right way might be to ensure that they cannot be used with osteams.

[massberg]

An alternative is to cast the char8_t* to char_t* in the << operator of the ostream of to get back the old behavior, e.g. something like

raw_ostream &operator<<(const char8_t *Str) {
    return this->operator<<(reinterpret_cast<const char *>(Str));
}

[massberg]

Delete insertion operator of raw_ostream for char8_t.

[ilya-biryukov]

LGTM, but please address the NITs before submitting.

llvm/include/llvm/Support/raw_ostream.h

231

NIT: is there a typo? should this be 'from C++20 on' or 'from C++20 onward'?

236

NIT: there's a typo, should be reinterpret_cast

llvm/unittests/Support/formatted_raw_ostream_test.cpp

102

NIT: reinterpret_cast seems better there as the character code is directly written above

109

NIT: reinterpret_cast also seems better here

115

NIT: reinterpret_cast also seems better here

122

NIT: reinterpret_cast also seems better here

140

NIT: reinterpret_cast also seems better here

148

NIT: reinterpret_cast also seems better here

[massberg]

Fixed nits. Thanks for the comments!

[massberg]

Thanks for the comments!

llvm/include/llvm/Support/raw_ostream.h

raw_ostream &operator<<(StringRef Str) {
 
     if (Size) {
       memcpy(OutBufCur, Str.data(), Size);
       OutBufCur += Size;
     }
     return *this;
   }
 
+#if defined(__cpp_char8_t)
+  // When using `char8_t *` integers or pointers are written to the ostream
+  // instead of UTF-8 code as one might expect. This might lead to unexpected
+  // behavior, especially as `u8""` literals are of type `char8_t*` instead of
+  // type `char_t*` from C++20 onwards. Thus we disallow using them with
+  // raw_ostreams.
+  // If you have u8"" literals to stream, you can rewrite them as ordinary
+  // literals with escape sequences
+  // e.g.  replace `u8"\u00a0"` by `"\xc2\xa0"`
+  // or use `reinterpret_cast`:
+  // e.g. replace `u8"\u00a0"` by `reinterpret_cast<const char *>(u8"\u00a0")`
+  raw_ostream &operator<<(const char8_t *Str) = delete;
+#endif
+
   raw_ostream &operator<<(const char *Str) {
     // Inline fast path, particularly for constant strings where a sufficiently
     // smart compiler will simplify strlen.
 
     return this->operator<<(StringRef(Str));
   }
 
   raw_ostream &operator<<(const std::string &Str) {

llvm/unittests/Support/formatted_raw_ostream_test.cpp

 
 TEST(formatted_raw_ostreamTest, Test_UTF8) {
   SmallString<128> A;
   raw_svector_ostream B(A);
   B.SetBufferSize(32);
   formatted_raw_ostream C(B);
 
   // U+00A0 Non-breaking space: encoded as two bytes, but only one column wide.
-  C << u8"\u00a0";
+  C << "\xc2\xa0";
   EXPECT_EQ(0U, C.getLine());
   EXPECT_EQ(1U, C.getColumn());
   EXPECT_EQ(2U, C.GetNumBytesInBuffer());
 
   // U+2468 CIRCLED DIGIT NINE: encoded as three bytes, but only one column
   // wide.
-  C << u8"\u2468";
+  C << reinterpret_cast<const char *>(u8"\u2468");
   EXPECT_EQ(0U, C.getLine());
   EXPECT_EQ(2U, C.getColumn());
   EXPECT_EQ(5U, C.GetNumBytesInBuffer());
 
   // U+00010000 LINEAR B SYLLABLE B008 A: encoded as four bytes, but only one
   // column wide.
-  C << u8"\U00010000";
+  C << reinterpret_cast<const char *>(u8"\U00010000");
   EXPECT_EQ(0U, C.getLine());
   EXPECT_EQ(3U, C.getColumn());
   EXPECT_EQ(9U, C.GetNumBytesInBuffer());
 
   // U+55B5, CJK character, encodes as three bytes, takes up two columns.
-  C << u8"\u55b5";
+  C << reinterpret_cast<const char *>(u8"\u55b5");
   EXPECT_EQ(0U, C.getLine());
   EXPECT_EQ(5U, C.getColumn());
   EXPECT_EQ(12U, C.GetNumBytesInBuffer());
 
   // U+200B, zero-width space, encoded as three bytes but has no effect on the
   // column or line number.
-  C << u8"\u200b";
+  C << reinterpret_cast<const char *>(u8"\u200b");
   EXPECT_EQ(0U, C.getLine());
   EXPECT_EQ(5U, C.getColumn());
   EXPECT_EQ(15U, C.GetNumBytesInBuffer());
 }
 
 TEST(formatted_raw_ostreamTest, Test_UTF8Buffered) {
   SmallString<128> A;
   raw_svector_ostream B(A);
   B.SetBufferSize(4);
   formatted_raw_ostream C(B);
 
   // U+2468 encodes as three bytes, so will cause the buffer to be flushed after
   // the first byte (4 byte buffer, 3 bytes already written). We need to save
   // the first part of the UTF-8 encoding until after the buffer is cleared and
   // the remaining two bytes are written, at which point we can check the
   // display width. In this case the display width is 1, so we end at column 4,
   // with 6 bytes written into total, 2 of which are in the buffer.
-  C << u8"123\u2468";
+  C << reinterpret_cast<const char *>(u8"123\u2468");
   EXPECT_EQ(0U, C.getLine());
   EXPECT_EQ(4U, C.getColumn());
   EXPECT_EQ(2U, C.GetNumBytesInBuffer());
   C.flush();
   EXPECT_EQ(6U, A.size());
 
   // Same as above, but with a CJK character which displays as two columns.
-  C << u8"123\u55b5";
+  C << reinterpret_cast<const char *>(u8"123\u55b5");
   EXPECT_EQ(0U, C.getLine());
   EXPECT_EQ(9U, C.getColumn());
   EXPECT_EQ(2U, C.GetNumBytesInBuffer());
   C.flush();
   EXPECT_EQ(12U, A.size());
 }
 
 TEST(formatted_raw_ostreamTest, Test_UTF8TinyBuffer) {
   SmallString<128> A;
   raw_svector_ostream B(A);
   B.SetBufferSize(1);
   formatted_raw_ostream C(B);
 
   // The stream has a one-byte buffer, so it gets flushed multiple times while
   // printing a single Unicode character.
-  C << u8"\u2468";
+  C << "\xe2\x91\xa8";
   EXPECT_EQ(0U, C.getLine());
   EXPECT_EQ(1U, C.getColumn());
   EXPECT_EQ(0U, C.GetNumBytesInBuffer());
   C.flush();
   EXPECT_EQ(3U, A.size());
 }
 }