Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-09-16 Thread strager via cfe-commits 
strager marked 4 inline comments as done.


Comment at: lib/Format/UnwrappedLineParser.cpp:1057-1058
@@ +1056,4 @@
+  while (!eof()) {
+// FIXME: Once we have an expression parser in the UnwrappedLineParser,
+// replace this by using parseAssigmentExpression() inside.
+if (FormatTok->is(tok::l_paren)) {

strager wrote:
> djasper wrote:
> > I very much doubt that we'll have an Expression parser here anytime soon. 
> > So, I don't think that this FIXME makes much sense. Instead, please provide 
> > a comment on what this is actually doing and in which cases it might fail.
> I copied the comment from elsewhere in the file.
I expanded the comment, including a reference to the other reference to 
`parseAssigmentExpression`.


Comment at: lib/Format/UnwrappedLineParser.cpp:1061
@@ +1060,3 @@
+  parseParens();
+} else if (FormatTok->isOneOf(tok::comma, tok::r_square)) {
+  break;

strager wrote:
> djasper wrote:
> > I think this list should be extended to figure out certain cases where we 
> > know something is fishy. In particular:
> > * If you find an l_square or less, call into parseSquare and parseAngle 
> > respectively.
> > * If you find an r_brace or semi, something is wrong, break.
> > 
> Will do.
Handling r_brace and semi is a bit weird, since we end up aborting mid-stream 
and what's left becomes unparsable/incomplete by clang-format.

parseAngle doesn't exist, and even if it did, the less-than operator wouldn't 
be handled properly.

I added l_square and l_brace support.


http://reviews.llvm.org/D11693



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

Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-09-16 Thread strager via cfe-commits 
strager updated this revision to Diff 34946.
strager marked 2 inline comments as done.
strager added a comment.

Address @djasper's comments.


http://reviews.llvm.org/D11693

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -10057,6 +10057,18 @@
   // More complex introducers.
   verifyFormat("return [i, args...] {};");
 
+  // Lambdas with generalized captures.
+  verifyFormat("auto f = [b = d]() {};\n");
+  verifyFormat("auto f = [b = std::move(d)]() {};\n");
+  verifyFormat("auto f = [b = c, d = e, g]() {};\n");
+  verifyFormat("auto f = [b = a[3]]() {};\n");
+  verifyFormat("auto f = [InRange = a < b && b < c]() {};\n");
+  verifyFormat("auto f = [b = std::vector{1, 2, 3}]() {};\n");
+  verifyFormat("auto f = [b = std::vector{\n"
+   "  1, 2, 3,\n"
+   "}]() {};\n");
+  verifyFormat("return [b = std::vector()] {}();\n");
+
   // Not lambdas.
   verifyFormat("constexpr char hello[]{\"hello\"};");
   verifyFormat("double &operator[](int i) { return 0; }\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1054,6 +1054,27 @@
 nextToken();
 if (FormatTok->is(tok::ellipsis))
   nextToken();
+if (FormatTok->is(tok::equal)) {
+  // Generalized lambda capture.
+  nextToken();
+  while (!eof()) {
+// FIXME: Once we have an expression parser in the UnwrappedLineParser,
+// replace this by using parseAssigmentExpression() inside. See also
+// parseBracedList. For now, parsing matching braces ([], (), {}) is
+// good enough.
+if (FormatTok->is(tok::l_paren)) {
+  parseParens();
+} else if (FormatTok->is(tok::l_square)) {
+  parseSquare();
+} else if (FormatTok->is(tok::l_brace)) {
+  parseBracedList(false);
+} else if (FormatTok->isOneOf(tok::comma, tok::r_square)) {
+  break;
+} else {
+  nextToken();
+}
+  }
+}
 if (FormatTok->is(tok::comma)) {
   nextToken();
 } else if (FormatTok->is(tok::r_square)) {
@@ -1110,7 +1131,8 @@
   nextToken();
 
   // FIXME: Once we have an expression parser in the UnwrappedLineParser,
-  // replace this by using parseAssigmentExpression() inside.
+  // replace this by using parseAssigmentExpression() inside. See also
+  // tryToParseLambdaIntroducer.
   do {
 if (Style.Language == FormatStyle::LK_JavaScript) {
   if (FormatTok->is(Keywords.kw_function)) {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -10057,6 +10057,18 @@
   // More complex introducers.
   verifyFormat("return [i, args...] {};");
 
+  // Lambdas with generalized captures.
+  verifyFormat("auto f = [b = d]() {};\n");
+  verifyFormat("auto f = [b = std::move(d)]() {};\n");
+  verifyFormat("auto f = [b = c, d = e, g]() {};\n");
+  verifyFormat("auto f = [b = a[3]]() {};\n");
+  verifyFormat("auto f = [InRange = a < b && b < c]() {};\n");
+  verifyFormat("auto f = [b = std::vector{1, 2, 3}]() {};\n");
+  verifyFormat("auto f = [b = std::vector{\n"
+   "  1, 2, 3,\n"
+   "}]() {};\n");
+  verifyFormat("return [b = std::vector()] {}();\n");
+
   // Not lambdas.
   verifyFormat("constexpr char hello[]{\"hello\"};");
   verifyFormat("double &operator[](int i) { return 0; }\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1054,6 +1054,27 @@
 nextToken();
 if (FormatTok->is(tok::ellipsis))
   nextToken();
+if (FormatTok->is(tok::equal)) {
+  // Generalized lambda capture.
+  nextToken();
+  while (!eof()) {
+// FIXME: Once we have an expression parser in the UnwrappedLineParser,
+// replace this by using parseAssigmentExpression() inside. See also
+// parseBracedList. For now, parsing matching braces ([], (), {}) is
+// good enough.
+if (FormatTok->is(tok::l_paren)) {
+  parseParens();
+} else if (FormatTok->is(tok::l_square)) {
+  parseSquare();
+} else if (FormatTok->is(tok::l_brace)) {
+  parseBracedList(false);
+} else if (FormatTok->isOneOf(tok::comma, tok::r_square)) {
+  break;
+} else {
+  nextToken();
+}
+  }
+}
 if (FormatTok->is(tok::comma)) {
   nextToken();
 } else if (FormatTok->is(tok::r_square)) {
@@ -1110,7 +1131,8 @@
   nextToken();
 
   // FIXME: Once we have an expressi

Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.

2015-09-16 Thread strager via cfe-commits 
strager updated this revision to Diff 34947.
strager added a comment.

Rebase.


http://reviews.llvm.org/D10370

Files:
  docs/ClangFormatStyleOptions.rst
  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
@@ -4658,6 +4658,44 @@
"  \"c\";");
 }
 
+TEST_F(FormatTest, DeclarationReturnTypeBreakingStyle) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_TopLevel;
+  verifyFormat("class C {\n"
+   "  int f();\n"
+   "};\n"
+   "int\n"
+   "f();",
+   Style);
+  Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+  verifyFormat("class C {\n"
+   "  int\n"
+   "  f();\n"
+   "};\n"
+   "int\n"
+   "f();",
+   Style);
+  verifyFormat("const char *f(void) { return \"\"; }\n"
+   "const char *\n"
+   "bar(void);\n",
+   Style);
+  verifyFormat("template  T *f(T &c) { return NULL; }\n"
+   "template \n"
+   "T *\n"
+   "f(T &c);\n",
+   Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Stroustrup;
+  verifyFormat("const char *f(void) { return \"\"; }\n"
+   "const char *\n"
+   "bar(void);\n",
+   Style);
+  verifyFormat("template  T *f(T &c) { return NULL; }\n"
+   "template \n"
+   "T *\n"
+   "f(T &c);\n",
+   Style);
+}
+
 TEST_F(FormatTest, DefinitionReturnTypeBreakingStyle) {
   FormatStyle Style = getLLVMStyle();
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_TopLevel;
@@ -4738,6 +4776,46 @@
Style);
 }
 
+TEST_F(FormatTest, AlwaysBreakAfterDeclarationAndDefinitionReturnTypeMixed) {
+  FormatStyle AfterType = getLLVMStyle();
+  AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_None;
+  AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+  verifyFormat("void f(void) {\n" // No break here.
+   "  f();\n"
+   "  f();\n"
+   "}\n"
+   "void bar(void);\n", // No break here.
+   AfterType);
+  AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+  AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+  verifyFormat("void f(void) {\n" // No break here.
+   "  f();\n"
+   "  f();\n"
+   "}\n"
+   "void\n"
+   "bar(void);\n", // Break here.
+   AfterType);
+  AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_None;
+  AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+  verifyFormat("void\n"
+   "f(void) {\n" // Break here.
+   "  f();\n"
+   "  f();\n"
+   "}\n"
+   "void bar(void);\n", // No break here.
+   AfterType);
+  AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+  AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+  verifyFormat("void\n"
+   "f(void) {\n" // Break here.
+   "  f();\n"
+   "  f();\n"
+   "}\n"
+   "void\n"
+   "bar(void);\n", // Break here.
+   AfterType);
+}
+
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
   FormatStyle NoBreak = getLLVMStyle();
   NoBreak.AlwaysBreakBeforeMultilineStrings = false;
@@ -9409,6 +9487,15 @@
   CHECK_PARSE("BreakBeforeBraces: GNU", BreakBeforeBraces, FormatStyle::BS_GNU);
   CHECK_PARSE("BreakBeforeBraces: WebKit", BreakBeforeBraces, FormatStyle::BS_WebKit);
 
+  Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+  CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: None",
+  AlwaysBreakAfterDeclarationReturnType, FormatStyle::DRTBS_None);
+  CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: All",
+  AlwaysBreakAfterDeclarationReturnType, FormatStyle::DRTBS_All);
+  CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: TopLevel",
+  AlwaysBreakAfterDeclarationReturnType,
+  FormatStyle::DRTBS_TopLevel);
+
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
   CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: None",
   AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_None);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1593,15 +1593,18 @@

Re: [PATCH] D10371: clang-format: Support @synchronized.

2015-09-16 Thread strager via cfe-commits 
strager updated this revision to Diff 34957.
strager added a comment.

Rebase. Fix style issues.


http://reviews.llvm.org/D10371

Files:
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2419,6 +2419,30 @@
Style);
 }
 
