Fixed column shift when formatting line containing bit shift operators

[idlecode]

Summary

During clang-format source lexing >> and << operators are split and
treated as two less/greater operators but column position of following
tokens was not adjusted accordingly.

Fixes pr26887

[mprobst]

Looks good to me, but Daniel is a better owner for this code.

[djasper]

test/Format/bitshift-operator-width.cpp

1 ↗

(On Diff #74143)

Could you move this test into unittests/Format/FormatTest.cpp?

[idlecode]

Sure, I will upload diff as soon as I test it

[djasper]

Generally, always upload diffs with the full file as context to phabricator. That way, it is easier to see how the diff fits into the rest of the file. Thanks for fixing this!!

lib/Format/FormatTokenLexer.cpp

528 ↗

(On Diff #74412)

++Column;

533 ↗

(On Diff #74412)

++Column;

unittests/Format/FormatTest.cpp

11364 ↗

(On Diff #74412)

Could you add this right underneath the test "UnderstandsTemplateParameters"?

11365 ↗

(On Diff #74412)

It's always useful to have some other formatting being done in the same test. We repeatedly ran into cases in the past where a test only passed because some change effectively disabled formatting for a specific line. I suggest writing these as:

EXPECT_EQ("int a = 1 << 2; /* foo\n"
          "                   bar */",
          format("int    a=1<<2;  /* foo\n"
                 "                   bar */"));

[idlecode]

Thanks for pointing it out, just a minute ago I found a proper document mentioning it (I have no idea how I could miss it).
I hope to be more use in future :)

unittests/Format/FormatTest.cpp

11365 ↗

(On Diff #74412)

Oh, that is worth mentioning, thanks :)

11365 ↗

(On Diff #74412)

Oh, that is good to know; Done

[djasper]

Looks good. Thank you!

[malcolm.parsons]

Do you have commit access?

[idlecode]

No, not yet - I was about to ask someone to commit this for me :)

cfe/trunk/lib/Format/FormatTokenLexer.cpp

if (Style.Language == FormatStyle::LK_Java &&
                FormatTok->isOneOf(tok::kw_struct, tok::kw_union,
                                   tok::kw_operator)) {
       FormatTok->Tok.setKind(tok::identifier);
       FormatTok->Tok.setIdentifierInfo(nullptr);
     }
   } else if (FormatTok->Tok.is(tok::greatergreater)) {
     FormatTok->Tok.setKind(tok::greater);
     FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+    ++Column;
     StateStack.push(LexerState::TOKEN_STASHED);
   } else if (FormatTok->Tok.is(tok::lessless)) {
     FormatTok->Tok.setKind(tok::less);
     FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+    ++Column;
     StateStack.push(LexerState::TOKEN_STASHED);
   }
 
   // Now FormatTok is the next non-whitespace token.
 
   StringRef Text = FormatTok->TokenText;
   size_t FirstNewlinePos = Text.find('\n');
   if (FirstNewlinePos == StringRef::npos) {

cfe/trunk/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, UnderstandsTemplateParameters) {
   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)}...) {}");
   verifyFormat("< < < < < < < < < < < < < < < < < < < < < < < < < < < < < <");
 }
 
+TEST_F(FormatTest, BitshiftOperatorWidth) {
+  EXPECT_EQ("int a = 1 << 2; /* foo\n"
+            "                   bar */",
+            format("int    a=1<<2;  /* foo\n"
+                   "                   bar */"));
+
+  EXPECT_EQ("int b = 256 >> 1; /* foo\n"
+            "                     bar */",
+            format("int  b  =256>>1 ;  /* foo\n"
+                   "                      bar */"));
+}
+
 TEST_F(FormatTest, UnderstandsBinaryOperators) {
   verifyFormat("COMPARE(a, ==, b);");
   verifyFormat("auto s = sizeof...(Ts) - 1;");
 }
 
 TEST_F(FormatTest, UnderstandsPointersToMembers) {
   verifyFormat("int A::*x;");
   verifyFormat("int (S::*func)(void *);");