[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

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

Rebase


https://reviews.llvm.org/D33589

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9475,6 +9475,60 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -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/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -885,6 +885,7 @@
 for (std::deque::iterator I = Path.begin(), E = Path.end();
  I != E; ++I) {
   unsigned Penalty = 0;
+  State.Reflow = (*I)->State.Reflow;
   formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
   Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
 
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,11 @@
   unsigned breakProtrudingToken(const FormatToken , LineState ,
 bool DryRun);
 
+  unsigned reflowProtrudingToken(const FormatToken & Current, LineState & State,
+ std::unique_ptr & Token,
+ unsigned ColumnLimit, bool DryRun);
+
+
   /// \brief Appends the next token to \p State and updates information
   /// necessary for indentation.
   ///
@@ -350,6 +356,8 @@
   /// \brief The indent of the first token.
   unsigned FirstIndent;
 
+  bool Reflow = true;
+
   /// \brief The line that is being formatted.
   ///
   /// Does not need to be considered for memoization because it doesn't change.
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1219,92 +1219,10 @@
   return 0;
 }
 
-unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken ,
-LineState ,
-bool DryRun) {
-  // Don't break multi-line tokens other than block comments. Instead, just
-  // update the state.
-  if (Current.isNot(TT_BlockComment) && Current.IsMultiline)
-return addMultilineToken(Current, State);
-
-  // Don't break implicit string literals or 

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

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

Move code out of optimizer, directly into 
ContinuationIndenter::breakProtrudingToken(), to minimize impact on performance.
Comment reflowing of breakable items which break the limit will be slightly 
slower, since we now consider the two options; however this change has no 
impact on the performance of processing non-breakable items, and may actually 
increase the operformance of processing breakable items which do not need to be 
reflowed.


https://reviews.llvm.org/D33589

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9393,6 +9393,60 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -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/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -866,6 +866,7 @@
 for (std::deque::iterator I = Path.begin(), E = Path.end();
  I != E; ++I) {
   unsigned Penalty = 0;
+  State.Reflow = (*I)->State.Reflow;
   formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
   Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
 
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,11 @@
   unsigned breakProtrudingToken(const FormatToken , LineState ,
 bool DryRun);
 
+  unsigned reflowProtrudingToken(const FormatToken & Current, LineState & State,
+ std::unique_ptr & Token,
+ unsigned ColumnLimit, bool DryRun);
+
+
   /// \brief Appends the next token to \p State and updates information
   /// necessary for indentation.
   ///
@@ -350,6 +356,8 @@
   /// \brief The indent of the first token.
   unsigned FirstIndent;
 
+  bool Reflow = true;
+
   /// \brief The line that is being formatted.
   ///
   /// Does not need to be considered for memoization because it doesn't change.
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1219,6 +1219,112 @@
   return 0;
 }
 
+unsigned 

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz marked 2 inline comments as done.
Typz added inline comments.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:723
   FormatDecision LastFormat = Node->State.NextToken->Decision;
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,

Typz wrote:
> djasper wrote:
> > This is almost certainly a no-go. It turns the algorithm from exploring a 
> > state space with a branching factor of 2 into something with a branching 
> > factor of 4.
> > 
> > Even assuming we can perfectly reuse results from computations (or in other 
> > words hit the same state through different path using Dijkstra's 
> > algorithm), the additional bool in StateNode doubles the number of states 
> > we are going to visit. You don't even seem to make a distinction of whether 
> > the token under investigation can possibly be split. You do look at 
> > NoLineBreak(InOperand), but even if those are false, the vast majority of 
> > tokens will never be split.
> > 
> > However, even if you narrow that down, I am not convinced that fixing this 
> > inconsistency around the formatting penalties is worth the runtime 
> > performance hit. I am generally fine with changing the behavior in the way 
> > you are proposing, but only if it has zero (negative) effect on performance.
> Making the distinction to skip some path is done at the beginning of 
> addNextStateToQueue(), though indeed not very precisely at the moment.
> 
> I can improve the check (i.e. by copying all the early return conditions from 
> the beginning of `ContinuationIndenter::breakProtrudingToken()`, which will 
> greatly reduce the number of possible state, but stilll have a small impact.
> 
> The alternative would be modify ContinuationIndenter::breakProtrudingToken(), 
> so that it computes the penalty for reflowing as well as not-reflowing, and 
> updates the LineState with the best solution. It would certainly have an 
> impact on performance, but would not increase the size of the state space.
> 
> Another issue with that approach is that the information is not passed from 
> the DRYRUN phase to the actual rewriting phase. I could store this 
> information in the LineState, to re-use it in the reconstruction phase, but 
> this seems really wrong (and would work only in the exact case of the 
> optimizing line formatter).
> 
> What do you think? Should I keep the same structure but reduce the number of 
> explored state; or move the decision into ContinuationIndenter, possibly 
> storing the result in LineState?
> 
Actually, it seems it cannot be done inside 
ContinuationIndenter::breakProtrudingToken(), since this causes the whitespace 
manager to be called twice for the same token.


https://reviews.llvm.org/D33589



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35483: clang-format: fix block OpeningLineIndex around preprocessor

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Herald added a subscriber: klimek.

The current code would return an incorrect value when a preprocessor
directive is present immediately after the opening brace: this causes
the nanespace end comment fixer to break in some places, for exemple it
would not add the comment in this case:

  namespace a {
  #define FOO
  }

Fixing the computation is simple enough, but it was breaking a feature,
as it would cause comments to be added also when the namespace
declaration was dependant on conditional compilation.

To fix this, a hash of the current preprocessor stack/branches is
computed at the beginning of parseBlock(), so that we explicitely do not
store the OpeningLineIndex when the beginning and end of the block are
not in the same preprocessor conditions.

Tthe hash is computed based on the line, but this could propbably be
improved by using the actual condition, so that clang-format would be
able to match multiple identical #ifdef blocks.


https://reviews.llvm.org/D35483

Files:
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -509,6 +509,51 @@
 "}\n"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddEndCommentForNamespacesAroundMacros) {
+  EXPECT_EQ("namespace A {\n"
+"#if 0\n"
+"int i;\n"
+"#endif\n"
+"}// namespace A",
+fixNamespaceEndComments("namespace A {\n"
+"#if 0\n"
+"int i;\n"
+"#endif\n"
+"}"));
+  EXPECT_EQ("#if 0\n"
+"#endif\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A",
+fixNamespaceEndComments("#if 0\n"
+"#endif\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A\n"
+"#if 0\n"
+"#endif",
+fixNamespaceEndComments("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+"#if 0\n"
+"#endif"));
+  EXPECT_EQ("namespace A {\n"
+"#define FOO\n"
+"int i;\n"
+"}// namespace A",
+fixNamespaceEndComments("namespace A {\n"
+"#define FOO\n"
+"int i;\n"
+"}"));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest,
DoesNotAddEndCommentForNamespacesInMacroDeclarations) {
   EXPECT_EQ("#ifdef 1\n"
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -155,6 +155,7 @@
   void conditionalCompilationEnd();
 
   bool isOnNewLine(const FormatToken );
+  size_t computePPHash() const;
 
   // FIXME: We are constantly running into bugs where Line.Level is incorrectly
   // subtracted from beyond 0. Introduce a method to subtract from Line.Level
@@ -174,7 +175,7 @@
 
   // Preprocessor directives are parsed out-of-order from other unwrapped lines.
   // Thus, we need to keep a list of preprocessor directives to be reported
-  // after an unwarpped line that has been started was finished.
+  // after an unwrapped line that has been started was finished.
   SmallVector PreprocessorDirectives;
 
   // New unwrapped lines are added via CurrentLines.
@@ -207,8 +208,15 @@
 PP_Unreachable  // #if 0 or a conditional preprocessor block inside #if 0
   };
 
+  struct PPBranch {
+PPBranch(PPBranchKind Kind, size_t Line) : Kind(Kind), Line(Line) {}
+bool operator==(PPBranchKind Kind) const { return Kind == this->Kind; }
+PPBranchKind Kind;
+size_t Line;
+  };
+
   // Keeps a stack of currently active preprocessor branching directives.
-  SmallVector PPStack;
+  SmallVector PPStack;
 
   // The \c UnwrappedLineParser re-parses the code for each combination
   // of preprocessor branches that can be taken.
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -452,23 +452,43 @@
   FormatTok = 

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 106221.
Typz marked 3 inline comments as done.
Typz added a comment.

Rename option to AlignAfterOperator


https://reviews.llvm.org/D32478

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -102,7 +102,7 @@
   verifyFormat("var x = a() in\n"
".aa.aa;");
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
-  Style.AlignOperands = true;
+  Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("var x = a() in\n"
".aa.aa;",
Style);
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2702,6 +2702,9 @@
"  > c) {\n"
"}",
Style);
+  verifyFormat("return a\n"
+   "   && bbb;",
+   Style);
   verifyFormat("return (a)\n"
"   // comment\n"
"   + b;",
@@ -2730,11 +2733,103 @@
 
   Style.ColumnLimit = 60;
   verifyFormat("zz\n"
-   "= b\n"
+   "= \n"
"  >> (aa);",
Style);
 }
 
+TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Style.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+
+  verifyFormat("bool value = a\n"
+   "   + a\n"
+   "   + a\n"
+   "  == a\n"
+   " * b\n"
+   " + b\n"
+   "  && a\n"
+   " * a\n"
+   " > c;",
+   Style);
+  verifyFormat("if (a\n"
+   "* \n"
+   "+ aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "+ \n"
+   "  * aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "== \n"
+   "   * aa\n"
+   "   + bbb) {\n}",
+   Style);
+  verifyFormat("if () {\n"
+   "} else if (a\n"
+   "   && b // break\n"
+   "  > c) {\n"
+   "}",
+   Style);
+  verifyFormat("return a\n"
+   "&& bbb;",
+   Style);
+  verifyFormat("return (a)\n"
+   " // comment\n"
+   " + b;",
+   Style);
+  verifyFormat(
+  "int aa = aa\n"
+  "   * bbb\n"
+  "   + cc;",
+  Style);
+
+  verifyFormat("a\n"
+   "=  + ;",
+   Style);
+
+  verifyFormat("return boost::fusion::at_c<0>().second\n"
+   "== boost::fusion::at_c<1>().second;",
+   Style);
+
+  Style.ColumnLimit = 60;
+  verifyFormat("z\n"
+   "= \n"
+   "   >> 

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

I renamed the option to `AlignAfterOperator`, is it acceptable now?
(I also thought of `UnindentOperator`, but I think it is better to keep the 
notion of alignment)

Note that this style is used in multiple open-source projects: Skia 
, parts of QtCreator 
 code base 
(e.g. in multiple plugins: clang, cpptools, android...), the OpenEXR 
 library...


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement and namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:723
   FormatDecision LastFormat = Node->State.NextToken->Decision;
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,

djasper wrote:
> This is almost certainly a no-go. It turns the algorithm from exploring a 
> state space with a branching factor of 2 into something with a branching 
> factor of 4.
> 
> Even assuming we can perfectly reuse results from computations (or in other 
> words hit the same state through different path using Dijkstra's algorithm), 
> the additional bool in StateNode doubles the number of states we are going to 
> visit. You don't even seem to make a distinction of whether the token under 
> investigation can possibly be split. You do look at NoLineBreak(InOperand), 
> but even if those are false, the vast majority of tokens will never be split.
> 
> However, even if you narrow that down, I am not convinced that fixing this 
> inconsistency around the formatting penalties is worth the runtime 
> performance hit. I am generally fine with changing the behavior in the way 
> you are proposing, but only if it has zero (negative) effect on performance.
Making the distinction to skip some path is done at the beginning of 
addNextStateToQueue(), though indeed not very precisely at the moment.

I can improve the check (i.e. by copying all the early return conditions from 
the beginning of `ContinuationIndenter::breakProtrudingToken()`, which will 
greatly reduce the number of possible state, but stilll have a small impact.

The alternative would be modify ContinuationIndenter::breakProtrudingToken(), 
so that it computes the penalty for reflowing as well as not-reflowing, and 
updates the LineState with the best solution. It would certainly have an impact 
on performance, but would not increase the size of the state space.

Another issue with that approach is that the information is not passed from the 
DRYRUN phase to the actual rewriting phase. I could store this information in 
the LineState, to re-use it in the reconstruction phase, but this seems really 
wrong (and would work only in the exact case of the optimizing line formatter).

What do you think? Should I keep the same structure but reduce the number of 
explored state; or move the decision into ContinuationIndenter, possibly 
storing the result in LineState?




Comment at: lib/Format/UnwrappedLineFormatter.cpp:764
   return;
+if (!BreakToken && !Indenter->canSplit(PreviousNode->State))
+  return;

djasper wrote:
> It's not clear to me why you'd return here.
This is needed to avoid trying to break when this is not supported: no point 
doing twice the same thing.


https://reviews.llvm.org/D33589



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35483: clang-format: fix block OpeningLineIndex around preprocessor

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 107030.
Typz marked an inline comment as done.
Typz added a comment.

Add more tests


https://reviews.llvm.org/D35483

Files:
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -509,6 +509,134 @@
 "}\n"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddEndCommentForNamespacesAroundMacros) {
+  // Conditional blocks around are fine
+  EXPECT_EQ("namespace A {\n"
+"#if 1\n"
+"int i;\n"
+"#endif\n"
+"}// namespace A",
+fixNamespaceEndComments("namespace A {\n"
+"#if 1\n"
+"int i;\n"
+"#endif\n"
+"}"));
+  EXPECT_EQ("#if 1\n"
+"#endif\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A",
+fixNamespaceEndComments("#if 1\n"
+"#endif\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A\n"
+"#if 1\n"
+"#endif",
+fixNamespaceEndComments("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+"#if 1\n"
+"#endif"));
+  EXPECT_EQ("#if 1\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A\n"
+"#endif",
+fixNamespaceEndComments("#if 1\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+"#endif"));
+
+  // Macro definition has no impact
+  EXPECT_EQ("namespace A {\n"
+"#define FOO\n"
+"int i;\n"
+"}// namespace A",
+fixNamespaceEndComments("namespace A {\n"
+"#define FOO\n"
+"int i;\n"
+"}"));
+  EXPECT_EQ("#define FOO\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A",
+fixNamespaceEndComments("#define FOO\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A\n"
+"#define FOO\n",
+fixNamespaceEndComments("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+"#define FOO\n"));
+
+  // No replacement if open & close in different conditional blocks
+  EXPECT_EQ("#if 1\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#if 1\n"
+"}\n"
+"#endif",
+fixNamespaceEndComments("#if 1\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#if 1\n"
+"}\n"
+"#endif"));
+  EXPECT_EQ("#ifdef A\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#ifdef B\n"
+"}\n"
+"#endif",
+fixNamespaceEndComments("#ifdef A\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#ifdef B\n"
+"}\n"
+"#endif"));
+
+  // No replacement inside unreachable conditional block
+  EXPECT_EQ("#if 0\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+

[PATCH] D35557: clang-format: merge short case labels with trailing comments

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Herald added a subscriber: klimek.

Allow merging short case labels when they actually end with a comment
(like a comment after the ``break``) and when followed by switch-level
comments (e.g. aligned with next case):

  switch(a) {
  case 0: break; // comment at end of case
  case 1: return value;
  // comment related to next case
  // comment related to next case
  case 2:
  }


https://reviews.llvm.org/D35557

Files:
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -897,6 +897,38 @@
"}",
Style);
   verifyFormat("switch (a) {\n"
+   "case 0: return; // comment\n"
+   "case 1: break;  // comment\n"
+   "case 2: return;\n"
+   "// comment\n"
+   "case 3: return;\n"
+   "// comment 1\n"
+   "// comment 2\n"
+   "// comment 3\n"
+   "case 4: break; /* comment */\n"
+   "case 5:\n"
+   "  // comment\n"
+   "  break;\n"
+   "}",
+   Style);
+  EXPECT_EQ("switch (a) {\n"
+"case 1:\n"
+"  x = 8;\n"
+"  // fall through\n"
+"case 2: x = 8;\n"
+"// comment\n"
+"default:\n"
+"}",
+format("switch (a) {\n"
+   "case 1: x = 8;\n"
+   "  // fall through\n"
+   "case 2:\n"
+   "  x = 8;\n"
+   "// comment\n"
+   "default:\n"
+   "}",
+   Style));
+  verifyFormat("switch (a) {\n"
"#if FOO\n"
"case 0: return 0;\n"
"#endif\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -382,7 +382,9 @@
   return 0;
 unsigned NumStmts = 0;
 unsigned Length = 0;
+bool EndsWithComment = false;
 bool InPPDirective = I[0]->InPPDirective;
+const unsigned Level = I[0]->Level;
 for (; NumStmts < 3; ++NumStmts) {
   if (I + 1 + NumStmts == E)
 break;
@@ -392,9 +394,26 @@
   if (Line->First->isOneOf(tok::kw_case, tok::kw_default, tok::r_brace))
 break;
   if (Line->First->isOneOf(tok::kw_if, tok::kw_for, tok::kw_switch,
-   tok::kw_while, tok::comment) ||
-  Line->Last->is(tok::comment))
+   tok::kw_while) ||
+  EndsWithComment)
 return 0;
+  if (Line->First->is(tok::comment)) {
+if (Level != Line->Level)
+  return 0;
+SmallVectorImpl::const_iterator J = I + 2 + NumStmts;
+for (; J != E; ++J) {
+  const AnnotatedLine *Line = J[0];
+  if (Line->InPPDirective != InPPDirective)
+break;
+  if (Line->First->isOneOf(tok::kw_case, tok::kw_default, 
tok::r_brace))
+break;
+  if (Line->First->isNot(tok::comment) || Level != Line->Level)
+return 0;
+}
+break;
+  }
+  if (Line->Last->is(tok::comment))
+EndsWithComment = true;
   Length += I[1 + NumStmts]->Last->TotalLength + 1; // 1 for the space.
 }
 if (NumStmts == 0 || NumStmts == 3 || Length > Limit)


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -897,6 +897,38 @@
"}",
Style);
   verifyFormat("switch (a) {\n"
+   "case 0: return; // comment\n"
+   "case 1: break;  // comment\n"
+   "case 2: return;\n"
+   "// comment\n"
+   "case 3: return;\n"
+   "// comment 1\n"
+   "// comment 2\n"
+   "// comment 3\n"
+   "case 4: break; /* comment */\n"
+   "case 5:\n"
+   "  // comment\n"
+   "  break;\n"
+   "}",
+   Style);
+  EXPECT_EQ("switch (a) {\n"
+"case 1:\n"
+"  x = 8;\n"
+"  // fall through\n"
+"case 2: x = 8;\n"
+"// comment\n"
+"default:\n"
+"}",
+format("switch (a) {\n"
+   "case 1: x = 8;\n"
+   "  // fall through\n"
+   "case 2:\n"
+   "  x = 8;\n"
+   "// comment\n"
+   "default:\n"
+   "}",
+   Style));
+  verifyFormat("switch (a) {\n"
"#if FOO\n"
"case 0: return 0;\n"

[PATCH] D33440: clang-format: better handle statement and namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

t>>! In https://reviews.llvm.org/D33440#812645, @djasper wrote:

> So, there are two things in this patch: Statement macros and namespace 
> macros. Lets break this out and handle them individually. They really aren't 
> related that much.

Indeed, the only "relation" is the implementation, which now uses the exact 
same list (internally) to match all macros. Phabricator makes it very difficult 
to work with related changes pushed as multiple reviews, so I ended up merging 
the two features.

> Statement macros:
>  I think clang-format's handling here is good enough. clang-format does not 
> insert the line break, but it also doesn't remove it. I am not 100% sure 
> here, so I an be convinced. But I want to understand the use cases better. Do 
> you expect people to run into this frequently? I am essentially trying to 
> understand whether the cost of an extra option is worth the benefit it is 
> giving.

This happens relatively often, for example when fixing "unused parameter 
warning" on an inlined function: ``int f(int a) { return 0; }`` often gets 
fixed to ``int f(int a) { Q_UNUSED(a) return 0; }`` and clang-format does not 
fix the formatting...

> Namespace macros:
>  How important are the automatic closing comments to you? I'd say that we 
> should punt on that and leave it to the user to fix comments of these. And 
> then, we could try to make the things we already have in MacroBlockBegin 
> detect whether it ends with an opening brace and not need an extra list here. 
> What do you think?

This is not just about automatic closing comments, there are may differences: 
indentation of namespaces, 'compacting' of namespaces when `CompactNamespaces` 
is used, detection of inline functions (for putting on a single line with 
SFS_Inline), handling of empty blocks (i.e. use 
`BraceWrappingFlags.SplitEmptyNamespace`)...


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35557: clang-format: merge short case labels with trailing comments

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 107841.
Typz marked 4 inline comments as done.
Typz added a comment.

Address review comments


https://reviews.llvm.org/D35557

Files:
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -897,6 +897,77 @@
"}",
Style);
   verifyFormat("switch (a) {\n"
+   "case 0: return; // comment\n"
+   "case 1: break;  // comment\n"
+   "case 2: return;\n"
+   "// comment\n"
+   "case 3: return;\n"
+   "// comment 1\n"
+   "// comment 2\n"
+   "// comment 3\n"
+   "case 4: break; /* comment */\n"
+   "case 5:\n"
+   "  // comment\n"
+   "  break;\n"
+   "case 6: /* comment */ x = 1; break;\n"
+   "case 7: x = /* comment */ 1; break;\n"
+   "case 8:\n"
+   "  x = 1; /* comment */\n"
+   "  break;\n"
+   "case 9:\n"
+   "  break; // comment line 1\n"
+   " // comment line 2\n"
+   "}",
+   Style);
+  EXPECT_EQ("switch (a) {\n"
+"case 1:\n"
+"  x = 8;\n"
+"  // fall through\n"
+"case 2: x = 8;\n"
+"// comment\n"
+"case 3:\n"
+"  return; /* comment line 1\n"
+"   * comment line 2 */\n"
+"case 4: i = 8;\n"
+"// something else\n"
+"#if FOO\n"
+"case 5: break;\n"
+"#endif\n"
+"}",
+format("switch (a) {\n"
+   "case 1: x = 8;\n"
+   "  // fall through\n"
+   "case 2:\n"
+   "  x = 8;\n"
+   "// comment\n"
+   "case 3:\n"
+   "  return; /* comment line 1\n"
+   "   * comment line 2 */\n"
+   "case 4:\n"
+   "  i = 8;\n"
+   "// something else\n"
+   "#if FOO\n"
+   "case 5: break;\n"
+   "#endif\n"
+   "}",
+   Style));
+  EXPECT_EQ("switch (a) {\n" "case 0:\n"
+"  return; // long long long long long long long long long long long long comment\n"
+"  // line\n" "}",
+format("switch (a) {\n"
+   "case 0: return; // long long long long long long long long long long long long comment line\n"
+   "}",
+   Style));
+  EXPECT_EQ("switch (a) {\n"
+"case 0:\n"
+"  return; /* long long long long long long long long long long long long comment\n"
+" line */\n"
+"}",
+format("switch (a) {\n"
+   "case 0: return; /* long long long long long long long long long long long long comment line */\n"
+   "}",
+   Style));
+  verifyFormat("switch (a) {\n"
"#if FOO\n"
"case 0: return 0;\n"
"#endif\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -382,7 +382,9 @@
   return 0;
 unsigned NumStmts = 0;
 unsigned Length = 0;
+bool EndsWithComment = false;
 bool InPPDirective = I[0]->InPPDirective;
+const unsigned Level = I[0]->Level;
 for (; NumStmts < 3; ++NumStmts) {
   if (I + 1 + NumStmts == E)
 break;
@@ -392,9 +394,26 @@
   if (Line->First->isOneOf(tok::kw_case, tok::kw_default, tok::r_brace))
 break;
   if (Line->First->isOneOf(tok::kw_if, tok::kw_for, tok::kw_switch,
-   tok::kw_while, tok::comment) ||
-  Line->Last->is(tok::comment))
+   tok::kw_while) ||
+  EndsWithComment)
 return 0;
+  if (Line->First->is(tok::comment)) {
+if (Level != Line->Level)
+  return 0;
+SmallVectorImpl::const_iterator J = I + 2 + NumStmts;
+for (; J != E; ++J) {
+  Line = *J;
+  if (Line->InPPDirective != InPPDirective)
+break;
+  if (Line->First->isOneOf(tok::kw_case, tok::kw_default, tok::r_brace))
+break;
+  if (Line->First->isNot(tok::comment) || Level != Line->Level)
+return 0;
+}
+break;
+  }
+  if (Line->Last->is(tok::comment))
+EndsWithComment = true;
   Length += I[1 + NumStmts]->Last->TotalLength + 1; // 1 for the space.
 }
 if (NumStmts == 0 || NumStmts == 3 || Length > Limit)

[PATCH] D35557: clang-format: merge short case labels with trailing comments

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: unittests/Format/FormatTest.cpp:912
+   "  break;\n"
+   "}",
+   Style);

krasimir wrote:
> I'd suggest adding more cases here, like:
> ```
>"case 6: /* comment */ x = 1; break;\n"
>"case 7: x = /* comment */ 1; break;\n"
>"case 8:\n"
>"  x = 1; /* comment */\n"
>"  break;\n"
>"case 9:\n"
>"  break; // comment line 1\n"
>" // comment line 2\n"
>"case 10:\n"
>"  return; /* comment line 1\n"
>"   * comment line 2 */\n"
>"}",
> ```
> I'm interested also why `case 10` here fails.
`case 10` fails because of test::messUp(), which removes the line break inside 
the comment.


https://reviews.llvm.org/D35557



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35483: clang-format: fix block OpeningLineIndex around preprocessor

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 107843.
Typz marked 3 inline comments as done.
Typz added a comment.

Address review commentsx


https://reviews.llvm.org/D35483

Files:
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -509,6 +509,134 @@
 "}\n"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddEndCommentForNamespacesAroundMacros) {
+  // Conditional blocks around are fine
+  EXPECT_EQ("namespace A {\n"
+"#if 1\n"
+"int i;\n"
+"#endif\n"
+"}// namespace A",
+fixNamespaceEndComments("namespace A {\n"
+"#if 1\n"
+"int i;\n"
+"#endif\n"
+"}"));
+  EXPECT_EQ("#if 1\n"
+"#endif\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A",
+fixNamespaceEndComments("#if 1\n"
+"#endif\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A\n"
+"#if 1\n"
+"#endif",
+fixNamespaceEndComments("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+"#if 1\n"
+"#endif"));
+  EXPECT_EQ("#if 1\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A\n"
+"#endif",
+fixNamespaceEndComments("#if 1\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+"#endif"));
+
+  // Macro definition has no impact
+  EXPECT_EQ("namespace A {\n"
+"#define FOO\n"
+"int i;\n"
+"}// namespace A",
+fixNamespaceEndComments("namespace A {\n"
+"#define FOO\n"
+"int i;\n"
+"}"));
+  EXPECT_EQ("#define FOO\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A",
+fixNamespaceEndComments("#define FOO\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A\n"
+"#define FOO\n",
+fixNamespaceEndComments("namespace A {\n"
+"int i;\n"
+"int j;\n"
+"}\n"
+"#define FOO\n"));
+
+  // No replacement if open & close in different conditional blocks
+  EXPECT_EQ("#if 1\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#if 1\n"
+"}\n"
+"#endif",
+fixNamespaceEndComments("#if 1\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#if 1\n"
+"}\n"
+"#endif"));
+  EXPECT_EQ("#ifdef A\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#ifdef B\n"
+"}\n"
+"#endif",
+fixNamespaceEndComments("#ifdef A\n"
+"namespace A {\n"
+"#endif\n"
+"int i;\n"
+"int j;\n"
+"#ifdef B\n"
+"}\n"
+"#endif"));
+
+  // No replacement inside unreachable conditional block
+  EXPECT_EQ("#if 0\n"
+"namespace A {\n"
+"int i;\n"
+"int j;\n"
+

[PATCH] D35483: clang-format: fix block OpeningLineIndex around preprocessor

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL309369: clang-format: fix block OpeningLineIndex around 
preprocessor (authored by Typz).

Changed prior to commit:
  https://reviews.llvm.org/D35483?vs=107843=108598#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35483

Files:
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.h
  cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: cfe/trunk/lib/Format/UnwrappedLineParser.h
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.h
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h
@@ -160,6 +160,11 @@
 
   bool isOnNewLine(const FormatToken );
 
+  // Compute hash of the current preprocessor branch.
+  // This is used to identify the different branches, and thus track if block
+  // open and close in the same branch.
+  size_t computePPHash() const;
+
   // FIXME: We are constantly running into bugs where Line.Level is incorrectly
   // subtracted from beyond 0. Introduce a method to subtract from Line.Level
   // and use that everywhere in the Parser.
@@ -178,7 +183,7 @@
 
   // Preprocessor directives are parsed out-of-order from other unwrapped lines.
   // Thus, we need to keep a list of preprocessor directives to be reported
-  // after an unwarpped line that has been started was finished.
+  // after an unwrapped line that has been started was finished.
   SmallVector PreprocessorDirectives;
 
   // New unwrapped lines are added via CurrentLines.
@@ -211,8 +216,14 @@
 PP_Unreachable  // #if 0 or a conditional preprocessor block inside #if 0
   };
 
+  struct PPBranch {
+PPBranch(PPBranchKind Kind, size_t Line) : Kind(Kind), Line(Line) {}
+PPBranchKind Kind;
+size_t Line;
+  };
+
   // Keeps a stack of currently active preprocessor branching directives.
-  SmallVector PPStack;
+  SmallVector PPStack;
 
   // The \c UnwrappedLineParser re-parses the code for each combination
   // of preprocessor branches that can be taken.
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -452,23 +452,43 @@
   FormatTok = Tokens->setPosition(StoredPosition);
 }
 
+template 
+static inline void hash_combine(std::size_t , const T ) {
+  std::hash hasher;
+  seed ^= hasher(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
+}
+
+size_t UnwrappedLineParser::computePPHash() const {
+  size_t h = 0;
+  for (const auto  : PPStack) {
+hash_combine(h, size_t(i.Kind));
+hash_combine(h, i.Line);
+  }
+  return h;
+}
+
 void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
  bool MunchSemi) {
   assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
  "'{' or macro block token expected");
   const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);
   FormatTok->BlockKind = BK_Block;
 
+  size_t PPStartHash = computePPHash();
+
   unsigned InitialLevel = Line->Level;
   nextToken(/*LevelDifference=*/AddLevel ? 1 : 0);
 
   if (MacroBlock && FormatTok->is(tok::l_paren))
 parseParens();
 
+  size_t NbPreprocessorDirectives =
+  CurrentLines ==  ? PreprocessorDirectives.size() : 0;
   addUnwrappedLine();
-  size_t OpeningLineIndex = CurrentLines->empty()
-? (UnwrappedLine::kInvalidIndex)
-: (CurrentLines->size() - 1);
+  size_t OpeningLineIndex =
+  CurrentLines->empty()
+  ? (UnwrappedLine::kInvalidIndex)
+  : (CurrentLines->size() - 1 - NbPreprocessorDirectives);
 
   ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
   MustBeDeclaration);
@@ -486,6 +506,8 @@
 return;
   }
 
+  size_t PPEndHash = computePPHash();
+
   // Munch the closing brace.
   nextToken(/*LevelDifference=*/AddLevel ? -1 : 0);
 
@@ -495,11 +517,14 @@
   if (MunchSemi && FormatTok->Tok.is(tok::semi))
 nextToken();
   Line->Level = InitialLevel;
-  Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
-  if (OpeningLineIndex != UnwrappedLine::kInvalidIndex) {
-// Update the opening line to add the forward reference as well
-(*CurrentLines)[OpeningLineIndex].MatchingOpeningBlockLineIndex =
-CurrentLines->size() - 1;
+
+  if (PPStartHash == PPEndHash) {
+Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
+if (OpeningLineIndex != UnwrappedLine::kInvalidIndex) {
+  // Update the opening line to add the forward reference as well
+  (*CurrentLines)[OpeningLineIndex].MatchingOpeningBlockLineIndex =
+  CurrentLines->size() - 1;
+}
   }
 }
 
@@ -607,10 +632,15 @@
 }
 
 void UnwrappedLineParser::conditionalCompilationCondition(bool Unreachable) {

[PATCH] D35557: clang-format: merge short case labels with trailing comments

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL309370: clang-format: merge short case labels with trailing 
comments (authored by Typz).

Repository:
  rL LLVM

https://reviews.llvm.org/D35557

Files:
  cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -382,7 +382,9 @@
   return 0;
 unsigned NumStmts = 0;
 unsigned Length = 0;
+bool EndsWithComment = false;
 bool InPPDirective = I[0]->InPPDirective;
+const unsigned Level = I[0]->Level;
 for (; NumStmts < 3; ++NumStmts) {
   if (I + 1 + NumStmts == E)
 break;
@@ -392,9 +394,26 @@
   if (Line->First->isOneOf(tok::kw_case, tok::kw_default, tok::r_brace))
 break;
   if (Line->First->isOneOf(tok::kw_if, tok::kw_for, tok::kw_switch,
-   tok::kw_while, tok::comment) ||
-  Line->Last->is(tok::comment))
+   tok::kw_while) ||
+  EndsWithComment)
 return 0;
+  if (Line->First->is(tok::comment)) {
+if (Level != Line->Level)
+  return 0;
+SmallVectorImpl::const_iterator J = I + 2 + NumStmts;
+for (; J != E; ++J) {
+  Line = *J;
+  if (Line->InPPDirective != InPPDirective)
+break;
+  if (Line->First->isOneOf(tok::kw_case, tok::kw_default, tok::r_brace))
+break;
+  if (Line->First->isNot(tok::comment) || Level != Line->Level)
+return 0;
+}
+break;
+  }
+  if (Line->Last->is(tok::comment))
+EndsWithComment = true;
   Length += I[1 + NumStmts]->Last->TotalLength + 1; // 1 for the space.
 }
 if (NumStmts == 0 || NumStmts == 3 || Length > Limit)
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -907,6 +907,77 @@
"}",
Style);
   verifyFormat("switch (a) {\n"
+   "case 0: return; // comment\n"
+   "case 1: break;  // comment\n"
+   "case 2: return;\n"
+   "// comment\n"
+   "case 3: return;\n"
+   "// comment 1\n"
+   "// comment 2\n"
+   "// comment 3\n"
+   "case 4: break; /* comment */\n"
+   "case 5:\n"
+   "  // comment\n"
+   "  break;\n"
+   "case 6: /* comment */ x = 1; break;\n"
+   "case 7: x = /* comment */ 1; break;\n"
+   "case 8:\n"
+   "  x = 1; /* comment */\n"
+   "  break;\n"
+   "case 9:\n"
+   "  break; // comment line 1\n"
+   " // comment line 2\n"
+   "}",
+   Style);
+  EXPECT_EQ("switch (a) {\n"
+"case 1:\n"
+"  x = 8;\n"
+"  // fall through\n"
+"case 2: x = 8;\n"
+"// comment\n"
+"case 3:\n"
+"  return; /* comment line 1\n"
+"   * comment line 2 */\n"
+"case 4: i = 8;\n"
+"// something else\n"
+"#if FOO\n"
+"case 5: break;\n"
+"#endif\n"
+"}",
+format("switch (a) {\n"
+   "case 1: x = 8;\n"
+   "  // fall through\n"
+   "case 2:\n"
+   "  x = 8;\n"
+   "// comment\n"
+   "case 3:\n"
+   "  return; /* comment line 1\n"
+   "   * comment line 2 */\n"
+   "case 4:\n"
+   "  i = 8;\n"
+   "// something else\n"
+   "#if FOO\n"
+   "case 5: break;\n"
+   "#endif\n"
+   "}",
+   Style));
+  EXPECT_EQ("switch (a) {\n" "case 0:\n"
+"  return; // long long long long long long long long long long long long comment\n"
+"  // line\n" "}",
+format("switch (a) {\n"
+   "case 0: return; // long long long long long long long long long long long long comment line\n"
+   "}",
+   Style));
+  EXPECT_EQ("switch (a) {\n"
+"case 0:\n"
+"  return; /* long long long long long long long long long long long long comment\n"
+" line */\n"
+"}",
+format("switch (a) {\n"
+   "case 0: return; /* long long long long long long long long long long long long comment line */\n"
+   "}",
+

[PATCH] D35483: clang-format: fix block OpeningLineIndex around preprocessor

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:464
+  for (const auto  : PPStack) {
+hash_combine(h, i.Kind);
+hash_combine(h, i.Line);

krasimir wrote:
> When I patch this, I get an `UnwrappedLineParser.cpp:457:16 error: implicit 
> instantiation of undefined template 
> 'std::hash'`. 
Seems to work fine with latest Ubuntu, but not on macos.


https://reviews.llvm.org/D35483



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34395: clang-format: add options to merge empty record body

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL306874: clang-format: add options to merge empty record body 
(authored by Typz).

Repository:
  rL LLVM

https://reviews.llvm.org/D34395

Files:
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/FormatToken.h
  cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -717,7 +717,29 @@
 ///   }
 /// \endcode
 ///
-bool SplitEmptyFunctionBody;
+bool SplitEmptyFunction;
+/// \brief If ``false``, empty record (e.g. class, struct or union) body
+/// can be put on a single line. This option is used only if the opening
+/// brace of the record has already been wrapped, i.e. the `AfterClass`
+/// (for classes) brace wrapping mode is set.
+/// \code
+///   class Foo   vs.  class Foo
+///   {}   {
+///}
+/// \endcode
+///
+bool SplitEmptyRecord;
+/// \brief If ``false``, empty namespace body can be put on a single line.
+/// This option is used only if the opening brace of the namespace has
+/// already been wrapped, i.e. the `AfterNamespace` brace wrapping mode is
+/// set.
+/// \code
+///   namespace Foo   vs.  namespace Foo
+///   {}   {
+///}
+/// \endcode
+///
+bool SplitEmptyNamespace;
   };
 
   /// \brief Control of individual brace wrapping cases.
Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -136,10 +136,7 @@
 
 bool isNamespaceDeclaration(const AnnotatedLine *Line) {
   const FormatToken *NamespaceTok = Line->First;
-  // Detect "(inline)? namespace" in the beginning of a line.
-  if (NamespaceTok->is(tok::kw_inline))
-NamespaceTok = NamespaceTok->getNextNonComment();
-  return NamespaceTok && NamespaceTok->is(tok::kw_namespace);
+  return NamespaceTok && NamespaceTok->getNamespaceToken();
 }
 
 bool isEndOfNamespace(const AnnotatedLine *Line,
@@ -216,10 +213,31 @@
 
 if (TheLine->Last->is(TT_FunctionLBrace) &&
 TheLine->First == TheLine->Last &&
-!Style.BraceWrapping.SplitEmptyFunctionBody &&
+!Style.BraceWrapping.SplitEmptyFunction &&
 I[1]->First->is(tok::r_brace))
   return tryMergeSimpleBlock(I, E, Limit);
 
+// Handle empty record blocks where the brace has already been wrapped
+if (TheLine->Last->is(tok::l_brace) && TheLine->First == TheLine->Last &&
+I != AnnotatedLines.begin()) {
+  bool EmptyBlock = I[1]->First->is(tok::r_brace);
+
+  const FormatToken *Tok = I[-1]->First;
+  if (Tok && Tok->is(tok::comment))
+Tok = Tok->getNextNonComment();
+
+  if (Tok && Tok->getNamespaceToken())
+return !Style.BraceWrapping.SplitEmptyNamespace && EmptyBlock
+? tryMergeSimpleBlock(I, E, Limit) : 0;
+
+  if (Tok && Tok->is(tok::kw_typedef))
+Tok = Tok->getNextNonComment();
+  if (Tok && Tok->isOneOf(tok::kw_class, tok::kw_struct, tok::kw_union,
+  Keywords.kw_interface))
+return !Style.BraceWrapping.SplitEmptyRecord && EmptyBlock
+? tryMergeSimpleBlock(I, E, Limit) : 0;
+}
+
 // FIXME: TheLine->Level != 0 might or might not be the right check to do.
 // If necessary, change to something smarter.
 bool MergeShortFunctions =
Index: cfe/trunk/lib/Format/FormatToken.h
===
--- cfe/trunk/lib/Format/FormatToken.h
+++ cfe/trunk/lib/Format/FormatToken.h
@@ -476,6 +476,19 @@
 return MatchingParen && MatchingParen->opensBlockOrBlockTypeList(Style);
   }
 
+  /// \brief Return the actual namespace token, if this token starts a namespace
+  /// block.
+  const FormatToken *getNamespaceToken() const {
+const FormatToken *NamespaceTok = this;
+if (is(tok::comment))
+  NamespaceTok = NamespaceTok->getNextNonComment();
+// Detect "(inline)? namespace" in the beginning of a line.
+if (NamespaceTok && NamespaceTok->is(tok::kw_inline))
+  NamespaceTok = NamespaceTok->getNextNonComment();
+return NamespaceTok && NamespaceTok->is(tok::kw_namespace) ? NamespaceTok
+   : nullptr;
+  }
+
 private:
   // Disallow copying.
   FormatToken(const FormatToken &) = delete;
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ 

[PATCH] D34238: clang-format: Do not binpack initialization lists

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL306868: clang-format: Do not binpack initialization lists 
(authored by Typz).

Changed prior to commit:
  https://reviews.llvm.org/D34238?vs=103057=104914#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34238

Files:
  cfe/trunk/lib/Format/ContinuationIndenter.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp
  cfe/trunk/unittests/Format/FormatTestJava.cpp


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -1063,12 +1063,12 @@
Current.MatchingParen->Previous &&
Current.MatchingParen->Previous->is(tok::comma);
 AvoidBinPacking =
-(Current.is(TT_ArrayInitializerLSquare) && EndsInComma) ||
-Current.is(TT_DictLiteral) ||
+EndsInComma || Current.is(TT_DictLiteral) ||
 Style.Language == FormatStyle::LK_Proto || !Style.BinPackArguments ||
 (NextNoComment &&
  NextNoComment->isOneOf(TT_DesignatedInitializerPeriod,
 TT_DesignatedInitializerLSquare));
+BreakBeforeParameter = EndsInComma;
 if (Current.ParameterCount > 1)
   NestedBlockIndent = std::max(NestedBlockIndent, State.Column + 1);
   } else {
Index: cfe/trunk/unittests/Format/FormatTestJava.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJava.cpp
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp
@@ -237,7 +237,10 @@
 TEST_F(FormatTestJava, ArrayInitializers) {
   verifyFormat("new int[] {1, 2, 3, 4};");
   verifyFormat("new int[] {\n"
-   "1, 2, 3, 4,\n"
+   "1,\n"
+   "2,\n"
+   "3,\n"
+   "4,\n"
"};");
 
   FormatStyle Style = getStyleWithColumns(65);
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -6004,7 +6004,10 @@
 TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
   verifyFormat("vector x{1, 2, 3, 4};");
   verifyFormat("vector x{\n"
-   "1, 2, 3, 4,\n"
+   "1,\n"
+   "2,\n"
+   "3,\n"
+   "4,\n"
"};");
   verifyFormat("vector x{{}, {}, {}, {}};");
   verifyFormat("f({1, 2});");
@@ -6049,6 +6052,17 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Binpacking only if there is no trailing comma
+  verifyFormat("const Aa a = {aa, bb,\n"
+   "  cc, dd};",
+  getLLVMStyleWithColumns(50));
+  verifyFormat("const Aa a = {\n"
+   "aaa,\n"
+   "bbb,\n"
+   "ccc,\n"
+   "ddd,\n"
+   "};", getLLVMStyleWithColumns(50));
+
   // Cases where distinguising braced lists and blocks is hard.
   verifyFormat("vector v{12} GUARDED_BY(mutex);");
   verifyFormat("void f() {\n"
@@ -6128,10 +6142,12 @@
"   // Second element:\n"
"   2};",
getLLVMStyleWithColumns(30)));
-  // A trailing comma should still lead to an enforced line break.
+  // A trailing comma should still lead to an enforced line break and no
+  // binpacking.
   EXPECT_EQ("vector SomeVector = {\n"
 "// aaa\n"
-"1, 2,\n"
+"1,\n"
+"2,\n"
 "};",
 format("vector SomeVector = { // aaa\n"
"1, 2, };"));
@@ -6297,7 +6313,7 @@
   " , a, aa, a, aaa}};");
 
   // No column layout should be used here.
-  verifyFormat("aaa = {aaa, 0, 0,\n"
+  verifyFormat("aaa = {a, 0, 0,\n"
"   b};");
 
   verifyNoCrash("a<,");


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -1063,12 +1063,12 @@
Current.MatchingParen->Previous &&
Current.MatchingParen->Previous->is(tok::comma);
 AvoidBinPacking =
-(Current.is(TT_ArrayInitializerLSquare) && EndsInComma) ||
-Current.is(TT_DictLiteral) ||
+EndsInComma || Current.is(TT_DictLiteral) ||
 Style.Language == FormatStyle::LK_Proto || !Style.BinPackArguments ||
 (NextNoComment &&
 

[PATCH] D33440: clang-format: better handle statement and namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: include/clang/Format/Format.h:167
+/// \endcode
+OAS_StrictAlign,
+  };

djasper wrote:
> The name is not intuitive. I don't think this is any more or less strict than 
> the other version.
It is a bit stricter in the sense that it really aligns operands, not operator 
with first operand... But this is indeed not very intuitive.

Any suggestion?


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D33589



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement and namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

I don't have commit access, can someone please commit this patch?


https://reviews.llvm.org/D32524



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32480: [clang-format] Add BinPackNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: include/clang/Format/Format.h:153
+  /// \endcode
+  bool AllowSemicolonAfterNamespace;
+

djasper wrote:
> While I am not entirely opposed to this feature, I think it should be a 
> separate patch.
I totally agree, which is why I created 2 commits; however, since they modify 
the same code (and I am new to Phabricator) I could not find a way to upload 
them separately.

Is there a way? Or should I upload one, and upload the next one once the first 
one has been accepted?


https://reviews.llvm.org/D32480



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33282: clang-format: fix prefix for doxygen comments after member

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Herald added a subscriber: klimek.

Doxygen supports putting documentation blocks after member, by adding
an additional < marker in the comment block. This patch makes sure
this marker is used in lines which are introduced by breaking the
comment.

  int foo; ///< Some very long comment.

becomes:

  int foo; ///< Some very long
   ///< comment.


https://reviews.llvm.org/D33282

Files:
  lib/Format/BreakableToken.cpp
  unittests/Format/FormatTestComments.cpp


Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1198,6 +1198,16 @@
 format("/* long long long long\n"
" * long */",
getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("///< long long long\n"
+"///< long long\n",
+format("///< long long long long\n"
+   "///< long\n",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("//!< long long long\n"
+"//!< long long\n",
+format("//!< long long long long\n"
+   "//!< long\n",
+   getLLVMStyleWithColumns(20)));
 
   // Don't bring leading whitespace up while reflowing.
   EXPECT_EQ("/*  long long long\n"
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -41,7 +41,8 @@
 }
 
 static StringRef getLineCommentIndentPrefix(StringRef Comment) {
-  static const char *const KnownPrefixes[] = {"///", "//", "//!"};
+  static const char *const KnownPrefixes[] = {
+  "///<", "//!<", "///", "//", "//!"};
   StringRef LongestPrefix;
   for (StringRef KnownPrefix : KnownPrefixes) {
 if (Comment.startswith(KnownPrefix)) {
@@ -692,6 +693,10 @@
   Prefix[i] = "/// ";
 else if (Prefix[i] == "//!")
   Prefix[i] = "//! ";
+else if (Prefix[i] == "///<")
+  Prefix[i] = "///< ";
+else if (Prefix[i] == "//!<")
+  Prefix[i] = "//!< ";
   }
 
   Tokens[i] = LineTok;


Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1198,6 +1198,16 @@
 format("/* long long long long\n"
" * long */",
getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("///< long long long\n"
+"///< long long\n",
+format("///< long long long long\n"
+   "///< long\n",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("//!< long long long\n"
+"//!< long long\n",
+format("//!< long long long long\n"
+   "//!< long\n",
+   getLLVMStyleWithColumns(20)));
 
   // Don't bring leading whitespace up while reflowing.
   EXPECT_EQ("/*  long long long\n"
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -41,7 +41,8 @@
 }
 
 static StringRef getLineCommentIndentPrefix(StringRef Comment) {
-  static const char *const KnownPrefixes[] = {"///", "//", "//!"};
+  static const char *const KnownPrefixes[] = {
+  "///<", "//!<", "///", "//", "//!"};
   StringRef LongestPrefix;
   for (StringRef KnownPrefix : KnownPrefixes) {
 if (Comment.startswith(KnownPrefix)) {
@@ -692,6 +693,10 @@
   Prefix[i] = "/// ";
 else if (Prefix[i] == "//!")
   Prefix[i] = "//! ";
+else if (Prefix[i] == "///<")
+  Prefix[i] = "///< ";
+else if (Prefix[i] == "//!<")
+  Prefix[i] = "//!< ";
   }
 
   Tokens[i] = LineTok;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33285: clang-format: do not reflow bullet lists

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Herald added a subscriber: klimek.

This patch prevents reflowing bullet lists in block comments.

It handles all lists supported by doxygen and markdown, e.g. bullet
lists starting with '-', '*', '+', as well as numbered lists starting
with -# or a number followed by a dot.


https://reviews.llvm.org/D33285

Files:
  lib/Format/BreakableToken.cpp
  unittests/Format/FormatTestComments.cpp


Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1629,6 +1629,38 @@
"//  long",
getLLVMStyleWithColumns(20)));
 
+  // Don't reflow separate bullets in list
+  EXPECT_EQ("// - long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+   "// - long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// * long long long\n"
+"// long\n"
+"// * long",
+format("// * long long long long\n"
+   "// * long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// + long long long\n"
+"// long\n"
+"// + long",
+format("// + long long long long\n"
+   "// + long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// 1. long long long\n"
+"// long\n"
+"// 2. long",
+format("// 1. long long long long\n"
+   "// 2. long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// -# long long long\n"
+"// long\n"
+"// -# long",
+format("// -# long long long long\n"
+   "// -# long",
+   getLLVMStyleWithColumns(20)));
+
   // Don't break or reflow after implicit string literals.
   verifyFormat("#include  // l l l\n"
" // l",
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -298,15 +298,22 @@
 static bool mayReflowContent(StringRef Content) {
   Content = Content.trim(Blanks);
   // Lines starting with '@' commonly have special meaning.
-  static const SmallVector kSpecialMeaningPrefixes = {
-  "@", "TODO", "FIXME", "XXX"};
+  // Lines starting with '-', '-#', '+' or '*' are bulleted/numbered lists.
+  static const SmallVector kSpecialMeaningPrefixes = {
+  "@", "TODO", "FIXME", "XXX", "-# ", "- ", "+ ", "* " };
   bool hasSpecialMeaningPrefix = false;
   for (StringRef Prefix : kSpecialMeaningPrefixes) {
 if (Content.startswith(Prefix)) {
   hasSpecialMeaningPrefix = true;
   break;
 }
   }
+
+  // Numbered lists may also start with a number followed by '.'
+  static const char *kNumberedListPattern = "^[0-9]+\\. ";
+  hasSpecialMeaningPrefix = hasSpecialMeaningPrefix ||
+llvm::Regex(kNumberedListPattern).match(Content);
+
   // Simple heuristic for what to reflow: content should contain at least two
   // characters and either the first or second character must be
   // non-punctuation.


Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1629,6 +1629,38 @@
"//  long",
getLLVMStyleWithColumns(20)));
 
+  // Don't reflow separate bullets in list
+  EXPECT_EQ("// - long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+   "// - long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// * long long long\n"
+"// long\n"
+"// * long",
+format("// * long long long long\n"
+   "// * long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// + long long long\n"
+"// long\n"
+"// + long",
+format("// + long long long long\n"
+   "// + long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// 1. long long long\n"
+"// long\n"
+"// 2. long",
+format("// 1. long long long long\n"
+   "// 2. long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// -# long long long\n"
+"// long\n"
+"// -# long",
+format("// -# long long long long\n"
+   "// -# long",
+   getLLVMStyleWithColumns(20)));
+
   // Don't break or reflow after implicit string literals.
   verifyFormat("#include  // l l l\n"
" // l",
Index: lib/Format/BreakableToken.cpp

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

we are using this style at our company, not sure if it is used elsewhere; I 
will check.

however, it seems to me that this behavior does not match the name of the 
option : AlignOperands does not align the operands anymore when 
BreakBeforeBinaryOperators is set...
so, for this patch to go forward, should I change AlignOperands into 
OperandAlignment enum, with 3 options? Something like

- OA_NotAligned, same as AlignOperands=false
- OA_AlignOperator, same the current AlignOperands=true
- OA_AlignOperands, same as AlignOperands=true when 
BreakBeforeBinaryOperators=false but my "new" mode when 
BreakBeforeBinaryOperators=true




Comment at: unittests/Format/FormatTest.cpp:2476
   "bool value = a\n"
-  " + a\n"
-  " + a\n"
-  " == a\n"
-  "* b\n"
-  "+ b\n"
-  " && a\n"
-  "* a\n"
-  "> c;",
+  "   + a\n"
+  "   + a\n"

djasper wrote:
> This looks very inconsistent to me.
not sure what you mean, I do not really understand how this expression was 
aligned before the patch...
it is not so much better in this case with the patch, but the '&&' is actually 
right-aligned with the '=' sign.


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: unittests/Format/FormatTest.cpp:2476
   "bool value = a\n"
-  " + a\n"
-  " + a\n"
-  " == a\n"
-  "* b\n"
-  "+ b\n"
-  " && a\n"
-  "* a\n"
-  "> c;",
+  "   + a\n"
+  "   + a\n"

Typz wrote:
> djasper wrote:
> > This looks very inconsistent to me.
> not sure what you mean, I do not really understand how this expression was 
> aligned before the patch...
> it is not so much better in this case with the patch, but the '&&' is 
> actually right-aligned with the '=' sign.
Seeing the test just before, I see (when breaking after operators) that the 
operands are actually right-aligned, e.g. all operators are on the same column.

So should it not be the same when breaking before the operator as well 
(independently from my patch, actually)?

  bool value = a\n"
 + a\n"
 + a\n"
== a\n"
 * b\n"
 + b\n"
&& a\n"
 * a\n"
 > c;

Not sure I like this right-alignment thing, but at least I start to understand 
how we get this output (and this may be another option to prefer 
left-alignment?)


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32477: [clang-format] Allow customizing the penalty for breaking assignment

[Francois Ferrand via Phabricator via cfe-commits]

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

Add test to verify the option actually has some effect


https://reviews.llvm.org/D32477

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3402,6 +3402,18 @@
"1;");
 }
 
+TEST_F(FormatTest, ConfigurableBreakAssignmentPenalty) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat("int aa =\n"
+   "bb + cc;",
+   Style);
+
+  Style.PenaltyBreakAssignment = 20;
+  verifyFormat("int aa = bb 
+\n"
+   " cc;",
+   Style);
+}
+
 TEST_F(FormatTest, AlignsAfterAssignments) {
   verifyFormat(
   "int Result = a + a +\n"
@@ -8728,6 +8740,8 @@
   CHECK_PARSE("ObjCBlockIndentWidth: 1234", ObjCBlockIndentWidth, 1234u);
   CHECK_PARSE("ColumnLimit: 1234", ColumnLimit, 1234u);
   CHECK_PARSE("MaxEmptyLinesToKeep: 1234", MaxEmptyLinesToKeep, 1234u);
+  CHECK_PARSE("PenaltyBreakAssignment: 1234",
+  PenaltyBreakAssignment, 1234u);
   CHECK_PARSE("PenaltyBreakBeforeFirstCallParameter: 1234",
   PenaltyBreakBeforeFirstCallParameter, 1234u);
   CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2081,9 +2081,10 @@
   if (Left.is(TT_ConditionalExpr))
 return prec::Conditional;
   prec::Level Level = Left.getPrecedence();
-  if (Level != prec::Unknown)
-return Level;
-  Level = Right.getPrecedence();
+  if (Level == prec::Unknown)
+Level = Right.getPrecedence();
+  if (Level == prec::Assignment)
+return Style.PenaltyBreakAssignment;
   if (Level != prec::Unknown)
 return Level;
 
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -330,6 +330,8 @@
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
+IO.mapOptional("PenaltyBreakAssignment",
+   Style.PenaltyBreakAssignment);
 IO.mapOptional("PenaltyBreakBeforeFirstCallParameter",
Style.PenaltyBreakBeforeFirstCallParameter);
 IO.mapOptional("PenaltyBreakComment", Style.PenaltyBreakComment);
@@ -569,6 +571,7 @@
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpacesInAngles = false;
 
+  LLVMStyle.PenaltyBreakAssignment = prec::Assignment;
   LLVMStyle.PenaltyBreakComment = 300;
   LLVMStyle.PenaltyBreakFirstLessLess = 120;
   LLVMStyle.PenaltyBreakString = 1000;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -1116,6 +1116,9 @@
   /// ``Foo `` instead of ``Foo``.
   bool ObjCSpaceBeforeProtocolList;
 
+  /// \brief The penalty for breaking around an assignment operator.
+  unsigned PenaltyBreakAssignment;
+
   /// \brief The penalty for breaking a function call after ``call(``.
   unsigned PenaltyBreakBeforeFirstCallParameter;
 
@@ -1403,6 +1406,8 @@
ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty &&
ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList &&
+   PenaltyBreakAssignment ==
+   R.PenaltyBreakAssignment &&
PenaltyBreakBeforeFirstCallParameter ==
R.PenaltyBreakBeforeFirstCallParameter &&
PenaltyBreakComment == R.PenaltyBreakComment &&


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3402,6 +3402,18 @@
"1;");
 }
 
+TEST_F(FormatTest, ConfigurableBreakAssignmentPenalty) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat("int aa =\n"
+   "bb + cc;",
+   Style);
+
+  Style.PenaltyBreakAssignment = 20;
+  verifyFormat("int aa = bb +\n"
+   " cc;",
+   Style);
+}
+
 TEST_F(FormatTest, AlignsAfterAssignments) {
   verifyFormat(
   "int Result = 

[PATCH] D33282: clang-format: fix prefix for doxygen comments after member

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

I don't have commit access, can someone please commit this patch?


https://reviews.llvm.org/D33282



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33314: clang-format: Add option to remove semicolon at end of namespace

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Herald added a subscriber: klimek.

This option allows cleaning up namespace declaration, by removing the
extra semicolon after namespace closing brace.


https://reviews.llvm.org/D33314

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -214,6 +214,51 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, RemoveSemicolon) {
+  FormatStyle LLVMStyleWithoutSemicolon = getLLVMStyle();
+  LLVMStyleWithoutSemicolon.AllowSemicolonAfterNamespace = false;
+
+  EXPECT_EQ("namespace {\n"
+"  int i;\n"
+"  int j;\n"
+"}// namespace",
+fixNamespaceEndComments("namespace {\n"
+"  int i;\n"
+"  int j;\n"
+"};",
+LLVMStyleWithoutSemicolon));
+  EXPECT_EQ("namespace A {\n"
+"  int i;\n"
+"  int j;\n"
+"}// namespace A",
+fixNamespaceEndComments("namespace A {\n"
+"  int i;\n"
+"  int j;\n"
+"};",
+LLVMStyleWithoutSemicolon));
+  EXPECT_EQ("namespace A {\n"
+"  int i;\n"
+"  int j;\n"
+"}// namespace A\n"
+"// unrelated",
+fixNamespaceEndComments("namespace A {\n"
+"  int i;\n"
+"  int j;\n"
+"};\n"
+"// unrelated",
+LLVMStyleWithoutSemicolon));
+  // case without semicolon is not affected
+  EXPECT_EQ("namespace A {\n"
+"  int i;\n"
+"  int j;\n"
+"}// namespace A",
+fixNamespaceEndComments("namespace A {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+LLVMStyleWithoutSemicolon));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8670,6 +8670,7 @@
   CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
+  CHECK_PARSE_BOOL(AllowSemicolonAfterNamespace);
   CHECK_PARSE_BOOL(AllowShortBlocksOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);
Index: lib/Format/NamespaceEndCommentsFixer.cpp
===
--- lib/Format/NamespaceEndCommentsFixer.cpp
+++ lib/Format/NamespaceEndCommentsFixer.cpp
@@ -107,6 +107,19 @@
  << llvm::toString(std::move(Err)) << "\n";
   }
 }
+
+void removeTrailingSemicolon(const FormatToken *SemiColonTok,
+ const SourceManager ,
+ tooling::Replacements *Fixes) {
+  assert(SemiColonTok->is(tok::semi));
+  auto Range = CharSourceRange::getCharRange(SemiColonTok->Tok.getLocation(),
+ SemiColonTok->Tok.getEndLoc());
+  auto Err = Fixes->add(tooling::Replacement(SourceMgr, Range, StringRef()));
+  if (Err) {
+llvm::errs() << "Error while removing trailing semicolon at end of namespace: "
+ << llvm::toString(std::move(Err)) << "\n";
+  }
+}
 } // namespace
 
 NamespaceEndCommentsFixer::NamespaceEndCommentsFixer(const Environment ,
@@ -144,6 +157,8 @@
 // comments to the semicolon tokens.
 if (RBraceTok->Next && RBraceTok->Next->is(tok::semi)) {
   EndCommentPrevTok = RBraceTok->Next;
+  if (!Style.AllowSemicolonAfterNamespace)
+removeTrailingSemicolon(RBraceTok->Next, SourceMgr, );
 }
 // The next token in the token stream after the place where the end comment
 // token must be. This is either the next token on the current line or the
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -252,6 +252,7 @@
 IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
 

[PATCH] D32480: [clang-format] Add BinPackNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

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

clang-format: Add CompactNamespaces option

- Change option name to CompactNamespaces
- Clarify & test behavior when wrapping is needed
- Separate from the 'remove semicolon' patch


https://reviews.llvm.org/D32480

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1306,6 +1306,61 @@
"} // namespace in\n"
"} // namespace out",
Style));
+
+  FormatStyle LLVMWithCompactNamespaces = getLLVMStyle();
+  LLVMWithCompactNamespaces.CompactNamespaces = true;
+
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "} // namespace out",
+   LLVMWithCompactNamespaces));
+
+  EXPECT_EQ("namespace out { namespace in1 {\n"
+"} // namespace in1\n"
+"namespace in2 {\n"
+"}} // namespace out::in2",
+format("namespace out {\n"
+   "namespace in1 {\n"
+   "} // namespace in1\n"
+   "namespace in2 {\n"
+   "} // namespace in2\n"
+   "} // namespace out",
+   LLVMWithCompactNamespaces));
+
+  EXPECT_EQ("namespace out {\n"
+"int i;\n"
+"namespace in {\n"
+"int j;\n"
+"} // namespace in\n"
+"int k;\n"
+"} // namespace out",
+format("namespace out { int i;\n"
+   "namespace in { int j; } // namespace in\n"
+   "int k; } // namespace out",
+   LLVMWithCompactNamespaces));
+
+  EXPECT_EQ("namespace  {\n"
+			"namespace  {\n"
+"}} // namespace ::",
+format("namespace  {\n"
+   "namespace  {\n"
+   "} // namespace \n"
+   "} // namespace ",
+   LLVMWithCompactNamespaces));
+
+  EXPECT_EQ("namespace a { namespace b {\n"
+			"namespace c {\n"
+"}}} // namespace a::b::c",
+format("namespace a {\n"
+   "namespace b {\n"
+   "namespace c {\n"
+   "} // namespace c\n"
+   "} // namespace b\n"
+   "} // namespace a",
+   LLVMWithCompactNamespaces));
 }
 
 TEST_F(FormatTest, FormatsExternC) { verifyFormat("extern \"C\" {\nint a;"); }
@@ -8683,6 +8738,7 @@
   CHECK_PARSE_BOOL(BreakConstructorInitializersBeforeComma);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(BreakBeforeInheritanceComma)
+  CHECK_PARSE_BOOL(CompactNamespaces);
   CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
   CHECK_PARSE_BOOL(DerivePointerAlignment);
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -127,12 +127,33 @@
   unsigned Indent = 0;
 };
 
+bool isNamespaceToken(const FormatToken *NamespaceTok) {
+  // Detect "(inline)? namespace" in the beginning of a line.
+  if (NamespaceTok->is(tok::kw_inline))
+NamespaceTok = NamespaceTok->getNextNonComment();
+  if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))
+return false;
+  return true;
+}
+
+bool isEndOfNamespace(const AnnotatedLine *line,
+  const SmallVectorImpl ) {
+  if (!line->startsWith(tok::r_brace))
+return false;
+  size_t StartLineIndex = line->MatchingOpeningBlockLineIndex;
+  if (StartLineIndex == UnwrappedLine::kInvalidIndex)
+return false;
+  assert(StartLineIndex < AnnotatedLines.size());
+  const FormatToken *NamespaceTok = AnnotatedLines[StartLineIndex]->First;
+  return isNamespaceToken(NamespaceTok);
+}
+
 class LineJoiner {
 public:
   LineJoiner(const FormatStyle , const AdditionalKeywords ,
  const SmallVectorImpl )
   : Style(Style), Keywords(Keywords), End(Lines.end()),
-Next(Lines.begin()) {}
+Next(Lines.begin()), AnnotatedLines(Lines) {}
 
   /// \brief 

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz marked 2 inline comments as done.
Typz added inline comments.



Comment at: include/clang/Format/Format.h:153
+  /// \endcode
+  bool AllowSemicolonAfterNamespace;
+

Typz wrote:
> djasper wrote:
> > While I am not entirely opposed to this feature, I think it should be a 
> > separate patch.
> I totally agree, which is why I created 2 commits; however, since they modify 
> the same code (and I am new to Phabricator) I could not find a way to upload 
> them separately.
> 
> Is there a way? Or should I upload one, and upload the next one once the 
> first one has been accepted?
OK, found the answer here: 
https://medium.com/@kurtisnusbaum/stacked-diffs-keeping-phabricator-diffs-small-d9964f4dcfa6



Comment at: include/clang/Format/Format.h:358
+  /// \endcode
+  bool BinPackNamespaces;
+

djasper wrote:
> This is not bin packing at all. Maybe CompactNamespaces? Or 
> SingleLineNamespaces?
> 
> Also, could you clarify what happens if the namespaces don't fit within the 
> column limit (both in the comment describing the flag and by adding tests for 
> it)?
This is not binpacking, but has a similar effect and made the option name 
somewhat consistent with the other binpack options :-)
I'll change to CompactNamespaces then.


https://reviews.llvm.org/D32480



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

I tried to add some test, but could not find a simple way: I could not find any 
'parser' tests from which to start, and I don't see with current master how 
this can be an issue (though it becomes an issue with some of other patches).
Any hint how to implement some test?


https://reviews.llvm.org/D32524



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D32525



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32477: [clang-format] Allow customizing the penalty for breaking assignment

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D32477



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32480: [clang-format] Add BinPackNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D32480



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32479: [clang-format] Add BreakConstructorInitializersBeforeColon option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Or would it be better to replace (i.e. deprecate) the 
BreakConstructorInitializersBeforeComma option with a multiple choice option, 
as suggested here:
http://clang-developers.42468.n3.nabble.com/clang-format-Proposal-for-a-new-style-for-constructor-and-initializers-line-break-td4041595.html

  /// \brief Different ways to break initializers.
  enum BreakConstructorInitializersStyle
  {
/// Constructor()
/// : initializer1(),
///   initializer2()
BCIS_BeforeColonAfterComma,
/// Constructor()
/// : initializer1()
/// , initializer2()
BCIS_BeforeColonAndComma,
/// Constructor() :
/// initializer1(),
/// initializer2()
BCIS_AfterColonAndComma
  };
  BreakConstructorInitializersStyle BreakConstructorInitializers


https://reviews.llvm.org/D32479



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

[Francois Ferrand via Phabricator via cfe-commits]

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

Reformat and remove unneeded comment


https://reviews.llvm.org/D32524

Files:
  lib/Format/UnwrappedLineParser.cpp


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -428,8 +428,9 @@
 parseParens();
 
   addUnwrappedLine();
-  size_t OpeningLineIndex =
-  Lines.empty() ? (UnwrappedLine::kInvalidIndex) : (Lines.size() - 1);
+  size_t OpeningLineIndex = CurrentLines->empty()
+? (UnwrappedLine::kInvalidIndex)
+: (CurrentLines->size() - 1);
 
   ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
   MustBeDeclaration);


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -428,8 +428,9 @@
 parseParens();
 
   addUnwrappedLine();
-  size_t OpeningLineIndex =
-  Lines.empty() ? (UnwrappedLine::kInvalidIndex) : (Lines.size() - 1);
+  size_t OpeningLineIndex = CurrentLines->empty()
+? (UnwrappedLine::kInvalidIndex)
+: (CurrentLines->size() - 1);
 
   ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
   MustBeDeclaration);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32479: [clang-format] Add BreakConstructorInitializersBeforeColon option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D32479



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D33589



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D32480



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D33447



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34395: clang-format: add options to merge empty record body

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Herald added a subscriber: klimek.

This patch introduces a few extra BraceWrapping options, similar to
`SplitEmptyFunction`, to allow merging empty 'record' bodies (e.g.
class, struct, union and namespace):

- SplitEmptyClass
- SplitEmptyStruct
- SplitEmptyUnion
- SplitEmptyNamespace

The `SplitEmptyFunction` option name has also been simplified/
shortened (from `SplitEmptyFunctionBody`).

These options are helpful when the correspond AfterXXX option is
enabled, to allow merging the empty record:

  class Foo
  {};

In addition, this fixes an unexpected merging of short records, when
the After options are used, which caused to be formatted like
this:

  class Foo
  { void Foo(); };

This is now properly formatted as:

  class Foo
  {
 void Foo();
  };


https://reviews.llvm.org/D34395

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6509,12 +6509,12 @@
MergeInlineOnly);
 }
 
-TEST_F(FormatTest, SplitEmptyFunctionBody) {
+TEST_F(FormatTest, SplitEmptyFunction) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterFunction = true;
-  Style.BraceWrapping.SplitEmptyFunctionBody = false;
+  Style.BraceWrapping.SplitEmptyFunction = false;
   Style.ColumnLimit = 40;
 
   verifyFormat("int f()\n"
@@ -6577,6 +6577,178 @@
Style);
 }
 
+TEST_F(FormatTest, SplitEmptyClass) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = true;
+  Style.BraceWrapping.SplitEmptyClass = false;
+
+  verifyFormat("class Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ class Foo\n"
+   "{};",
+   Style);
+  verifyFormat("template  class Foo\n"
+   "{};",
+   Style);
+  verifyFormat("class Foo\n"
+   "{\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef class Foo\n"
+   "{\n"
+   "} Foo_t;",
+   Style);
+}
+
+TEST_F(FormatTest, SplitEmptyStruct) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterStruct = true;
+  Style.BraceWrapping.SplitEmptyStruct = false;
+
+  verifyFormat("struct Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ struct Foo\n"
+   "{};",
+   Style);
+  verifyFormat("template  struct Foo\n"
+   "{};",
+   Style);
+  verifyFormat("struct Foo\n"
+   "{\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef struct Foo\n"
+   "{\n"
+   "} Foo_t;",
+   Style);
+  //typedef struct Bar {} Bar_t;
+}
+
+TEST_F(FormatTest, SplitEmptyUnion) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterUnion = true;
+  Style.BraceWrapping.SplitEmptyUnion = false;
+
+  verifyFormat("union Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ union Foo\n"
+   "{};",
+   Style);
+  verifyFormat("union Foo\n"
+   "{\n"
+   "  A,\n"
+   "};",
+   Style);
+  verifyFormat("typedef union Foo\n"
+   "{\n"
+   "} Foo_t;",
+   Style);
+}
+
+TEST_F(FormatTest, SplitEmptyNamespace) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterNamespace = true;
+  Style.BraceWrapping.SplitEmptyNamespace = false;
+
+  verifyFormat("namespace Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ namespace Foo\n"
+   "{};",
+   Style);
+  verifyFormat("inline namespace Foo\n"
+   "{};",
+   Style);
+  verifyFormat("namespace Foo\n"
+   "{\n"
+   "void Bar();\n"
+   "};",
+   Style);
+}
+
+TEST_F(FormatTest, NeverMergeShortRecords) {
+  FormatStyle Style = getLLVMStyle();
+
+  verifyFormat("class Foo {\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef class Foo {\n"
+   "  Foo();\n"
+   "} Foo_t;",
+   Style);
+  verifyFormat("struct Foo {\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef struct Foo {\n"
+   "  

[PATCH] D33491: clang-format: Fix C99 designated initializers corner cases

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305696: clang-format: Fix C99 designated initializers corner 
cases (authored by Typz).

Changed prior to commit:
  https://reviews.llvm.org/D33491?vs=103018=103041#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33491

Files:
  cfe/trunk/lib/Format/ContinuationIndenter.cpp
  cfe/trunk/lib/Format/FormatToken.h
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/lib/Format/FormatToken.h
===
--- cfe/trunk/lib/Format/FormatToken.h
+++ cfe/trunk/lib/Format/FormatToken.h
@@ -39,6 +39,7 @@
   TYPE(ConflictStart) \
   TYPE(CtorInitializerColon) \
   TYPE(CtorInitializerComma) \
+  TYPE(DesignatedInitializerLSquare) \
   TYPE(DesignatedInitializerPeriod) \
   TYPE(DictLiteral) \
   TYPE(ForEachMacro) \
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -340,6 +340,9 @@
  Contexts.back().ContextKind == tok::l_brace &&
  Parent->isOneOf(tok::l_brace, tok::comma)) {
 Left->Type = TT_JsComputedPropertyName;
+  } else if (Style.isCpp() && Contexts.back().ContextKind == tok::l_brace &&
+ Parent && Parent->isOneOf(tok::l_brace, tok::comma)) {
+Left->Type = TT_DesignatedInitializerLSquare;
   } else if (CurrentToken->is(tok::r_square) && Parent &&
  Parent->is(TT_TemplateCloser)) {
 Left->Type = TT_ArraySubscriptLSquare;
@@ -392,7 +395,8 @@
   if (CurrentToken->isOneOf(tok::r_paren, tok::r_brace))
 return false;
   if (CurrentToken->is(tok::colon)) {
-if (Left->is(TT_ArraySubscriptLSquare)) {
+if (Left->isOneOf(TT_ArraySubscriptLSquare,
+  TT_DesignatedInitializerLSquare)) {
   Left->Type = TT_ObjCMethodExpr;
   StartsObjCMethodExpr = true;
   Contexts.back().ColonIsObjCMethodExpr = true;
@@ -1975,7 +1979,8 @@
 if (Right.is(TT_LambdaLSquare) && Left.is(tok::equal))
   return 35;
 if (!Right.isOneOf(TT_ObjCMethodExpr, TT_LambdaLSquare,
-   TT_ArrayInitializerLSquare))
+   TT_ArrayInitializerLSquare,
+   TT_DesignatedInitializerLSquare))
   return 500;
   }
 
@@ -2196,7 +2201,8 @@
 (Style.SpacesInSquareBrackets &&
  Right.MatchingParen->is(TT_ArraySubscriptLSquare)));
   if (Right.is(tok::l_square) &&
-  !Right.isOneOf(TT_ObjCMethodExpr, TT_LambdaLSquare) &&
+  !Right.isOneOf(TT_ObjCMethodExpr, TT_LambdaLSquare,
+ TT_DesignatedInitializerLSquare) &&
   !Left.isOneOf(tok::numeric_constant, TT_DictLiteral))
 return false;
   if (Left.is(tok::l_brace) && Right.is(tok::r_brace))
@@ -2396,8 +2402,9 @@
   if (Left.is(tok::greater) && Right.is(tok::greater))
 return Right.is(TT_TemplateCloser) && Left.is(TT_TemplateCloser) &&
(Style.Standard != FormatStyle::LS_Cpp11 || Style.SpacesInAngles);
-  if (Right.isOneOf(tok::arrow, tok::period, tok::arrowstar, tok::periodstar) ||
-  Left.isOneOf(tok::arrow, tok::period, tok::arrowstar, tok::periodstar))
+  if (Right.isOneOf(tok::arrow, tok::arrowstar, tok::periodstar) ||
+  Left.isOneOf(tok::arrow, tok::period, tok::arrowstar, tok::periodstar) ||
+  (Right.is(tok::period) && Right.isNot(TT_DesignatedInitializerPeriod)))
 return false;
   if (!Style.SpaceBeforeAssignmentOperators &&
   Right.getPrecedence() == prec::Assignment)
Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -1050,7 +1050,9 @@
 (Current.is(TT_ArrayInitializerLSquare) && EndsInComma) ||
 Current.is(TT_DictLiteral) ||
 Style.Language == FormatStyle::LK_Proto || !Style.BinPackArguments ||
-(NextNoComment && NextNoComment->is(TT_DesignatedInitializerPeriod));
+(NextNoComment &&
+ NextNoComment->isOneOf(TT_DesignatedInitializerPeriod,
+TT_DesignatedInitializerLSquare));
 if (Current.ParameterCount > 1)
   NestedBlockIndent = std::max(NestedBlockIndent, State.Column + 1);
   } else {
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1704,6 +1704,19 @@
".eee = 5};");
 
   verifyGoogleFormat("const struct A a = {.a = 1, .b = 2};");
+
+  verifyFormat("const struct A a = {[0] = 1, [1] = 2};");
+  verifyFormat("const struct A a = {[1] = aa,\n"
+   

[PATCH] D34399: clang-format: introduce InlineOnly short function style

[Francois Ferrand via Phabricator via cfe-commits]

Typz marked 3 inline comments as done.
Typz added inline comments.



Comment at: include/clang/Format/Format.h:234
 
+  bool allowEmptyFunctionsOnASingleLine() const {
+  return AllowShortFunctionsOnASingleLine >= ShortFunctionStyle::SFS_Empty;

djasper wrote:
> I'd prefer these functions not to be in the public header file. They are 
> implementation details. Either find a header/cpp-file in the lib/ directory 
> to place them in or just remove them for now. They are both still quite small 
> and called only twice each.
This is similar to the isCpp() method, to provide a better abstraction in the 
implementation.
I cannot find any better place, so I will remove them.


https://reviews.llvm.org/D34399



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34399: clang-format: introduce InlineOnly short function style

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Herald added subscribers: rengolin, klimek.

This is the same as Inline, except it does not imply all empty
functions are merged: with this style, empty functions are merged only
if they also match the 'inline' criteria (i.e. defined in a class).

This is helpful to avoid inlining functions in implementations files.


https://reviews.llvm.org/D34399

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6509,6 +6509,49 @@
MergeInlineOnly);
 }
 
+TEST_F(FormatTest, PullInlineOnlyFunctionDefinitionsIntoSingleLine) {
+  FormatStyle MergeInlineOnly = getLLVMStyle();
+  MergeInlineOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_InlineOnly;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_InlineOnly does not imply SFS_Empty
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {\n"
+   "}", MergeInlineOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeInlineOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_InlineOnly does not imply SFS_Empty
+  verifyFormat("int f()\n"
+   "{\n"
+   "}", MergeInlineOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+}
+
 TEST_F(FormatTest, SplitEmptyFunctionBody) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -224,9 +224,9 @@
 // If necessary, change to something smarter.
 bool MergeShortFunctions =
 Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All ||
-(Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
+(Style.allowEmptyFunctionsOnASingleLine() &&
  I[1]->First->is(tok::r_brace)) ||
-(Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline &&
+(Style.allowShortInlineFunctionsOnASingleLine() &&
  TheLine->Level != 0);
 
 if (Style.CompactNamespaces) {
@@ -282,7 +282,7 @@
 
   unsigned MergedLines = 0;
   if (MergeShortFunctions ||
-  (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
+  (Style.allowEmptyFunctionsOnASingleLine() &&
I[1]->First == I[1]->Last && I + 2 != E &&
I[2]->First->is(tok::r_brace))) {
 MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2480,8 +2480,7 @@
   return Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_None ||
  Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Empty ||
  (Left.NestingLevel == 0 && Line.Level == 0 &&
-  Style.AllowShortFunctionsOnASingleLine ==
-  FormatStyle::SFS_Inline);
+  Style.allowShortInlineFunctionsOnASingleLine());
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Right.is(tok::plus) && Left.is(tok::string_literal) && Right.Next &&
 Right.Next->is(tok::string_literal))
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -96,6 +96,7 @@
 IO.enumCase(Value, "All", FormatStyle::SFS_All);
 IO.enumCase(Value, "true", FormatStyle::SFS_All);
 IO.enumCase(Value, "Inline", FormatStyle::SFS_Inline);
+IO.enumCase(Value, "InlineOnly", FormatStyle::SFS_InlineOnly);
 IO.enumCase(Value, "Empty", FormatStyle::SFS_Empty);
   }
 };
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -184,9 +184,23 @@
   enum ShortFunctionStyle {
 /// \brief Never merge functions into a 

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

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

Rebase & fix indentation when newline is inserted after return or equal.


https://reviews.llvm.org/D32478

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -102,7 +102,7 @@
   verifyFormat("var x = a() in\n"
".aa.aa;");
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
-  Style.AlignOperands = true;
+  Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("var x = a() in\n"
".aa.aa;",
Style);
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2702,6 +2702,9 @@
"  > c) {\n"
"}",
Style);
+  verifyFormat("return a\n"
+   "   && bbb;",
+   Style);
   verifyFormat("return (a)\n"
"   // comment\n"
"   + b;",
@@ -2730,11 +2733,103 @@
 
   Style.ColumnLimit = 60;
   verifyFormat("zz\n"
-   "= b\n"
+   "= \n"
"  >> (aa);",
Style);
 }
 
+TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Style.AlignOperands = FormatStyle::OAS_StrictAlign;
+
+  verifyFormat("bool value = a\n"
+   "   + a\n"
+   "   + a\n"
+   "  == a\n"
+   " * b\n"
+   " + b\n"
+   "  && a\n"
+   " * a\n"
+   " > c;",
+   Style);
+  verifyFormat("if (a\n"
+   "* \n"
+   "+ aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "+ \n"
+   "  * aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "== \n"
+   "   * aa\n"
+   "   + bbb) {\n}",
+   Style);
+  verifyFormat("if () {\n"
+   "} else if (a\n"
+   "   && b // break\n"
+   "  > c) {\n"
+   "}",
+   Style);
+  verifyFormat("return a\n"
+   "&& bbb;",
+   Style);
+  verifyFormat("return (a)\n"
+   "   // comment\n"
+   " + b;",
+   Style);
+  verifyFormat(
+  "int aa = aa\n"
+  "   * bbb\n"
+  "   + cc;",
+  Style);
+
+  verifyFormat("a\n"
+   "=  + ;",
+   Style);
+
+  verifyFormat("return boost::fusion::at_c<0>().second\n"
+   "== boost::fusion::at_c<1>().second;",
+   Style);
+
+  Style.ColumnLimit = 60;
+  verifyFormat("z\n"
+   "= \n"
+   "   >> 

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz marked 7 inline comments as done.
Typz added a comment.

This style is used in the Skia  project.


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34395: clang-format: add options to merge empty record body

[Francois Ferrand via Phabricator via cfe-commits]

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

Enable merging records for Mozilla style


https://reviews.llvm.org/D34395

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6509,12 +6509,12 @@
MergeInlineOnly);
 }
 
-TEST_F(FormatTest, SplitEmptyFunctionBody) {
+TEST_F(FormatTest, SplitEmptyFunction) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterFunction = true;
-  Style.BraceWrapping.SplitEmptyFunctionBody = false;
+  Style.BraceWrapping.SplitEmptyFunction = false;
   Style.ColumnLimit = 40;
 
   verifyFormat("int f()\n"
@@ -6577,6 +6577,178 @@
Style);
 }
 
+TEST_F(FormatTest, SplitEmptyClass) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = true;
+  Style.BraceWrapping.SplitEmptyClass = false;
+
+  verifyFormat("class Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ class Foo\n"
+   "{};",
+   Style);
+  verifyFormat("template  class Foo\n"
+   "{};",
+   Style);
+  verifyFormat("class Foo\n"
+   "{\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef class Foo\n"
+   "{\n"
+   "} Foo_t;",
+   Style);
+}
+
+TEST_F(FormatTest, SplitEmptyStruct) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterStruct = true;
+  Style.BraceWrapping.SplitEmptyStruct = false;
+
+  verifyFormat("struct Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ struct Foo\n"
+   "{};",
+   Style);
+  verifyFormat("template  struct Foo\n"
+   "{};",
+   Style);
+  verifyFormat("struct Foo\n"
+   "{\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef struct Foo\n"
+   "{\n"
+   "} Foo_t;",
+   Style);
+  //typedef struct Bar {} Bar_t;
+}
+
+TEST_F(FormatTest, SplitEmptyUnion) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterUnion = true;
+  Style.BraceWrapping.SplitEmptyUnion = false;
+
+  verifyFormat("union Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ union Foo\n"
+   "{};",
+   Style);
+  verifyFormat("union Foo\n"
+   "{\n"
+   "  A,\n"
+   "};",
+   Style);
+  verifyFormat("typedef union Foo\n"
+   "{\n"
+   "} Foo_t;",
+   Style);
+}
+
+TEST_F(FormatTest, SplitEmptyNamespace) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterNamespace = true;
+  Style.BraceWrapping.SplitEmptyNamespace = false;
+
+  verifyFormat("namespace Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ namespace Foo\n"
+   "{};",
+   Style);
+  verifyFormat("inline namespace Foo\n"
+   "{};",
+   Style);
+  verifyFormat("namespace Foo\n"
+   "{\n"
+   "void Bar();\n"
+   "};",
+   Style);
+}
+
+TEST_F(FormatTest, NeverMergeShortRecords) {
+  FormatStyle Style = getLLVMStyle();
+
+  verifyFormat("class Foo {\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef class Foo {\n"
+   "  Foo();\n"
+   "} Foo_t;",
+   Style);
+  verifyFormat("struct Foo {\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef struct Foo {\n"
+   "  Foo();\n"
+   "} Foo_t;",
+   Style);
+  verifyFormat("union Foo {\n"
+   "  A,\n"
+   "};",
+   Style);
+  verifyFormat("typedef union Foo {\n"
+   "  A,\n"
+   "} Foo_t;",
+   Style);
+  verifyFormat("namespace Foo {\n"
+   "void Bar();\n"
+   "};",
+   Style);
+
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = true;
+  Style.BraceWrapping.AfterStruct = true;
+  Style.BraceWrapping.AfterUnion = true;
+  Style.BraceWrapping.AfterNamespace = true;
+  verifyFormat("class Foo\n"
+   "{\n"
+   "  

[PATCH] D34399: clang-format: introduce InlineOnly short function style

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 103357.
Typz marked an inline comment as done.
Typz added a comment.

Fix according to review comments


https://reviews.llvm.org/D34399

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6509,6 +6509,52 @@
MergeInlineOnly);
 }
 
+TEST_F(FormatTest, PullInlineOnlyFunctionDefinitionsIntoSingleLine) {
+  FormatStyle MergeInlineOnly = getLLVMStyle();
+  MergeInlineOnly.AllowShortFunctionsOnASingleLine =
+  FormatStyle::SFS_InlineOnly;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_InlineOnly does not imply SFS_Empty
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {\n"
+   "}",
+   MergeInlineOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeInlineOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_InlineOnly does not imply SFS_Empty
+  verifyFormat("int f()\n"
+   "{\n"
+   "}",
+   MergeInlineOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+}
+
 TEST_F(FormatTest, SplitEmptyFunctionBody) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -226,7 +226,7 @@
 Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All ||
 (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
  I[1]->First->is(tok::r_brace)) ||
-(Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline &&
+(Style.AllowShortFunctionsOnASingleLine & FormatStyle::SFS_InlineOnly &&
  TheLine->Level != 0);
 
 if (Style.CompactNamespaces) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2480,8 +2480,8 @@
   return Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_None ||
  Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Empty ||
  (Left.NestingLevel == 0 && Line.Level == 0 &&
-  Style.AllowShortFunctionsOnASingleLine ==
-  FormatStyle::SFS_Inline);
+  Style.AllowShortFunctionsOnASingleLine &
+  FormatStyle::SFS_InlineOnly);
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Right.is(tok::plus) && Left.is(tok::string_literal) && Right.Next &&
 Right.Next->is(tok::string_literal))
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -96,6 +96,7 @@
 IO.enumCase(Value, "All", FormatStyle::SFS_All);
 IO.enumCase(Value, "true", FormatStyle::SFS_All);
 IO.enumCase(Value, "Inline", FormatStyle::SFS_Inline);
+IO.enumCase(Value, "InlineOnly", FormatStyle::SFS_InlineOnly);
 IO.enumCase(Value, "Empty", FormatStyle::SFS_Empty);
   }
 };
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -184,9 +184,23 @@
   enum ShortFunctionStyle {
 /// \brief Never merge functions into a single line.
 SFS_None,
+/// \brief Only merge functions defined inside a class. Same as "inline",
+/// except it does not implies "empty": i.e. top level empty functions
+/// are not merged either.
+/// \code
+///   class Foo {
+/// void f() { foo(); }
+///   };
+///   void f() {
+/// foo();
+///   }
+///   void f() {
+///   }
+/// \endcode
+SFS_InlineOnly,
 /// \brief Only merge empty functions.
 /// \code
-///   void f() { bar(); }
+///   void f() {}
 ///   void f2() {
 /// bar2();
 ///   }
@@ -197,6 +211,10 @@
 ///  

[PATCH] D34238: clang-format: Do not binpack initialization lists

[Francois Ferrand via Phabricator via cfe-commits]

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

Fix indentation


https://reviews.llvm.org/D34238

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


Index: unittests/Format/FormatTestJava.cpp
===
--- unittests/Format/FormatTestJava.cpp
+++ unittests/Format/FormatTestJava.cpp
@@ -237,7 +237,10 @@
 TEST_F(FormatTestJava, ArrayInitializers) {
   verifyFormat("new int[] {1, 2, 3, 4};");
   verifyFormat("new int[] {\n"
-   "1, 2, 3, 4,\n"
+   "1,\n"
+   "2,\n"
+   "3,\n"
+   "4,\n"
"};");
 
   FormatStyle Style = getStyleWithColumns(65);
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5574,7 +5574,10 @@
 TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
   verifyFormat("vector x{1, 2, 3, 4};");
   verifyFormat("vector x{\n"
-   "1, 2, 3, 4,\n"
+   "1,\n"
+   "2,\n"
+   "3,\n"
+   "4,\n"
"};");
   verifyFormat("vector x{{}, {}, {}, {}};");
   verifyFormat("f({1, 2});");
@@ -5617,6 +5620,17 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Binpacking only if there is no trailing comma
+  verifyFormat("const Aa a = {aa, bb,\n"
+   "  cc, dd};",
+  getLLVMStyleWithColumns(50));
+  verifyFormat("const Aa a = {\n"
+   "aaa,\n"
+   "bbb,\n"
+   "ccc,\n"
+   "ddd,\n"
+   "};", getLLVMStyleWithColumns(50));
+
   // Cases where distinguising braced lists and blocks is hard.
   verifyFormat("vector v{12} GUARDED_BY(mutex);");
   verifyFormat("void f() {\n"
@@ -5696,10 +5710,12 @@
"   // Second element:\n"
"   2};",
getLLVMStyleWithColumns(30)));
-  // A trailing comma should still lead to an enforced line break.
+  // A trailing comma should still lead to an enforced line break and no
+  // binpacking.
   EXPECT_EQ("vector SomeVector = {\n"
 "// aaa\n"
-"1, 2,\n"
+"1,\n"
+"2,\n"
 "};",
 format("vector SomeVector = { // aaa\n"
"1, 2, };"));
@@ -5863,7 +5879,7 @@
   " , a, aa, a, aaa}};");
 
   // No column layout should be used here.