+TEST_F(FormatTest, FormatObjCSynchronized) {
+  FormatStyle Style = getLLVMStyleWithColumns(32);
+  verifyFormat("@synchronized(foo) {\n"
+   "  f();\n"
+   "}\n",
+   Style);
+  verifyFormat("@synchronized([self\n"
+   "veryLongMethodNameWithParameters:\n"
+   "YES]) {\n"
+   "  f();\n"
+   "}\n",
+   Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  verifyFormat("@synchronized(foo)\n"
+   "{\n"
+   "  f();\n"
+   "}\n"
+   "@synchronized(foo)\n"
+   "{\n"
+   "  f();\n"
+   "}\n",
+   Style);
+}
+
 TEST_F(FormatTest, StaticInitializers) {
   verifyFormat("static SomeClass SC = {1, 'a'};");
 
@@ -8373,6 +8397,10 @@
"  break;\n"
"}",
NoSpace);
+  verifyFormat("@synchronized(x) {\n"
+   "  do_something();\n"
+   "}",
+   NoSpace);
   verifyFormat("auto i = std::make_unique(5);", NoSpace);
   verifyFormat("size_t x = sizeof(x);", NoSpace);
   verifyFormat("auto f(int x) -> decltype(x);", NoSpace);
@@ -8408,6 +8436,10 @@
"  break;\n"
"}",
Space);
+  verifyFormat("@synchronized (x) {\n"
+   "  do_something ();\n"
+   "}",
+   Space);
   verifyFormat("A::A () : a (1) {}", Space);
   verifyFormat("void f () __attribute__ ((asdf));", Space);
   verifyFormat("*(&a + 1);\n"
@@ -8460,6 +8492,10 @@
"  break;\n"
"}",
Spaces);
+  verifyFormat("@synchronized( x ) {\n"
+   "  do_something();\n"
+   "}",
+   Spaces);
 
   Spaces.SpacesInParentheses = false;
   Spaces.SpacesInCStyleCastParentheses = true;
