[PATCH] D6833: Clang-format: Braces Indent Style Whitesmith

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

I updated this patch to remove all of the code from `ContinuationIndenter` and 
to use the newer `BraceWrapping` style option instead of setting each one 
individually in `UnwrappedLineParser`.

I still have the same issue with enums. Looking at the `RootToken` in 
`UnwrappedLineFormatter::formatFirstToken` when parsing an enum, the first pass 
through it has the entire line for the enum in it with all of the newlines 
stripped from my test case. It then doesn't ever return the braces as separate 
lines. If someone could give me a pointer as to how/why enums are tokenized 
differently from everything else, I can likely fix it. The test case I'm using 
looks like:

  verifyFormat("enum X\n"
   "  {\n"
   "  Y = 0,\n"
   "  }\n",
   WhitesmithsBraceStyle);

I also have the issue with a `break` after a block inside of a case statement, 
as mentioned in https://reviews.llvm.org/D6833#107539. Other than those two, 
it's mostly working correctly. I removed the test for lambdas because I'm not 
entirely sure how that should even be formatted with Whitesmiths, but I can add 
it back in again.

Should I update this thread with the new diff against HEAD, or should I open a 
new one and close this one as dead?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D6833/new/

https://reviews.llvm.org/D6833



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

[PATCH] D67627: Clang-format: Add Whitesmiths indentation style

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch adds support for the Whitesmiths indentation style to clang-format. 
It’s an update to a patch submitted in 2015 (D6833 
), but reworks it to use the newer API.

There are still some issues with this patch, primarily around `switch` and 
`case` support. The added unit test won’t currently pass because of the 
remaining issues.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67627

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2401,6 +2401,16 @@
"  // something\n"
"}",
Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Whitesmiths;
+  verifyFormat("try\n"
+   "  {\n"
+   "  // something white\n"
+   "  }\n"
+   "catch (...)\n"
+   "  {\n"
+   "  // something white\n"
+   "  }",
+   Style);
   Style.BreakBeforeBraces = FormatStyle::BS_GNU;
   verifyFormat("try\n"
"  {\n"
@@ -4880,6 +4890,13 @@
"}",
Style);
 
+  Style.BreakBeforeBraces = FormatStyle::BS_Whitesmiths;
+  verifyFormat("void someLongFunction(\n"
+   "int someLongParameter) const\n"
+   "  {\n"
+   "  }",
+   Style);
+
   // Unless these are unknown annotations.
   verifyFormat("void SomeFunction(aa aaa,\n"
"  aa aaa)\n"
@@ -11347,6 +11364,238 @@
BreakBeforeBraceShortIfs);
 }
 
+TEST_F(FormatTest, WhitesmithsBraceBreaking) {
+  FormatStyle WhitesmithsBraceStyle = getLLVMStyle();
+  WhitesmithsBraceStyle.BreakBeforeBraces = FormatStyle::BS_Whitesmiths;
+
+  // Make a few changes to the style for testing purposes
+  WhitesmithsBraceStyle.AllowShortFunctionsOnASingleLine =
+  FormatStyle::SFS_Empty;
+  WhitesmithsBraceStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  WhitesmithsBraceStyle.ColumnLimit = 0;
+
+  verifyFormat("class A;\n"
+   "namespace B\n"
+   "  {\n"
+   "class C;\n"
+   "// Comment\n"
+   "class D\n"
+   "  {\n"
+   "public:\n"
+   "  D();\n"
+   "  ~D() {}\n"
+   "private:\n"
+   "  enum E\n"
+   "{\n"
+   "F\n"
+   "}\n"
+   "  };\n"
+   "  } // namespace B\n",
+   WhitesmithsBraceStyle);
+
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  verifyFormat("void f()\n"
+   "  {\n"
+   "  if (true)\n"
+   "{\n"
+   "a();\n"
+   "}\n"
+   "  else if (false)\n"
+   "{\n"
+   "b();\n"
+   "}\n"
+   "  else\n"
+   "{\n"
+   "c();\n"
+   "}\n"
+   "  }\n",
+   WhitesmithsBraceStyle);
+
+  verifyFormat("void f()\n"
+   "  {\n"
+   "  for (int i = 0; i < 10; ++i)\n"
+   "{\n"
+   "a();\n"
+   "}\n"
+   "  while (false)\n"
+   "{\n"
+   "b();\n"
+   "}\n"
+   "  do\n"
+   "{\n"
+   "c();\n"
+   "} while (false)\n"
+   "  }\n",
+   WhitesmithsBraceStyle);
+
+  verifyFormat("void f(int a)\n"
+   "  {\n"
+   "  switch (a)\n"
+   "{\n"
+   "case 2:\n"
+   "  {\n"
+   "  }\n"
+   "  break;\n"
+   "}\n"
+   "  }\n",
+   WhitesmithsBraceStyle);
+

[PATCH] D67627: Clang-format: Add Whitesmiths indentation style

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 220610.
timwoj added a comment.

Marked failing tests as FIXME


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67627/new/

https://reviews.llvm.org/D67627

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2401,6 +2401,16 @@
"  // something\n"
"}",
Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Whitesmiths;
+  verifyFormat("try\n"
+   "  {\n"
+   "  // something white\n"
+   "  }\n"
+   "catch (...)\n"
+   "  {\n"
+   "  // something white\n"
+   "  }",
+   Style);
   Style.BreakBeforeBraces = FormatStyle::BS_GNU;
   verifyFormat("try\n"
"  {\n"
@@ -4880,6 +4890,13 @@
"}",
Style);
 
+  Style.BreakBeforeBraces = FormatStyle::BS_Whitesmiths;
+  verifyFormat("void someLongFunction(\n"
+   "int someLongParameter) const\n"
+   "  {\n"
+   "  }",
+   Style);
+
   // Unless these are unknown annotations.
   verifyFormat("void SomeFunction(aa aaa,\n"
"  aa aaa)\n"
@@ -11347,6 +11364,251 @@
BreakBeforeBraceShortIfs);
 }
 