-  verifyFormat("aaa = {aaa, 0, 0,\n"
+  verifyFormat("aaa = {a, 0, 0,\n"
"   b};");
 
   verifyNoCrash("a<,");
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1000,10 +1000,10 @@
Current.MatchingParen->Previous &&
Current.MatchingParen->Previous->is(tok::comma);
 AvoidBinPacking =
-(Current.is(TT_ArrayInitializerLSquare) && EndsInComma) ||
-Current.is(TT_DictLiteral) ||
+EndsInComma || Current.is(TT_DictLiteral) ||
 Style.Language == FormatStyle::LK_Proto || !Style.BinPackArguments ||
 (NextNoComment && NextNoComment->is(TT_DesignatedInitializerPeriod));
+BreakBeforeParameter = EndsInComma;
 if (Current.ParameterCount > 1)
   NestedBlockIndent = std::max(NestedBlockIndent, State.Column + 1);
   } else {


Index: unittests/Format/FormatTestJava.cpp
===
--- unittests/Format/FormatTestJava.cpp
+++ unittests/Format/FormatTestJava.cpp
@@ -237,7 +237,10 @@
 TEST_F(FormatTestJava, ArrayInitializers) {
   verifyFormat("new int[] {1, 2, 3, 4};");
   verifyFormat("new int[] {\n"
-   "1, 2, 3, 4,\n"
+   "1,\n"
+   "2,\n"
+   "3,\n"
+   "4,\n"
"};");
 
   FormatStyle Style = getStyleWithColumns(65);
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5574,7 +5574,10 @@
 TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
   verifyFormat("vector x{1, 2, 3, 4};");
   verifyFormat("vector x{\n"
-   "1, 2, 3, 4,\n"
+   "1,\n"
+   "2,\n"
+   "3,\n"
+   "4,\n"
"};");
   verifyFormat("vector 

[PATCH] D34238: clang-format: Do not binpack initialization lists

[Francois Ferrand via Phabricator via cfe-commits]

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

Fix case where the content fits on a line, by wrapping after each comma, like 
this:

static int types[] = {
0,
  1,
  2,
};


https://reviews.llvm.org/D34238

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


Index: unittests/Format/FormatTestJava.cpp
===
--- unittests/Format/FormatTestJava.cpp
+++ unittests/Format/FormatTestJava.cpp
@@ -237,7 +237,10 @@
 TEST_F(FormatTestJava, ArrayInitializers) {
   verifyFormat("new int[] {1, 2, 3, 4};");
   verifyFormat("new int[] {\n"
-   "1, 2, 3, 4,\n"
+   "1,\n"
+   "2,\n"
+   "3,\n"
+   "4,\n"
"};");
 
   FormatStyle Style = getStyleWithColumns(65);
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5574,7 +5574,10 @@
 TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
   verifyFormat("vector x{1, 2, 3, 4};");
   verifyFormat("vector x{\n"
-   "1, 2, 3, 4,\n"
+   "1,\n"
+  "2,\n"
+  "3,\n"
+  "4,\n"
"};");
   verifyFormat("vector x{{}, {}, {}, {}};");
   verifyFormat("f({1, 2});");
@@ -5617,6 +5620,17 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Binpacking only if there is no trailing comma
+  verifyFormat("const Aa a = {aa, bb,\n"
+   "  cc, dd};",
+  getLLVMStyleWithColumns(50));
+  verifyFormat("const Aa a = {\n"
+   "aaa,\n"
+   "bbb,\n"
+   "ccc,\n"
+   "ddd,\n"
+   "};", getLLVMStyleWithColumns(50));
+
   // Cases where distinguising braced lists and blocks is hard.
   verifyFormat("vector v{12} GUARDED_BY(mutex);");
   verifyFormat("void f() {\n"
@@ -5696,10 +5710,12 @@
"   // Second element:\n"
"   2};",
getLLVMStyleWithColumns(30)));
-  // A trailing comma should still lead to an enforced line break.
+  // A trailing comma should still lead to an enforced line break and no
+  // binpacking.
   EXPECT_EQ("vector SomeVector = {\n"
 "// aaa\n"
-"1, 2,\n"
+"1,\n"
+"2,\n"
 "};",
 format("vector SomeVector = { // aaa\n"
"1, 2, };"));
@@ -5784,7 +5800,7 @@
"X86::RAX, X86::RDX, X86::RCX, X86::RSI, X86::RDI,\n"
"X86::R8,  X86::R9,  X86::R10, X86::R11, 0};");
   verifyFormat("static const uint16_t CallerSavedRegs64Bit[] = {\n"
-   "X86::RAX, X86::RDX, X86::RCX, X86::RSI, X86::RDI,\n"
+  "X86::RAX, X86::RDX, X86::RCX, X86::RSI, 
X86::RDI,\n"
"// Separating comment.\n"
"X86::R8, X86::R9, X86::R10, X86::R11, 0};");
   verifyFormat("static const uint16_t CallerSavedRegs64Bit[] = {\n"
@@ -5863,7 +5879,7 @@
   " , a, aa, a, aaa}};");
 
   // No column layout should be used here.
-  verifyFormat("aaa = {aaa, 0, 0,\n"
+  verifyFormat("aaa = {a, 0, 0,\n"
"   b};");
 
   verifyNoCrash("a<,");
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1000,10 +1000,10 @@
Current.MatchingParen->Previous &&
Current.MatchingParen->Previous->is(tok::comma);
 AvoidBinPacking =
-(Current.is(TT_ArrayInitializerLSquare) && EndsInComma) ||
-Current.is(TT_DictLiteral) ||
+EndsInComma || Current.is(TT_DictLiteral) ||
 Style.Language == FormatStyle::LK_Proto || !Style.BinPackArguments ||
 (NextNoComment && NextNoComment->is(TT_DesignatedInitializerPeriod));
+BreakBeforeParameter = EndsInComma;
 if (Current.ParameterCount > 1)
   NestedBlockIndent = std::max(NestedBlockIndent, State.Column + 1);
   } else {


Index: unittests/Format/FormatTestJava.cpp
===
--- unittests/Format/FormatTestJava.cpp
+++ unittests/Format/FormatTestJava.cpp
@@ -237,7 +237,10 @@
 TEST_F(FormatTestJava, ArrayInitializers) {
   verifyFormat("new int[] {1, 2, 

[PATCH] D34399: clang-format: introduce InlineOnly short function style

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305912: clang-format: introduce InlineOnly short function 
style (authored by Typz).

Changed prior to commit:
  https://reviews.llvm.org/D34399?vs=103357=103370#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34399

Files:
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -184,9 +184,23 @@
   enum ShortFunctionStyle {
 /// \brief Never merge functions into a single line.
 SFS_None,
+/// \brief Only merge functions defined inside a class. Same as "inline",
+/// except it does not implies "empty": i.e. top level empty functions
+/// are not merged either.
+/// \code
+///   class Foo {
+/// void f() { foo(); }
+///   };
+///   void f() {
+/// foo();
+///   }
+///   void f() {
+///   }
+/// \endcode
+SFS_InlineOnly,
 /// \brief Only merge empty functions.
 /// \code
-///   void f() { bar(); }
+///   void f() {}
 ///   void f2() {
 /// bar2();
 ///   }
@@ -197,6 +211,10 @@
 ///   class Foo {
 /// void f() { foo(); }
 ///   };
+///   void f() {
+/// foo();
+///   }
+///   void f() {}
 /// \endcode
 SFS_Inline,
 /// \brief Merge all functions fitting on a single line.
Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -226,7 +226,7 @@
 Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All ||
 (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
  I[1]->First->is(tok::r_brace)) ||
-(Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline &&
+(Style.AllowShortFunctionsOnASingleLine & FormatStyle::SFS_InlineOnly &&
  TheLine->Level != 0);
 
 if (Style.CompactNamespaces) {
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -97,6 +97,7 @@
 IO.enumCase(Value, "All", FormatStyle::SFS_All);
 IO.enumCase(Value, "true", FormatStyle::SFS_All);
 IO.enumCase(Value, "Inline", FormatStyle::SFS_Inline);
+IO.enumCase(Value, "InlineOnly", FormatStyle::SFS_InlineOnly);
 IO.enumCase(Value, "Empty", FormatStyle::SFS_Empty);
   }
 };
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2480,8 +2480,8 @@
   return Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_None ||
  Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Empty ||
  (Left.NestingLevel == 0 && Line.Level == 0 &&
-  Style.AllowShortFunctionsOnASingleLine ==
-  FormatStyle::SFS_Inline);
+  Style.AllowShortFunctionsOnASingleLine &
+  FormatStyle::SFS_InlineOnly);
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Right.is(tok::plus) && Left.is(tok::string_literal) && Right.Next &&
 Right.Next->is(tok::string_literal))
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -6509,6 +6509,52 @@
MergeInlineOnly);
 }
 
+TEST_F(FormatTest, PullInlineOnlyFunctionDefinitionsIntoSingleLine) {
+  FormatStyle MergeInlineOnly = getLLVMStyle();
+  MergeInlineOnly.AllowShortFunctionsOnASingleLine =
+  FormatStyle::SFS_InlineOnly;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_InlineOnly does not imply SFS_Empty
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {\n"
+   "}",
+   MergeInlineOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeInlineOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D33589#789002, @alexfh wrote:

> why do we want to make an exception for comments and not for regular code?


This is not an exception for comments: the `PenaltyExcessCharacter` is used 
whenever the code is longer than the `ColumnLimit`, and used to compute the 
best solution.
The default value is extremely high, so there is no practical difference: code 
does not go beyond the column, except for specific exceptions.

However, the difference becomes apparent when reducing this penalty: in that 
case, code can break the column limit instead of wrapping, but it does not 
happen with comments. This patch tries to restore the parity, and let comments 
break the comment limit as well.


https://reviews.llvm.org/D33589



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: properly handle Q_UNUSED and QT_REQUIRE_VERSION

[Francois Ferrand via Phabricator via cfe-commits]

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

Complete refactor to make the processing much more generic


https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+Style));
+  

[PATCH] D34395: clang-format: add options to merge empty record body

[Francois Ferrand via Phabricator via cfe-commits]

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

Merge `SplitEmptyClass/Struct/Union` options into a single `SplitEmptyRecord` 
option.


https://reviews.llvm.org/D34395

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6509,12 +6509,12 @@
MergeInlineOnly);
 }
 