@@ -8497,6 +8533,10 @@
"  break;\n"
"}",
Spaces);
+  verifyFormat("@synchronized(x) {\n"
+   "  do_something( );\n"
+   "}",
+   Spaces);
 
   // Run the first set of tests again with:
   Spaces.SpaceAfterCStyleCast = true;
@@ -8523,6 +8563,10 @@
"  break;\n"
"}",
Spaces);
+  verifyFormat("@synchronized(x) {\n"
+   "  do_something( );\n"
+   "}",
+   Spaces);
 
   // Run subset of tests again with:
   Spaces.SpacesInCStyleCastParentheses = false;
@@ -8534,6 +8578,10 @@
"  do_something((int) i);\n"
"} while (something( ));",
Spaces);
+  verifyFormat("@synchronized((NSLock) x) {\n"
+   "  do_something( );\n"
+   "}",
+   Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInSquareBrackets) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -679,8 +679,12 @@
   addUnwrappedLine();
   return;
 case tok::objc_autoreleasepool:
+case tok::objc_synchronized:
   nextToken();
-  if (FormatTok->Tok.is(tok::l_brace)) {
+  if (FormatTok->is(tok::l_paren)) {
+parseParens();
+  }
+  if (FormatTok->is(tok::l_brace)) {
 if (Style.BreakBeforeBraces == FormatStyle::BS_Allman ||
 Style.BreakBeforeBraces == FormatStyle::BS_GNU)
   addUnwrappedLine();
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -122,7 +122,7 @@
 if (Left->Previous &&
 (Left->Previous->isOneOf(tok::kw_static_assert, tok::kw_decltype,
  tok::kw_if, tok::kw_while, tok::l_paren,
- tok::comma) ||
+ tok::comma, Keywords.kw_synchronized) ||
  Left->Previous->is(TT_BinaryOperator))) {
   // static_assert, if and while usually contain expressions.
   Contexts.back().IsExpression = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D12921: clang-format: Support 'template<>' (no space).

2015-09-16 Thread strager via cfe-commits 
strager created this revision.
strager added a reviewer: djasper.
strager added subscribers: cfe-commits, abdulras, sas.
Herald added a subscriber: klimek.

Some styles don't put a space between 'template' and the
opening '<'. Introduce SpaceAfterTemplateKeyword which, when
set to false, causes 'template' and '<' to not have a space
between.

http://reviews.llvm.org/D12921

Files:
  docs/ClangFormatStyleOptions.rst
  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
@@ -5728,6 +5728,17 @@
   verifyFormat("template  void Foo(Ts*... ts) {}", PointersLeft);
 }
 
+TEST_F(FormatTest, SpaceAfterTemplate) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceAfterTemplateKeyword = false;
+  verifyFormat("template<> class Foo {}", Style);
+  verifyFormat("template<> void Foo() {}", Style);
+  verifyFormat("template class Foo {}", Style);
+  verifyFormat("template void Foo() {}", Style);
+  verifyFormat("template class Foo {}", Style);
+  verifyFormat("template void Foo() {}", Style);
+}
+
 TEST_F(FormatTest, AdaptivelyFormatsPointersAndReferences) {
   EXPECT_EQ("int *a;\n"
 "int *a;\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1827,7 +1827,7 @@
   if (Right.isOneOf(tok::semi, tok::comma))
 return false;
   if (Right.is(tok::less) &&
-  (Left.is(tok::kw_template) ||
+  ((Left.is(tok::kw_template) && Style.SpaceAfterTemplateKeyword) ||
(Line.Type == LT_ObjCDecl && Style.ObjCSpaceBeforeProtocolList)))
 return true;
   if (Left.isOneOf(tok::exclaim, tok::tilde))
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -268,6 +268,7 @@
Style.PenaltyReturnTypeOnItsOwnLine);
 IO.mapOptional("PointerAlignment", Style.PointerAlignment);
 IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
+IO.mapOptional("SpaceAfterTemplateKeyword", 
Style.SpaceAfterTemplateKeyword);
 IO.mapOptional("SpaceBeforeAssignmentOperators",
Style.SpaceBeforeAssignmentOperators);
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
@@ -398,6 +399,7 @@
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
   LLVMStyle.SpaceAfterCStyleCast = false;
+  LLVMStyle.SpaceAfterTemplateKeyword = true;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpacesInAngles = false;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -367,6 +367,10 @@
   /// \brief If \c true, a space may be inserted after C style casts.
   bool SpaceAfterCStyleCast;
 
+  /// \brief If \c true, a space may be inserted between the 'template' keyword
+  /// and the following '<'.
+  bool SpaceAfterTemplateKeyword;
+
   /// \brief If \c false, spaces will be removed before assignment operators.
   bool SpaceBeforeAssignmentOperators;
 
@@ -512,6 +516,7 @@
PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
PointerAlignment == R.PointerAlignment &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
+   SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators 
&&
SpaceBeforeParens == R.SpaceBeforeParens &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -481,6 +481,10 @@
 **SpaceAfterCStyleCast** (``bool``)
   If ``true``, a space may be inserted after C style casts.
 
+**SpaceAfterTemplateKeyword** (``bool``)
+  If ``true``, a space may be inserted between the 'template' keyword
+  and the following '<'.
+
 **SpaceBeforeAssignmentOperators** (``bool``)
   If ``false``, spaces will be removed before assignment operators.
 


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5728,6 +5728,17 @@
   verifyFormat("template  void Foo(Ts*... ts) {}", PointersLeft);
 }
 
+TEST_F(FormatTest, SpaceAfterTemplate) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceAfterTemplateKeyword = false;
+  verifyFormat("template<> class Foo {}", Style);
+  verifyFormat("template<> v

Re: [PATCH] D12921: clang-format: Support 'template<>' (no space).

2015-09-17 Thread strager via cfe-commits 
strager added a comment.

Should we remove `ObjCSpaceBeforeProtocolList`? It has the same problem as the 
`SpaceAfterTemplateKeyword` I am introducing.


http://reviews.llvm.org/D12921



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

Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-09-17 Thread strager via cfe-commits 
strager marked 2 inline comments as done.


Comment at: lib/Format/UnwrappedLineParser.cpp:1060-1061
@@ +1059,4 @@
+  nextToken();
+  while (!eof()) {
+// FIXME: Once we have an expression parser in the UnwrappedLineParser,
+// replace this by using parseAssigmentExpression() inside. See also

djasper wrote:
> Again, please remove the FIXME. We aren't going to have an expression parser 
> here (anytime soon) and shouldn't add (more) comments that make people think 
> otherwise.
> We aren't going to have an expression parser here (anytime soon) and 
> shouldn't add (more) comments that make people think otherwise.

If there is enough need for the function, perhaps it will be written.

I don't think the comment implies some code will be written soon.


Comment at: lib/Format/UnwrappedLineParser.cpp:1064
@@ +1063,3 @@
+// parseBracedList. For now, parsing matching braces ([], (), {}) is
+// good enough.
+if (FormatTok->is(tok::l_paren)) {

djasper wrote:
> Ah, parseAngle doesn't exist here. I was thinking about the TokenAnnotator.
> 
> I don't understand your comment about mid-stream. This is precisely about the 
> case where the input is corrupt so that clang-format can recover and doesn't 
> just parse the reset of the file input the lambda introducer.
> This is precisely about the case where the input is corrupt so that 
> clang-format can recover and doesn't just parse the reset of the file input 
> the lambda introducer.

If I write this test:

```
verifyFormat("return [}] {};\n");
```

I get this output:

```
/Users/strager/Projects/llvm/tools/clang/unittests/Format/FormatTest.cpp:42: 
Failure
Value of: IncompleteFormat
  Actual: true
Expected: ExpectedIncompleteFormat
Which is: false
return [}] {};



/Users/strager/Projects/llvm/tools/clang/unittests/Format/FormatTest.cpp:65: 
Failure
Value of: format(test::messUp(Code), Style)
  Actual: "return [\n}] {};\n"
Expected: Code.str()
Which is: "return [}] {};\n"
```

How can I fix this?


http://reviews.llvm.org/D11693



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

Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.

2015-09-17 Thread strager via cfe-commits 
strager added inline comments.


Comment at: docs/ClangFormatStyleOptions.rst:221-235
@@ -220,3 +220,17 @@
 
-**AlwaysBreakAfterDefinitionReturnType** 
(``DefinitionReturnTypeBreakingStyle``)
+**AlwaysBreakAfterDeclarationReturnType** (``ReturnTypeBreakingStyle``)
+  The function declaration return type breaking style to use.
+
+  Possible values:
+
+  * ``DRTBS_None`` (in configuration: ``None``)
+Break after return type automatically.
+``PenaltyReturnTypeOnItsOwnLine`` is taken into account.
+  * ``DRTBS_All`` (in configuration: ``All``)
+Always break after the return type.
+  * ``DRTBS_TopLevel`` (in configuration: ``TopLevel``)
+Always break after the return types of top level functions.
+
+
+**AlwaysBreakAfterDefinitionReturnType** (``ReturnTypeBreakingStyle``)
   The function definition return type breaking style to use.

djasper wrote:
> Same as I am arguing on some of your other patches. Fewer options are easier 
> to maintain and easier to discover.
I think having separate options for separate cases is easier to maintain.

Current method (separate option):

```
  FormatStyle::ReturnTypeBreakingStyle BreakStyle =
  Line.mightBeFunctionDefinition()
  ? Style.AlwaysBreakAfterDefinitionReturnType
  : Style.AlwaysBreakAfterDeclarationReturnType;
  if ((BreakStyle == FormatStyle::DRTBS_All ||
   (BreakStyle == FormatStyle::DRTBS_TopLevel && Line.Level == 0)))
Current->MustBreakBefore = true;
```

Proposed method:

```
  auto BreakStyle = Style.AlwaysBreakAfterReturnType;
  if (BreakStyle == FormatStyle::DRTBS_All ||
  (BreakStyle == FormatStyle::DRTBS_TopLevel && Line.Level == 0) ||
  (!Line->mightBeFunctionDefinition() &&
   (BreakStyle == FormatStyle::DRTBS_AllDeclarations) ||
   (BreakStyle == FormatStyle::DRTBS_TopLevelDeclarations &&
Line.Level == 0)))
Current->MustBreakBefore = true;
```


http://reviews.llvm.org/D10370



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

Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.

2015-09-17 Thread strager via cfe-commits 
strager updated this revision to Diff 35038.
strager added a comment.

Fix missing IsDefinition check in ContinuationIndenter::canBreak.


http://reviews.llvm.org/D10370

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4658,6 +4658,44 @@
"  \"c\";");
 }
 
+TEST_F(FormatTest, DeclarationReturnTypeBreakingStyle) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_TopLevel;
+  verifyFormat("class C {\n"
+   "  int f();\n"
+   "};\n"
+   "int\n"
+   "f();",
+   Style);
+  Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+  verifyFormat("class C {\n"
+   "  int\n"
+   "  f();\n"
+   "};\n"
+   "int\n"
+   "f();",
+   Style);
+  verifyFormat("const char *f(void) { return \"\"; }\n"
+   "const char *\n"
+   "bar(void);\n",
+   Style);
+  verifyFormat("template  T *f(T &c) { return NULL; }\n"
+   "template \n"
+   "T *\n"
+   "f(T &c);\n",
+   Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Stroustrup;
+  verifyFormat("const char *f(void) { return \"\"; }\n"
+   "const char *\n"
+   "bar(void);\n",
+   Style);
+  verifyFormat("template  T *f(T &c) { return NULL; }\n"
+   "template \n"
+   "T *\n"
+   "f(T &c);\n",
+   Style);
+}
+
 TEST_F(FormatTest, DefinitionReturnTypeBreakingStyle) {
   FormatStyle Style = getLLVMStyle();
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_TopLevel;
@@ -4738,6 +4776,46 @@
Style);
 }
 
+TEST_F(FormatTest, AlwaysBreakAfterDeclarationAndDefinitionReturnTypeMixed) {
+  FormatStyle AfterType = getLLVMStyle();
+  AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_None;
+  AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+  verifyFormat("void f(void) {\n" // No break here.
+   "  f();\n"
+   "  f();\n"
+   "}\n"
+   "void bar(void);\n", // No break here.
+   AfterType);
+  AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+  AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+  verifyFormat("void f(void) {\n" // No break here.
+   "  f();\n"
+   "  f();\n"
+   "}\n"
+   "void\n"
+   "bar(void);\n", // Break here.
+   AfterType);
+  AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_None;
+  AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+  verifyFormat("void\n"
+   "f(void) {\n" // Break here.
+   "  f();\n"
+   "  f();\n"
+   "}\n"
+   "void bar(void);\n", // No break here.
+   AfterType);
+  AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+  AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+  verifyFormat("void\n"
+   "f(void) {\n" // Break here.
+   "  f();\n"
+   "  f();\n"
+   "}\n"
+   "void\n"
+   "bar(void);\n", // Break here.
+   AfterType);
+}
+
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
   FormatStyle NoBreak = getLLVMStyle();
   NoBreak.AlwaysBreakBeforeMultilineStrings = false;
@@ -9409,6 +9487,15 @@
   CHECK_PARSE("BreakBeforeBraces: GNU", BreakBeforeBraces, FormatStyle::BS_GNU);
   CHECK_PARSE("BreakBeforeBraces: WebKit", BreakBeforeBraces, FormatStyle::BS_WebKit);
 
+  Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+  CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: None",
+  AlwaysBreakAfterDeclarationReturnType, FormatStyle::DRTBS_None);
+  CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: All",
+  AlwaysBreakAfterDeclarationReturnType, FormatStyle::DRTBS_All);
+  CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: TopLevel",
+  AlwaysBreakAfterDeclarationReturnType,
+  FormatStyle::DRTBS_TopLevel);
+
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
   CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: None",
   AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_None);
