[clang-format] Handle shifts within conditions

[Saldivarcher]

Summary

In some situation shifts can be treated as a template, and is thus formatted as one. So, by doing a couple extra checks to assure that the condition doesn't contain a template, and is in fact a bit shift should solve this problem.

This is a fix for bug 46969

[MyDeveloperDay]

I think I like your solution better than mine D79293: [clang-format] [PR45218] Fix an issue where < and > and >> in a for loop gets incorrectly interpreted at a TemplateOpener/Closer, could you look at my unit tests and consider adding them here?

[JakeMerdichAMD]

Agreed that this is a very nice solution for this case. LGTM too assuming it passes @MyDeveloperDay's tests.

Minor heads up: there are still some cases I can dream up where we'd currently spit out invalid code from splitting a right shift even after this fix. Long story short, C++14 made some cases completely ambiguous and impossible to detect without semantic analysis (https://godbolt.org/z/rxEqTG). I'll be having a fix out for that soon, but it's rather complementary to this one-- yours improves template detection, mine acts more conservative in case we guessed wrong about it being a template.

[Saldivarcher]

Added some more tests.

[Saldivarcher]

MyDeveloperDay, I added your tests. Sorry to step on your toes, I didn't know there was a patch open for this already :(. Yeah, JakeMerdichAMD, it's kinda tough to handle those types of cases without some sort of parsing. I'm glad you're working on that, and my code doesn't get in the way.

[MyDeveloperDay]

I think this LGTM and the solution is much more elegant than mine! You were not stepping on my toes, I'm always happy for someone else to take things over and this is a much smarter solution than mine.

This bug has been reported multiple times I think this is a good first step towards a solution, and I don't think "perfection should be the enemy of good", I think for most cases this will suffice.

anything else more obtuse I wonder why people write code in that form when its hard for the non author to read, but we can address those cases when we hit them.

[MyDeveloperDay]

@Saldivarcher do you have commit access?

[Saldivarcher]

@MyDeveloperDay thanks for the kind words, it's much appreciated! I'm just glad I was able to help. Nope, I don't have commit access, can you do me the favor of commiting for me?

[MyDeveloperDay]

@Saldivarcher this should be landed now, please validate

[Saldivarcher]

@MyDeveloperDay looks like it's in master, thanks for committing for me!

clang/lib/Format/TokenAnnotator.cpp

if (Left->Previous && Left->Previous->is(tok::greater) &&
     Left = Left->Previous->MatchingParen;
 
   // Check for `[...]`.
   return Left->Previous && Left->Previous->is(tok::r_square) &&
          Left->Previous->MatchingParen &&
          Left->Previous->MatchingParen->is(TT_LambdaLSquare);
 }
 
+/// Returns \c true if the token is followed by a boolean condition, \c false
+/// otherwise.
+static bool isKeywordWithCondition(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
+                     tok::kw_constexpr, tok::kw_catch);
+}
+
 /// A parser that gathers additional information about tokens.
 ///
 /// The \c TokenAnnotator tries to match parenthesis and square brakets and
 /// store a parenthesis levels. It also tries to resolve matching "<" and ">"
 /// into template parameter lists.
 class AnnotatingParser {
 public:
   AnnotatingParser(const FormatStyle &Style, AnnotatedLine &Line,
Contexts.back().InTemplateArgument =
         Left->Previous && Left->Previous->Tok.isNot(tok::kw_template);
 
     if (Style.Language == FormatStyle::LK_Java &&
         CurrentToken->is(tok::question))
       next();
 
     while (CurrentToken) {
       if (CurrentToken->is(tok::greater)) {
+        // Try to do a better job at looking for ">>" within the condition of
+        // a statement.
+        if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
+            Left->ParentBracket != tok::less &&
+            isKeywordWithCondition(*Line.First))
+          return false;
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
         // In TT_Proto, we must distignuish between:
         //   map<key, value>
         //   msg < item: data >
         //   msg: < item: data >
         // In TT_TextProto, map<key, value> does not occur.
         if (Style.Language == FormatStyle::LK_TextProto ||

 }
 
 bool TokenAnnotator::spaceRequiredBeforeParens(const FormatToken &Right) const {
   return Style.SpaceBeforeParens == FormatStyle::SBPO_Always ||
          (Style.SpaceBeforeParens == FormatStyle::SBPO_NonEmptyParentheses &&
           Right.ParameterCount > 0);
 }
 
-/// Returns \c true if the token is followed by a boolean condition, \c false
-/// otherwise.
-static bool isKeywordWithCondition(const FormatToken &Tok) {
-  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
-                     tok::kw_constexpr, tok::kw_catch);
-}
-
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
                                           const FormatToken &Left,
                                           const FormatToken &Right) {
   if (Left.is(tok::kw_return) && Right.isNot(tok::semi))
     return true;
   if (Left.is(Keywords.kw_assert) && Style.Language == FormatStyle::LK_Java)
     return true;
   if (Style.ObjCSpaceAfterProperty && Line.Type == LT_ObjCProperty &&

clang/unittests/Format/FormatTest.cpp

verifyFormat("f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
                getLLVMStyleWithColumns(35));
 
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("void f() {\n"
                "  while (a < b && c > d) {\n"
                "  }\n"
                "}");
+  verifyFormat("int j = 10;\n"
+               "for (int i = 0; i < j >> 1; i++)\n"
+               "  ;");
   verifyFormat("template <typename... Types>\n"
                "typename enable_if<0 < sizeof...(Types)>::type Foo() {}");
 
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
                "    aaaaaaaaaaaaaaaaaaaaaaaaaaaaa >> aaaaa);",
                getLLVMStyleWithColumns(60));
   verifyFormat("static_assert(is_convertible<A &&, B>::value, \"AAA\");");
   verifyFormat("Constructor(A... a) : a_(X<A>{std::forward<A>(a)}...) {}");