[PATCH] D19058: clang-format: Pointer `*` is seen as multiplication in C-style casts

2016-04-13 Thread Maxime Beaulieu via cfe-commits
mxbOctasic created this revision.
mxbOctasic added a reviewer: djasper.
mxbOctasic added subscribers: cameron314, cfe-commits.
Herald added a subscriber: klimek.

The `*` was treated as multiplication operator in complex pointer type casts.

e.g. 
(type *const)bar
(type *restrict)bar

Patch by cameron314

http://reviews.llvm.org/D19058

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5882,6 +5882,15 @@
"operator()() && {}");
   verifyGoogleFormat("template \n"
  "auto x() & -> int {}");
+
+  // Some casts look like they might be multiplications, but they are 
definitely
+  // not!
+  // Here, foo could be a macro containing e.g. `const`
+  verifyFormat("(type *foo)bar");
+  verifyFormat("(type *const)bar");
+  verifyFormat("type *x = (type *foo)bar");
+  verifyFormat("type *const x = (type *const)bar");
+  verifyFormat("type *x = (type *foo)(bar)");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1167,6 +1167,14 @@
 if (ParensAreType && !ParensCouldEndDecl)
   return true;
 
+// Some more obscure casts can leak through the above heuristic, like
+// (type* const)bar or (type* MACRO)bar
+if (Tok.Next->is(tok::identifier) && Tok.Previous &&
+Tok.Previous->isOneOf(tok::kw_const, tok::identifier) &&
+Tok.Previous->Previous &&
+Tok.Previous->Previous->is(TT_PointerOrReference))
+  return true;
+
 // At this point, we heuristically assume that there are no casts at the
 // start of the line. We assume that we have found most cases where there
 // are by the logic above, e.g. "(void)x;".
@@ -1260,6 +1268,17 @@
 if (NextNextToken && NextNextToken->is(tok::arrow))
   return TT_BinaryOperator;
 
+// Casts are never binary operators, regardless of IsExpression
+if (NextToken->isOneOf(tok::r_paren, tok::greater))
+  return TT_PointerOrReference;
+if (NextToken->isOneOf(tok::identifier, tok::kw_const) && NextNextToken &&
+NextNextToken->is(tok::r_paren) && NextNextToken->Next &&
+NextNextToken->Next->isOneOf(tok::identifier, tok::l_paren) &&
+(!PrevToken->Previous || !PrevToken->Previous->is(tok::l_paren) ||
+ !PrevToken->Previous->Previous ||
+ !PrevToken->Previous->Previous->is(tok::kw_decltype)))
+  return TT_PointerOrReference;
+
 // It is very unlikely that we are going to find a pointer or reference 
type
 // definition on the RHS of an assignment.
 if (IsExpression && !Contexts.back().CaretFound)


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5882,6 +5882,15 @@
"operator()() && {}");
   verifyGoogleFormat("template \n"
  "auto x() & -> int {}");
+
+  // Some casts look like they might be multiplications, but they are definitely
+  // not!
+  // Here, foo could be a macro containing e.g. `const`
+  verifyFormat("(type *foo)bar");
+  verifyFormat("(type *const)bar");
+  verifyFormat("type *x = (type *foo)bar");
+  verifyFormat("type *const x = (type *const)bar");
+  verifyFormat("type *x = (type *foo)(bar)");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1167,6 +1167,14 @@
 if (ParensAreType && !ParensCouldEndDecl)
   return true;
 
+// Some more obscure casts can leak through the above heuristic, like
+// (type* const)bar or (type* MACRO)bar
+if (Tok.Next->is(tok::identifier) && Tok.Previous &&
+Tok.Previous->isOneOf(tok::kw_const, tok::identifier) &&
+Tok.Previous->Previous &&
+Tok.Previous->Previous->is(TT_PointerOrReference))
+  return true;
+
 // At this point, we heuristically assume that there are no casts at the
 // start of the line. We assume that we have found most cases where there
 // are by the logic above, e.g. "(void)x;".
@@ -1260,6 +1268,17 @@
 if (NextNextToken && NextNextToken->is(tok::arrow))
   return TT_BinaryOperator;
 
