[PATCH] D39900: Refactor ContinuationIndenter's breakProtrudingToken logic into slightly moreorthogonal pieces.

Krasimir Georgiev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 10 07:25:25 PST 2017


krasimir added inline comments.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1028
+  unsigned Penalty =
+      handleEndOfLine(Current, State, DryRun, AllowBreak);
 
----------------
Why `handleEndOfLine`? Is it guaranteed that here we've reached the end of the line?


================
Comment at: lib/Format/ContinuationIndenter.cpp:1284
+  unsigned StartColumn = State.Column - Current.ColumnWidth;
+  auto Delimiter = *getRawStringDelimiter(Current.TokenText);
   // The text of a raw string is between the leading 'R"delimiter(' and the
----------------
At this point, we compute `getRawStringDelimiter` twice: once here, and once in `getRawStringStyle` in the caller side. Ideally, we'd only like to compute it once. That's why it was a parameter before.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1408
+std::unique_ptr<BreakableToken> ContinuationIndenter::createBreakableToken(
+    const FormatToken &Current, LineState &State, bool AllowBreak) {
   unsigned StartColumn = State.Column - Current.ColumnWidth;
----------------
If `AllowBreak == false`, in which cases will this return a non-`nullptr`?


================
Comment at: unittests/Format/FormatTest.cpp:6322
                "#include \"some long include\" // with a comment\n"
-               "#include \"some very long include paaaaaaaaaaaaaaaaaaaaaaath\"",
+               "#include \"some very long include path\"\n"
+               "#include <some/very/long/include/path>\n",
----------------
What happened to the old test line?


================
Comment at: unittests/Format/FormatTestComments.cpp:683
             "  //\\\n"
-            "  // long 1 2 3 4\n"
-            "  // 5\n"
+            "  // long 1 2 3 4 5\n"
             "}",
----------------
Please also add a test case adding a ` 6`, which actually gets broken.


https://reviews.llvm.org/D39900





More information about the cfe-commits mailing list