[Clang] Treat invalid UDL as two tokens

[rZhBoYao]

Summary

In contrast to Clang-17, we treat an invalid ud-suffix as if whitespace preceded
it only if it can be seen as a macro and the preceding string literal is non-empty.

#define E "!"
const char
  *operator""E(const char*),
  // ""E is a single token as it should be
  *s = "not empty"E;
  // treated as if whitespace preceds E hence a string concat:
  // = "not empty!"

This addresses comments in D153156.

[cor3ntin]

clang/include/clang/Basic/DiagnosticSemaKinds.td

9322

Can you remove <Error> and adapt the calling code to adjust the index?

clang/lib/Lex/Lexer.cpp

1992–2006

I missed that in the previous review, is the FIX-IT here still relevant?

2017

This sounds brittle. I think we are better off looking ahead to the next non-identifier character rather than assuming a size here

2024

This is also sort of brittle, it assumes standard UDL don't have unicode... but that sounds more reasonable today.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticLexKinds.td

283–284

[rZhBoYao]

clang/lib/Lex/Lexer.cpp

1992–2006

Yes, I also put back the fixit test.

[rsmith]

clang/docs/ReleaseNotes.rst

97

I don't think this is accurate. Clang supported CWG1473 before these changes, as far as I can see: all valid code under CWG1473 was accepted, and invalid code was diagnosed (by default). Rather, what has changed is the behavior for invalid code: instead of treating an invalid ""blah as two tokens always, in order to accept as much old code as possible, we now treat it as two tokens only when blah is defined as a macro name.

This is still a breaking change in some cases, for users of -Wno-deprecated-literal-operator, eg:

#define FOO(xyz) ""xyz

... now will be lexed as a single invalid token rather than two tokens.

I'm not sure what the motivation for making changes here was, and D153156's description doesn't really help me understand it. Is the goal to improve the diagnostic quality for these kinds of errors on invalid code? Is there some example for which Clang's behavior with regard to CWG1473 was non-conforming? Something else?

clang/lib/Lex/Lexer.cpp

2020

Reverse the order of these checks to do the cheaper check first and to avoid the possibility of reading off the end of the input.

2027–2029

That's still a breaking change compared to what we designed -Wno-reserved-literal-operator to do. How often does it happen in practice that someone has both an ill-formed literal operator *and* a macro defined to the same name as the suffix?

2035

This doesn't check whether the identifier is currently defined as a macro, in the presence of modules. It also won't be correct if the lexer has got substantially ahead of the preprocessor, and the #define has been lexed but not yet preprocessed.

Overall, it's not really possible to tell from here whether an identifier is defined as a macro. To do this properly, you should instead produce a single token here and teach the preprocessor to consider splitting it into two tokens if the suffix is a reserved ud-suffix naming a defined macro. In principle you could also check from the preprocessor whether the previous produced token was operator and use that as part of the decision...

clang/test/CXX/drs/dr14xx.cpp

487

I don't think this is correct. As far as I can tell, Clang has correctly implemented CWG1473 since version 3.2.

[tahonermann]

Given Richard's comments, it seems that changes are needed.

[rZhBoYao]

OK, will do it by the end of this week.

clang/docs/ReleaseNotes.rst

 
 - Attributes now expect unevaluated strings in attributes parameters that are string literals.
   This is applied to both C++ standard attributes, and other attributes supported by Clang.
   This completes the implementation of `P2361R6 Unevaluated Strings <https://wg21.link/P2361R6>`_
 
 
 Resolutions to C++ Defect Reports
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-- Implemented `CWG1473 <https://wg21.link/CWG1473>`_ which allows spaces after ``operator""``.
-  Clang used to err on the lack of space when the literal suffix identifier was invalid in
-  all the language modes, which contradicted the deprecation of the whitespaces.
-  Also turn on ``-Wdeprecated-literal-operator`` by default in all the language modes.
+- Implemented `CWG1473 <https://wg21.link/CWG1473>`_ allowing lack of space after ``operator""``.
+  Clang used to err on the lack of space when the literal suffix identifier was invalid,
+  contradicting ``-Wdeprecated-literal-operator`` which is now default on.
+  Instead, Clang now emits error only if the invalid suffix looks like a macro and the preceding
+  string literal is not empty, and then treat the suffix as if it were preceded by whitespace.
+
+  .. code-block:: cpp
+
+    #define E "!"
+    const char 
+      *operator""E(const char*),
+      // ""E is a single token as it should be pedantically
+      *s = "not empty"E;
+      // treated as if whitespace preceds E hence a string concat:
+      // = "not empty!"
 
 C Language Changes
 ------------------
 - ``structs``, ``unions``, and ``arrays`` that are const may now be used as
   constant expressions.  This change is more consistent with the behavior of
   GCC.
 
 C23 Feature Support