+TEST_F(FormatTest, WhitesmithsBraceBreaking) {
+  FormatStyle WhitesmithsBraceStyle = getLLVMStyle();
+  WhitesmithsBraceStyle.BreakBeforeBraces = FormatStyle::BS_Whitesmiths;
+
+  // Make a few changes to the style for testing purposes
+  WhitesmithsBraceStyle.AllowShortFunctionsOnASingleLine =
+  FormatStyle::SFS_Empty;
+  WhitesmithsBraceStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  WhitesmithsBraceStyle.ColumnLimit = 0;
+
+  // FIXME: this test case can't decide whether there should be a blank line
+  // after the ~D() line or not. It adds one if one doesn't exist in the test
+  // and it removes the line if one exists.
+  /*
+  verifyFormat("class A;\n"
+   "namespace B\n"
+   "  {\n"
+   "class C;\n"
+   "// Comment\n"
+   "class D\n"
+   "  {\n"
+   "public:\n"
+   "  D();\n"
+   "  ~D() {}\n"
+   "private:\n"
+   "  enum E\n"
+   "{\n"
+   "F\n"
+   "}\n"
+   "  };\n"
+   "  } // namespace B\n",
+   WhitesmithsBraceStyle);
+  */
+
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  verifyFormat("void f()\n"
+   "  {\n"
+   "  if (true)\n"
+   "{\n"
+   "a();\n"
+   "}\n"
+   "  else if (false)\n"
+   "{\n"
+   "b();\n"
+   "}\n"
+   "  else\n"
+   "{\n"
+   "c();\n"
+   "}\n"
+   "  }\n",
+   WhitesmithsBraceStyle);
+
+  verifyFormat("void f()\n"
+   "  {\n"
+   "  for (int i = 0; i < 10; ++i)\n"
+   "{\n"
+   "a();\n"
+   "}\n"
+   "  while (false)\n"
+   "{\n"
+   "b();\n"
+   "}\n"
+   "  do\n"
+   "{\n"
+   "c();\n"
+   "} while (false)\n"
+   "  }\n",
+   WhitesmithsBraceStyle);
+
+  // FIXME: the block and the break under case 2 in this test don't get indented correctly
+  /*
+  verifyFormat("void switchTest1(int a)\n"
+   "  {\n"
+   "  switch (a)\n"
+   "{\n"
+   "case 2:\n"
+   "  {\n"
+   "  }\n"
+   "  break;\n"
+   "}\n"
+   "  }\n",
+   Whitesmiths

[PATCH] D67627: Clang-format: Add Whitesmiths indentation style

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

I don't have push access for `master`. Can someone merge this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67627/new/

https://reviews.llvm.org/D67627



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

[PATCH] D68346: Clang-format: Add new option to add spaces around conditions

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This diff adds a new option SpacesAroundConditions that inserts spaces inside 
the braces for conditional statements. It's used by the Zeek project 
(https://github.com/zeek/zeek) as described in their style guide 
(https://github.com/zeek/zeek-docs/blob/master/devel/style_guide.rst#conditional-expressions)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68346

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12177,6 +12177,7 @@
   CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
   CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
   CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
+  CHECK_PARSE_BOOL(SpacesAroundConditions);
 
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterCaseLabel);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterClass);
@@ -14373,6 +14374,16 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+TEST_F(FormatTest, SpacesAroundConditions) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpacesAroundConditions = true;
+  verifyFormat("if ( !a )\n  return;", Spaces);
+  verifyFormat("if ( a )\n  return;", Spaces);
+  verifyFormat("while ( a )\n  return;", Spaces);
+  verifyFormat("while ( (a && b) )\n  return;", Spaces);
+  verifyFormat("do {\n} while ( 1 != 0 );", Spaces);
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2505,6 +2505,18 @@
 return Right.is(tok::hash);
   if (Left.is(tok::l_paren) && Right.is(tok::r_paren))
 return Style.SpaceInEmptyParentheses;
+  if (Style.SpacesAroundConditions) {
+const auto is_cond_kw = [&](const FormatToken *t) {
+  return t->isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
+tok::kw_switch, tok::kw_constexpr, TT_ForEachMacro);
+};
+if (Left.is(tok::l_paren) && Left.Previous && is_cond_kw(Left.Previous))
+  return true;
+if (Right.is(tok::r_paren) && Right.MatchingParen &&
+Right.MatchingParen->Previous &&
+is_cond_kw(Right.MatchingParen->Previous))
+  return true;
+  }
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
 return (Right.is(TT_CastRParen) ||
 (Left.MatchingParen && Left.MatchingParen->is(TT_CastRParen)))
@@ -2903,7 +2915,8 @@
 // The identifier might actually be a macro name such as ALWAYS_INLINE. If
 // this turns out to be too lenient, add analysis of the identifier itself.
 return Right.WhitespaceRange.getBegin() != Right.WhitespaceRange.getEnd();
-  if (Right.is(tok::coloncolon) && !Left.isOneOf(tok::l_brace, tok::comment))
+  if (Right.is(tok::coloncolon) &&
+  !Left.isOneOf(tok::l_brace, tok::comment, tok::l_paren))
 return (Left.is(TT_TemplateOpener) &&
 Style.Standard < FormatStyle::LS_Cpp11) ||
!(Left.isOneOf(tok::l_paren, tok::r_paren, tok::l_square,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -517,6 +517,7 @@
Style.SpacesInCStyleCastParentheses);
 IO.mapOptional("SpacesInParentheses", Style.SpacesInParentheses);
 IO.mapOptional("SpacesInSquareBrackets", Style.SpacesInSquareBrackets);
+IO.mapOptional("SpacesAroundConditions", Style.SpacesAroundConditions);
 IO.mapOptional("Standard", Style.Standard);
 IO.mapOptional("StatementMacros", Style.StatementMacros);
 IO.mapOptional("TabWidth", Style.TabWidth);
@@ -775,6 +776,7 @@
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpaceBeforeCpp11BracedList = false;
   LLVMStyle.SpacesInAngles = false;
+  LLVMStyle.SpacesAroundConditions = false;
 
   LLVMStyle.PenaltyBreakAssignment = prec::Assignment;
   LLVMStyle.PenaltyBreakComment = 300;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1945,6 +1945,10 @@
   /// \endcode
   bool SpacesInSquareBrackets;
 
+  /// \brief If ``true``, spaces will be inserted around if/for/while (and
+  /// similar) conditions.
+  bool SpacesAroundConditions;
+
   /// Supported language standards.
   enum LanguageStandard {
 /// Use C++03-compatible syntax.
@@ -2081,6 +2085,7 @@
SpacesInCStyleCastParentheses == R.SpacesInCStyleCastParentheses &&
  

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 224890.
timwoj added a comment.

Changed option name to SpacesInConditionalStatement, added 
isKeywordWithCondition method, added some extra tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68346/new/

https://reviews.llvm.org/D68346

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12165,6 +12165,7 @@
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
   CHECK_PARSE_BOOL(SpacesInAngles);
+  CHECK_PARSE_BOOL(SpacesInConditionalStatement);
   CHECK_PARSE_BOOL(SpaceInEmptyBlock);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
@@ -14373,6 +14374,23 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+TEST_F(FormatTest, SpacesInConditionalStatement) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpacesInConditionalStatement = true;
+  verifyFormat("for ( int i = 0; i; i++ )\n  continue;", Spaces);
+  verifyFormat("if ( !a )\n  return;", Spaces);
+  verifyFormat("if ( a )\n  return;", Spaces);
+  verifyFormat("if constexpr ( a )\n  return;", Spaces);
+  verifyFormat("switch ( a )\ncase 1:\n  return;", Spaces);
+  verifyFormat("while ( a )\n  return;", Spaces);
+  verifyFormat("while ( (a && b) )\n  return;", Spaces);
+  verifyFormat("do {\n} while ( 1 != 0 );", Spaces);
+
+  // Check that space on the left of "::" is inserted as expected at beginning
+  // of condition.
+  verifyFormat("while ( ::func() )\n  return;", Spaces);
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2489,6 +2489,13 @@
   Right.ParameterCount > 0);
 }
 
+/// Returns \c true if the token is followed by a boolean condition, \c false
+/// otherwise.
+static bool isKeywordWithCondition(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
+ tok::kw_constexpr);
+};
+
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
   const FormatToken &Left,
   const FormatToken &Right) {
@@ -2505,6 +2512,16 @@
 return Right.is(tok::hash);
   if (Left.is(tok::l_paren) && Right.is(tok::r_paren))
 return Style.SpaceInEmptyParentheses;
+  if (Style.SpacesInConditionalStatement) {
+if (Left.is(tok::l_paren) && Left.Previous &&
+isKeywordWithCondition(*Left.Previous) )
+  return true;
+if (Right.is(tok::r_paren) &&
+Right.MatchingParen &&
+Right.MatchingParen->Previous &&
+isKeywordWithCondition(*Right.MatchingParen->Previous))
+  return true;
+  }
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
 return (Right.is(TT_CastRParen) ||
 (Left.MatchingParen && Left.MatchingParen->is(TT_CastRParen)))
@@ -2903,7 +2920,8 @@
 // The identifier might actually be a macro name such as ALWAYS_INLINE. If
 // this turns out to be too lenient, add analysis of the identifier itself.
 return Right.WhitespaceRange.getBegin() != Right.WhitespaceRange.getEnd();
-  if (Right.is(tok::coloncolon) && !Left.isOneOf(tok::l_brace, tok::comment))
+  if (Right.is(tok::coloncolon) &&
+  !Left.isOneOf(tok::l_brace, tok::comment, tok::l_paren))
 return (Left.is(TT_TemplateOpener) &&
 Style.Standard < FormatStyle::LS_Cpp11) ||
!(Left.isOneOf(tok::l_paren, tok::r_paren, tok::l_square,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -511,6 +511,7 @@
 IO.mapOptional("SpacesBeforeTrailingComments",
Style.SpacesBeforeTrailingComments);
 IO.mapOptional("SpacesInAngles", Style.SpacesInAngles);
+IO.mapOptional("SpacesInConditionalStatement", Style.SpacesInConditionalStatement);
 IO.mapOptional("SpacesInContainerLiterals",
Style.SpacesInContainerLiterals);
 IO.mapOptional("SpacesInCStyleCastParentheses",
@@ -775,6 +776,7 @@
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpaceBeforeCpp11BracedList = false;
   LLVMStyle.SpacesInAngles = false;
+  LLVMStyle.SpacesInConditionalStatement = false;
 
   LLVMStyle.PenaltyBreakAssignment = prec::Assignment;
   LLVMStyle.PenaltyBreakComment = 300;
Index: clang/include/clang/Format/Format.h

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

In D68346#1708384 , @timwoj wrote:

> Changed option name to SpacesInConditionalStatement, added 
> isKeywordWithCondition method, added some extra tests


Sorry for the delay on getting back to this. I'm not the original author and I 
was waiting for changes from him, and then I slacked on getting them submitted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68346/new/

https://reviews.llvm.org/D68346



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

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 225345.
timwoj added a comment.

Fix ordering


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68346/new/

https://reviews.llvm.org/D68346

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12165,6 +12165,7 @@
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
   CHECK_PARSE_BOOL(SpacesInAngles);
+  CHECK_PARSE_BOOL(SpacesInConditionalStatement);
   CHECK_PARSE_BOOL(SpaceInEmptyBlock);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
@@ -14373,6 +14374,23 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+TEST_F(FormatTest, SpacesInConditionalStatement) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpacesInConditionalStatement = true;
+  verifyFormat("for ( int i = 0; i; i++ )\n  continue;", Spaces);
+  verifyFormat("if ( !a )\n  return;", Spaces);
+  verifyFormat("if ( a )\n  return;", Spaces);
+  verifyFormat("if constexpr ( a )\n  return;", Spaces);
+  verifyFormat("switch ( a )\ncase 1:\n  return;", Spaces);
+  verifyFormat("while ( a )\n  return;", Spaces);
+  verifyFormat("while ( (a && b) )\n  return;", Spaces);
+  verifyFormat("do {\n} while ( 1 != 0 );", Spaces);
+
+  // Check that space on the left of "::" is inserted as expected at beginning
+  // of condition.
+  verifyFormat("while ( ::func() )\n  return;", Spaces);
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2489,6 +2489,13 @@
   Right.ParameterCount > 0);
 }
 
+/// Returns \c true if the token is followed by a boolean condition, \c false
+/// otherwise.
+static bool isKeywordWithCondition(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
+ tok::kw_constexpr);
+};
+
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
   const FormatToken &Left,
   const FormatToken &Right) {
@@ -2505,6 +2512,16 @@
 return Right.is(tok::hash);
   if (Left.is(tok::l_paren) && Right.is(tok::r_paren))
 return Style.SpaceInEmptyParentheses;
+  if (Style.SpacesInConditionalStatement) {
+if (Left.is(tok::l_paren) && Left.Previous &&
+isKeywordWithCondition(*Left.Previous) )
+  return true;
+if (Right.is(tok::r_paren) &&
+Right.MatchingParen &&
+Right.MatchingParen->Previous &&
+isKeywordWithCondition(*Right.MatchingParen->Previous))
+  return true;
+  }
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
 return (Right.is(TT_CastRParen) ||
 (Left.MatchingParen && Left.MatchingParen->is(TT_CastRParen)))
@@ -2903,7 +2920,8 @@
 // The identifier might actually be a macro name such as ALWAYS_INLINE. If
 // this turns out to be too lenient, add analysis of the identifier itself.
 return Right.WhitespaceRange.getBegin() != Right.WhitespaceRange.getEnd();
-  if (Right.is(tok::coloncolon) && !Left.isOneOf(tok::l_brace, tok::comment))
+  if (Right.is(tok::coloncolon) &&
+  !Left.isOneOf(tok::l_brace, tok::comment, tok::l_paren))
 return (Left.is(TT_TemplateOpener) &&
 Style.Standard < FormatStyle::LS_Cpp11) ||
!(Left.isOneOf(tok::l_paren, tok::r_paren, tok::l_square,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -511,6 +511,7 @@
 IO.mapOptional("SpacesBeforeTrailingComments",
Style.SpacesBeforeTrailingComments);
 IO.mapOptional("SpacesInAngles", Style.SpacesInAngles);
+IO.mapOptional("SpacesInConditionalStatement", Style.SpacesInConditionalStatement);
 IO.mapOptional("SpacesInContainerLiterals",
Style.SpacesInContainerLiterals);
 IO.mapOptional("SpacesInCStyleCastParentheses",
@@ -775,6 +776,7 @@
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpaceBeforeCpp11BracedList = false;
   LLVMStyle.SpacesInAngles = false;
+  LLVMStyle.SpacesInConditionalStatement = false;
 
   LLVMStyle.PenaltyBreakAssignment = prec::Assignment;
   LLVMStyle.PenaltyBreakComment = 300;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include

[PATCH] D72793: [clang-format] Expand the SpacesAroundConditions option to include catch statements

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This diff expands the SpacesAroundConditions option added in D68346 
 to include adding spaces to catch statements.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72793

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14927,6 +14927,7 @@
   verifyFormat("while ( a )\n  return;", Spaces);
   verifyFormat("while ( (a && b) )\n  return;", Spaces);
   verifyFormat("do {\n} while ( 1 != 0 );", Spaces);
+  verifyFormat("try {\n} catch ( const std::exception & ) {\n}", Spaces);
   // Check that space on the left of "::" is inserted as expected at beginning
   // of condition.
   verifyFormat("while ( ::func() )\n  return;", Spaces);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2596,7 +2596,7 @@
 /// otherwise.
 static bool isKeywordWithCondition(const FormatToken &Tok) {
   return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
- tok::kw_constexpr);
+ tok::kw_constexpr, tok::kw_catch);
 }
 
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14927,6 +14927,7 @@
   verifyFormat("while ( a )\n  return;", Spaces);
   verifyFormat("while ( (a && b) )\n  return;", Spaces);
   verifyFormat("do {\n} while ( 1 != 0 );", Spaces);
+  verifyFormat("try {\n} catch ( const std::exception & ) {\n}", Spaces);
   // Check that space on the left of "::" is inserted as expected at beginning
   // of condition.
   verifyFormat("while ( ::func() )\n  return;", Spaces);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2596,7 +2596,7 @@
 /// otherwise.
 static bool isKeywordWithCondition(const FormatToken &Tok) {
   return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
- tok::kw_constexpr);
+ tok::kw_constexpr, tok::kw_catch);
 }
 
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 319170.
timwoj added a comment.

Restored lost single-nesting namespace test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13543,6 +13543,7 @@
WhitesmithsBraceStyle);
   */
 
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
   verifyFormat("namespace a\n"
"  {\n"
"class A\n"
@@ -13567,6 +13568,89 @@
"  } // namespace a",
WhitesmithsBraceStyle);
 
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "  class A\n"
+   "{\n"
+   "void f()\n"
+   "  {\n"
+   "  if (true)\n"
+   "{\n"
+   "a();\n"
+   "b();\n"
+   "}\n"
+   "  }\n"
+   "void g()\n"
+   "  {\n"
+   "  return;\n"
+   "  }\n"
+   "};\n"
+   "  struct B\n"
+   "{\n"
+   "int x;\n"
+   "};\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "  namespace b\n"
+   "{\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "} // namespace b\n"
+   "  }   // namespace a",
+   WhitesmithsBraceStyle);
+
   verifyFormat("void f()\n"
"  {\n"
"  if (true)\n"
@@ -13601,7 +13685,7 @@
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
+  WhitesmithsBraceStyle.IndentCaseLabels = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -13609,7 +13693,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13619,7 +13703,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -13627,9 +13711,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13642,17 +13726,17 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"d

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2967
+  // needs to be indented.
+  bool ClosesBlock =
+  Line->MatchingOpeningBlockLineIndex != UnwrappedLine::kInvalidIndex &&

curdeius wrote:
> Please move below to the place of use. Also, just naming it `ClosesBlock` is 
> a bit misleading as it is only for Whitesmiths style.
I agree with renaming it, but I'm not sure on moving it to where it's used. It 
relies on `Line->MatchingOpeningBlockLineIndex` which is reset shortly after 
this boolean value is calculated.



Comment at: clang/unittests/Format/FormatTest.cpp:13688
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
+  WhitesmithsBraceStyle.IndentCaseLabels = true;
   verifyFormat("void switchTest1(int a)\n"

curdeius wrote:
> Hmm, you don't test the same thing anymore...
This change was intentional. According to the clang-format documentation, 
`IndentCaseBlocks` effectively just changes the brace breaking for the block 
after a case label. Given that it always breaks for Whitesmiths, 
`IndentCaseBlocks` can be effectively ignored for that style. I changed it to 
test the `IndentCaseLabels` instead because that tests indentation for the 
whole block.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 319716.
timwoj added a comment.

- Make LineLevel an enum class
- Combine multiple duplicate checks into a named variable
- Minor formatting fixes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13543,6 +13543,7 @@
WhitesmithsBraceStyle);
   */
 
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
   verifyFormat("namespace a\n"
"  {\n"
"class A\n"
@@ -13567,6 +13568,89 @@
"  } // namespace a",
WhitesmithsBraceStyle);
 
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "  class A\n"
+   "{\n"
+   "void f()\n"
+   "  {\n"
+   "  if (true)\n"
+   "{\n"
+   "a();\n"
+   "b();\n"
+   "}\n"
+   "  }\n"
+   "void g()\n"
+   "  {\n"
+   "  return;\n"
+   "  }\n"
+   "};\n"
+   "  struct B\n"
+   "{\n"
+   "int x;\n"
+   "};\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "  namespace b\n"
+   "{\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "} // namespace b\n"
+   "  }   // namespace a",
+   WhitesmithsBraceStyle);
+
   verifyFormat("void f()\n"
"  {\n"
"  if (true)\n"
@@ -13601,7 +13685,7 @@
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
+  WhitesmithsBraceStyle.IndentCaseLabels = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -13609,7 +13693,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13619,7 +13703,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -13627,9 +13711,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13642,17 +13726,17 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj marked 8 inline comments as done.
timwoj added a comment.

No problem, sorry it took a bit of back-and-forth there. Is there any way this 
can sneak into 12.0?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

https://bugs.llvm.org/show_bug.cgi?id=48668 already exists as a bug report 
linked to this review. I'm not sure "the Zeek project would like to start using 
this" is a good enough reason to block an LLVM release though. I'm fairly 
certain there aren't a whole lot of people in the world using Whitesmiths. That 
said, there's already at least one clang-format bug in the release-blocker list.

I'll get the release notes updated shortly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 322477.
timwoj added a comment.

Added release note entry


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13543,6 +13543,7 @@
WhitesmithsBraceStyle);
   */
 
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
   verifyFormat("namespace a\n"
"  {\n"
"class A\n"
@@ -13567,6 +13568,89 @@
"  } // namespace a",
WhitesmithsBraceStyle);
 
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "  class A\n"
+   "{\n"
+   "void f()\n"
+   "  {\n"
+   "  if (true)\n"
+   "{\n"
+   "a();\n"
+   "b();\n"
+   "}\n"
+   "  }\n"
+   "void g()\n"
+   "  {\n"
+   "  return;\n"
+   "  }\n"
+   "};\n"
+   "  struct B\n"
+   "{\n"
+   "int x;\n"
+   "};\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "  namespace b\n"
+   "{\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "} // namespace b\n"
+   "  }   // namespace a",
+   WhitesmithsBraceStyle);
+
   verifyFormat("void f()\n"
"  {\n"
"  if (true)\n"
@@ -13601,7 +13685,7 @@
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
+  WhitesmithsBraceStyle.IndentCaseLabels = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -13609,7 +13693,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13619,7 +13703,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -13627,9 +13711,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13642,17 +13726,17 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
   

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 322689.
timwoj added a comment.

Rebased on main


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14721,6 +14721,7 @@
WhitesmithsBraceStyle);
   */
 
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
   verifyFormat("namespace a\n"
"  {\n"
"class A\n"
@@ -14745,6 +14746,89 @@
"  } // namespace a",
WhitesmithsBraceStyle);
 
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "  class A\n"
+   "{\n"
+   "void f()\n"
+   "  {\n"
+   "  if (true)\n"
+   "{\n"
+   "a();\n"
+   "b();\n"
+   "}\n"
+   "  }\n"
+   "void g()\n"
+   "  {\n"
+   "  return;\n"
+   "  }\n"
+   "};\n"
+   "  struct B\n"
+   "{\n"
+   "int x;\n"
+   "};\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "  namespace b\n"
+   "{\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "} // namespace b\n"
+   "  }   // namespace a",
+   WhitesmithsBraceStyle);
+
   verifyFormat("void f()\n"
"  {\n"
"  if (true)\n"
@@ -14779,7 +14863,7 @@
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
+  WhitesmithsBraceStyle.IndentCaseLabels = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -14787,7 +14871,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -14797,7 +14881,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -14805,9 +14889,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -14820,17 +14904,17 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"   

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

> A crash (https://bugs.llvm.org/show_bug.cgi?id=50663) in 12.0.1 is fixed by 
> your changes here when merged to the 12 branch, I'm not sure if this needs to 
> be backported to 12 I guess it might depend on if we think it's critical 
> enough or if there will more 12 release candidates before 13 @timwoj any 
> thoughts?

It should apply cleanly to 12.0 as far as I know, since if I remember right I 
had asked if it could be merged before 12.0 was released. I don't have a whole 
lot of free cycles at the moment to help with it if it needs a lot of changes 
though. My vote is yes to backport it, if it counts for anything.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

In D94500#2592448 , 
@HazardyKnusperkeks wrote:

> Do you need someone to push this?

Yes I do. I don't have committer access.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

In D94500#2594394 , 
@HazardyKnusperkeks wrote:

> In D94500#2593229 , @timwoj wrote:
>
>> In D94500#2592448 , 
>> @HazardyKnusperkeks wrote:
>>
>>> Do you need someone to push this?
>>
>> Yes I do. I don't have committer access.
>
> Please state the name and mail for the commit.

Tim Wojtulewicz
tim...@gmail.com


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 328308.
timwoj added a comment.

Rebase onto master again


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14767,6 +14767,7 @@
WhitesmithsBraceStyle);
   */
 
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
   verifyFormat("namespace a\n"
"  {\n"
"class A\n"
@@ -14791,6 +14792,89 @@
"  } // namespace a",
WhitesmithsBraceStyle);
 
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "  class A\n"
+   "{\n"
+   "void f()\n"
+   "  {\n"
+   "  if (true)\n"
+   "{\n"
+   "a();\n"
+   "b();\n"
+   "}\n"
+   "  }\n"
+   "void g()\n"
+   "  {\n"
+   "  return;\n"
+   "  }\n"
+   "};\n"
+   "  struct B\n"
+   "{\n"
+   "int x;\n"
+   "};\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "  namespace b\n"
+   "{\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "} // namespace b\n"
+   "  }   // namespace a",
+   WhitesmithsBraceStyle);
+
   verifyFormat("void f()\n"
"  {\n"
"  if (true)\n"
@@ -14825,7 +14909,7 @@
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
+  WhitesmithsBraceStyle.IndentCaseLabels = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -14833,7 +14917,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -14843,7 +14927,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -14851,9 +14935,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -14866,17 +14950,17 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
   

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

In D94500#2605515 , 
@HazardyKnusperkeks wrote:

> In D94500#2604754 , @timwoj wrote:
>
>> Rebase onto master again
>
> `master` or `main`? Please rebase on `main`. That is where the pushes land. 
> (And as far as I can see `master` hasn't been updated in quite some time.

Yes, `main`, sorry. It was late in my work day and I was fried already.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

Thanks for the consideration on committer access, but I'm going to have to pass 
for the time being.

That said, can someone land this patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68346/new/

https://reviews.llvm.org/D68346



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

[PATCH] D72793: [clang-format] Expand the SpacesAroundConditions option to include catch statements

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

I know that 10.0 was branch recently. Any chance this can make it over?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72793/new/

https://reviews.llvm.org/D72793



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

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj created this revision.
timwoj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This commit removes the old way of handling Whitesmiths mode in favor of just 
setting the
levels during parsing and letting the formatter handle it from there. It 
requires a bit of
special-casing during the parsing, but ends up a bit cleaner than before. It 
also removes
some of switch/case unit tests that don't really make much sense when dealing 
with
Whitesmiths.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94500

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13525,7 +13525,7 @@
   /*
   verifyFormat("class A;\n"
"namespace B\n"
-   "  {\n"
+   "{\n"
"class C;\n"
"// Comment\n"
"class D\n"
@@ -13539,12 +13539,12 @@
"F\n"
"}\n"
"  };\n"
-   "  } // namespace B\n",
+   "} // namespace B\n",
WhitesmithsBraceStyle);
   */
 
   verifyFormat("namespace a\n"
-   "  {\n"
+   "{\n"
"class A\n"
"  {\n"
"  void f()\n"
@@ -13564,7 +13564,7 @@
"  {\n"
"  int x;\n"
"  };\n"
-   "  } // namespace a",
+   "} // namespace a",
WhitesmithsBraceStyle);
 
   verifyFormat("void f()\n"
@@ -13597,11 +13597,11 @@
"  do\n"
"{\n"
"c();\n"
-   "} while (false)\n"
+   "} while (false);\n"
+   "  return;\n"
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -13609,7 +13609,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13619,7 +13619,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -13627,9 +13627,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13642,65 +13642,12 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
"  {\n"
"  foo(1);\n"
"  }\n"
-   "break;\n"
-   "}\n"
-   "  }\n",
-   WhitesmithsBraceStyle);
-
-  WhitesmithsBraceStyle.IndentCaseBlocks = false;
-
-  verifyFormat("void switchTest4(int a)\n"
-   "  {\n"
-   "  switch (a)\n"
-   "{\n"
-   "  case 2:\n"
-   "{\n"
-   "}\n"
-   "break;\n"
-   "}\n"
-   "  }\n",
-   WhitesmithsBraceStyle);
-
-  verifyFormat("void switchTest5(int a)\n"
-   "  {\n"
-   "  switch (a)\n"
-   "{\n"
-   "  case 0:\n"
-   "break;\n"
-   "  case 1:\n"
-   "{\n"
-   "foo();\n"
-   "break;\n"
-   "}\n"
-   "  case 2:\n"
-   "{\n"
-   "}\n"
-   "break;\n"
-   "  default:\n"
-   "break;\n"
-   "}\n"
-   "  }\n",
-   WhitesmithsBraceStyle);
-
-  verifyFormat("void switchTest6(int a)\n"
-   "  {\n"
-   "  switch (a)\n"
-   "{\n"
-   "  case 0:\n"
-   "{\n"
-   "foo(x);\n"
-   "}\n"
-   "break;\n"
-   "  default:\n"
-   "{\n"
-   "foo(1

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

This fixes https://bugs.llvm.org/show_bug.cgi?id=48569, amongst other things.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added inline comments.



Comment at: clang/lib/Format/TokenAnalyzer.cpp:71
Env.getFirstStartColumn(), Style, Encoding, Allocator,
-
IdentTable);

HazardyKnusperkeks wrote:
> Unrelated change (although I think it's good one).
Should I split that into a separate commit?



Comment at: clang/unittests/Format/FormatTest.cpp:13528
"namespace B\n"
-   "  {\n"
+   "{\n"
"class C;\n"

HazardyKnusperkeks wrote:
> So until now it has formatted that always wrong?
Hm, that's a good question. I'll have to think about that one. I guess for 
Whitesmiths and it's always wanting to indent braces, this should be fixed back 
to way that it was.



Comment at: clang/unittests/Format/FormatTest.cpp:13707
WhitesmithsBraceStyle);
 
   verifyFormat("enum X\n"

MyDeveloperDay wrote:
> any reason why this is being removed?
This is an artifact of there not being any sort of actual guide for 
Whitesmiths. I have it set now to always indent the case labels (and it should 
just ignore the IndentCaseLabels option entirely, like it does for 
IndentCaseBlocks). I can certainly fix that option to work again though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.h:143
   bool tryToParseSimpleAttribute();
-  void addUnwrappedLine();
+  void addUnwrappedLine(bool RemoveLevel = true);
   bool eof() const;

HazardyKnusperkeks wrote:
> A `bool` parameter for something like that is not very nice. An `enum 
> (class)` is much nicer.
> 
> If one does not know the declaration `addUnwrappedLine(false)` seems a bit 
> odd, `addUnwrappedLine(LineLevel::Keep)` looks better. (The naming may be 
> changed.)
Noted, I'll make it an enum in the next update.



Comment at: clang/unittests/Format/FormatTest.cpp:13707
WhitesmithsBraceStyle);
 
   verifyFormat("enum X\n"

MyDeveloperDay wrote:
> timwoj wrote:
> > MyDeveloperDay wrote:
> > > any reason why this is being removed?
> > This is an artifact of there not being any sort of actual guide for 
> > Whitesmiths. I have it set now to always indent the case labels (and it 
> > should just ignore the IndentCaseLabels option entirely, like it does for 
> > IndentCaseBlocks). I can certainly fix that option to work again though.
> see the images from {D93806}, you are saying that anyone with 
> bracewrappingsytle of Whitesmith cannot decide if they can or cannot indent 
> case labels?
> 
> Could you show me a spec that show Whitesmiths is the other way around?
Part of the problem is that there isn't any sort of official spec for 
Whitesmiths that I've been able to find. I'm working on a project that uses it, 
and have been basically going off their layout as the proper way of doing 
things. Looking at those images, I agree that the option should continue to 
function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 317765.
timwoj added a comment.

Updates from review:

- Fixed handling of IndentCaseLabels
- Fixed indentation of namespace braces
- Replaced bool argument to addUnwrappedLine with an enum


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

Files:
  clang/lib/Format/TokenAnalyzer.cpp


Index: clang/lib/Format/TokenAnalyzer.cpp
===
--- clang/lib/Format/TokenAnalyzer.cpp
+++ clang/lib/Format/TokenAnalyzer.cpp
@@ -68,7 +68,6 @@
   IdentifierTable IdentTable(getFormattingLangOpts(Style));
   FormatTokenLexer Lex(Env.getSourceManager(), Env.getFileID(),
Env.getFirstStartColumn(), Style, Encoding, Allocator,
-
IdentTable);
   ArrayRef Toks(Lex.lex());
   SmallVector Tokens(Toks.begin(), Toks.end());


Index: clang/lib/Format/TokenAnalyzer.cpp
===
--- clang/lib/Format/TokenAnalyzer.cpp
+++ clang/lib/Format/TokenAnalyzer.cpp
@@ -68,7 +68,6 @@
   IdentifierTable IdentTable(getFormattingLangOpts(Style));
   FormatTokenLexer Lex(Env.getSourceManager(), Env.getFileID(),
Env.getFirstStartColumn(), Style, Encoding, Allocator,
-
IdentTable);
   ArrayRef Toks(Lex.lex());
   SmallVector Tokens(Toks.begin(), Toks.end());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 317766.
timwoj added a comment.

Fixing diff to look at the right commit


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13601,7 +13601,7 @@
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
+  WhitesmithsBraceStyle.IndentCaseLabels = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -13609,7 +13609,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13619,7 +13619,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -13627,9 +13627,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13642,17 +13642,17 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
"  {\n"
"  foo(1);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = false;
+  WhitesmithsBraceStyle.IndentCaseLabels = false;
 
   verifyFormat("void switchTest4(int a)\n"
"  {\n"
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -86,7 +86,7 @@
   void parseFile();
   void parseLevel(bool HasOpeningBrace);
   void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
-  bool MunchSemi = true);
+  bool MunchSemi = true, bool NamespaceBlock = false);
   void parseChildBlock();
   void parsePPDirective(unsigned Level);
   void parsePPDefine();
@@ -140,7 +140,12 @@
   bool tryToParsePropertyAccessor();
   void tryToParseJSFunction();
   bool tryToParseSimpleAttribute();
-  void addUnwrappedLine();
+
+  // Used by addUnwrappedLine to denote whether to keep or remove a level
+  // when resetting the line state.
+  enum LineLevel { LL_Remove, LL_Keep };
+
+  void addUnwrappedLine(LineLevel AdjustLevel = LineLevel::LL_Remove);
   bool eof() const;
   // LevelDifference is the difference of levels after and before the current
   // token. For example:
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -580,12 +580,17 @@
 }
 
 void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
- bool MunchSemi) {
+ bool MunchSemi, bool NamespaceBlock) {
   assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
  "'{' or macro block token expected");
   const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);
   FormatTok->setBlockKind(BK_Block);
 