-TEST_F(FormatTest, SplitEmptyFunctionBody) {
+TEST_F(FormatTest, SplitEmptyFunction) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterFunction = true;
-  Style.BraceWrapping.SplitEmptyFunctionBody = false;
+  Style.BraceWrapping.SplitEmptyFunction = false;
   Style.ColumnLimit = 40;
 
   verifyFormat("int f()\n"
@@ -6577,6 +6577,178 @@
Style);
 }
 
+TEST_F(FormatTest, SplitEmptyClass) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = true;
+  Style.BraceWrapping.SplitEmptyRecord = false;
+
+  verifyFormat("class Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ class Foo\n"
+   "{};",
+   Style);
+  verifyFormat("template  class Foo\n"
+   "{};",
+   Style);
+  verifyFormat("class Foo\n"
+   "{\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef class Foo\n"
+   "{\n"
+   "} Foo_t;",
+   Style);
+}
+
+TEST_F(FormatTest, SplitEmptyStruct) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterStruct = true;
+  Style.BraceWrapping.SplitEmptyRecord = false;
+
+  verifyFormat("struct Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ struct Foo\n"
+   "{};",
+   Style);
+  verifyFormat("template  struct Foo\n"
+   "{};",
+   Style);
+  verifyFormat("struct Foo\n"
+   "{\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef struct Foo\n"
+   "{\n"
+   "} Foo_t;",
+   Style);
+  //typedef struct Bar {} Bar_t;
+}
+
+TEST_F(FormatTest, SplitEmptyUnion) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterUnion = true;
+  Style.BraceWrapping.SplitEmptyRecord = false;
+
+  verifyFormat("union Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ union Foo\n"
+   "{};",
+   Style);
+  verifyFormat("union Foo\n"
+   "{\n"
+   "  A,\n"
+   "};",
+   Style);
+  verifyFormat("typedef union Foo\n"
+   "{\n"
+   "} Foo_t;",
+   Style);
+}
+
+TEST_F(FormatTest, SplitEmptyNamespace) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterNamespace = true;
+  Style.BraceWrapping.SplitEmptyNamespace = false;
+
+  verifyFormat("namespace Foo\n"
+   "{};",
+   Style);
+  verifyFormat("/* something */ namespace Foo\n"
+   "{};",
+   Style);
+  verifyFormat("inline namespace Foo\n"
+   "{};",
+   Style);
+  verifyFormat("namespace Foo\n"
+   "{\n"
+   "void Bar();\n"
+   "};",
+   Style);
+}
+
+TEST_F(FormatTest, NeverMergeShortRecords) {
+  FormatStyle Style = getLLVMStyle();
+
+  verifyFormat("class Foo {\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef class Foo {\n"
+   "  Foo();\n"
+   "} Foo_t;",
+   Style);
+  verifyFormat("struct Foo {\n"
+   "  Foo();\n"
+   "};",
+   Style);
+  verifyFormat("typedef struct Foo {\n"
+   "  Foo();\n"
+   "} Foo_t;",
+   Style);
+  verifyFormat("union Foo {\n"
+   "  A,\n"
+   "};",
+   Style);
+  verifyFormat("typedef union Foo {\n"
+   "  A,\n"
+   "} Foo_t;",
+   Style);
+  verifyFormat("namespace Foo {\n"
+   "void Bar();\n"
+   "};",
+   Style);
+
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = true;
+  Style.BraceWrapping.AfterStruct = true;
+  Style.BraceWrapping.AfterUnion = true;
+  Style.BraceWrapping.AfterNamespace = true;
+  verifyFormat("class 

[PATCH] D33440: clang-format: better handle statement and namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

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

Fix typo


https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+Style));
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+  

