[PATCH] D80144: [clang-format] @lefticus just taught the world how to use [[unlikely]] but we forgot to teach clang-format

Krasimir Georgiev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 25 09:05:45 PDT 2020


krasimir added a comment.

Unfortunately this introduced a bad regression with ObjC method expressions in `if` bodies:

  if (argc > 5) [obj func:arg];

is now formatted as:

  if (argc > 5)[obj func:arg]
    ;

It seems that at this stage of processing it would be non-trivial to distinguish between general annotations and Objective-C call expressions; the general heuristics are at a later stage of clang-format's pipeline, in TokenAnnotator: https://github.com/llvm/llvm-project/blob/7c298c104bfe725d4315926a656263e8a5ac3054/clang/lib/Format/TokenAnnotator.cpp#L174.

The problem is that these two cases fundamentally require look ahead: we need to keep `[[likely]]` on the same unwrapped line as the `if (argc > 5)`, but we need to put the `[pbj func:arg]` on a new unwrapped line.

Maybe we can try manually parsing forward looking for a `[[identifier]]` pattern, and if we don't find it, reset the token position, reset the token sequence to the point before the first left square brace; similarly to https://github.com/llvm/llvm-project/blob/7c298c104bfe725d4315926a656263e8a5ac3054/clang/lib/Format/UnwrappedLineParser.cpp#L1496.

I know that we didn't have a test for this in ObjC land, but the regression seems severe enough to revert this, add a regression test and work on adding if [[likely]]/[[unlikely]] support compatible with ObjC method expressions as a follow-up.

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80144





More information about the cfe-commits mailing list