mclow.lists added inline comments.

================
Comment at: lib/Lex/LiteralSupport.cpp:768
       .Cases("il", "i", "if", true)
+      .Case("sv", true)
       .Default(false);
----------------
aaron.ballman wrote:
> malcolm.parsons wrote:
> > aaron.ballman wrote:
> > > mclow.lists wrote:
> > > > malcolm.parsons wrote:
> > > > > This is in `NumericLiteralParser::isValidUDSuffix()`.
> > > > > 
> > > > > If a change is needed for `"sv"`, wouldn't it be in 
> > > > > `StringLiteralParser`?
> > > > Would it be?  I don't know.  `"s"` appears here, which is used for both 
> > > > 'string' and 'seconds'.
> > > 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`.
> > `s` for `std::string` seems to be handled in `Lexer::LexUDSuffix()`:
> > 
> > ```
> >             IsUDSuffix = (Chars == 1 && Buffer[0] == 's') ||
> >                          NumericLiteralParser::isValidUDSuffix(
> >                              getLangOpts(), StringRef(Buffer, Chars));
> > ```
> Good catch, there too (I haven't done a complete tour to see where else may 
> need changing).
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)


https://reviews.llvm.org/D26667



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to