[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Armen Khachkinaev via Phabricator via cfe-commits 
futuarmo added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:4810
+
+  verifyFormat("#define A LOOONG() LOOONG()\n",
+   ZeroColumn);

owenpan wrote:
> Please remove the newline and re-run `FormatTests`.
FormatTests pass again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Armen Khachkinaev via Phabricator via cfe-commits 
futuarmo added a comment.

@curdeius, all tests pass on fresh build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-12 Thread Armen Khachkinaev via Phabricator via cfe-commits 
futuarmo added a comment.

I will build fresh main branch tomorrow (it tooks 1+ hour) and will comment 
here about test results
I don't have commit rights, so it will be great if someone commit it with user 
"Armen Khachkinaev" and email "armen...@yandex.ru"
Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: Fix for: clang-format: break added to macro define with ColumnLimit: 0

2022-01-12 Thread Armen Khachkinaev via Phabricator via cfe-commits 
futuarmo added a comment.

Yes. Looks like I understood wrong how pre-merge clang-format check works, 
thanks everyone for help


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: Fix for: clang-format: break added to macro define with ColumnLimit: 0

2022-01-12 Thread Armen Khachkinaev via Phabricator via cfe-commits 
futuarmo updated this revision to Diff 399394.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4804,6 +4804,13 @@
Style);
 }
 
+TEST_F(FormatTest, FormatMacroWithZeroColumnWidth) {
+  FormatStyle ZeroColumn = getLLVMStyleWithColumns(0);
+
+  verifyFormat("#define A LOOONG() LOOONG()\n",
+   ZeroColumn);
+}
+
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
   verifyFormat("#define A \\\n"
"  f({ \\\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -485,7 +485,8 @@
 // different LineFormatter would be used otherwise.
 if (Previous.ClosesTemplateDeclaration)
   return Style.AlwaysBreakTemplateDeclarations != FormatStyle::BTDS_No;
-if (Previous.is(TT_FunctionAnnotationRParen))
+if (Previous.is(TT_FunctionAnnotationRParen) &&
+State.Line->Type != LT_PreprocessorDirective)
   return true;
 if (Previous.is(TT_LeadingJavaAnnotation) && Current.isNot(tok::l_paren) &&
 Current.isNot(TT_LeadingJavaAnnotation))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4804,6 +4804,13 @@
Style);
 }
 
+TEST_F(FormatTest, FormatMacroWithZeroColumnWidth) {
+  FormatStyle ZeroColumn = getLLVMStyleWithColumns(0);
+
+  verifyFormat("#define A LOOONG() LOOONG()\n",
+   ZeroColumn);
+}
+
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
   verifyFormat("#define A \\\n"
"  f({ \\\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -485,7 +485,8 @@
 // different LineFormatter would be used otherwise.
 if (Previous.ClosesTemplateDeclaration)
   return Style.AlwaysBreakTemplateDeclarations != FormatStyle::BTDS_No;
-if (Previous.is(TT_FunctionAnnotationRParen))
+if (Previous.is(TT_FunctionAnnotationRParen) &&
+State.Line->Type != LT_PreprocessorDirective)
   return true;
 if (Previous.is(TT_LeadingJavaAnnotation) && Current.isNot(tok::l_paren) &&
 Current.isNot(TT_LeadingJavaAnnotation))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116859: Fix for: clang-format: break added to macro define with ColumnLimit: 0

2022-01-10 Thread Armen Khachkinaev via Phabricator via cfe-commits 
futuarmo updated this revision to Diff 398695.
futuarmo edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4804,6 +4804,15 @@
Style);
 }
 
+TEST_F(FormatTest, FormatMacroWithZeroColumnWidth) {
+  FormatStyle ZeroColumn = getLLVMStyleWithColumns(0);
+
+  // clang-format off
+  verifyFormat("#define A LOOONG() LOOONG()\n",
+   ZeroColumn);
+  // clang-format on
+}
+
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
   verifyFormat("#define A \\\n"
"  f({ \\\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -485,7 +485,8 @@
 // different LineFormatter would be used otherwise.
 if (Previous.ClosesTemplateDeclaration)
   return Style.AlwaysBreakTemplateDeclarations != FormatStyle::BTDS_No;
-if (Previous.is(TT_FunctionAnnotationRParen))
+if (Previous.is(TT_FunctionAnnotationRParen) &&
+State.Line->Type != LT_PreprocessorDirective)
   return true;
 if (Previous.is(TT_LeadingJavaAnnotation) && Current.isNot(tok::l_paren) &&
 Current.isNot(TT_LeadingJavaAnnotation))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4804,6 +4804,15 @@
Style);
 }
 
+TEST_F(FormatTest, FormatMacroWithZeroColumnWidth) {
+  FormatStyle ZeroColumn = getLLVMStyleWithColumns(0);
+
+  // clang-format off
+  verifyFormat("#define A LOOONG() LOOONG()\n",
+   ZeroColumn);
+  // clang-format on
+}
+
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
   verifyFormat("#define A \\\n"
"  f({ \\\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -485,7 +485,8 @@
 // different LineFormatter would be used otherwise.
 if (Previous.ClosesTemplateDeclaration)
   return Style.AlwaysBreakTemplateDeclarations != FormatStyle::BTDS_No;
-if (Previous.is(TT_FunctionAnnotationRParen))
+if (Previous.is(TT_FunctionAnnotationRParen) &&
+State.Line->Type != LT_PreprocessorDirective)
   return true;
 if (Previous.is(TT_LeadingJavaAnnotation) && Current.isNot(tok::l_paren) &&
 Current.isNot(TT_LeadingJavaAnnotation))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116859: Fix for: clang-format: break added to macro define with ColumnLimit: 0

2022-01-08 Thread Armen Khachkinaev via Phabricator via cfe-commits 
futuarmo created this revision.
futuarmo added reviewers: djasper, MyDeveloperDay.
futuarmo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix for #49164  issue.
Alse added test for this case


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116859

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4804,6 +4804,14 @@
Style);
 }
 