+// Casts are never binary operators, regardless of IsExpression
+if (NextToken->isOneOf(tok::r_paren, tok::greater))
+  return TT_PointerOrReference;
+if (NextToken->isOneOf(tok::identifier, tok::kw_const) && NextNextToken &&
+NextNextToken->is(tok::r_paren) && NextNextToken->Next &&
+NextNextToken->Next->isOneOf(tok::identifier, tok::l_paren) &&
+(!PrevToken->Previous

[PATCH] D19064: clang-format: Allow include of clangFormat.h in managed context

2016-04-13 Thread Maxime Beaulieu via cfe-commits
mxbOctasic created this revision.
mxbOctasic added a reviewer: djasper.
mxbOctasic added subscribers: cameron314, cfe-commits.
Herald added a subscriber: klimek.

Including `VirtualFileSystem.h` in the clangFormat.h indirectly includes 
``.
This header is blocked when compiling with /clr.
I added a forward declaration of `vfs::FileSystem` in clangFormat.h and moved 
the include to clangFormat.cpp.


http://reviews.llvm.org/D19064

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp

Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Allocator.h"
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -16,7 +16,6 @@
 #define LLVM_CLANG_FORMAT_FORMAT_H
 
 #include "clang/Basic/LangOptions.h"
-#include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/ArrayRef.h"
 #include 
@@ -27,6 +26,11 @@
 class SourceManager;
 class DiagnosticConsumer;
 
+namespace vfs {
+
+class FileSystem;
+}
+
 namespace format {
 
 enum class ParseError { Success = 0, Error, Unsuitable };


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Allocator.h"
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -16,7 +16,6 @@
 #define LLVM_CLANG_FORMAT_FORMAT_H
 
 #include "clang/Basic/LangOptions.h"
-#include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/ArrayRef.h"
 #include 
@@ -27,6 +26,11 @@
 class SourceManager;
 class DiagnosticConsumer;
 
+namespace vfs {
+
+class FileSystem;
+}
+
 namespace format {
 
 enum class ParseError { Success = 0, Error, Unsuitable };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D19065: clang-format: Last line in incomplete block is indented incorrectly

2016-04-13 Thread Maxime Beaulieu via cfe-commits
mxbOctasic created this revision.
mxbOctasic added a reviewer: djasper.
mxbOctasic added subscribers: cameron314, cfe-commits.
Herald added a subscriber: klimek.

This bug occurred when the end of the file was reached while parsing a block.
The indentation of the last line was reset to the initial indentation of the 
block.

http://reviews.llvm.org/D19065

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7110,10 +7110,9 @@
 }
 
 TEST_F(FormatTest, IndentLineCommentsInStartOfBlockAtEndOfFile) {
-  // FIXME: This is not what we want...
   verifyFormat("{\n"
-   "// a"
-   "// b");
+   "  // a\n"
+   "  // b");
 }
 
 TEST_F(FormatTest, FormatStarDependingOnContext) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -430,6 +430,9 @@
 ++Line->Level;
   parseLevel(/*HasOpeningBrace=*/true);
 
+  if (eof())
+return;
+
   if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
  : !FormatTok->is(tok::r_brace)) {
 Line->Level = InitialLevel;


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7110,10 +7110,9 @@
 }
 
 TEST_F(FormatTest, IndentLineCommentsInStartOfBlockAtEndOfFile) {
-  // FIXME: This is not what we want...
   verifyFormat("{\n"
-   "// a"
-   "// b");
+   "  // a\n"
+   "  // b");
 }
 
 TEST_F(FormatTest, FormatStarDependingOnContext) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -430,6 +430,9 @@
 ++Line->Level;
   parseLevel(/*HasOpeningBrace=*/true);
 
