rsmith added inline comments.
================
Comment at: lib/Lex/LiteralSupport.cpp:768
.Cases("il", "i", "if", true)
+ .Case("sv", true)
.Default(false);
----------------
aaron.ballman wrote:
> mclow.lists wrote:
> > 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)
> 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.
We should add a `StringLiteralParser::isValidUDSuffix` and use it from
`Lexer::LexUDSuffix` (and anywhere else that wants to know).
https://reviews.llvm.org/D26667
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