[PATCH] D34395: clang-format: add options to merge empty record body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

I don't know if some style would want different styles, and I agree with you on 
principle; but since the brace wrapping is already configured for each kind of 
record, I choose to keep things consistent and have flags for each kind of 
record.
But I can merge the options, and keep only `SplitEmptyRecord` and 
`SplitEmptyNamespace`, if you really think it is better.


https://reviews.llvm.org/D34395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34238: clang-format: Do not binpack initialization lists

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D34238



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34395: clang-format: add options to merge empty record body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D34395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33285: clang-format: do not reflow bullet lists

[Francois Ferrand via Phabricator via cfe-commits]

Typz marked an inline comment as done.
Typz added inline comments.



Comment at: lib/Format/BreakableToken.cpp:313
+  // Numbered lists may also start with a number followed by '.'
+  static const char *kNumberedListPattern = "^[0-9]+\\. ";
+  hasSpecialMeaningPrefix = hasSpecialMeaningPrefix ||

krasimir wrote:
> A problem with this is that sometimes you have a sentence ending with a 
> number, like this one, in **2016.** If this sentence also happens to just go 
> over the column width, its last part would be reflown and during subsequent 
> passes it will be seen as a numbered list, which is super unfortunate. I'd 
> like us to come up with a more refined strategy of handling this case. Maybe 
> we should look at how others are doing it?
Looking at doxygen, it seems there is no extra protection: just a number 
followed by a dot...
So it means:
  * We should never break before a such a sequence, to avoid the issue.
  * We may also limit the expression to limit the size of the number: I am not 