+  // For Whitesmiths mode, jump to the next level prior to skipping over the
+  // braces.
+  if (AddLevel && Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths)
+++Line->Level;
+
   size_t PPStartHash = computePPHash();
 
   unsigned InitialLevel = Line->Level;
@@ -602,9 +607,16 @@
   ? (UnwrappedLine::kInvalidIndex)
   : (CurrentLines->size() - 1 - NbPreprocessorDirectives);
 
+  // Whitesmiths is weird here. The brace needs to be indented for the namespace
+  // block, but the block itself may not be indented depending on the style
+  // settings. This allows the format to back up one level in those cases.
+  if (!AddLevel && NamespaceBlock &&
+  Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths)
+--Line->Level;
+
   ScopedDeclarationState DeclarationState(*Line, Declar

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj updated this revision to Diff 317924.
timwoj added a comment.

Add tests for Whitesmiths with all of the NamespaceIndentation options


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94500/new/

https://reviews.llvm.org/D94500

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13543,7 +13543,10 @@
WhitesmithsBraceStyle);
   */
 
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_None;
   verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
"  {\n"
"class A\n"
"  {\n"
@@ -13564,9 +13567,66 @@
"  {\n"
"  int x;\n"
"  };\n"
+   "  } // namespace b\n"
"  } // namespace a",
WhitesmithsBraceStyle);
 
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "namespace b\n"
+   "  {\n"
+   "  class A\n"
+   "{\n"
+   "void f()\n"
+   "  {\n"
+   "  if (true)\n"
+   "{\n"
+   "a();\n"
+   "b();\n"
+   "}\n"
+   "  }\n"
+   "void g()\n"
+   "  {\n"
+   "  return;\n"
+   "  }\n"
+   "};\n"
+   "  struct B\n"
+   "{\n"
+   "int x;\n"
+   "};\n"
+   "  } // namespace b\n"
+   "  } // namespace a",
+   WhitesmithsBraceStyle);
+
+  WhitesmithsBraceStyle.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("namespace a\n"
+   "  {\n"
+   "  namespace b\n"
+   "{\n"
+   "class A\n"
+   "  {\n"
+   "  void f()\n"
+   "{\n"
+   "if (true)\n"
+   "  {\n"
+   "  a();\n"
+   "  b();\n"
+   "  }\n"
+   "}\n"
+   "  void g()\n"
+   "{\n"
+   "return;\n"
+   "}\n"
+   "  };\n"
+   "struct B\n"
+   "  {\n"
+   "  int x;\n"
+   "  };\n"
+   "} // namespace b\n"
+   "  }   // namespace a",
+   WhitesmithsBraceStyle);
+
   verifyFormat("void f()\n"
"  {\n"
"  if (true)\n"
@@ -13601,7 +13661,7 @@
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = true;
+  WhitesmithsBraceStyle.IndentCaseLabels = true;
   verifyFormat("void switchTest1(int a)\n"
"  {\n"
"  switch (a)\n"
@@ -13609,7 +13669,7 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13619,7 +13679,7 @@
"  switch (a)\n"
"{\n"
"case 0:\n"
-   "break;\n"
+   "  break;\n"
"case 1:\n"
"  {\n"
"  break;\n"
@@ -13627,9 +13687,9 @@
"case 2:\n"
"  {\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
@@ -13642,17 +13702,17 @@
"  {\n"
"  foo(x);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"default:\n"
"  {\n"
"  foo(1);\n"
"  }\n"
-   "break;\n"
+   "  break;\n"
"}\n"
"  }\n",
WhitesmithsBraceStyle);
 
-  WhitesmithsBraceStyle.IndentCaseBlocks = false;
+  WhitesmithsBraceStyle.IndentCaseLabels = false;
 
   verifyFormat("void switchTest4(int a)\n"
"  {\n"
Index: clang/lib/Format/UnwrappedLineParser.h