+  if (eof())
+return;
+
   if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
  : !FormatTok->is(tok::r_brace)) {
 Line->Level = InitialLevel;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D19063: clang-format: Fixed line merging of more than two lines

2016-04-13 Thread Maxime Beaulieu via cfe-commits
mxbOctasic created this revision.
mxbOctasic added a reviewer: djasper.
mxbOctasic added subscribers: cameron314, cfe-commits.
Herald added a subscriber: klimek.

`getNextMergedLine` merged pairs of adjacent lines instead of merging them all 
with the first one.

Consider `AnnotatedLine` `A`, `B` and `C`.

The function merges `A` with `B` then `B` with `C` and returns a pointer to `A`.
`A.Last` is not updated when `B` and `C` are merged together. (`A.Last == 
B.Last`)

Subsequent lines need to be merged with the first.
By merging `A` with `B` then with `C`, `A.Last` points to the proper token. 
(`A.Last == C.Last`)



http://reviews.llvm.org/D19063

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -273,6 +273,30 @@
"int i;\n"
"\n"
"}  // namespace"));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  Style.MaxEmptyLinesToKeep = 2;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = true;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.KeepEmptyLinesAtTheStartOfBlocks = false;
+
+  EXPECT_EQ("class Foo\n"
+"{\n"
+"  Foo() {}\n"
+"\n"
+"  void funk() {}\n"
+"};",
+format("class Foo\n"
+   "{\n"
+   "  Foo()\n"
+   "  {\n"
+   "  }\n"
+   "\n"
+   "  void funk() {}\n"
+   "};",
+   Style));
 }
 
 TEST_F(FormatTest, RecognizesBinaryOperatorKeywords) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -150,7 +150,7 @@
   MergedLines = 0;
 if (!DryRun)
   for (unsigned i = 0; i < MergedLines; ++i)
-join(*Next[i], *Next[i + 1]);
+join(*Next[0], *Next[i + 1]);
 Next = Next + MergedLines + 1;
 return Current;
   }


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -273,6 +273,30 @@
"int i;\n"
"\n"
"}  // namespace"));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  Style.MaxEmptyLinesToKeep = 2;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = true;
+  Style.BraceWrapping.AfterFunction = true;
+  Style.KeepEmptyLinesAtTheStartOfBlocks = false;
+
+  EXPECT_EQ("class Foo\n"
+"{\n"
+"  Foo() {}\n"
+"\n"
+"  void funk() {}\n"
+"};",
+format("class Foo\n"
+   "{\n"
+   "  Foo()\n"
+   "  {\n"
+   "  }\n"
+   "\n"
+   "  void funk() {}\n"
+   "};",
+   Style));
 }
 
 TEST_F(FormatTest, RecognizesBinaryOperatorKeywords) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -150,7 +150,7 @@
   MergedLines = 0;
 if (!DryRun)
   for (unsigned i = 0; i < MergedLines; ++i)
-join(*Next[i], *Next[i + 1]);
+join(*Next[0], *Next[i + 1]);
 Next = Next + MergedLines + 1;
 return Current;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D19066: clang-format: `SpaceAfterTemplate` and `SpacesInBraces` options

2016-04-13 Thread Maxime Beaulieu via cfe-commits
mxbOctasic created this revision.
mxbOctasic added a reviewer: djasper.
mxbOctasic added subscribers: cameron314, cfe-commits.
Herald added a subscriber: klimek.

This patch adds two new spacing options.

`SpaceAfterTemplate` inserts a space between the template keyword and the 
opening chevron.

`SpacesInBraces` inserts spaces in short and empty brace blocks.

Patch by cameron314.

http://reviews.llvm.org/D19066

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11058,6 +11058,24 @@
   verifyFormat("A>();", Spaces);
 }
 
+TEST_F(FormatTest, SpaceAfterTemplate) {
+  FormatStyle Style = getLLVMStyle();
+
+  verifyFormat("template  void foo();", Style);
+
+  Style.SpaceAfterTemplate = false;
+  verifyFormat("template void foo();", Style);
+}
+
+TEST_F(FormatTest, SpacesInBraces) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  Style.SpacesInBraces = true;
+
+  verifyFormat("int x[] = { 1, 2, 3, 4 };", Style);
+  verifyFormat("void foo() { }", Style);
+}
+
 TEST_F(FormatTest, TripleAngleBrackets) {
   verifyFormat("f<<<1, 1>>>();");
   verifyFormat("f<<<1, 1, 1, s>>>();");
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -358,7 +358,7 @@
 (Tok->getNextNonComment() == nullptr ||
  Tok->getNextNonComment()->is(tok::semi))) {
   // We merge empty blocks even if the line exceeds the column limit.
-  Tok->SpacesRequiredBefore = 0;
+  Tok->SpacesRequiredBefore = Style.SpacesInBraces ? 1 : 0;
   Tok->CanBreakBefore = true;
   return 1;
 } else if (Limit != 0 && !Line.startsWith(tok::kw_namespace) &&
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1938,7 +1938,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.SpaceAfterTemplate ||
(Line.Type == LT_ObjCDecl && Style.ObjCSpaceBeforeProtocolList)))
 return true;
   if (Left.isOneOf(tok::exclaim, tok::tilde))
@@ -2001,11 +2001,12 @@
   !Left.isOneOf(tok::numeric_constant, TT_DictLiteral))
 return false;
   if (Left.is(tok::l_brace) && Right.is(tok::r_brace))
-return !Left.Children.empty(); // No spaces in "{}".
+return Style.SpacesInBraces ? true
+: !Left.Children.empty(); // No spaces in "{}".
   if ((Left.is(tok::l_brace) && Left.BlockKind != BK_Block) ||
   (Right.is(tok::r_brace) && Right.MatchingParen &&
Right.MatchingParen->BlockKind != BK_Block))
-return !Style.Cpp11BracedListStyle;
+return Style.SpacesInBraces || !Style.Cpp11BracedListStyle;
   if (Left.is(TT_BlockComment))
 return !Left.TokenText.endswith("=*/");
   if (Right.is(tok::l_paren)) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -328,13 +328,15 @@
 IO.mapOptional("ReflowComments", Style.ReflowComments);
 IO.mapOptional("SortIncludes", Style.SortIncludes);
 IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
+IO.mapOptional("SpaceAfterTemplate", Style.SpaceAfterTemplate);
 IO.mapOptional("SpaceBeforeAssignmentOperators",
Style.SpaceBeforeAssignmentOperators);
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
 IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses);
 IO.mapOptional("SpacesBeforeTrailingComments",
Style.SpacesBeforeTrailingComments);
 IO.mapOptional("SpacesInAngles", Style.SpacesInAngles);