sure there are cases where bullet lists with hundreds of items are used, esp. 
with explicit values (uses the auto-numbering -# would be much simpler in that 
case). Maybe a limit of 2 digits? The same limit would be applied to prevent 
breaking before a number followed by a dot.

What do you think?



Comment at: lib/Format/BreakableToken.cpp:315
+  hasSpecialMeaningPrefix = hasSpecialMeaningPrefix ||
+llvm::Regex(kNumberedListPattern).match(Content);
+

krasimir wrote:
> This builds an `llvm::Regex` on each invocation, which is wasteful.
I did this to keep the function re-entrant; but since the code is not 
multi-thread I can use a static variable instead.


https://reviews.llvm.org/D33285



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Indeed, I don't have commit access. But I was wondering if I should not get it, 
to simplify landing the patches after review.
I read http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access about 
this, but I am still wondering what is considered a "track record of submitting 
high quality patches"...

I have uploaded 8 patches at the moment, if that is track record enough I will 
request access; otherwise please submit already :-)
Thanks,


https://reviews.llvm.org/D32524



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33285: clang-format: do not reflow bullet lists

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 99436.
Typz marked an inline comment as done.
Typz added a comment.

Limit to 2 digits and not break before a matching numbered list sequence 
followed by a fullstop, to avoid interpreting numbers at the end of sentence as 
numbered bullets (and thus preventing reflow).


