[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