[Clang] Produce a warning instead of an error in unevaluated strings before C++26

[cor3ntin]

Format + Add FixIt tests

[aaron.ballman]

Please add more details to the patch summary explaining why you're making these changes. Also, I assume this is to be backported to Clang 17? If so, I'd like @hubert.reinterpretcast to mention if this works for his needs.

[cor3ntin]

Oups, sorry, the context is this PR https://reviews.llvm.org/D105759#4543246

[aaron.ballman]

In D156596#4546787, @cor3ntin wrote:

Oups, sorry, the context is this PR https://reviews.llvm.org/D105759#4543246

I kind of figured. :-) But we want the context in the patch summary since that's what typically makes it to the commit message, and we don't want folks doing code archeology to have to click through to figure that context out.

[hubert.reinterpretcast]

clang/test/SemaCXX/static-assert.cpp

44–45

I doubt that it is a problem, but Clang 16 accepted such hex escapes.

[cor3ntin]

clang/test/CXX/dcl.dcl/dcl.link/p2.cpp

11–14

Note that was ill-formed in clang 16, despite being well-formed in the standard.

clang/test/SemaCXX/static-assert.cpp

44–45

Yes, I only downgraded to a warning what you had reports about.
I haven't seen that pattern in the wild and i don't think anyone who would use that isn't confused on some level.

[hubert.reinterpretcast]

In D156596#4546742, @aaron.ballman wrote:

If so, I'd like @hubert.reinterpretcast to mention if this works for his needs.

We are putting together a build for the (known) affected groups. I will report back when we have results.

[hubert.reinterpretcast]

clang/include/clang/Basic/DiagnosticLexKinds.td

290

I am not seeing any tests covering C mode.

@aaron.ballman, can you confirm that you are okay with the warning when processing C code (and, moreover, the newly-added errors for numeric escape sequences)?

[aaron.ballman]

clang/include/clang/Basic/DiagnosticLexKinds.td

290

Good call on C test coverage, thank you for bringing that up!

I think the behavior in C is reasonable, even if it's not mandated by the standard. We're going from issuing an error in C to issuing a warning and the warning text helpfully omits the C++2c part of the diagnostic, so it seems like we're more relaxed in C than we previously were.

Do you have concerns about the behavior in C? (Or are you saying you'd prefer this to be an error in C as it was before?)

[hubert.reinterpretcast]

clang/include/clang/Basic/DiagnosticLexKinds.td

290

My question was with respect to the change from the Clang 16 status quo for numeric escapes introduced by the prior change and carried forward with this one: https://godbolt.org/z/aneGr1Yfb

_Static_assert(1, "\x30"); // error in Clang 17

[cor3ntin]

clang/include/clang/Basic/DiagnosticLexKinds.td

290

Yes, this is intended, numeric escape sequences in unevaluated string literals are not allowed in any language modes - the motivation for C is the same as C++.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticLexKinds.td

290

Barring evidence that we're breaking working code without workarounds, I'm happy with the behavior in C. I don't think numeric escape sequences make a lot of sense in the contexts where these strings appear in C, same as for C++. However, if we had evidence that this breaks working code in problematic ways, we could consider changing it to warning-defaults-to-error.

[cor3ntin]

Note: we are waiting for feedback from @hubert.reinterpretcast's people before progressing this.

[hubert.reinterpretcast]

In D156596#4558097, @cor3ntin wrote:

Note: we are waiting for feedback from @hubert.reinterpretcast's people before progressing this.

The "full" build hit some (unrelated) snags, but the initial results look good. The patch is awaiting C test cases.

[cor3ntin]

Add C tests

[aaron.ballman]

LGTM but please wait for @hubert.reinterpretcast to confirm the new tests cover what he was looking for.

[hubert.reinterpretcast]

In D156596#4573354, @aaron.ballman wrote:

LGTM but please wait for @hubert.reinterpretcast to confirm the new tests cover what he was looking for.

LGTM; thanks!

clang/include/clang/Basic/DiagnosticLexKinds.td

def ext_reserved_user_defined_literal : ExtWarn<
   "invalid suffix on literal; C++11 requires a space between literal and "
   "identifier">, InGroup<ReservedUserDefinedLiteral>, DefaultError;
 def ext_ms_reserved_user_defined_literal : ExtWarn<
   "invalid suffix on literal; C++11 requires a space between literal and "
   "identifier">, InGroup<ReservedUserDefinedLiteral>;
 def err_unsupported_string_concat : Error<
   "unsupported non-standard concatenation of string literals">;
 
+def warn_unevaluated_string_prefix : Warning<
+  "encoding prefix '%0' on an unevaluated string literal has no effect"
+  "%select{| and is incompatible with c++2c}1">,
+  InGroup<DiagGroup<"invalid-unevaluated-string">>;
 def err_unevaluated_string_prefix : Error<
   "an unevaluated string literal cannot have an encoding prefix">;
 def err_unevaluated_string_udl : Error<
   "an unevaluated string literal cannot be a user-defined literal">;
 def err_unevaluated_string_invalid_escape_sequence : Error<
   "invalid escape sequence '%0' in an unevaluated string literal">;
 def err_string_concat_mixed_suffix : Error<
   "differing user-defined suffixes ('%0' and '%1') in string literal "

clang/lib/Lex/LiteralSupport.cpp

static unsigned getCharWidth(tok::TokenKind kind, const TargetInfo &Target) {
   case tok::utf16_string_literal:
     return Target.getChar16Width();
   case tok::utf32_char_constant:
   case tok::utf32_string_literal:
     return Target.getChar32Width();
   }
 }
 