https://reviews.llvm.org/D33285

Files:
  lib/Format/BreakableToken.cpp
  unittests/Format/FormatTestComments.cpp

Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1534,7 +1534,7 @@
" *\n"
" * long */",
getLLVMStyleWithColumns(20)));
-  
+
   // Don't reflow lines having content that is a single character.
   EXPECT_EQ("// long long long\n"
 "// long\n"
@@ -1559,7 +1559,7 @@
 format("// long long long long\n"
"// @param arg",
getLLVMStyleWithColumns(20)));
-  
+
   // Don't reflow lines starting with 'TODO'.
   EXPECT_EQ("// long long long\n"
 "// long\n"
@@ -1628,6 +1628,69 @@
"//  long",
getLLVMStyleWithColumns(20)));
 
+  // Don't reflow separate bullets in list
+  EXPECT_EQ("// - long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+   "// - long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// * long long long\n"
+"// long\n"
+"// * long",
+format("// * long long long long\n"
+   "// * long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// + long long long\n"
+"// long\n"
+"// + long",
+format("// + long long long long\n"
+   "// + long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// 1. long long long\n"
+"// long\n"
+"// 2. long",
+format("// 1. long long long long\n"
+   "// 2. long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// -# long long long\n"
+"// long\n"
+"// -# long",
+format("// -# long long long long\n"
+   "// -# long",
+   getLLVMStyleWithColumns(20)));
+
+  EXPECT_EQ("// - long long long\n"
+"// long long long\n"
+"// - long",
+format("// - long long long long\n"
+   "// long long\n"
+   "// - long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// - long long long\n"
+"// long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+   "// long long long\n"
+   "// - long",
+   getLLVMStyleWithColumns(20)));
+
+  // Large number (>2 digits) are not list items
+  EXPECT_EQ("// long long long\n"
+"// long 1024. long.",
+format("// long long long long\n"
+   "// 1024. long.",
+   getLLVMStyleWithColumns(20)));
+
+  // Do not break before number, to avoid introducing a non-reflowable doxygen
+  // list item.
+  EXPECT_EQ("// long long\n"
+"// long 10. long.",
+format("// long long long 10.\n"
+   "// long.",
+   getLLVMStyleWithColumns(20)));
+
   // Don't break or reflow after implicit string literals.
   verifyFormat("#include  // l l l\n"
" // l",
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -77,6 +77,14 @@
   }
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
+
+  // Do not split before a number followed by a dot: this would be interpreted
+  // as a numbered list, which would prevent re-flowing in subsequent passes.
+  static llvm::Regex kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\.");
+  if (SpaceOffset != StringRef::npos &&
+  kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks)))
+SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+
   if (SpaceOffset == StringRef::npos ||
   // Don't break at leading whitespace.
   Text.find_last_not_of(Blanks, SpaceOffset) == StringRef::npos) {
@@ -298,15 +306,24 @@
 static bool mayReflowContent(StringRef Content) {
   Content = Content.trim(Blanks);
   // Lines starting with '@' commonly have special meaning.
-  static const SmallVector kSpecialMeaningPrefixes = {
-  "@", "TODO", "FIXME", "XXX"};
+  // Lines starting with '-', '-#', '+' or '*' are bulleted/numbered lists.
+  static const SmallVector kSpecialMeaningPrefixes = {
+  "@", "TODO", "FIXME", "XXX", "-# 

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

I checked the code of POCO, and it indeed follows this convention (though there 
does not seem to be any C++11 for loop indeed).
We also use this style at our company.

> Also see my comment.

I could not find your comment... can you please re-post?

> It's very hard to even name this option without it being confusing to users.

What do you mean? 'space before colon' seems quite explicit to me... The only 
thing I can think of is that it does not apply to ternary operator and labels 
(though I don't think that would be misleading for user); do you think it 
should be a flag with different modes, similar to BreakBeforeBinaryOperators 
(e.g. Never, LabelsAndOperators, Always) ?


https://reviews.llvm.org/D32525



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33285: clang-format: do not reflow bullet lists

[Francois Ferrand via Phabricator via cfe-commits]

Typz marked 3 inline comments as done.
Typz added inline comments.



Comment at: lib/Format/BreakableToken.cpp:313
+  // Numbered lists may also start with a number followed by '.'
+  static const char *kNumberedListPattern = "^[0-9]+\\. ";
+  hasSpecialMeaningPrefix = hasSpecialMeaningPrefix ||

krasimir wrote:
> Typz wrote:
> > krasimir wrote:
> > > A problem with this is that sometimes you have a sentence ending with a 
> > > number, like this one, in **2016.** If this sentence also happens to just 
> > > go over the column width, its last part would be reflown and during 
> > > subsequent passes it will be seen as a numbered list, which is super 
> > > unfortunate. I'd like us to come up with a more refined strategy of 
> > > handling this case. Maybe we should look at how others are doing it?
> > Looking at doxygen, it seems there is no extra protection: just a number 
> > followed by a dot...
> > So it means:
> >   * We should never break before a such a sequence, to avoid the issue.
> >   * We may also limit the expression to limit the size of the number: I am 
> > not sure there are cases where bullet lists with hundreds of items are 
> > used, esp. with explicit values (uses the auto-numbering -# would be much 
> > simpler in that case). Maybe a limit of 2 digits? The same limit would be 
> > applied to prevent breaking before a number followed by a dot.
> > 
> > What do you think?
> I like the combination of the two options: let's limit to 2 digits and not 
> break before a matching numbered list sequence followed by a fullstop. That 
> would require also a little change to `BreakableToken::getCommentSplit`.
Done, but I could find a use-case where this would break subsequent passes, 
apart from re-running clang-format ; but in this case it is fine, since the 
comments are already formatted to fit, and will thus not be reflowed... 


https://reviews.llvm.org/D33285



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Nop, it's formatted like this:

  bool a = aa   //
==  //
&& c;
  
  bool a = aa //
==    //
   + c;


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D33447#765610, @djasper wrote:

> I think it's just wrong that WebKit inherits this. The style guide explicitly 
> says that this is wrong:
>
>   MyOtherClass::MyOtherClass() : MySuperClass() {}


I think this exemple applies to constructors only.
Looking at webkit code, there are many shortl functions which are indeed 
formatted on a single line (at least 22097 occurences, actually)

> So I still think we can make this work with the existing options without 
> regressing a behavior that anyone is relying on.


https://reviews.llvm.org/D33447



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

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

fix style


https://reviews.llvm.org/D32478

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -102,7 +102,7 @@
   verifyFormat("var x = a() in\n"
".aa.aa;");
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
-  Style.AlignOperands = true;
+  Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("var x = a() in\n"
".aa.aa;",
Style);
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2503,6 +2503,9 @@
"  > c) {\n"
"}",
Style);
+  verifyFormat("return a\n"
+   "   && bbb;",
+   Style);
   verifyFormat("return (a)\n"
"   // comment\n"
"   + b;",
@@ -2531,11 +2534,103 @@
 
   Style.ColumnLimit = 60;
   verifyFormat("zz\n"
-   "= b\n"
+   "= \n"
"  >> (aa);",
Style);
 }
 
+TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Style.AlignOperands = FormatStyle::OAS_StrictAlign;
+
+  verifyFormat("bool value = a\n"
+   "   + a\n"
+   "   + a\n"
+   "  == a\n"
+   " * b\n"
+   " + b\n"
+   "  && a\n"
+   " * a\n"
+   " > c;",
+   Style);
+  verifyFormat("if (a\n"
+   "* \n"
+   "+ aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "+ \n"
+   "  * aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "== \n"
+   "   * aa\n"
+   "   + bbb) {\n}",
+   Style);
+  verifyFormat("if () {\n"
+   "} else if (a\n"
+   "   && b // break\n"
+   "  > c) {\n"
+   "}",
+   Style);
+  verifyFormat("return a\n"
+   "&& bbb;",
+   Style);
+  verifyFormat("return (a)\n"
+   "   // comment\n"
+   " + b;",
+   Style);
+  verifyFormat(
+  "int aa = aa\n"
+  "   * bbb\n"
+  "   + cc;",
+  Style);
+
+  verifyFormat("a\n"
+   "=  + ;",
+   Style);
+
+  verifyFormat("return boost::fusion::at_c<0>().second\n"
+   "== boost::fusion::at_c<1>().second;",
+   Style);
+
+  Style.ColumnLimit = 60;
+  verifyFormat("z\n"
+   "= \n"
+   "   >> (aa);",
+   Style);
+
+  // 

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D32478#765537, @djasper wrote:

> In all honesty, I think this style isn't thought out well enough. It really 
> is a special case for only "=" and "return" and even there, it has many cases 
> where it simply doesn't make sense. And then you have cases like this:
>
>   bool = aa //
>   ==  //
>   && c;
>   
>
> Where the syntactic structure is lost entirely.




  bool a = aa   //
==  //
&& c;

> On top of that it has runtime downsides for all clang-format users because 
> ParenState gets larger and more costly compare. As such, I am against moving 
> forward with this. Can you remind me again, which coding style suggests this 
> format?

This is just a single extra bit (and there are still less than 16 such bits), 
so it does change the size of ParenState. As for the compare cost, I think it 
is within reach of the compiler's optimization, but it may indeed have a slight 
impact.


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

I actually don't know how, but it still manages somehow : I rebuilt this exact 
patch to ensure I gave you the correct output.
And the same behavior can be seen in the test cases, where the operator with 
highest precedence is aligned with the equal sign.


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

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

update mozilla style to use empty function blocks


https://reviews.llvm.org/D33447

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6006,6 +6006,36 @@
getLLVMStyleWithColumns(23));
 }
 
+TEST_F(FormatTest, PullEmptyFunctionDefinitionsIntoSingleLine) {
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {\n"
+			   "return 42;\n"
+			   "  }\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("int f() {}",
+   MergeEmptyOnly);
+  verifyFormat("int f() {\n"
+   "  return 42;\n"
+   "}",
+   MergeEmptyOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeEmptyOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeEmptyOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("int f() {}", MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+}
+
 TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
   FormatStyle MergeInlineOnly = getLLVMStyle();
   MergeInlineOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
@@ -6017,6 +6047,104 @@
"  return 42;\n"
"}",
MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {}",
+   MergeInlineOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeInlineOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("int f() {}", MergeInlineOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+}
+
+TEST_F(FormatTest, AllowEmptyFunctionBodyOnASingleLine) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  Style.AllowEmptyFunctionBodyOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.ColumnLimit = 40;
+
+  verifyFormat("int f()\n"
+   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  return 42;\n"
+   "}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  // some comment\n"
+   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("int f() {}", Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+  verifyFormat("class Foo {\n"
+			   "  int f() {}\n"
+			   "};\n",
+   Style);
+verifyFormat("class Foo {\n"
+			   "  int f() { return 0; }\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {}\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {\n"
+			   "return 0;\n"
+			   "  }\n"
+			   "};\n",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("int f() {}",
+   Style);
+  verifyFormat("int f() { return 0; }",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
 }
 
 TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) {
@@ -8670,6 +8798,7 @@
   CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
+  

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

> Ah damn. Didn't think about the precedences. What I wanted was for the 
> highest level to have a one char operator, e.g.
> 
>   bool a = aa //
>  <<    //
>  | c;
>

Almost, but not quite:

  bool a = aa   //
<<  //
 | c;

i.e. the identifiers is indented by 4 characters, then the operator is 
'unindented.

> I would still be interested in a coding style that recommends this format.

We are using this at our compagny, but this is neither public nor open source.
I am sure I saw this style somewhere else, but I cannot remember where...
(This is also the style recommended for webkit's when returning booleans : 
https://webkit.org/code-style-guidelines/#indentation-wrap-bool-op, but 
probably because the continuation indent is 4...
and I am not sure this guide in Boost's Spirit library counts either: 
http://www.boost.org/doc/libs/1_56_0/libs/spirit/doc/html/spirit/notes/style_guide.html
 )


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 100420.
Typz marked 2 inline comments as done.
Typz added a comment.

fix indent & rename option to SplitEmptyFunctionBody


https://reviews.llvm.org/D33447

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6006,6 +6006,35 @@
getLLVMStyleWithColumns(23));
 }
 
+TEST_F(FormatTest, PullEmptyFunctionDefinitionsIntoSingleLine) {
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {\n"
+   "return 42;\n"
+   "  }\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("int f() {}", MergeEmptyOnly);
+  verifyFormat("int f() {\n"
+   "  return 42;\n"
+   "}",
+   MergeEmptyOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeEmptyOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeEmptyOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("int f() {}", MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+}
+
 TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
   FormatStyle MergeInlineOnly = getLLVMStyle();
   MergeInlineOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
@@ -6017,6 +6046,101 @@
"  return 42;\n"
"}",
MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {}", MergeInlineOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeInlineOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("int f() {}", MergeInlineOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+}
+
+TEST_F(FormatTest, SplitEmptyFunctionBody) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.SplitEmptyFunctionBody = false;
+  Style.ColumnLimit = 40;
+
+  verifyFormat("int f()\n"
+   "{}",
+   Style);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 42;\n"
+   "}",
+   Style);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  // some comment\n"
+   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("int f() {}", Style);
+  verifyFormat("int aa(int bb)\n"
+   "{}",
+   Style);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 0;\n"
+   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+  verifyFormat("class Foo {\n"
+   "  int f() {}\n"
+   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+   "  int f() { return 0; }\n"
+   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+   "  int aa(int bb)\n"
+   "  {}\n"
+   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+   "  int aa(int bb)\n"
+   "  {\n"
+   "return 0;\n"
+   "  }\n"
+   "};\n",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("int f() {}", Style);
+  verifyFormat("int f() { return 0; }", Style);
+  verifyFormat("int aa(int bb)\n"
+   "{}",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+   "{\n"
+   "  return 0;\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) {
@@ -8715,6 +8839,7 @@
   

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

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

move option to BraceWrapping


https://reviews.llvm.org/D33447

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6006,6 +6006,36 @@
getLLVMStyleWithColumns(23));
 }
 
+TEST_F(FormatTest, PullEmptyFunctionDefinitionsIntoSingleLine) {
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {\n"
+			   "return 42;\n"
+			   "  }\n"
+   "};",
+   MergeEmptyOnly);
+  verifyFormat("int f() {}",
+   MergeEmptyOnly);
+  verifyFormat("int f() {\n"
+   "  return 42;\n"
+   "}",
+   MergeEmptyOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeEmptyOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeEmptyOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("int f() {}", MergeEmptyOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeEmptyOnly);
+}
+
 TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
   FormatStyle MergeInlineOnly = getLLVMStyle();
   MergeInlineOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
@@ -6017,6 +6047,104 @@
"  return 42;\n"
"}",
MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f() {}",
+   MergeInlineOnly);
+
+  // Also verify behavior when BraceWrapping.AfterFunction = true
+  MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+  MergeInlineOnly.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "  int f() { return 42; }\n"
+   "};",
+   MergeInlineOnly);
+  verifyFormat("int f()\n"
+   "{\n"
+   "  return 42;\n"
+   "}",
+   MergeInlineOnly);
+
+  // SFS_Inline implies SFS_Empty
+  verifyFormat("int f() {}", MergeInlineOnly);
+  verifyFormat("class C {\n"
+   "  int f() {}\n"
+   "};",
+   MergeInlineOnly);
+}
+
+TEST_F(FormatTest, AllowEmptyFunctionBody) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.AllowEmptyFunctionBody = true;
+  Style.ColumnLimit = 40;
+
+  verifyFormat("int f()\n"
+   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  return 42;\n"
+   "}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+   "  // some comment\n"
+   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
+  verifyFormat("int f() {}", Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int f()\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+  verifyFormat("class Foo {\n"
+			   "  int f() {}\n"
+			   "};\n",
+   Style);
+verifyFormat("class Foo {\n"
+			   "  int f() { return 0; }\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {}\n"
+			   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+			   "  int aa(int bb)\n"
+			   "  {\n"
+			   "return 0;\n"
+			   "  }\n"
+			   "};\n",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("int f() {}",
+   Style);
+  verifyFormat("int f() { return 0; }",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{}",
+   Style);
+  verifyFormat("int aa(int bb)\n"
+			   "{\n"
+			   "  return 0;\n"
+			   "}",
+   Style);
 }
 
 TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) {
@@ -8712,6 +8840,7 @@
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterObjCDeclaration);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterStruct);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterUnion);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, 

[PATCH] D33447: clang-format: add option to merge empty function body

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Webkit inherits AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All from 
LLVM style, so merging the options would introduce these explicitely-forbidden 
empty blocks.
But the empty blocks should actually be used in code following Mozilla or Qt 
style.


https://reviews.llvm.org/D33447



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D32478#758258, @djasper wrote:

> When you say "this doesn't happen in tests", do you mean this never happens 
> when there are parentheses around the expression?


By 'test' I meant 'conditional construct' : it happens when there are 
parentheses around the expression, but it does not happen when these 
parentheses are the parentheses from a `if (...)`


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32477: clang-format: Allow customizing the penalty for breaking assignment

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL303534: clang-format: Allow customizing the penalty for 
breaking assignment (authored by Typz).

Changed prior to commit:
  https://reviews.llvm.org/D32477?vs=99415=99720#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32477

Files:
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -1133,6 +1133,9 @@
   /// ``Foo `` instead of ``Foo``.
   bool ObjCSpaceBeforeProtocolList;
 
+  /// \brief The penalty for breaking around an assignment operator.
+  unsigned PenaltyBreakAssignment;
+
   /// \brief The penalty for breaking a function call after ``call(``.
   unsigned PenaltyBreakBeforeFirstCallParameter;
 
@@ -1420,6 +1423,8 @@
ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty &&
ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList &&
+   PenaltyBreakAssignment ==
+   R.PenaltyBreakAssignment &&
PenaltyBreakBeforeFirstCallParameter ==
R.PenaltyBreakBeforeFirstCallParameter &&
PenaltyBreakComment == R.PenaltyBreakComment &&
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2093,9 +2093,10 @@
   if (Left.is(TT_ConditionalExpr))
 return prec::Conditional;
   prec::Level Level = Left.getPrecedence();
-  if (Level != prec::Unknown)
-return Level;
-  Level = Right.getPrecedence();
+  if (Level == prec::Unknown)
+Level = Right.getPrecedence();
+  if (Level == prec::Assignment)
+return Style.PenaltyBreakAssignment;
   if (Level != prec::Unknown)
 return Level;
 
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -343,6 +343,8 @@
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
+IO.mapOptional("PenaltyBreakAssignment",
+   Style.PenaltyBreakAssignment);
 IO.mapOptional("PenaltyBreakBeforeFirstCallParameter",
Style.PenaltyBreakBeforeFirstCallParameter);
 IO.mapOptional("PenaltyBreakComment", Style.PenaltyBreakComment);
@@ -582,6 +584,7 @@
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpacesInAngles = false;
 
+  LLVMStyle.PenaltyBreakAssignment = prec::Assignment;
   LLVMStyle.PenaltyBreakComment = 300;
   LLVMStyle.PenaltyBreakFirstLessLess = 120;
   LLVMStyle.PenaltyBreakString = 1000;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -3457,6 +3457,18 @@
"1;");
 }
 
+TEST_F(FormatTest, ConfigurableBreakAssignmentPenalty) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat("int aa =\n"
+   "bb + cc;",
+   Style);
+
+  Style.PenaltyBreakAssignment = 20;
+  verifyFormat("int aa = bb 
+\n"
+   " cc;",
+   Style);
+}
+
 TEST_F(FormatTest, AlignsAfterAssignments) {
   verifyFormat(
   "int Result = a + a +\n"
@@ -8802,6 +8814,8 @@
   CHECK_PARSE("ObjCBlockIndentWidth: 1234", ObjCBlockIndentWidth, 1234u);
   CHECK_PARSE("ColumnLimit: 1234", ColumnLimit, 1234u);
   CHECK_PARSE("MaxEmptyLinesToKeep: 1234", MaxEmptyLinesToKeep, 1234u);
+  CHECK_PARSE("PenaltyBreakAssignment: 1234",
+  PenaltyBreakAssignment, 1234u);
   CHECK_PARSE("PenaltyBreakBeforeFirstCallParameter: 1234",
   PenaltyBreakBeforeFirstCallParameter, 1234u);
   CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u);


Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -1133,6 +1133,9 @@
   /// ``Foo `` instead of ``Foo``.
   bool ObjCSpaceBeforeProtocolList;
 
+  /// \brief The penalty for breaking around an assignment operator.
+  unsigned PenaltyBreakAssignment;
+
   /// \brief The penalty for breaking a function call after ``call(``.
   

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Merging the 2 options is definitely a "safe" option, as it prevents ensures 
only the most obvious behavior is accessible.

However, it has significant (IMO) drawbacks:

- "Compact" is a not an namespace indentation type, this will make the option 
quite confusing
- If indentation inside compact namespaces is needed, it cannot easily be 
added: we would need an extra mode NI_CompactWithIndent

All in all, I think I prefer the current behavior of the patch (a separate 
CompactNamespace options, with consistent [if not useful] indentation 
settings); but it is your call, just let me know how to proceed.


https://reviews.llvm.org/D32480



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: include/clang/Format/Format.h:358
+  /// \endcode
+  bool BinPackNamespaces;
+

djasper wrote:
> Typz wrote:
> > djasper wrote:
> > > Typz wrote:
> > > > djasper wrote:
> > > > > This is not bin packing at all. Maybe CompactNamespaces? Or 
> > > > > SingleLineNamespaces?
> > > > > 
> > > > > Also, could you clarify what happens if the namespaces don't fit 
> > > > > within the column limit (both in the comment describing the flag and 
> > > > > by adding tests for it)?
> > > > This is not binpacking, but has a similar effect and made the option 
> > > > name somewhat consistent with the other binpack options :-)
> > > > I'll change to CompactNamespaces then.
> > > How does this option interact with NamespaceIndentation. Do all the 
> > > values you can set there make sense and work as expected?
> > > 
> > > I am wondering whether we should generally rename this to NamespaceStyle 
> > > and make it an enum. That way, we can start to also support C++17 
> > > namespace, but people that don't want to use C++17 yet, can still use 
> > > this style of compact namespace.
> > > How does this option interact with NamespaceIndentation. Do all the 
> > > values you can set there make sense and work as expected?
> > 
> > NamespaceIndentation is not affected, indent is done as before (e.g. just 
> > "counting" the imbricated namespaces).
> > 
> > In 'NI_All' we may want to reduce the indent when multiple namespaces are 
> > declared in the same line, but this would become inconsistent if the 
> > beginning and end of all namespaces do not match:
> > 
> >   namepace A { namespace B {
> >   int i;
> >   } // namespace B
> > int i;
> >   } // namespace A
> > 
> > So I think reducing indent in that case should be (if ever needed) another 
> > value in NamespaceIndentation...
> > 
> > > I am wondering whether we should generally rename this to NamespaceStyle 
> > > and make it an enum. That way, we can start to also support C++17 
> > > namespace, but people that don't want to use C++17 yet, can still use 
> > > this style of compact namespace.
> > 
> > As for C++17, I am not sure we need another option: just having 
> > CompactNamespaces=true and Standard=C++17 would use the "real" C++17 mode. 
> > That said converting to C++17 namespace blocks is slightly more 
> > restrictive, as it will require that both the beginning and end of the 
> > inner & outer blocks to match...
> > 
> > I will keep the boolean flag for now, just let me know if you prefer to 
> > have the enum in case other modes are needed and I will update the patch.
> Yeah, this is probably something nobody will ever want:
> 
>   namepace A { namespace B {
>   int i;
>   } // namespace B
> int i;
>   } // namespace A
> 
> And you have the same problem for NI_Inner as soon as you have more than two 
> levels of namespace.
> 
> I see two ways forward:
> 
> 1. Make "compacted" namespaces always add at most one level of indentation.
> 2. Assume that this can only ever usefully work with the behavior of NI_None 
> and add an additional enum value NI_Compact.
> 
> The problem I have with #1 is that it increases the complexity quite a bit 
> and the behavior is even difficult to predict to users. Remove a comment 
> somewhere might enable clang-format to make two namespaces "compact" and then 
> remove indentation throughout the file.
> 
> So I would lean more towards solution #2. The name NamespaceIndentation might 
> then be a bit confusing, but not sure whether it's worth changing it. And of 
> course I don't know whether some people would want indentation in compacted 
> namespaces.
> 
> What do you think?
I think current behavior is at least consistent (and simple to implement), and 
I agree that #2 would be the way to go to implement special nanespace indent 
rules for compacted namespaces.

Personnally, I don't need namespace indentation (we use NI_None), but i can add 
try to add an extra style NI_Compact to indent one level when in any number of 
namespaces (e.g. indentLevel = namespaceDepth>0).

That should probably be a separate patch however?


https://reviews.llvm.org/D32480



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33285: clang-format: do not reflow bullet lists

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL303556: clang-format: do not reflow bullet lists (authored 
by Typz).

Changed prior to commit:
  https://reviews.llvm.org/D33285?vs=99436=99764#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33285

Files:
  cfe/trunk/lib/Format/BreakableToken.cpp
  cfe/trunk/unittests/Format/FormatTestComments.cpp

Index: cfe/trunk/unittests/Format/FormatTestComments.cpp
===
--- cfe/trunk/unittests/Format/FormatTestComments.cpp
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp
@@ -1577,7 +1577,7 @@
" *\n"
" * long */",
getLLVMStyleWithColumns(20)));
-  
+
   // Don't reflow lines having content that is a single character.
   EXPECT_EQ("// long long long\n"
 "// long\n"
@@ -1602,7 +1602,7 @@
 format("// long long long long\n"
"// @param arg",
getLLVMStyleWithColumns(20)));
-  
+
   // Don't reflow lines starting with 'TODO'.
   EXPECT_EQ("// long long long\n"
 "// long\n"
@@ -1671,6 +1671,69 @@
"//  long",
getLLVMStyleWithColumns(20)));
 