+IO.mapOptional("SpacesInBraces", Style.SpacesInBraces);
 IO.mapOptional("SpacesInContainerLiterals",
Style.SpacesInContainerLiterals);
 IO.mapOptional("SpacesInCStyleCastParentheses",
@@ -541,9 +543,11 @@
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
   LLVMStyle.SpaceAfterCStyleCast = false;
+  LLVMStyle.SpaceAfterTemplate = true;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpacesInAngles = false;
+  LLVMStyle.SpacesInBraces = false;
 
   LLVMStyle.PenaltyBreakComment = 300;
   LLVMStyle.PenaltyBreakFirstLessLess = 120;
Index: include/clang/Format/Format.h
===

[PATCH] D19069: clang-format: Fixed various brace wrapping and block merging bugs

2016-04-13 Thread Maxime Beaulieu via cfe-commits
mxbOctasic created this revision.
mxbOctasic added a reviewer: djasper.
mxbOctasic added subscribers: cameron314, cfe-commits.
Herald added a subscriber: klimek.

Multiple brace wrapping flag combination were broken. 

First, IndentBraces + !AfterControlStatement caused the whole If-Else construct 
to be indented.

Currently, only function blocks can be merged into one line when the opening 
brace wrapped.
The short block merging logic checks the AfterFunction flag for all block types.
With AllowShortFunctions (All), !BraceWrapping.AfterFunction and 
BraceWrapping.AfterControlStatement I ended up with this: 
  if(true)
  {}
This happened with all other non-function blocks with the opening brace wrapped 
on its own line.


http://reviews.llvm.org/D19069

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -423,6 +423,119 @@
"  f();\n"
"}",
AllowSimpleBracedStatements);
+
+  AllowSimpleBracedStatements.BreakBeforeBraces = FormatStyle::BS_Custom;
+  AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true;
+  AllowSimpleBracedStatements.BraceWrapping.BeforeElse = true;
+
+  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = false;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
+
+  verifyFormat("if (true)\n"
+   "{\n"
+   "  f();\n"
+   "}\n"
+   "else\n"
+   "{\n"
+   "  f();\n"
+   "}\n",
+   AllowSimpleBracedStatements);
+
+  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = false;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
+
+  verifyFormat("if (true)\n"
+   "{\n"
+   "  f();\n"
+   "}\n"
+   "else\n"
+   "{\n"
+   "  f();\n"
+   "}\n",
+   AllowSimpleBracedStatements);
+
+  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
+
+  verifyFormat("if (true)\n"
+   "{\n"
+   "  f();\n"
+   "}\n"
+   "else\n"
+   "{\n"
+   "  f();\n"
+   "}\n",
+   AllowSimpleBracedStatements);
+
+  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
+
+  EXPECT_EQ("if (true) { f(); }\nelse { f(); }",
+format("if (true)\n"
+   "{\n"
+   "  f();\n"
+   "}\n"
+   "else\n"
+   "{\n"
+   "  f();\n"
+   "}",
+   AllowSimpleBracedStatements));
+
+  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = false;
+  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = false;
+
+  verifyFormat("while (true)\n"
+   "{\n"
+   "  f();\n"
+   "}",
+   AllowSimpleBracedStatements);
+  verifyFormat("for (;;)\n"
+   "{\n"
+   "  f();\n"
+   "}",
+   AllowSimpleBracedStatements);
+
+  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = false;
+  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
+
+  verifyFormat("while (true)\n"
+   "{\n"
+   "  f();\n"
+   "}",
+   AllowSimpleBracedStatements);
+  verifyFormat("for (;;)\n"
+   "{\n"
+   "  f();\n"
+   "}",
+   AllowSimpleBracedStatements);
+
+  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = false;
+
+  verifyFormat("while (true)\n"
+   "{\n"
+   "  f();\n"
+   "}",
+   AllowSimpleBracedStatements);
+  verifyFormat("for (;;)\n"
+   "{\n"
+   "  f();\n"
+   "}",
+   AllowSimpleBracedStatements);
+
+  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
+
+  EXPECT_EQ("while (true) { f(); }", format("while (true)\n"
+"{\n"
+"  f();\n"
+"}",
+AllowSimpleBracedStatements));
+  EXPECT_EQ("for (;;) { f(); }", format("for (;;)\n"
+"{\n"
+  

Re: [PATCH] D19069: clang-format: Fixed various brace wrapping and block merging bugs

2016-04-13 Thread Maxime Beaulieu via cfe-commits
mxbOctasic added inline comments.


Comment at: lib/Format/UnwrappedLineParser.cpp:789-790
@@ -773,4 +788,4 @@
   if (FormatTok->Tok.is(tok::l_brace)) {
-if (Style.BraceWrapping.AfterObjCDeclaration)
-  addUnwrappedLine();
+CompoundStatementIndenter Indenter(
+this, Style, Line->Level, 
Style.BraceWrapping.AfterObjCDeclaration);
 parseBlock(/*MustBeDeclaration=*/false);

We may not want this.


Comment at: lib/Format/UnwrappedLineParser.cpp:1388-1390
@@ -1372,3 +1387,5 @@
 parseBlock(/*MustBeDeclaration=*/false);
-if (Style.BraceWrapping.BeforeElse)
+if (Style.BraceWrapping.BeforeElse ||
+Style.BraceWrapping.AfterControlStatement &&
+Style.BraceWrapping.IndentBraces)
   addUnwrappedLine();

IndentBraces can only work properly when both AfterControlStatement and 
BeforeElse are active.
The opening brace can only be indented when it is wrapped on its own line, and 
the else keyword has to be on another line too.
We have to decide which option has precedence over the others.
Right now I'm overriding BeforeElse when the opening brace is wrapped.


Comment at: unittests/Format/FormatTest.cpp:2454-2456
@@ -2340,3 +2453,5 @@
   // Function-level try statements.
-  verifyFormat("int f() try { return 4; } catch (...) {\n"
+  verifyFormat("int f() try {\n"
+   "  return 4;\n"
+   "} catch (...) {\n"
"  return 5;\n"

This test probably should not have been changed.
However, it's strange to have the try on one line but not the catch.


http://reviews.llvm.org/D19069



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


Re: [PATCH] D19064: clang-format: Allow include of clangFormat.h in managed context

2016-04-13 Thread Maxime Beaulieu via cfe-commits
mxbOctasic updated this revision to Diff 53608.
mxbOctasic added a comment.

Removed empty line.


http://reviews.llvm.org/D19064

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp

Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Allocator.h"
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -16,7 +16,6 @@
 #define LLVM_CLANG_FORMAT_FORMAT_H
 
 #include "clang/Basic/LangOptions.h"
-#include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/ArrayRef.h"
 #include 
@@ -27,6 +26,10 @@
 class SourceManager;
 class DiagnosticConsumer;
 
+namespace vfs {
+class FileSystem;
+}
+
 namespace format {
 
 enum class ParseError { Success = 0, Error, Unsuitable };


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Allocator.h"
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -16,7 +16,6 @@
 #define LLVM_CLANG_FORMAT_FORMAT_H
 
 #include "clang/Basic/LangOptions.h"
-#include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/ArrayRef.h"
 #include 
@@ -27,6 +26,10 @@
 class SourceManager;
 class DiagnosticConsumer;
 
+namespace vfs {
+class FileSystem;
+}
+
 namespace format {
 
 enum class ParseError { Success = 0, Error, Unsuitable };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D19063: clang-format: Fixed line merging of more than two lines

2016-04-13 Thread Maxime Beaulieu via cfe-commits
mxbOctasic added inline comments.


Comment at: unittests/Format/FormatTest.cpp:285
@@ +284,3 @@
+
+  EXPECT_EQ("class Foo\n"
+"{\n"

djasper wrote:
> How does this break? I generally add an Before and After of one of the test 
> cases into my patch descriptions.
This is a fun one.

The newline between `Foo() {}` and `void Funk() {}` is removed.
This is a combination of the line merge problem and 
`KeepEmptyLinesAtTheStartOfBlocks`.

`Last` on the merged line for `Foo() {}` points to `{` instead of `}`, so the 
newline before `void` is seen as a newline at the start of a block. (Checks 
`PreviousLine->Last->is(tok::l_brace)`)


http://reviews.llvm.org/D19063



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


Re: [PATCH] D19069: clang-format: Fixed various brace wrapping and block merging bugs

2016-04-14 Thread Maxime Beaulieu via cfe-commits
mxbOctasic added inline comments.


Comment at: lib/Format/UnwrappedLineParser.cpp:789-790
@@ -773,4 +788,4 @@
   if (FormatTok->Tok.is(tok::l_brace)) {
-if (Style.BraceWrapping.AfterObjCDeclaration)
-  addUnwrappedLine();
+CompoundStatementIndenter Indenter(
+this, Style, Line->Level, 
Style.BraceWrapping.AfterObjCDeclaration);
 parseBlock(/*MustBeDeclaration=*/false);

mxbOctasic wrote:
> We may not want this.
Type is set to TT_CompoundStatementLBrace by the CompoundStatementIndenter.

Should it be TT_ObjCBlockLBrace instead?

Should the braces be affected by BraceWrapping.IndentBraces?


Comment at: lib/Format/UnwrappedLineParser.cpp:1388-1390
@@ -1372,3 +1387,5 @@
 parseBlock(/*MustBeDeclaration=*/false);
-if (Style.BraceWrapping.BeforeElse)
+if (Style.BraceWrapping.BeforeElse ||
+Style.BraceWrapping.AfterControlStatement &&
+Style.BraceWrapping.IndentBraces)
   addUnwrappedLine();

djasper wrote:
> mxbOctasic wrote:
> > IndentBraces can only work properly when both AfterControlStatement and 
> > BeforeElse are active.
> > The opening brace can only be indented when it is wrapped on its own line, 
> > and the else keyword has to be on another line too.
> > We have to decide which option has precedence over the others.
> > Right now I'm overriding BeforeElse when the opening brace is wrapped.
> Maybe we should put logic like this into expandPresets (possibly renaming it)?
Something like "applyBraceWrappingFlags"?
The preset styles do not set incompatible flags (yet?).
Do we want to perform fix-ups like this only on custom styles?

I will update the \brief in Format.h to mention that some flags can be 
overridden in certain edge cases.


Comment at: unittests/Format/FormatTest.cpp:474
@@ +473,3 @@
+  EXPECT_EQ("if (true) { f(); }\nelse { f(); }",
+format("if (true)\n"
+   "{\n"

djasper wrote:
> I'd remove all the line breaks and "\n"s here.
Should we consider the BeforeElse flag when merging this statement?




http://reviews.llvm.org/D19069



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


Re: [PATCH] D19058: clang-format: Pointer `*` is seen as multiplication in C-style casts

2016-04-14 Thread Maxime Beaulieu via cfe-commits
mxbOctasic updated this revision to Diff 53755.
mxbOctasic added a comment.

Moved cast detection logic to `rParenEndsCast`.


http://reviews.llvm.org/D19058

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5882,6 +5882,24 @@
"operator()() && {}");
   verifyGoogleFormat("template \n"
  "auto x() & -> int {}");
+
+  // Some casts look like they might be multiplications, but they are 
definitely
+  // not!
+  // Here, foo could be a macro containing e.g. `const`
+  verifyFormat("(type *foo)bar");
+  verifyFormat("(type *const)bar");
+  verifyFormat("(type *restrict)bar");
+  verifyFormat("(type *foo)(bar)");
+  verifyFormat("(type *const)(bar)");
+  verifyFormat("(type *restrict)(bar)");
+  verifyFormat("type *x = (type *foo)bar");
+  verifyFormat("type *const x = (type *const)bar");
+  verifyFormat("type *x = (type *foo)(bar)");
+
+  FormatStyle LeftPointer = getLLVMStyle();
+  LeftPointer.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("int x = a[(sint16*)pIn - j]", LeftPointer);
+  verifyFormat("int x = a[(int*)pIn - j]", LeftPointer);
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -998,8 +998,14 @@
 Current.Type = TT_LineComment;
   }
 } else if (Current.is(tok::r_paren)) {
-  if (rParenEndsCast(Current))
+  if (rParenEndsCast(Current)) {
 Current.Type = TT_CastRParen;
+for (FormatToken *tok = Current.MatchingParen; tok && tok != &Current;
+ tok = tok->Next)
+  // `*` in casts are never binary operators
+  if (tok->is(tok::star) && tok->is(TT_BinaryOperator))
+tok->Type = TT_PointerOrReference;
+  }
   if (Current.MatchingParen && Current.Next &&
   !Current.Next->isBinaryOperator() &&
   !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
@@ -1167,6 +1173,13 @@
 if (ParensAreType && !ParensCouldEndDecl)
   return true;
 
+// Some more obscure casts can leak through the above heuristic, like
+// (type* const)bar or (type* MACRO)(bar)
+if (Tok.Next->isOneOf(tok::identifier, tok::l_paren) && Tok.Previous &&
+Tok.Previous->isOneOf(tok::kw_const, tok::identifier) &&
+Tok.Previous->Previous && Tok.Previous->Previous->is(tok::star))
+  return true;
+
 // At this point, we heuristically assume that there are no casts at the
 // start of the line. We assume that we have found most cases where there
 // are by the logic above, e.g. "(void)x;".


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5882,6 +5882,24 @@
"operator()() && {}");
   verifyGoogleFormat("template \n"
  "auto x() & -> int {}");
+
+  // Some casts look like they might be multiplications, but they are definitely
+  // not!
+  // Here, foo could be a macro containing e.g. `const`
+  verifyFormat("(type *foo)bar");
+  verifyFormat("(type *const)bar");
+  verifyFormat("(type *restrict)bar");
+  verifyFormat("(type *foo)(bar)");
+  verifyFormat("(type *const)(bar)");
+  verifyFormat("(type *restrict)(bar)");
+  verifyFormat("type *x = (type *foo)bar");
+  verifyFormat("type *const x = (type *const)bar");
+  verifyFormat("type *x = (type *foo)(bar)");
+
+  FormatStyle LeftPointer = getLLVMStyle();
+  LeftPointer.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("int x = a[(sint16*)pIn - j]", LeftPointer);
+  verifyFormat("int x = a[(int*)pIn - j]", LeftPointer);
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -998,8 +998,14 @@
 Current.Type = TT_LineComment;
   }
 } else if (Current.is(tok::r_paren)) {
-  if (rParenEndsCast(Current))
+  if (rParenEndsCast(Current)) {
 Current.Type = TT_CastRParen;
+for (FormatToken *tok = Current.MatchingParen; tok && tok != &Current;
+ tok = tok->Next)
+  // `*` in casts are never binary operators
+  if (tok->is(tok::star) && tok->is(TT_BinaryOperator))
+tok->Type = TT_PointerOrReference;
+  }
   if (Current.MatchingParen && Current.Next &&
   !Current.Next->isBinaryOperator() &&
   !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
@@ -1167,6 +1173,13 @@
 if (ParensAreType && !ParensCouldEndDecl)
   return true;
 
+// Some more obscure casts can leak through