Index: lib/Format/TokenAnnotator.h
===
--- lib/For

Re: [PATCH] D10371: clang-format: Support @synchronized.

2015-08-11 Thread strager via cfe-commits 
strager added a comment.

ping


http://reviews.llvm.org/D10371



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

Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-08-11 Thread strager via cfe-commits 
strager planned changes to this revision.


Comment at: lib/Format/UnwrappedLineParser.cpp:1057-1058
@@ +1056,4 @@
+  while (!eof()) {
+// FIXME: Once we have an expression parser in the UnwrappedLineParser,
+// replace this by using parseAssigmentExpression() inside.
+if (FormatTok->is(tok::l_paren)) {

djasper wrote:
> I very much doubt that we'll have an Expression parser here anytime soon. So, 
> I don't think that this FIXME makes much sense. Instead, please provide a 
> comment on what this is actually doing and in which cases it might fail.
I copied the comment from elsewhere in the file.


Comment at: lib/Format/UnwrappedLineParser.cpp:1061
@@ +1060,3 @@
+  parseParens();
+} else if (FormatTok->isOneOf(tok::comma, tok::r_square)) {
+  break;

djasper wrote:
> I think this list should be extended to figure out certain cases where we 
> know something is fishy. In particular:
> * If you find an l_square or less, call into parseSquare and parseAngle 
> respectively.
> * If you find an r_brace or semi, something is wrong, break.
> 
Will do.


http://reviews.llvm.org/D11693



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

Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.

2015-08-07 Thread strager via cfe-commits 
strager added a comment.

(Sorry for the late feedback.)



Comment at: docs/ClangFormatStyleOptions.rst:221
@@ -220,2 +220,3 @@
 
-**AlwaysBreakAfterDefinitionReturnType** 
(``DefinitionReturnTypeBreakingStyle``)
+**AlwaysBreakAfterDeclarationReturnType** (``ReturnTypeBreakingStyle``)
+  The function declaration return type breaking style to use.

djasper wrote:
> Do you think it'll ever make sense to break differently for declarations and 
> definitions? I think having two entirely independent configuration flags 
> gives us 9 combinations (assuming the three enum values will remain) out of 
> which many will never be used.
> 
> I see two alternatives:
> Add an additional bool flag "TreatDeclarationsLikeDefinitions" (name might 
> not be ideal yet).
> Change existing flag name to AlwaysBreakAfterReturnType and use five enum 
> values (None, TopLevel, All, TopLevelDefinitions, AllDefinitions).
> 
> What do you think?
> 
> Sorry for being very picky on this. The problem is that these options stick 
> around for a long time and we need to think about them carefully.
> I think having two entirely independent configuration flags gives us 9 
> combinations (assuming the three enum values will remain) out of which many 
> will never be used.

I agree that many combinations will be unused. I don't see why we should 
conflate the two if they are configured independently in practice, though.

> Add an additional bool flag "TreatDeclarationsLikeDefinitions" (name might 
> not be ideal yet).
> Change existing flag name to AlwaysBreakAfterReturnType and use five enum 
> values (None, TopLevel, All, TopLevelDefinitions, AllDefinitions).

What about styles where declarations have return types on their own line, but 
definitions don't? I don't see a reason to prevent that style.


Comment at: lib/Format/ContinuationIndenter.cpp:129-132
@@ -128,5 +128,6 @@
   if (Current.is(TT_FunctionDeclarationName) &&
   Style.AlwaysBreakAfterDefinitionReturnType == FormatStyle::DRTBS_None &&
+  Style.AlwaysBreakAfterDeclarationReturnType == FormatStyle::DRTBS_None &&
   State.Column < 6)
 return false;
 

djasper wrote:
> What do you mean exactly?
See `IsDefinition` in `lib/Format/TokenAnnotator.cpp` introduced by this diff. 
The logic here and there should match, I think.


http://reviews.llvm.org/D10370



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

Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-08-07 Thread strager via cfe-commits 
strager added a comment.

ping


http://reviews.llvm.org/D11693



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