+  // Don't reflow separate bullets in list
+  EXPECT_EQ("// - long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+   "// - long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// * long long long\n"
+"// long\n"
+"// * long",
+format("// * long long long long\n"
+   "// * long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// + long long long\n"
+"// long\n"
+"// + long",
+format("// + long long long long\n"
+   "// + long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// 1. long long long\n"
+"// long\n"
+"// 2. long",
+format("// 1. long long long long\n"
+   "// 2. long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// -# long long long\n"
+"// long\n"
+"// -# long",
+format("// -# long long long long\n"
+   "// -# long",
+   getLLVMStyleWithColumns(20)));
+
+  EXPECT_EQ("// - long long long\n"
+"// long long long\n"
+"// - long",
+format("// - long long long long\n"
+   "// long long\n"
+   "// - long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// - long long long\n"
+"// long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+   "// long long long\n"
+   "// - long",
+   getLLVMStyleWithColumns(20)));
+
+  // Large number (>2 digits) are not list items
+  EXPECT_EQ("// long long long\n"
+"// long 1024. long.",
+format("// long long long long\n"
+   "// 1024. long.",
+   getLLVMStyleWithColumns(20)));
+
+  // Do not break before number, to avoid introducing a non-reflowable doxygen
+  // list item.
+  EXPECT_EQ("// long long\n"
+"// long 10. long.",
+format("// long long long 10.\n"
+   "// long.",
+   getLLVMStyleWithColumns(20)));
+
   // Don't break or reflow after implicit string literals.
   verifyFormat("#include  // l l l\n"
" // l",
Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -78,6 +78,14 @@
   }
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
+
+  // Do not split before a number followed by a dot: this would be interpreted
+  // as a numbered list, which would prevent re-flowing in subsequent passes.
+  static llvm::Regex kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\.");
+  if (SpaceOffset != StringRef::npos &&
+  kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks)))
+SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+
   if (SpaceOffset == StringRef::npos ||
   // Don't break at leading whitespace.
   Text.find_last_not_of(Blanks, SpaceOffset) == StringRef::npos) {
@@ -299,15 +307,24 @@
 static bool mayReflowContent(StringRef Content) {
   Content = Content.trim(Blanks);
   // Lines starting with '@' commonly have special meaning.
-  static const SmallVector kSpecialMeaningPrefixes = {
-  "@", "TODO", "FIXME", "XXX"};
+  // Lines starting with '-', '-#', '+' or '*' are bulleted/numbered lists.
+  static const SmallVector kSpecialMeaningPrefixes = {
+ 

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

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

Remove dependency on https://reviews.llvm.org/D33314


https://reviews.llvm.org/D32480

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1309,6 +1309,140 @@
Style));
 }
 
+TEST_F(FormatTest, FormatsCompactNamespaces) {
+  FormatStyle Style = getLLVMStyle();
+  Style.CompactNamespaces = true;
+
+  verifyFormat("namespace A { namespace B {\n"
+			   "}} // namespace A::B",
+			   Style);
+
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "} // namespace out",
+   Style));
+
+  EXPECT_EQ("namespace out { namespace in1 {\n"
+"} // namespace in1\n"
+"namespace in2 {\n"
+"}} // namespace out::in2",
+format("namespace out {\n"
+   "namespace in1 {\n"
+   "} // namespace in1\n"
+   "namespace in2 {\n"
+   "} // namespace in2\n"
+   "} // namespace out",
+   Style));
+
+  EXPECT_EQ("namespace out {\n"
+"int i;\n"
+"namespace in {\n"
+"int j;\n"
+"} // namespace in\n"
+"int k;\n"
+"} // namespace out",
+format("namespace out { int i;\n"
+   "namespace in { int j; } // namespace in\n"
+   "int k; } // namespace out",
+   Style));
+
+  EXPECT_EQ("namespace A { namespace B { namespace C {\n"
+"}}} // namespace A::B::C\n",
+format("namespace A { namespace B {\n"
+   "namespace C {\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n",
+   Style));
+
+  EXPECT_EQ("namespace  {\n"
+			"namespace  {\n"
+"}} // namespace ::",
+format("namespace  {\n"
+   "namespace  {\n"
+   "} // namespace \n"
+   "} // namespace ",
+   Style));
+
+  EXPECT_EQ("namespace a { namespace b {\n"
+			"namespace c {\n"
+"}}} // namespace a::b::c",
+format("namespace a {\n"
+   "namespace b {\n"
+   "namespace c {\n"
+   "} // namespace c\n"
+   "} // namespace b\n"
+   "} // namespace a",
+   Style));
+
+  // Missing comments are added
+  EXPECT_EQ("namespace out { namespace in {\n"
+			"int i;\n"
+			"int j;\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "int i;\n"
+   "int j;\n"
+   "}\n"
+   "}",
+   Style));
+
+  // Incorrect comments are fixed
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+   "}} // namespace out",
+   Style));
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+   "}} // namespace in",
+   Style));
+
+  // Extra semicolon after 'inner' closing brace prevents merging
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}; } // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "}; // namespace in\n"
+   "} // namespace out",
+   Style));
+
+  // Extra semicolon after 'outer' closing brace is conserved
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}}; // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "}; // namespace out",
+   Style));
+
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  EXPECT_EQ("namespace out { namespace in {\n"
+"int i;\n"
+"}} // namespace out::in",
+

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

It indeed does not happens inside any parenthesis (it would actually make 
things completely unreadable if there are imbricated parenthesis), and may get 
indented less than the ContinuationIndentWidth.


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:949
+ Previous->is(tok::kw_return)))
+  NewParenState.UnindentOperator = true;
 

djasper wrote:
> I am not yet convinced you need a new flag in ParenState here. I guess you 
> need it because the operators can have varying length and so you cannot just 
> change LastSpace here?
exactly. This is set when passing through the equal/return sign, but indent 
must be adjusted for each line individually based on the actual size of that 
line's leading operator.


https://reviews.llvm.org/D32478



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Herald added a subscriber: klimek.

This patch tries to improve the optimizer a bit, to avoid splitting
tokens (e.g. comments/strings) if only there are only few characters
beyond the ColumnLimit.

Previously, comments/strings would be split whenever they went beyond
the ColumnLimit, without consideration for the PenaltyBreakComment/
String and PenaltyExcessCharacter. With this patch, the
OptimizingLineFormatter will also consider not splitting each token,
so that these 'small' offenders get a chance:

  // With ColumnLimit=20
  int a; // the
 // comment
  
  // With ColumnLimit=20 and PenaltyExcessCharacter = 10
  int a; // the comment

This patch does not fully optimize the reflowing, as it will only try
reflowing the whole comment or not reflowing it at all (instead of
trying each split, to allow some lines of overflow ColumnLimit even
when reflowing).


https://reviews.llvm.org/D33589

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8542,6 +8542,36 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 20;
+
+  verifyFormat("int a; // the\n"
+			   "   // comment", Style);
+  verifyFormat("int a; /* first line\n"
+   "* second\n"
+			   "* line third\n"
+   "* line\n"
+   "*/", Style);
+
+  Style.PenaltyExcessCharacter = 30;
+  verifyFormat("int a; // the comment", Style);
+  EXPECT_EQ("int a; // the\n"
+"   // comment aa",
+format("int a; // the comment aa", Style));
+  verifyFormat("int a; /* first line\n"
+   "* second line\n"
+			   "* third line\n"
+   "*/", Style);
+  EXPECT_EQ("int a; /* first line\n"
+"* second\n"
+			"* line third\n"
+"* line\n"
+"*/",
+format("int a; /* first line second line third line */",
+   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/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -590,7 +590,8 @@
   (Indenter->canBreak(State) && State.NextToken->NewlinesBefore > 0);
   unsigned Penalty = 0;
   formatChildren(State, Newline, /*DryRun=*/false, Penalty);
-  Indenter->addTokenToState(State, Newline, /*DryRun=*/false);
+  Indenter->addTokenToState(State, Newline, /*BreakToken=*/true,
+/*DryRun=*/false);
 }
 return 0;
   }
@@ -611,7 +612,8 @@
 LineState State = Indenter->getInitialState(FirstIndent, , DryRun);
 while (State.NextToken) {
   formatChildren(State, /*Newline=*/false, DryRun, Penalty);
-  Indenter->addTokenToState(State, /*Newline=*/false, DryRun);
+  Indenter->addTokenToState(State, /*Newline=*/false, /*BreakToken=*/true,
+DryRun);
 }
 return Penalty;
   }
@@ -658,10 +660,11 @@
   /// \brief An edge in the solution space from \c Previous->State to \c State,
   /// inserting a newline dependent on the \c NewLine.
   struct StateNode {
-StateNode(const LineState , bool NewLine, StateNode *Previous)
-: State(State), NewLine(NewLine), Previous(Previous) {}
+StateNode(const LineState , bool NewLine, bool BreakToken, StateNode *Previous)
+: State(State), NewLine(NewLine), BreakToken(BreakToken), Previous(Previous) {}
 LineState State;
 bool NewLine;
+bool BreakToken;
 StateNode *Previous;
   };
 
@@ -691,7 +694,7 @@
 
 // Insert start element into queue.
 StateNode *Node =
-new (Allocator.Allocate()) StateNode(InitialState, false, nullptr);
+new (Allocator.Allocate()) StateNode(InitialState, false, true, nullptr);
 Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
 ++Count;
 
@@ -718,9 +721,17 @@
 
   FormatDecision LastFormat = Node->State.NextToken->Decision;
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/false, , );
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,
+/*BreakToken=*/true, , );
   if (LastFormat == FD_Unformatted || 

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

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

fix indentation issues


https://reviews.llvm.org/D33589

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8542,6 +8542,35 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -C   -A"));
 }
 
+TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 20;
+
+  verifyFormat("int a; // the\n"
+   "   // comment", Style);
+  verifyFormat("int a; /* first line\n"
+   "* second\n"
+   "* line third\n"
+   "* line\n"
+   "*/", Style);
+
+  Style.PenaltyExcessCharacter = 30;
+  verifyFormat("int a; // the comment", Style);
+  EXPECT_EQ("int a; // the\n"
+"   // comment aa",
+format("int a; // the comment aa", Style));
+  verifyFormat("int a; /* first line\n"
+   "* second line\n"
+   "* third line\n"
+   "*/", Style);
+  EXPECT_EQ("int a; /* first line\n"
+"* second\n"
+"* line third\n"
+"* line\n"
+"*/",
+format("int a; /* first line second line third line */", 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/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -590,7 +590,8 @@
   (Indenter->canBreak(State) && State.NextToken->NewlinesBefore > 0);
   unsigned Penalty = 0;
   formatChildren(State, Newline, /*DryRun=*/false, Penalty);
-  Indenter->addTokenToState(State, Newline, /*DryRun=*/false);
+  Indenter->addTokenToState(State, Newline, /*BreakToken=*/true,
+/*DryRun=*/false);
 }
 return 0;
   }
@@ -611,7 +612,8 @@
 LineState State = Indenter->getInitialState(FirstIndent, , DryRun);
 while (State.NextToken) {
   formatChildren(State, /*Newline=*/false, DryRun, Penalty);
-  Indenter->addTokenToState(State, /*Newline=*/false, DryRun);
+  Indenter->addTokenToState(State, /*Newline=*/false, /*BreakToken=*/true,
+DryRun);
 }
 return Penalty;
   }
@@ -658,10 +660,11 @@
   /// \brief An edge in the solution space from \c Previous->State to \c State,
   /// inserting a newline dependent on the \c NewLine.
   struct StateNode {
-StateNode(const LineState , bool NewLine, StateNode *Previous)
-: State(State), NewLine(NewLine), Previous(Previous) {}
+StateNode(const LineState , bool NewLine, bool BreakToken, StateNode *Previous)
+: State(State), NewLine(NewLine), BreakToken(BreakToken), Previous(Previous) {}
 LineState State;
 bool NewLine;
+bool BreakToken;
 StateNode *Previous;
   };
 
@@ -691,7 +694,7 @@
 
 // Insert start element into queue.
 StateNode *Node =
-new (Allocator.Allocate()) StateNode(InitialState, false, nullptr);
+new (Allocator.Allocate()) StateNode(InitialState, false, true, nullptr);
 Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
 ++Count;
 
@@ -718,9 +721,17 @@
 
   FormatDecision LastFormat = Node->State.NextToken->Decision;
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/false, , );
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,
+/*BreakToken=*/true, , );
   if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/true, , );
+addNextStateToQueue(Penalty, Node, /*NewLine=*/true,
+/*BreakToken=*/true, , );
+  if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,
+/*BreakToken=*/false, , );
+  if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
+addNextStateToQueue(Penalty, Node, /*NewLine=*/true,
+/*BreakToken=*/false, , );
 }
 
 if (Queue.empty()) {
@@ -745,18 +756,20 @@
   /// Assume the current state is \p PreviousNode and has been reached with a
   /// penalty of \p Penalty. Insert a line break if \p NewLine is \c true.
 

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Still some issues with the patch, I would need some feedback first:

- Is this approach desirable, as a relatively easy fix?
- Or should this be fixed with a complete refactoring of the way the 
strings/comments are split, making multiple tokens out of them to let them be 
reflown 'directly' by the optimizing line formatter? (keep the optimizer, but 
changes the whole way comments are handled, and need to move all the 
information about the comment block and its relfowing into LineState)
- Or should the breakProtrudingToken() actually perform a "local" optimization 
of the splits? (keeps the same overall structure, but requires writing another 
optimizer)




Comment at: unittests/Format/FormatTest.cpp:8571
+"*/",
+format("int a; /* first line second line third line */", Style));
+}