clang/include/clang/Basic/DiagnosticLexKinds.td

def warn_cxx14_compat_u8_character_literal : Warning<
   InGroup<CXXPre17Compat>, DefaultIgnore;
 def warn_cxx11_compat_user_defined_literal : Warning<
   "identifier after literal will be treated as a user-defined literal suffix "
   "in C++11">, InGroup<CXX11Compat>, DefaultIgnore;
 def warn_cxx11_compat_reserved_user_defined_literal : Warning<
   "identifier after literal will be treated as a reserved user-defined literal "
   "suffix in C++11">,
   InGroup<CXX11CompatReservedUserDefinedLiteral>, DefaultIgnore;
+def ext_reserved_user_defined_literal : ExtWarn<
+  "invalid suffix on literal; C++11 requires a space between literal and "
+  "a macro">, InGroup<ReservedUserDefinedLiteral>, DefaultError;
+def ext_ms_reserved_user_defined_literal : ExtWarn<
+  ext_reserved_user_defined_literal.Summary>,
+  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<

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def err_literal_operator_template : Error<
   "template parameter list for literal operator must be either 'char...' or 'typename T, T...'">;
 def err_literal_operator_extern_c : Error<
   "literal operator must have C++ linkage">;
 def ext_string_literal_operator_template : ExtWarn<
   "string literal operator templates are a GNU extension">,
   InGroup<GNUStringLiteralOperatorTemplate>;
 def warn_user_literal_reserved : Warning<
-  "user-defined literal suffixes %select{<ERROR>|not starting with '_'|containing '__'}0 are reserved"
-  "%select{; no literal will invoke this operator|}1">,
+  "user-defined literal suffixes %select{not starting with '_'|containing '__'}0 are reserved">,
   InGroup<UserDefinedLiterals>;
 
 // C++ conversion functions
 def err_conv_function_not_member : Error<
   "conversion function must be a non-static member function">;
 def err_conv_function_return_type : Error<
   "conversion function cannot have a return type">;
 def err_conv_function_with_params : Error<

clang/include/clang/Basic/IdentifierTable.h

enum class ReservedIdentifierStatus {
   StartsWithUnderscoreAtGlobalScope,
   StartsWithUnderscoreAndIsExternC,
   StartsWithDoubleUnderscore,
   StartsWithUnderscoreFollowedByCapitalLetter,
   ContainsDoubleUnderscore,
 };
 
 enum class ReservedLiteralSuffixIdStatus {
-  NotReserved = 0,
-  NotStartsWithUnderscore,
+  NotStartsWithUnderscore = 0,
   ContainsDoubleUnderscore,
+  NotReserved,
 };
 
 /// Determine whether an identifier is reserved for use as a name at global
 /// scope. Such identifiers might be implementation-specific global functions
 /// or variables.
 inline bool isReservedAtGlobalScope(ReservedIdentifierStatus Status) {
   return Status != ReservedIdentifierStatus::NotReserved;
 }

