[clang] Allow lexer to handle string_view literals

[AntonBikineev]

Summary

This is to address p0403r0 proposal.

[EricWF]

Please add tests to cxx1y-user-defined-literals.cpp (and probably rename it).

Other than that this LGTM, but I don't feel comfortable approving clang patches.

[malcolm.parsons]

@mclow.lists is working on this in D26667.
The review comments from that apply here too.

[EricWF]

Should this review be closed then?

[AntonBikineev]

Just added a small test case

[malcolm.parsons]

Does Sema::CheckLiteralOperatorDeclaration need to check StringLiteralParser::isValidUDSuffix?

[AntonBikineev]

In D26829#599906, @malcolm.parsons wrote:

Does Sema::CheckLiteralOperatorDeclaration need to check StringLiteralParser::isValidUDSuffix?

Thanks, nice point! Just addressed it.

[rsmith]

include/clang/Basic/DiagnosticLexKinds.td

189–190

I don't see a need for a custom diagnostic for this. Using our normal "reserved ud-suffix" diagnostic seems fine, and it doesn't make sense to treat this suffix as a hard error but accept all the rest with a warning.

lib/Lex/LiteralSupport.cpp

1716–1717

Just make this call NumericLiteralParser::isValidUDSuffix and then check for the sv case. All the numeric suffixes are also valid string literal suffixes for the form operator""suffix.

[AntonBikineev]

lib/Lex/LiteralSupport.cpp

1716–1717

This makes sense for the call sites we currently have.

All the numeric suffixes are also valid string literal suffixes for the form operator""suffix.

Don't really understand this part. It seems inconsistent if one calls, say,

StringLiteralParser::isValidUDSuffix(LangOpts(), "if")

and gets

true

[AntonBikineev]

Richard, thanks, I addressed your comments.

[rsmith]

LGTM, thanks!

lib/Lex/Lexer.cpp

1717

I don't think the LangOpts variable is worthwhile; remove and replace the use of it below with getLangOpts().

[AntonBikineev]

@rsmith Richard, any plans to merge this or is there anything left?

[EricWF]

Committed on behalf of @AntonBikineev in r290744.

include/clang/Basic/DiagnosticLexKinds.td

   "hexadecimal floating literals are incompatible with "
   "C++ standards before C++1z">,
   InGroup<CXXPre1zCompatPedantic>, DefaultIgnore;
+def err_cxx1z_string_view_literal : Error<
+  "string_view literals are a C++1z feature">;
 def ext_binary_literal : Extension<
   "binary integer literals are a GNU extension">, InGroup<GNUBinaryLiteral>;
 def ext_binary_literal_cxx14 : Extension<

include/clang/Lex/LiteralSupport.h

     return UDSuffixOffset;
   }
 
+  enum class UDSuffixResult : uint8_t {
+    Valid, Invalid, SVIncompatible
+  };
+
+  static UDSuffixResult isValidUDSuffix(const LangOptions &LangOpts,
+                                        StringRef Suffix);
+
 private:
   void init(ArrayRef<Token> StringToks);
   bool CopyStringFragment(const Token &Tok, const char *TokBegin,

lib/Lex/Lexer.cpp

   // likely to be a ud-suffix than a macro, however, and accept that.
   if (!Consumed) {
     bool IsUDSuffix = false;
+    StringLiteralParser::UDSuffixResult IsStringUDSuffix =
+        StringLiteralParser::UDSuffixResult::Invalid;
     if (C == '_')
       IsUDSuffix = true;
     else if (IsStringLiteral && getLangOpts().CPlusPlus14) {

                                          getLangOpts());
         if (!isIdentifierBody(Next)) {
           // End of suffix. Check whether this is on the whitelist.
-          IsUDSuffix = (Chars == 1 && Buffer[0] == 's') ||
-                       NumericLiteralParser::isValidUDSuffix(
-                           getLangOpts(), StringRef(Buffer, Chars));
+          const StringRef CompleteSuffix = StringRef(Buffer, Chars);
+          const LangOptions &LangOpts = getLangOpts();
+          // First, check if the suffix is a numeric suffix (s,sv)
+          IsUDSuffix =
+              NumericLiteralParser::isValidUDSuffix(LangOpts, CompleteSuffix);
+          // If it is not, check for a string suffix
+          if (!IsUDSuffix) {
+            IsStringUDSuffix =
+                StringLiteralParser::isValidUDSuffix(LangOpts, CompleteSuffix);
+            IsUDSuffix =
+                IsStringUDSuffix == StringLiteralParser::UDSuffixResult::Valid;
+          }
           break;
         }
 

 
     if (!IsUDSuffix) {
       if (!isLexingRawMode())
-        Diag(CurPtr, getLangOpts().MSVCCompat
-                         ? diag::ext_ms_reserved_user_defined_literal
-                         : diag::ext_reserved_user_defined_literal)
+        if (IsStringUDSuffix ==
+            StringLiteralParser::UDSuffixResult::SVIncompatible) {
+          Diag(CurPtr, diag::err_cxx1z_string_view_literal)
             << FixItHint::CreateInsertion(getSourceLocation(CurPtr), " ");
+        } else {
+          Diag(CurPtr, getLangOpts().MSVCCompat
+                           ? diag::ext_ms_reserved_user_defined_literal
+                           : diag::ext_reserved_user_defined_literal)
+            << FixItHint::CreateInsertion(getSourceLocation(CurPtr), " ");
+        }
       return CurPtr;
     }
 

lib/Lex/LiteralSupport.cpp

 
   return SpellingPtr-SpellingStart;
 }
+
+/// Determine whether a suffix is a valid ud-suffix. We avoid treating reserved
+/// suffixes as ud-suffixes, because the diagnostic experience is better if we
+/// treat it as an invalid suffix.
+StringLiteralParser::UDSuffixResult
+StringLiteralParser::isValidUDSuffix(const LangOptions &LangOpts,
+                                     StringRef Suffix) {
+  if (!LangOpts.CPlusPlus11 || Suffix.empty())
+    return UDSuffixResult::Invalid;
+
+  // By C++11 [lex.ext]p10, ud-suffixes starting with an '_' are always valid.
+  if (Suffix[0] == '_')
+    return UDSuffixResult::Valid;
+
+  // In C++11, there are no library suffixes.
+  if (!LangOpts.CPlusPlus14)
+    return UDSuffixResult::Invalid;
+
+  // C++1z adds "sv" literals
+  if (Suffix == "sv")
+    return LangOpts.CPlusPlus1z ? UDSuffixResult::Valid
+                                : UDSuffixResult::SVIncompatible;
+
+  return Suffix == "s" ? UDSuffixResult::Valid : UDSuffixResult::Invalid;
+}