+TEST_F(FormatTest, FormatMacroWithZeroColumnWidth) {
+  FormatStyle ZeroColumn = getLLVMStyleWithColumns(0);
+
+  verifyFormat("#define STRINGIFY(t) #t\n"
+   "#define MAKEVERSIONSTRING(x, y, z, build) STRINGIFY(x) \".\" 
STRINGIFY(y) \".\" STRINGIFY(z) \".\" STRINGIFY(build)\n",
+   ZeroColumn);
+}
+
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
   verifyFormat("#define A \\\n"
"  f({ \\\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -485,7 +485,8 @@
 // different LineFormatter would be used otherwise.
 if (Previous.ClosesTemplateDeclaration)
   return Style.AlwaysBreakTemplateDeclarations != FormatStyle::BTDS_No;
-if (Previous.is(TT_FunctionAnnotationRParen))
+if (Previous.is(TT_FunctionAnnotationRParen) &&
+State.Line->Type != LT_PreprocessorDirective)
   return true;
 if (Previous.is(TT_LeadingJavaAnnotation) && Current.isNot(tok::l_paren) &&
 Current.isNot(TT_LeadingJavaAnnotation))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4804,6 +4804,14 @@
Style);
 }
 
+TEST_F(FormatTest, FormatMacroWithZeroColumnWidth) {
+  FormatStyle ZeroColumn = getLLVMStyleWithColumns(0);
+
+  verifyFormat("#define STRINGIFY(t) #t\n"
+   "#define MAKEVERSIONSTRING(x, y, z, build) STRINGIFY(x) \".\" STRINGIFY(y) \".\" STRINGIFY(z) \".\" STRINGIFY(build)\n",
+   ZeroColumn);
+}
+
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
   verifyFormat("#define A \\\n"
"  f({ \\\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -485,7 +485,8 @@
 // different LineFormatter would be used otherwise.
 if (Previous.ClosesTemplateDeclaration)
   return Style.AlwaysBreakTemplateDeclarations != FormatStyle::BTDS_No;
-if (Previous.is(TT_FunctionAnnotationRParen))
+if (Previous.is(TT_FunctionAnnotationRParen) &&
+State.Line->Type != LT_PreprocessorDirective)
   return true;
 if (Previous.is(TT_LeadingJavaAnnotation) && Current.isNot(tok::l_paren) &&
 Current.isNot(TT_LeadingJavaAnnotation))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116585: Fix wrong JS import linebreak

2022-01-07 Thread Armen Khachkinaev via Phabricator via cfe-commits 
futuarmo abandoned this revision.
futuarmo added a comment.

Issue was fixed by other patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116585

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

[PATCH] D116585: Fix wrong JS import linebreak

2022-01-04 Thread Armen Khachkinaev via Phabricator via cfe-commits 
futuarmo created this revision.
futuarmo added a reviewer: djasper.
futuarmo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes #52935 
Also I could fix it by wrapping 2 if-statements in UnwrappedLineFormatter.cpp 
(lines 1191 and 1196) but I added more checks to ContinuationIndenter.cpp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116585

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTestJS.cpp


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -1975,6 +1975,10 @@
"  }from\n"
"  'some/path/longer/than/column/limit/module.js'  ; ",
Style);
+  Style.ColumnLimit = 0;
+  Style.JavaScriptWrapImports = false;
+  verifyFormat("import {aaa, bbb, ccc} from 'abc';",
+   "import {aaa, bbb, ccc} from 'abc';", Style);
 }
 
 TEST_F(FormatTestJS, TemplateStrings) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -367,7 +367,10 @@
(!Style.BreakBeforeTernaryOperators &&
 Previous.is(TT_ConditionalExpr))) &&
   State.Stack.back().BreakBeforeParameter && !Current.isTrailingComment() 
&&
-  !Current.isOneOf(tok::r_paren, tok::r_brace))
+  !Current.isOneOf(tok::r_paren, tok::r_brace) &&
+  ((State.Line->Type == LT_ImportStatement &&
+Style.JavaScriptWrapImports) ||
+  State.Line->Type != LT_ImportStatement))
 return true;
   if (State.Stack.back().IsChainedConditional &&
   ((Style.BreakBeforeTernaryOperators && Current.is(TT_ConditionalExpr) &&


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -1975,6 +1975,10 @@
"  }from\n"
"  'some/path/longer/than/column/limit/module.js'  ; ",
Style);
+  Style.ColumnLimit = 0;
+  Style.JavaScriptWrapImports = false;
+  verifyFormat("import {aaa, bbb, ccc} from 'abc';",
+   "import {aaa, bbb, ccc} from 'abc';", Style);
 }
 
 TEST_F(FormatTestJS, TemplateStrings) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -367,7 +367,10 @@
(!Style.BreakBeforeTernaryOperators &&
 Previous.is(TT_ConditionalExpr))) &&
   State.Stack.back().BreakBeforeParameter && !Current.isTrailingComment() &&
-  !Current.isOneOf(tok::r_paren, tok::r_brace))
+  !Current.isOneOf(tok::r_paren, tok::r_brace) &&
+  ((State.Line->Type == LT_ImportStatement &&
+Style.JavaScriptWrapImports) ||
+  State.Line->Type != LT_ImportStatement))
 return true;
   if (State.Stack.back().IsChainedConditional &&
   ((Style.BreakBeforeTernaryOperators && Current.is(TT_ConditionalExpr) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits