Teach clang that 'sv' is a fine literal suffix

[mclow.lists]

Summary

Last week, WG21 approved a paper (http://wg21.link/P0403) for allowing literal suffixes for string_view. This means that should be able to write:

constexpr std::string_view sv = "1234"sv;

However, clang has a list of all the allowable suffixes. Update the list to include "sv"

[aaron.ballman]

Testcases?

[malcolm.parsons]

lib/Lex/LiteralSupport.cpp

768

This is in NumericLiteralParser::isValidUDSuffix().

If a change is needed for "sv", wouldn't it be in StringLiteralParser?

[mclow.lists]

lib/Lex/LiteralSupport.cpp

768

Would it be? I don't know. "s" appears here, which is used for both 'string' and 'seconds'.

[aaron.ballman]

lib/Lex/LiteralSupport.cpp

768

This function is used by the numeric literal parser, and so sv does not make sense there. It's also used by SemaDeclCXX.cpp in CheckLiteralOperatorDeclaration(), where this change would make sense.

I was asking about test cases; I was wondering what problem this was trying to solve. I suspect you want to modify CheckLiteralOperatorDeclaration() rather than NumericLiteralParser.

[malcolm.parsons]

lib/Lex/LiteralSupport.cpp

768

s for std::string seems to be handled in Lexer::LexUDSuffix():

IsUDSuffix = (Chars == 1 && Buffer[0] == 's') ||
             NumericLiteralParser::isValidUDSuffix(
                 getLangOpts(), StringRef(Buffer, Chars));

[aaron.ballman]

lib/Lex/LiteralSupport.cpp

768

Good catch, there too (I haven't done a complete tour to see where else may need changing).

[mclow.lists]

lib/Lex/LiteralSupport.cpp

768

That's really interesting; since I made the change I described and tested it and it works fine:

constexpr std::string_view sv = "1234"sv;
constexpr std::string_view sv = "ACBQ"sv;

"works fine" in this case means "compiles w/o error"

(Not that I'm disputing that there might be a better place for this)

[aaron.ballman]

lib/Lex/LiteralSupport.cpp

768

You modified NumericLiteralParser::isValidUDSuffix(), which is called by the code @malcolm.parsons posted. So the changes will work, but the design is wrong since the UD suffix has nothing to do with parsing numeric literals.

[rsmith]

lib/Lex/LiteralSupport.cpp

768

We should add a StringLiteralParser::isValidUDSuffix and use it from Lexer::LexUDSuffix (and anywhere else that wants to know).

[mclow.lists]

This was resolved by https://reviews.llvm.org/D26829

lib/Lex/LiteralSupport.cpp

       .Cases("h", "min", "s", true)
       .Cases("ms", "us", "ns", true)
       .Cases("il", "i", "if", true)
+      .Case("sv", true)
       .Default(false);
 }