+static unsigned getEncodingPrefixLen(tok::TokenKind kind) {
+  switch (kind) {
+  default:
+    llvm_unreachable("Unknown token type!");
+  case tok::char_constant:
+  case tok::string_literal:
+    return 0;
+  case tok::utf8_char_constant:
+  case tok::utf8_string_literal:
+    return 2;
+  case tok::wide_char_constant:
+  case tok::wide_string_literal:
+  case tok::utf16_char_constant:
+  case tok::utf16_string_literal:
+  case tok::utf32_char_constant:
+  case tok::utf32_string_literal:
+    return 1;
+  }
+}
+
 static CharSourceRange MakeCharSourceRange(const LangOptions &Features,
                                            FullSourceLoc TokLoc,
                                            const char *TokBegin,
                                            const char *TokRangeBegin,
                                            const char *TokRangeEnd) {
   SourceLocation Begin =
     Lexer::AdvanceToTokenCharacter(TokLoc, TokRangeBegin - TokBegin,
                                    TokLoc.getManager(), Features);
if (Delimited && Diags) {
     }
   }
 
   if (EvalMethod == StringLiteralEvalMethod::Unevaluated &&
       !IsEscapeValidInUnevaluatedStringLiteral(Escape)) {
     Diag(Diags, Features, Loc, ThisTokBegin, EscapeBegin, ThisTokBuf,
          diag::err_unevaluated_string_invalid_escape_sequence)
         << StringRef(EscapeBegin, ThisTokBuf - EscapeBegin);
+    HadError = true;
   }
