LiteralSupport: Don't assert() on invalid input

[DaanDeMeyer]

Summary

When using clangd, it's possible to trigger assertions in
NumericLiteralParser and CharLiteralParser when switching git branches.
This commit removes the initial asserts on invalid input and replaces
those asserts with the error handling mechanism from those respective
classes instead. This allows clangd to gracefully recover without
crashing.

See https://github.com/clangd/clangd/issues/888 for more information
on the clangd crashes.

[DaanDeMeyer]

Added some people that were recent reviewers of changes to this file and some clangd folks.

I don't have the time to properly test this unfortunately (aside from verifying that it fixes all the clangd crashes I'm having), but putting the patch up anyway in case anyone's interested.

[DaanDeMeyer]

clang-format

[beccadax]

err_lexing_string’s message is “failure when lexing a string”, which isn’t accurate here since you’re lexing a character literal or numeric literal instead. Could you emit a more appropriate message for this? That might mean adding additional diagnostics or modifying the existing one so you can insert information about the kind of literal.

(err_lexing_string is only used for “can’t happen” errors, so maybe you could change the message to something like failure when lexing a %0 literal; a file may have been modified during compilation.)

I’m otherwise pretty happy with this change—we’ve seen similar un-reproducible crashes in string literal parsing, and this kind of solution has worked well there.

[DaanDeMeyer]

Addressed comments by adding two new errors, one for character literals and one for numeric literals.

[sammccall]

Added a bit of analysis on https://github.com/clangd/clangd/issues/888.

The short version is we built a PCH. When consuming it, the parser decides to re-lex and this means sources must be loaded from disk.
These sources are inconsistent with the PCH, so the precondition that the input range describes a reasonable literal doesn't hold.

This patch fixes it by removing the precondition and defending against it instead.
This seems like an OK fallback unless there's hundreds of other places where we might re-lex and make similar assumptions.
I'm going to dig and try to find out but if anyone knows about this it'd be nice to chime in :-)

[beccadax]

The diagnostics are now much better—thanks!

[nridge]

This has been committed in https://github.com/llvm/llvm-project/commit/5a6dac66db67225e2443f4e61dfe9d2f96780611.

clang/include/clang/Basic/DiagnosticLexKinds.td

 def warn_bad_string_encoding : ExtWarn<
   "illegal character encoding in string literal">,
   InGroup<InvalidSourceEncoding>;
 def err_bad_character_encoding : Error<
   "illegal character encoding in character literal">;
 def warn_bad_character_encoding : ExtWarn<
   "illegal character encoding in character literal">,
   InGroup<InvalidSourceEncoding>;
-def err_lexing_string : Error<"failure when lexing a string">;
+def err_lexing_string : Error<"failure when lexing a string literal">;
+def err_lexing_char : Error<"failure when lexing a character literal">;
+def err_lexing_numeric : Error<"failure when lexing a numeric literal">;
 def err_placeholder_in_source : Error<"editor placeholder in source file">;
 
 //===----------------------------------------------------------------------===//
 // Preprocessor Diagnostics
 //===----------------------------------------------------------------------===//
 
 let CategoryName = "User-Defined Issue" in {
 def pp_hash_warning : Warning<"%0">,

clang/lib/Lex/LiteralSupport.cpp

NumericLiteralParser::NumericLiteralParser(StringRef TokSpelling,
                                            SourceLocation TokLoc,
                                            const SourceManager &SM,
                                            const LangOptions &LangOpts,
                                            const TargetInfo &Target,
                                            DiagnosticsEngine &Diags)
     : SM(SM), LangOpts(LangOpts), Diags(Diags),
       ThisTokBegin(TokSpelling.begin()), ThisTokEnd(TokSpelling.end()) {
 
-  // This routine assumes that the range begin/end matches the regex for integer
-  // and FP constants (specifically, the 'pp-number' regex), and assumes that
-  // the byte at "*end" is both valid and not part of the regex.  Because of
-  // this, it doesn't have to check for 'overscan' in various places.
-  assert(!isPreprocessingNumberBody(*ThisTokEnd) && "didn't maximally munch?");
-
   s = DigitsBegin = ThisTokBegin;
   saw_exponent = false;
   saw_period = false;
   saw_ud_suffix = false;
   saw_fixed_point_suffix = false;
   isLong = false;
   isUnsigned = false;
   isLongLong = false;
   isSizeT = false;
   isHalf = false;
   isFloat = false;
   isImaginary = false;
   isFloat16 = false;
   isFloat128 = false;
   MicrosoftInteger = 0;
   isFract = false;
   isAccum = false;
   hadError = false;
 
+  // This routine assumes that the range begin/end matches the regex for integer
+  // and FP constants (specifically, the 'pp-number' regex), and assumes that
+  // the byte at "*end" is both valid and not part of the regex.  Because of
+  // this, it doesn't have to check for 'overscan' in various places.
+  if (isPreprocessingNumberBody(*ThisTokEnd)) {
+    Diags.Report(TokLoc, diag::err_lexing_numeric);
+    hadError = true;
+    return;
+  }
+
   if (*s == '0') { // parse radix
     ParseNumberStartingWithZero(TokLoc);
     if (hadError)
       return;
   } else { // the first digit is non-zero
     radix = 10;
     s = SkipDigits(s);
     if (s == ThisTokEnd) {
CharLiteralParser::CharLiteralParser(const char *begin, const char *end,
 
   // Skip over wide character determinant.
   if (Kind != tok::char_constant)
     ++begin;
   if (Kind == tok::utf8_char_constant)
     ++begin;
 
   // Skip over the entry quote.
-  assert(begin[0] == '\'' && "Invalid token lexed");
+  if (begin[0] != '\'') {
+    PP.Diag(Loc, diag::err_lexing_char);
+    HadError = true;
+    return;
+  }
+
   ++begin;
 
   // Remove an optional ud-suffix.
   if (end[-1] != '\'') {
     const char *UDSuffixEnd = end;
     do {
       --end;
     } while (end[-1] != '\'');