Typz updated this revision to Diff 115830.
Typz added a comment.

Remove `Reflow` from LineState, and perform the decision again during 
reconstruction phase.


https://reviews.llvm.org/D33589

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9809,6 +9809,60 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragma    option   -C   -A"));
 }
 
+TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 20;
+
+  verifyFormat("int a; // the\n"
+               "       // comment", Style);
+  EXPECT_EQ("int a; /* first line\n"
+            "        * second\n"
+            "        * line third\n"
+            "        * line\n"
+            "        */",
+			format("int a; /* first line\n"
+                   "        * second\n"
+                   "        * line third\n"
+                   "        * line\n"
+                   "        */", Style));
+  EXPECT_EQ("int a; // first line\n"
+            "       // second\n"
+            "       // line third\n"
+            "       // line",
+            format("int a; // first line\n"
+                   "       // second line\n"
+                   "       // third line",
+                   Style));
+
+  Style.PenaltyExcessCharacter = 90;
+  verifyFormat("int a; // the comment", Style);
+  EXPECT_EQ("int a; // the\n"
+            "       // comment aa",
+            format("int a; // the comment aa", Style));
+  EXPECT_EQ("int a; /* first line\n"
+            "        * second line\n"
+            "        * third line\n"
+            "        */",
+			format("int a; /* first line\n"
+            	   "        * second line\n"
+			       "        * third line\n"
+            	   "        */", Style));
+  EXPECT_EQ("int a; // first line\n"
+            "       // second line\n"
+            "       // third line",
+            format("int a; // first line\n"
+                   "       // second line\n"
+                   "       // third line",
+                   Style));
+  EXPECT_EQ("int a; /* first line\n"
+            "        * second\n"
+            "        * line third\n"
+            "        * line\n"
+            "        */",
+            format("int a; /* first line second line third line"
+				   "\n*/", Style));
+}
+
 #define EXPECT_ALL_STYLES_EQUAL(Styles)                                        \
   for (size_t i = 1; i < Styles.size(); ++i)                                   \
   EXPECT_EQ(Styles[0], Styles[i]) << "Style #" << i << " of " << Styles.size() \
Index: lib/Format/ContinuationIndenter.h
===================================================================
--- lib/Format/ContinuationIndenter.h
+++ lib/Format/ContinuationIndenter.h
@@ -27,6 +27,7 @@
 namespace format {
 
 class AnnotatedLine;
+class BreakableToken;
 struct FormatToken;
 struct LineState;
 struct ParenState;
@@ -100,6 +101,16 @@
   unsigned breakProtrudingToken(const FormatToken &Current, LineState &State,
                                 bool DryRun);
 
+  /// \brief Perform the reflowing of a BreakableToken.
+  /// Reflow=true, then the function will reflow the token as needed. Otherwise, it simply computes
+  /// the penalty caused by this tokens characters.
+  ///
+  /// \returns The penalty of reflowing the token if State.Reflow=true; otherwise
+  /// the penalty of characters going beyond the column limit.
+  unsigned reflowProtrudingToken(const FormatToken & Current, LineState & State,
+                                 std::unique_ptr<clang::format::BreakableToken> & Token,
+                                 unsigned ColumnLimit, bool DryRun, bool Reflow);
+
   /// \brief Appends the next token to \p State and updates information
   /// necessary for indentation.
   ///
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1313,6 +1313,40 @@
   if (Current.UnbreakableTailLength >= ColumnLimit)
     return 0;
 
+  // Verify if the comment should be reflown
+  LineState PrevState = State;
+  unsigned Penalty = reflowProtrudingToken(Current, State, Token, ColumnLimit, true, true);
+  bool Reflow = true;
+  if (Penalty > 0) {
+    LineState NoReflowState = PrevState;
+    unsigned NoReflowPenalty = reflowProtrudingToken(Current, NoReflowState, Token, ColumnLimit,
+                                                     true, false);
+    if (NoReflowPenalty <= Penalty) {
+      Reflow = false;
+      State = NoReflowState;
+      Penalty = NoReflowPenalty;
+    }
+  }
+
+  // Actually do the reflow, if DryRun=false
+  if (!DryRun)
+    reflowProtrudingToken(Current, PrevState, Token, ColumnLimit, false, Reflow);
+
+  // Do not count the penalty twice, it will be added afterwards
+  if (State.Column > getColumnLimit(State)) {
+    unsigned ExcessCharacters = State.Column - getColumnLimit(State);
+    Penalty -= Style.PenaltyExcessCharacter * ExcessCharacters;
+  }
+
+  return Penalty;
+}
+
+unsigned ContinuationIndenter::reflowProtrudingToken(const FormatToken &Current, LineState &State,
+                                                     std::unique_ptr<BreakableToken> &Token,
+                                                     unsigned ColumnLimit, bool DryRun,
+                                                     bool Reflow) {
+  unsigned StartColumn = State.Column - Current.ColumnWidth;
+
   unsigned RemainingSpace = ColumnLimit - Current.UnbreakableTailLength;
   bool BreakInserted = false;
   // We use a conservative reflowing strategy. Reflow starts after a line is
@@ -1337,14 +1371,18 @@
                                      RemainingSpace, SplitBefore, Whitespaces);
     RemainingTokenColumns = Token->getLineLengthAfterSplitBefore(
         LineIndex, TailOffset, RemainingTokenColumns, ColumnLimit, SplitBefore);
+    if (!Reflow) {
+      if (RemainingTokenColumns > RemainingSpace)
+        Penalty += Style.PenaltyExcessCharacter *
+                   (RemainingTokenColumns - RemainingSpace);
+      continue;
+    }
     while (RemainingTokenColumns > RemainingSpace) {
       BreakableToken::Split Split = Token->getSplit(
           LineIndex, TailOffset, ColumnLimit, CommentPragmasRegex);
       if (Split.first == StringRef::npos) {
-        // The last line's penalty is handled in addNextStateToQueue().
-        if (LineIndex < EndIndex - 1)
-          Penalty += Style.PenaltyExcessCharacter *
-                     (RemainingTokenColumns - RemainingSpace);
+        Penalty += Style.PenaltyExcessCharacter *
+                   (RemainingTokenColumns - RemainingSpace);
         break;
       }
       assert(Split.first != 0);
@@ -1400,6 +1438,8 @@
   State.Column = RemainingTokenColumns;
 
   if (BreakInserted) {
+    assert(Reflow);
+
     // If we break the token inside a parameter list, we need to break before
     // the next parameter on all levels, so that the next parameter is clearly
     // visible. Line comments already introduce a break.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to