clang/lib/Lex/Lexer.cpp

 
 /// LexUDSuffix - Lex the ud-suffix production for user-defined literal suffixes
 /// in C++11, or warn on a ud-suffix in C++98.
 const char *Lexer::LexUDSuffix(Token &Result, const char *CurPtr,
                                bool IsStringLiteral) {
   assert(LangOpts.CPlusPlus);
 
   // Maximally munch an identifier.
+  const char *const TokStart = CurPtr;
   unsigned Size;
   char C = getCharAndSize(CurPtr, Size);
-  bool Consumed = false;
 
-  if (!isAsciiIdentifierStart(C)) {
-    if (C == '\\' && tryConsumeIdentifierUCN(CurPtr, Size, Result))
-      Consumed = true;
-    else if (!isASCII(C) && tryConsumeIdentifierUTF8Char(CurPtr))
-      Consumed = true;
-    else
-      return CurPtr;
-  }
+  if (isAsciiIdentifierStart(C)) {
+    CurPtr = ConsumeChar(CurPtr, Size, Result);
+  } else if (C == '\\' && tryConsumeIdentifierUCN(CurPtr, Size, Result)) {
+  } else if (!isASCII(C) && tryConsumeIdentifierUTF8Char(CurPtr)) {
+  } else
 
   if (!LangOpts.CPlusPlus11) {
     if (!isLexingRawMode())
-      Diag(CurPtr,
+      Diag(TokStart,
            C == '_' ? diag::warn_cxx11_compat_user_defined_literal
                     : diag::warn_cxx11_compat_reserved_user_defined_literal)
-        << FixItHint::CreateInsertion(getSourceLocation(CurPtr), " ");
-    return CurPtr;
+          << FixItHint::CreateInsertion(getSourceLocation(TokStart), " ");
+    return TokStart;
   }
 
-  // C++11 [lex.ext]p10, [usrlit.suffix]p1: A program containing a ud-suffix
-  // that does not start with an underscore is ill-formed. We assume a suffix
-  // beginning with a UCN or UTF-8 character is more likely to be a ud-suffix
-  // than a macro, however, and accept that.
-  if (!Consumed)
-    CurPtr = ConsumeChar(CurPtr, Size, Result);
-
-  Result.setFlag(Token::HasUDSuffix);
   while (true) {
     C = getCharAndSize(CurPtr, Size);
     if (isAsciiIdentifierContinue(C)) {
       CurPtr = ConsumeChar(CurPtr, Size, Result);
     } else if (C == '\\' && tryConsumeIdentifierUCN(CurPtr, Size, Result)) {
     } else if (!isASCII(C) && tryConsumeIdentifierUTF8Char(CurPtr)) {
     } else
       break;
   }
 
+  bool IsLiteralOperator =
+      StringRef(BufferPtr, 2).equals("\"\"") && BufferPtr + 2 == TokStart;
+  if (unsigned TokLen = CurPtr - TokStart;
+      StringLiteralParser::isValidUDSuffix(LangOpts, {TokStart, TokLen}))
+    Result.setFlag(Token::HasUDSuffix);
+  else if (!isLexingRawMode() && !IsLiteralOperator) {
+    // As a conforming extension, we treat invalid suffixes as if they had
+    // whitespace before them if doing so results in macro expansions.
+    // However, don't diagnose operator""E(...) even if E is a macro as it
+    // results in confusing error messages. Hence, ""E would not be treated as
+    // string concat; instead it's a single PP token (as it should be).
+    Result.setLength(TokLen);
+    Result.setLocation(getSourceLocation(TokStart, TokLen));
+    Result.setKind(tok::raw_identifier);
+    Result.setRawIdentifierData(TokStart);
+    IdentifierInfo *II = PP->LookUpIdentifierInfo(Result);
+    if (II->hasMacroDefinition()) {
+      Diag(TokStart, LangOpts.MSVCCompat
+                         ? diag::ext_ms_reserved_user_defined_literal
+                         : diag::ext_reserved_user_defined_literal)
+          << FixItHint::CreateInsertion(getSourceLocation(TokStart), " ");
+      return TokStart;
+    }
+  }
+
   return CurPtr;
 }
 
 /// LexStringLiteral - Lex the remainder of a string literal, after having lexed
 /// either " or L" or u8" or u" or U".
 bool Lexer::LexStringLiteral(Token &Result, const char *CurPtr,
                              tok::TokenKind Kind) {
   const char *AfterQuote = CurPtr;

clang/lib/Sema/SemaDeclCXX.cpp

bool Sema::CheckLiteralOperatorDeclaration(FunctionDecl *FnDecl) {
   if (Status != ReservedLiteralSuffixIdStatus::NotReserved &&
       !getSourceManager().isInSystemHeader(FnDecl->getLocation())) {
     // C++23 [usrlit.suffix]p1:
     //   Literal suffix identifiers that do not start with an underscore are
     //   reserved for future standardization. Literal suffix identifiers that
     //   contain a double underscore __ are reserved for use by C++
     //   implementations.
     Diag(FnDecl->getLocation(), diag::warn_user_literal_reserved)
-        << static_cast<int>(Status)
-        << StringLiteralParser::isValidUDSuffix(getLangOpts(), II->getName());
+        << static_cast<int>(Status);
   }
 
   return false;
 }
 
 /// ActOnStartLinkageSpecification - Parsed the beginning of a C++
 /// linkage specification, including the language and (if present)
 /// the '{'. ExternLoc is the location of the 'extern', Lang is the

clang/test/CXX/drs/dr14xx.cpp

 #endif
     f({uR"(abc)"});   // expected-error {{call to deleted function 'f'}}
     f({(UR"(abc)")}); // expected-error {{call to deleted function 'f'}}
   }
   } // namespace StringLiterals
 #endif
 } // dr1467
 
 namespace dr1473 { // dr1473: 18
                    // NB: sup 1762, test reused there
 #if __cplusplus >= 201103L
-  float operator ""_E(const char *);
-  float operator ""E(const char *); // don't err on the lack of spaces even when the literal suffix identifier is invalid
-  // expected-warning@-1 {{user-defined literal suffixes not starting with '_' are reserved; no literal will invoke this operator}}
+#define E "!"
+const char
+  *operator""_E(const char*),
+  *operator""E(const char*), // don't err on the lack of spaces even when the literal suffix identifier is invalid
+  // expected-warning@-1 {{user-defined literal suffixes not starting with '_' are reserved}}
+  *s = "not empty"E;
+  // expected-error@-1 {{invalid suffix on literal; C++11 requires a space between literal and a macro}}
+#undef E
 #endif
 }
 
 namespace dr1479 { // dr1479: yes
   int operator""_a(const char*, std::size_t = 0); // expected-error {{literal operator cannot have a default argument}}
 }
 
 namespace dr1482 { // dr1482: yes

clang/test/CXX/drs/dr17xx.cpp

#if __cplusplus >= 201103L
   } b;
   A a{b};
 #endif
 }
 
 namespace dr1762 { // dr1762: 14
                    // NB: reusing 1473 test
 #if __cplusplus >= 201103L
-  float operator ""_E(const char *);
-  float operator ""E(const char *);
-  // expected-warning@-1 {{user-defined literal suffixes not starting with '_' are reserved; no literal will invoke this operator}}
+#define E "!"
+const char
+  *operator""_E(const char*),
+  *operator""E(const char*), // don't err on the lack of spaces even when the literal suffix identifier is invalid
+  // expected-warning@-1 {{user-defined literal suffixes not starting with '_' are reserved}}
+  *s = "not empty"E;
+  // expected-error@-1 {{invalid suffix on literal; C++11 requires a space between literal and a macro}}
+#undef E
 #endif
 }
 
 namespace dr1778 { // dr1778: 9
   // Superseded by P1286R2.
 #if __cplusplus >= 201103L
   struct A { A() noexcept(true) = default; };
   struct B { B() noexcept(false) = default; };

clang/test/CXX/lex/lex.literal/lex.ext/p10.cpp

 // RUN: %clang_cc1 -std=c++11 -verify %s
 
 using size_t = decltype(sizeof(int));
-void operator ""wibble(const char *); // expected-warning {{user-defined literal suffixes not starting with '_' are reserved; no literal will invoke this operator}}
-void operator ""wibble(const char *, size_t); // expected-warning {{user-defined literal suffixes not starting with '_' are reserved; no literal will invoke this operator}}
+void operator ""wibble(const char *); // expected-warning {{user-defined literal suffixes not starting with '_' are reserved}}
+void operator ""wibble(const char *, size_t); // expected-warning {{user-defined literal suffixes not starting with '_' are reserved}}
 
 template<typename T>
 void f() {
   // A program containing a reserved ud-suffix is ill-formed.
   123wibble; // expected-error {{invalid suffix 'wibble'}}
   123.0wibble; // expected-error {{invalid suffix 'wibble'}}
   const char *p = ""wibble; // expected-error {{cannot initialize a variable of type 'const char *' with an rvalue of type 'void'}}
   const char *q = R"x("hello")x"wibble; // expected-error {{cannot initialize a variable of type 'const char *' with an rvalue of type 'void'}}
 }

clang/test/FixIt/fixit-c++11.cpp

#if __cplusplus <= 202002L
   // expected-warning@-3{{is a C++23 extension}}
 #endif
 
   delete []() { return new int; }(); // expected-error{{'[]' after delete interpreted as 'delete[]'}}
   delete [] { return new int; }(); // expected-error{{'[]' after delete interpreted as 'delete[]'}}
 }
 
 #define bar "bar"
-const char *p = "foo" bar;
+const char *p = "foo"bar; // expected-error {{requires a space between}}
 #define ord - '0'
-int k = '4' ord;
+int k = '4'ord; // expected-error {{requires a space between}}
 
 void operator"x" _y(char); // expected-error {{must be '""'}}
 void operator L"" _z(char); // expected-error {{encoding prefix}}
 void operator "x" "y" U"z" ""_whoops "z" "y"(char); // expected-error {{must be '""'}}
 
 void f() {
   'b'_y;
   'c'_z;