This is not working as expected, format return:

  int a; /* first line
* second *
line third
* line */

Any clue how things could go so wrong?


https://reviews.llvm.org/D33589



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: include/clang/Format/Format.h:358
+  /// \endcode
+  bool BinPackNamespaces;
+

djasper wrote:
> Typz wrote:
> > djasper wrote:
> > > This is not bin packing at all. Maybe CompactNamespaces? Or 
> > > SingleLineNamespaces?
> > > 
> > > Also, could you clarify what happens if the namespaces don't fit within 
> > > the column limit (both in the comment describing the flag and by adding 
> > > tests for it)?
> > This is not binpacking, but has a similar effect and made the option name 
> > somewhat consistent with the other binpack options :-)
> > I'll change to CompactNamespaces then.
> How does this option interact with NamespaceIndentation. Do all the values 
> you can set there make sense and work as expected?
> 
> I am wondering whether we should generally rename this to NamespaceStyle and 
> make it an enum. That way, we can start to also support C++17 namespace, but 
> people that don't want to use C++17 yet, can still use this style of compact 
> namespace.
> How does this option interact with NamespaceIndentation. Do all the values 
> you can set there make sense and work as expected?

NamespaceIndentation is not affected, indent is done as before (e.g. just 
"counting" the imbricated namespaces).

In 'NI_All' we may want to reduce the indent when multiple namespaces are 
declared in the same line, but this would become inconsistent if the beginning 
and end of all namespaces do not match:

  namepace A { namespace B {
  int i;
  } // namespace B
int i;
  } // namespace A

So I think reducing indent in that case should be (if ever needed) another 
value in NamespaceIndentation...

> I am wondering whether we should generally rename this to NamespaceStyle and 
> make it an enum. That way, we can start to also support C++17 namespace, but 
> people that don't want to use C++17 yet, can still use this style of compact 
> namespace.

As for C++17, I am not sure we need another option: just having 
CompactNamespaces=true and Standard=C++17 would use the "real" C++17 mode. That 
said converting to C++17 namespace blocks is slightly more restrictive, as it 
will require that both the beginning and end of the inner & outer blocks to 
match...

I will keep the boolean flag for now, just let me know if you prefer to have 
the enum in case other modes are needed and I will update the patch.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:139
+
+bool isEndOfNamespace(const AnnotatedLine *line,
+  const SmallVectorImpl ) {

djasper wrote:
> s/line/Line/
these methods (isEndOfNamespace, ...) are somewhat duplicated in 
UnwrapperLinesFormatter and NamespaceEndComments (not really, but very similar) 
: should I factorize these into helper methods of FormatToken?


https://reviews.llvm.org/D32480



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 99575.
Typz marked 11 inline comments as done.
Typz edited the summary of this revision.
Typz added a comment.

update for review comments


https://reviews.llvm.org/D32480

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1308,6 +1308,140 @@
Style));
 }
 
+TEST_F(FormatTest, FormatsCompactNamespaces) {
+  FormatStyle Style = getLLVMStyle();
+  Style.CompactNamespaces = true;
+
+  verifyFormat("namespace A { namespace B {\n"
+			   "}} // namespace A::B",
+			   Style);
+
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "} // namespace out",
+   Style));
+
+  EXPECT_EQ("namespace out { namespace in1 {\n"
+"} // namespace in1\n"
+"namespace in2 {\n"
+"}} // namespace out::in2",
+format("namespace out {\n"
+   "namespace in1 {\n"
+   "} // namespace in1\n"
+   "namespace in2 {\n"
+   "} // namespace in2\n"
+   "} // namespace out",
+   Style));
+
+  EXPECT_EQ("namespace out {\n"
+"int i;\n"
+"namespace in {\n"
+"int j;\n"
+"} // namespace in\n"
+"int k;\n"
+"} // namespace out",
+format("namespace out { int i;\n"
+   "namespace in { int j; } // namespace in\n"
+   "int k; } // namespace out",
+   Style));
+
+  EXPECT_EQ("namespace A { namespace B { namespace C {\n"
+"}}} // namespace A::B::C\n",
+format("namespace A { namespace B {\n"
+   "namespace C {\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n",
+   Style));
+
+  EXPECT_EQ("namespace  {\n"
+			"namespace  {\n"
+"}} // namespace ::",
+format("namespace  {\n"
+   "namespace  {\n"
+   "} // namespace \n"
+   "} // namespace ",
+   Style));
+
+  EXPECT_EQ("namespace a { namespace b {\n"
+			"namespace c {\n"
+"}}} // namespace a::b::c",
+format("namespace a {\n"
+   "namespace b {\n"
+   "namespace c {\n"
+   "} // namespace c\n"
+   "} // namespace b\n"
+   "} // namespace a",
+   Style));
+
+  // Missing comments are added
+  EXPECT_EQ("namespace out { namespace in {\n"
+			"int i;\n"
+			"int j;\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "int i;\n"
+   "int j;\n"
+   "}\n"
+   "}",
+   Style));
+
+  // Incorrect comments are fixed
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+   "}} // namespace out",
+   Style));
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+   "}} // namespace in",
+   Style));
+
+  // Extra semicolon after 'inner' closing brace prevents merging
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}; } // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "}; // namespace in\n"
+   "} // namespace out",
+   Style));
+
+  // Extra semicolon after 'outer' closing brace is conserved
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}}; // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "}; // namespace out",
+   Style));
+
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  EXPECT_EQ("namespace out { namespace in {\n"
+"int i;\n"
+  

[PATCH] D32479: [clang-format] Add BreakConstructorInitializersBeforeColon option

[Francois Ferrand via Phabricator via cfe-commits]

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

Deprecate BreakConstructorInitializersBeforeComma and replace it with a more 
generic BreakConstructorInitializers option.


https://reviews.llvm.org/D32479

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -56,16 +56,17 @@
 return *Result;
   }
 
-  FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
-FormatStyle Style = getLLVMStyle();
+  FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) {
 Style.ColumnLimit = ColumnLimit;
 return Style;
   }
 
+  FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
+return getStyleWithColumns(getLLVMStyle(), ColumnLimit);
+  }
+
   FormatStyle getGoogleStyleWithColumns(unsigned ColumnLimit) {
-FormatStyle Style = getGoogleStyle();
-Style.ColumnLimit = ColumnLimit;
-return Style;
+return getStyleWithColumns(getGoogleStyle(), ColumnLimit);
   }
 
   void verifyFormat(llvm::StringRef Code,
@@ -2699,6 +2700,128 @@
"() {}"));
 }
 
+TEST_F(FormatTest, BreakConstructorInitializersAfterColonAndComma) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColonAndComma;
+
+  verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}");
+  verifyFormat("Constructor() : Inttializer(FitsOnTheLine) {}",
+   getStyleWithColumns(Style, 45));
+  verifyFormat("Constructor() :\n"
+   "Inttializer(FitsOnTheLine) {}",
+   getStyleWithColumns(Style, 44));
+  verifyFormat("Constructor() :\n"
+   "Inttializer(FitsOnTheLine) {}",
+   getStyleWithColumns(Style, 43));
+
+  verifyFormat("template \n"
+   "Constructor() : Initializer(FitsOnTheLine) {}",
+   getStyleWithColumns(Style, 50));
+
+  verifyFormat(
+  "SomeClass::Constructor() :\n"
+  "a(aa), aaa() {}",
+	  Style);
+
+  verifyFormat(
+  "SomeClass::Constructor() :\n"
+  "a(aa), a(aa),\n"
+  "a(aa) {}",
+	  Style);
+  verifyFormat(
+  "SomeClass::Constructor() :\n"
+  "aa(aa),\n"
+  "aaa() {}",
+	  Style);
+  verifyFormat("Constructor(aa ,\n"
+   "aa ) :\n"
+   "aa(aa) {}",
+			   Style);
+
+  verifyFormat("Constructor() :\n"
+   "(aaa),\n"
+   "(aaa,\n"
+   " aaa),\n"
+   "aaa() {}",
+			   Style);
+
+  verifyFormat("Constructor() :\n"
+   "(\n"
+   "a) {}",
+			   Style);
+
+  verifyFormat("Constructor(int Parameter = 0) :\n"
+   "aa(a),\n"
+   "(a) {}",
+			   Style);
+  verifyFormat("Constructor() :\n"
+   "aa(a), (b) {\n"
+   "}",
+   getStyleWithColumns(Style, 60));
+  verifyFormat("Constructor() :\n"
+   "a(\n"
+   "a(, )) {}",
+			   Style);
+
+  // Here a line could be saved by splitting the second initializer onto two
+  // lines, but that is not desirable.
+  verifyFormat("Constructor() :\n"
+   "(),\n"
+   "aaa(aaa),\n"
+   "at() {}",
+			   Style);
+
+  FormatStyle OnePerLine = Style;
+  OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  OnePerLine.AllowAllParametersOfDeclarationOnNextLine = false;
+  verifyFormat("SomeClass::Constructor() :\n"
+   "a(aa),\n"
+   "a(aa),\n"
+   "a(aa) {}",
+   OnePerLine);
+  verifyFormat("SomeClass::Constructor() :\n"
+   "a(aa), // Some comment\n"
+   "a(aa),\n"
+   "

[PATCH] D33314: clang-format: Add option to remove semicolon at end of namespace

[Francois Ferrand via Phabricator via cfe-commits]

Typz abandoned this revision.
Typz added a comment.

ATM, in the presence of semicolons, the closing braces will simply not be 
merged : i will check if I can handle this case easily (in the 
CompactNamespaces patch), and I'll abandon this one for now.


https://reviews.llvm.org/D33314



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33285: clang-format: do not reflow bullet lists

[Francois Ferrand via Phabricator via cfe-commits]

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

- Use static regex to avoid recreating it each time
- Add more tests


https://reviews.llvm.org/D33285

Files:
  lib/Format/BreakableToken.cpp
  unittests/Format/FormatTestComments.cpp

Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1534,7 +1534,7 @@
" *\n"
" * long */",
getLLVMStyleWithColumns(20)));
-  
+
   // Don't reflow lines having content that is a single character.
   EXPECT_EQ("// long long long\n"
 "// long\n"
@@ -1559,7 +1559,7 @@
 format("// long long long long\n"
"// @param arg",
getLLVMStyleWithColumns(20)));
-  
+
   // Don't reflow lines starting with 'TODO'.
   EXPECT_EQ("// long long long\n"
 "// long\n"
@@ -1628,6 +1628,54 @@
"//  long",
getLLVMStyleWithColumns(20)));
 
+  // Don't reflow separate bullets in list
+  EXPECT_EQ("// - long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+   "// - long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// * long long long\n"
+"// long\n"
+"// * long",
+format("// * long long long long\n"
+   "// * long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// + long long long\n"
+"// long\n"
+"// + long",
+format("// + long long long long\n"
+   "// + long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// 1. long long long\n"
+"// long\n"
+"// 2. long",
+format("// 1. long long long long\n"
+   "// 2. long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// -# long long long\n"
+"// long\n"
+"// -# long",
+format("// -# long long long long\n"
+   "// -# long",
+   getLLVMStyleWithColumns(20)));
+
+  EXPECT_EQ("// - long long long\n"
+"// long long long\n"
+"// - long",
+format("// - long long long long\n"
+   "// long long\n"
+   "// - long",
+   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ("// - long long long\n"
+"// long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+   "// long long long\n"
+   "// - long",
+   getLLVMStyleWithColumns(20)));
+
   // Don't break or reflow after implicit string literals.
   verifyFormat("#include  // l l l\n"
" // l",
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -298,15 +298,22 @@
 static bool mayReflowContent(StringRef Content) {
   Content = Content.trim(Blanks);
   // Lines starting with '@' commonly have special meaning.
-  static const SmallVector kSpecialMeaningPrefixes = {
-  "@", "TODO", "FIXME", "XXX"};
+  // Lines starting with '-', '-#', '+' or '*' are bulleted/numbered lists.
+  static const SmallVector kSpecialMeaningPrefixes = {
+  "@", "TODO", "FIXME", "XXX", "-# ", "- ", "+ ", "* " };
   bool hasSpecialMeaningPrefix = false;
   for (StringRef Prefix : kSpecialMeaningPrefixes) {
 if (Content.startswith(Prefix)) {
   hasSpecialMeaningPrefix = true;
   break;
 }
   }
+
+  // Numbered lists may also start with a number followed by '.'
+  static llvm::Regex kNumberedListRegexp = llvm::Regex("^[1-9][0-9]*\\. ");
+  hasSpecialMeaningPrefix = hasSpecialMeaningPrefix ||
+kNumberedListRegexp.match(Content);
+
   // Simple heuristic for what to reflow: content should contain at least two
   // characters and either the first or second character must be
   // non-punctuation.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D32480



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz marked an inline comment as done.
Typz added inline comments.



Comment at: unittests/Format/FormatTest.cpp:8571
+"*/",
+format("int a; /* first line second line third line */", Style));
+}

Typz wrote:
> This is not working as expected, format return:
> 
>   int a; /* first line
> * second *
> line third
> * line */
> 
> Any clue how things could go so wrong?
This was a bug in tests: should not use test::messUp() with multi-line comments.


https://reviews.llvm.org/D33589



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 100698.
Typz marked an inline comment as done.
Typz added a comment.

fix code & tests


https://reviews.llvm.org/D33589

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8542,6 +8542,45 @@
   EXPECT_EQ("#pragma option -C -A", format("#pragmaoption   -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));
+
+  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\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/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -590,7 +590,8 @@
   (Indenter->canBreak(State) && State.NextToken->NewlinesBefore > 0);
   unsigned Penalty = 0;
   formatChildren(State, Newline, /*DryRun=*/false, Penalty);
-  Indenter->addTokenToState(State, Newline, /*DryRun=*/false);
+  Indenter->addTokenToState(State, Newline, /*BreakToken=*/true,
+/*DryRun=*/false);
 }
 return 0;
   }
@@ -611,7 +612,8 @@
 LineState State = Indenter->getInitialState(FirstIndent, , DryRun);
 while (State.NextToken) {
   formatChildren(State, /*Newline=*/false, DryRun, Penalty);
-  Indenter->addTokenToState(State, /*Newline=*/false, DryRun);
+  Indenter->addTokenToState(State, /*Newline=*/false, /*BreakToken=*/true,
+DryRun);
 }
 return Penalty;
   }
@@ -658,10 +660,11 @@
   /// \brief An edge in the solution space from \c Previous->State to \c State,
   /// inserting a newline dependent on the \c NewLine.
   struct StateNode {
-StateNode(const LineState , bool NewLine, StateNode *Previous)
-: State(State), NewLine(NewLine), Previous(Previous) {}
+StateNode(const LineState , bool NewLine, bool BreakToken, StateNode *Previous)
+: State(State), NewLine(NewLine), BreakToken(BreakToken), Previous(Previous) {}
 LineState State;
 bool NewLine;
+bool BreakToken;
 StateNode *Previous;
   };
 
@@ -691,7 +694,7 @@
 
 // Insert start element into queue.
 StateNode *Node =
-new (Allocator.Allocate()) StateNode(InitialState, false, nullptr);
+new (Allocator.Allocate()) StateNode(InitialState, false, true, nullptr);
 Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
 ++Count;
 
@@ -718,9 +721,17 @@
 
   FormatDecision LastFormat = Node->State.NextToken->Decision;
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/false, , );
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,
+/*BreakToken=*/true, , );
   if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/true, , );
+addNextStateToQueue(Penalty, Node, /*NewLine=*/true,
+/*BreakToken=*/true, , );
+  if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false,
+/*BreakToken=*/false, , );
+  if (LastFormat == 

[PATCH] D32480: clang-format: Add CompactNamespaces option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

That will be slightly more complicated to check, esp. we will need to keep 
track of the matching-closing-brace (currently only the matching-opening-brace) 
is stored.
But I can try to update the patch in that direction, if that is the consensus.


https://reviews.llvm.org/D32480



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34238: clang-format: Do not binpack initialization lists

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Herald added a subscriber: klimek.

This patch tries to avoid binpacking when initializing lists/arrays,
to allow things like:

  static int types[] = {
  registerType1(),
  registerType2(),
  registerType3(),
  };
  std::map x = {
  { 0, "foo fjakfjaklf kljj" },
  { 1, "bar fjakfjaklf kljj" },
  { 2, "stuff fjakfjaklf kljj" },
  };

This is similar to how dictionnaries are formatted, and actually
corresponds to the same conditions: when initializing a container (and
not just 'calling' a constructor).

Such formatting involves 2 things:

- Line breaks around the content of the block. This can be forced by

adding a comma or comment after the last element

- Elements should not be binpacked

This patch considers the block is an initializer list if it either
ends with a comma, or follows an assignment, which seems to provide a
sensible approximation.


https://reviews.llvm.org/D34238

Files:
  lib/Format/ContinuationIndenter.cpp


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -999,9 +999,13 @@
 bool EndsInComma = Current.MatchingParen &&
Current.MatchingParen->Previous &&
Current.MatchingParen->Previous->is(tok::comma);
+const FormatToken *PreviousNoComment = Current.getPreviousNonComment();
+bool IsAfterAssignment = PreviousNoComment &&
+ PreviousNoComment->getPrecedence() ==
+ prec::Assignment;
 AvoidBinPacking =
-(Current.is(TT_ArrayInitializerLSquare) && EndsInComma) ||
-Current.is(TT_DictLiteral) ||
+(/*Current.is(TT_ArrayInitializerLSquare) && */EndsInComma) ||
+IsAfterAssignment || Current.is(TT_DictLiteral) ||
 Style.Language == FormatStyle::LK_Proto || !Style.BinPackArguments ||
 (NextNoComment && NextNoComment->is(TT_DesignatedInitializerPeriod));
 if (Current.ParameterCount > 1)


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -999,9 +999,13 @@
 bool EndsInComma = Current.MatchingParen &&
Current.MatchingParen->Previous &&
Current.MatchingParen->Previous->is(tok::comma);
+const FormatToken *PreviousNoComment = Current.getPreviousNonComment();
+bool IsAfterAssignment = PreviousNoComment &&
+ PreviousNoComment->getPrecedence() ==
+ prec::Assignment;
 AvoidBinPacking =
-(Current.is(TT_ArrayInitializerLSquare) && EndsInComma) ||
-Current.is(TT_DictLiteral) ||
+(/*Current.is(TT_ArrayInitializerLSquare) && */EndsInComma) ||
+IsAfterAssignment || Current.is(TT_DictLiteral) ||
 Style.Language == FormatStyle::LK_Proto || !Style.BinPackArguments ||
 (NextNoComment && NextNoComment->is(TT_DesignatedInitializerPeriod));
 if (Current.ParameterCount > 1)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34238: clang-format: Do not binpack initialization lists

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

This patch is probably not complete, though it works fine in all situations I 
could think of: nested initializers, "short" statement (properly merged), 
column layout is still performed when needed...

  static int types[] = {
  SourcePrivate::registerTypes(),
  registerEnum(),
  registerEnum(),
  };
  static int types[] = {registerTypes(),
registerEnum(),
registerEnum()};
  static int types[]{
  SourcePrivate::registerTypes(),
  registerEnum(),
  registerEnum(),
  };
  static int types[]{SourcePrivate::registerTypes(),
 registerEnum(),
 registerEnum()};
  static int types[]{0, 1, 2};
  static int types[] = {0, 1, 2};
  static int types[] = {aaa,  bb,   ,
, eee,  fff,
gg,   , ii};
  static int types[] = {
  aaa, bb, , , eee,
  fff, gg, , ii,
  };
  std::map x = {
  {0, "foo fjakfjaklf kljj"},
  {1, "bar fjakfjaklf kljj"},
  {2, "stuff fjakfjaklf kljj"},
  };

I have seen 2 problems so far, but they do not seem to be related:

- indent is performed using `continuationIndentWidth`, while I think 
`IndentWidth` may be more appropriate
- When force-wrapping due to comma, all the content may be merged afterwards, 
leading to some strange layout. In this case, I think it should either not 
merge the content at all, or merge the braces as well: static int types[] = { 
0, 1, 2 };

but I would need some feedback:

- Would this be an interesting improvement?
- Should this come with an option, or should this be the new behavior?
- Any case in particular you think will break?
- Technically, is this the right approach, or should I introduce a new 
TokenType and try to determine it in TokenAnnotator? This may help factorize 
some code from TokenAnnotator (which adds the forced line breaks) and 
ContinuationIndenter (which prevents BinPacking) and possibly 
UnwrappedLineFormatter (to fix the issue where the items get merged but not the 
braces)




Comment at: lib/Format/ContinuationIndenter.cpp:1007
 AvoidBinPacking =
-(Current.is(TT_ArrayInitializerLSquare) && EndsInComma) ||
-Current.is(TT_DictLiteral) ||
+(/*Current.is(TT_ArrayInitializerLSquare) && */EndsInComma) ||
+IsAfterAssignment || Current.is(TT_DictLiteral) ||

I don't really understand what this "TT_ArrayInitializerLSquare" case is...


https://reviews.llvm.org/D34238



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits