[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:125
+                 CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+                     -1)))
           return false;
----------------
penagos wrote:
> MyDeveloperDay wrote:
> > I don't really understand what we are saying here? 
> Effectively we are checking that, barring intervening whitespace, we are analyzing 2 consecutive '>' tokens. If so, we treat such sequence as a binary op in lieu of a closing template angle bracket. If there's another more straightforward way of accomplishing this check, I'm open to that, but this seemed to be the most straightforward way at the time.
I'm worried that this may regress template code. How does this account for cases where two consecutive `>`-s are really two closing template brackets, e.g.,
`std::vector<std::decay_t<int& >> v;`?

In particular, one added test case is ambiguous: `>>` could really be two closing template brackets:
https://godbolt.org/z/v19hj9vKn

I have to say that my general feeling about trying to disambiguate between bitshifts and template closers is: don't try too hard inside clang-format as the heuristics are generally quite brittle and make the code harder to maintain; in cases where clang-format wrongly detects bitshift as templates, users should add parens around the bitshift, which IMO improves readability.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100778