[PATCH] D142412: [clang-format] Put peekNextToken(/*SkipComment=*/true) to good use

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 24 00:28:15 PST 2023


MyDeveloperDay accepted this revision.
MyDeveloperDay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3592-3601
     case tok::kw_bool:
       // bool is only allowed if it is directly followed by a paren for a cast:
       // concept C = bool(...);
       // and bool is the only type, all other types as cast must be inside a
       // cast to bool an thus are handled by the other cases.
       if (Tokens->peekNextToken()->isNot(tok::l_paren))
         return;
----------------
rymiel wrote:
> owenpan wrote:
> > rymiel wrote:
> > > rymiel wrote:
> > > > owenpan wrote:
> > > > > We should pass `true` to `peekNextToken()` on Line 3597 here, but removing this entire case would have no effect on the existing unit test (Line 23693 in FormatTest.cpp). @HazardyKnusperkeks any idea?
> > > > The peeking is there because of https://reviews.llvm.org/D134325, it has no format tests, only an annotator test
> > > Also, I don't think that function is involved in concept declarations since https://reviews.llvm.org/D140339
> > So which tests were meant to check the `kw_bool` case here?
> Well, originally the one you mentioned on line 23693, but since the code paths have been changed, I think it's really only the one added in https://reviews.llvm.org/D134325
I love the new annotator tests, but we should probably always try to add a verifyFormat test as well even if its trivial in the future, just so we have some context as to what type of code warranted the change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142412/new/

https://reviews.llvm.org/D142412



More information about the cfe-commits mailing list