+
   return ResultChar;
 }
 
 static void appendCodePoint(unsigned Codepoint,
                             llvm::SmallVectorImpl<char> &Str) {
   char ResultBuf[4];
   char *ResultPtr = ResultBuf;
   if (llvm::ConvertCodePointToUTF8(Codepoint, ResultPtr))
for (const Token &Tok : StringToks) {
 
     // Remember maximum string piece length.
     if (Tok.getLength() > MaxTokenLength)
       MaxTokenLength = Tok.getLength();
 
     // Remember if we see any wide or utf-8/16/32 strings.
     // Also check for illegal concatenations.
     if (isUnevaluated() && Tok.getKind() != tok::string_literal) {
-      if (Diags)
-        Diags->Report(Tok.getLocation(), diag::err_unevaluated_string_prefix);
+      if (Diags) {
+        SourceLocation PrefixEndLoc = Lexer::AdvanceToTokenCharacter(
+            Tok.getLocation(), getEncodingPrefixLen(Tok.getKind()), SM,
+            Features);
+        CharSourceRange Range =
+            CharSourceRange::getCharRange({Tok.getLocation(), PrefixEndLoc});
+        StringRef Prefix(SM.getCharacterData(Tok.getLocation()),
+                         getEncodingPrefixLen(Tok.getKind()));
+        Diags->Report(Tok.getLocation(),
+                      Features.CPlusPlus26
+                          ? diag::err_unevaluated_string_prefix
+                          : diag::warn_unevaluated_string_prefix)
+            << Prefix << Features.CPlusPlus << FixItHint::CreateRemoval(Range);
+      }
+      if (Features.CPlusPlus26)
         hadError = true;
     } else if (Tok.isNot(Kind) && Tok.isNot(tok::string_literal)) {
       if (isOrdinary()) {
         Kind = Tok.getKind();
       } else {
         if (Diags)
           Diags->Report(Tok.getLocation(), diag::err_unsupported_string_concat);
         hadError = true;
       }

clang/test/CXX/dcl.dcl/dcl.link/p2.cpp

 // RUN: %clang_cc1 -std=c++11 -verify %s
 
 extern "C" {
   extern R"(C++)" { }
 }
 
 #define plusplus "++"
 extern "C" plusplus {
 }
 
-extern u8"C" {}  // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
-extern L"C" {}   // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
-extern u"C++" {} // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
-extern U"C" {}   // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+extern u8"C" {}  // expected-warning {{encoding prefix 'u8' on an unevaluated string literal has no effect and is incompatible with c++2c}}
+extern L"C" {}   // expected-warning {{encoding prefix 'L' on an unevaluated string literal has no effect and is incompatible with c++2c}}
+extern u"C++" {} // expected-warning {{encoding prefix 'u' on an unevaluated string literal has no effect and is incompatible with c++2c}}
+extern U"C" {}   // expected-warning {{encoding prefix 'U' on an unevaluated string literal has no effect and is incompatible with c++2c}}

clang/test/CXX/dcl.dcl/p4-0x.cpp

struct U {
   constexpr operator long() const { return 0; } // expected-note {{candidate}}
 };
 
 static_assert(S(true), "");
 static_assert(S(false), "not so fast"); // expected-error {{not so fast}}
 static_assert(T(), "");
 static_assert(U(), ""); // expected-error {{ambiguous}}
 
-static_assert(false, L"\x14hi" // expected-error {{an unevaluated string literal cannot have an encoding prefix}} \
+static_assert(false, L"\x14hi" // expected-warning {{encoding prefix 'L' on an unevaluated string literal has no effect and is incompatible with c++2c}} \
                                // expected-error {{invalid escape sequence '\x14' in an unevaluated string literal}}
                      "!"
                      R"x(")x");

clang/test/FixIt/unevaluated-strings.cpp

+// RUN: %clang_cc1 -verify -std=c++2c %s
+// RUN: cp %s %t
+// RUN: not %clang_cc1 -x c++ -std=c++2c -fixit %t
+// RUN: %clang_cc1 -x c++ -std=c++2c %t
+// RUN: not %clang_cc1 -std=c++2c -x c++ -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+static_assert(true, L""); // expected-error{{an unevaluated string literal cannot have an encoding prefix}}
+// CHECK: fix-it:{{.*}}:{7:21-7:22}
+
+static_assert(true, u8""); // expected-error{{an unevaluated string literal cannot have an encoding prefix}}
+// CHECK: fix-it:{{.*}}:{10:21-10:23}
+
+static_assert(true, u""); // expected-error{{an unevaluated string literal cannot have an encoding prefix}}
+// CHECK: fix-it:{{.*}}:{13:21-13:22}
+
+static_assert(true, U""); // expected-error{{an unevaluated string literal cannot have an encoding prefix}}
+// CHECK: fix-it:{{.*}}:{16:21-16:22}

clang/test/Sema/static-assert.c

 // expected-warning@-8 {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
 // expected-warning@-8 {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
 // expected-warning@-8 {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
 // expected-warning@-8 {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
 // expected-warning@-8 {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
 // __builtin_strlen(literal) is considered an integer constant expression
 // and doesn't cause a pedantic warning
 #endif
+
+
+_Static_assert(0, L"\xFFFFFFFF"); // expected-warning {{encoding prefix 'L' on an unevaluated string literal has no effect}} \
+                                  // expected-error {{invalid escape sequence '\xFFFFFFFF' in an unevaluated string literal}} \
+                                  // expected-error {{hex escape sequence out of range}} \
+                                  // ext-warning {{'_Static_assert' is a C11 extension}}
+_Static_assert(0, L"\u1234"); // expected-warning {{encoding prefix 'L' on an unevaluated string literal has no effect}} \
+                              // expected-error {{static assertion failed: ሴ}} \
+                              // ext-warning {{'_Static_assert' is a C11 extension}}
+
+_Static_assert(0, L"\x1ff"      // expected-warning {{encoding prefix 'L' on an unevaluated string literal has no effect}} \
+                                // expected-error {{hex escape sequence out of range}} \
+                                // expected-error {{invalid escape sequence '\x1ff' in an unevaluated string literal}} \
+                                // ext-warning {{'_Static_assert' is a C11 extension}}
+                   "0\x123"     // expected-error {{invalid escape sequence '\x123' in an unevaluated string literal}}
+                   "fx\xfffff"  // expected-error {{invalid escape sequence '\xfffff' in an unevaluated string literal}}
+                   "goop");

clang/test/SemaCXX/static-assert.cpp

 template<typename T> struct S {
     static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}} \
                                                                      // expected-note {{1 > 1}}
 };
 
 S<char> s1; // expected-note {{in instantiation of template class 'S<char>' requested here}}
 S<int> s2;
 
-static_assert(false, L"\xFFFFFFFF"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}} \
-                                     // expected-error {{invalid escape sequence '\xFFFFFFFF' in an unevaluated string literal}}
-static_assert(false, u"\U000317FF"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
-// FIXME: render this as u8"\u03A9"
-static_assert(false, u8"Ω");     // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
-static_assert(false, L"\u1234"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
-static_assert(false, L"\x1ff"    // expected-error {{an unevaluated string literal cannot have an encoding prefix}} \
+static_assert(false, L"\xFFFFFFFF"); // expected-warning {{encoding prefix 'L' on an unevaluated string literal has no effect and is incompatible with c++2c}} \
+                                     // expected-error {{invalid escape sequence '\xFFFFFFFF' in an unevaluated string literal}} \
+                                     // expected-error {{hex escape sequence out of range}}
+static_assert(false, u"\U000317FF"); // expected-warning {{encoding prefix 'u' on an unevaluated string literal has no effect and is incompatible with c++2c}} \
+                                     // expected-error {{static assertion failed}}
+
+static_assert(false, u8"Ω");     // expected-warning {{encoding prefix 'u8' on an unevaluated string literal has no effect and is incompatible with c++2c}} \
+                                 // expected-error {{static assertion failed: Ω}}
+static_assert(false, L"\u1234"); // expected-warning {{encoding prefix 'L' on an unevaluated string literal has no effect and is incompatible with c++2c}} \
+                                 // expected-error {{static assertion failed: ሴ}}
+
+static_assert(false, L"\x1ff"    // expected-warning {{encoding prefix 'L' on an unevaluated string literal has no effect and is incompatible with c++2c}} \
+                                 // expected-error {{hex escape sequence out of range}} \
                                  // expected-error {{invalid escape sequence '\x1ff' in an unevaluated string literal}}
                      "0\x123"    // expected-error {{invalid escape sequence '\x123' in an unevaluated string literal}}
                      "fx\xfffff" // expected-error {{invalid escape sequence '\xfffff' in an unevaluated string literal}}
                      "goop");
 
 static_assert(false, "\'\"\?\\\a\b\f\n\r\t\v"); // expected-error {{'"?\<U+0007><U+0008>}}
 static_assert(true, "\xFF"); // expected-error {{invalid escape sequence '\xFF' in an unevaluated string literal}}
 static_assert(true, "\123"); // expected-error {{invalid escape sequence '\123' in an unevaluated string literal}}
 static_assert(true, "\pOh no, a Pascal string!"); // expected-warning {{unknown escape sequence '\p'}} \