[PATCH] D80547: [clang-format] Fix an ObjC regression introduced with new [[likely]][[unlikely]] support in if/else clauses
Marek Kurdej via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue May 26 05:54:48 PDT 2020
curdeius added a comment.
Minor remarks.
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2346
+// look to see if we have [[ by looking ahead if
+// its not then rewind to the original position
----------------
Nit: shouldn't comments be "Full phrases."?
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2348
+// its not then rewind to the original position
+bool UnwrappedLineParser::tryToParseAttribute() {
+ unsigned StoredPosition = Tokens->getPosition();
----------------
MyDeveloperDay wrote:
> MyDeveloperDay wrote:
> > This so makes me feel like we need a peekNextToken()
> which is like all the time I have to write
>
> ```
> Tok->Next && Tok->Next->is(tok::XXX)
> Tok->Previous && Tok->Previous ->is(tok::XXX)
> ```
>
> would be so much smaller code if we had
>
> ```
> Tok->isNext(tok::XXX)
> Tok->isPrevious(tok::XXX)
> ```
`peekNextToken()` probably should be done in a separate revision, nope?
It would be good to have it IMO.
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2351
+ FormatToken *Tok = Tokens->getNextToken();
+ FormatTok = Tokens->setPosition(StoredPosition);
+ if (Tok->is(tok::l_square)) {
----------------
Maybe add `assert(Tok);`. How can you be know for sure that there's a next token?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80547/new/
https://reviews.llvm.org/D80547
More information about the cfe-commits
mailing list