[PATCH] D25439: Fixed column shift when formatting line containing bit shift operators

[Daniel Jasper via cfe-commits]

djasper added a comment.

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!!



================
Comment at: lib/Format/FormatTokenLexer.cpp:528
     FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+    Column += 1;
     StateStack.push(LexerState::TOKEN_STASHED);
----------------
++Column;


================
Comment at: lib/Format/FormatTokenLexer.cpp:533
     FormatTok->TokenText = FormatTok->TokenText.substr(0, 1);
+    Column += 1;
     StateStack.push(LexerState::TOKEN_STASHED);
----------------
++Column;


================
Comment at: unittests/Format/FormatTest.cpp:11364
 
+TEST_F(FormatTest, BitshiftOperatorWidth) {
+  std::string left = "int a = 1 << 2; /* foo\n"
----------------
Could you add this right underneath the test "UnderstandsTemplateParameters"?


================
Comment at: unittests/Format/FormatTest.cpp:11365
+TEST_F(FormatTest, BitshiftOperatorWidth) {
+  std::string left = "int a = 1 << 2; /* foo\n"
+                     "                   bar */";
----------------
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 */"));


https://reviews.llvm.org/D25439