[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1360
+  if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator)
+NewParenState.UnindentOperator = true;
+  // Mark indentation as alignment if the expression is aligned.

fickert wrote:
> Typz wrote:
> > bad indent here : should be space-indented.
> Can you clarify? This is formatted with the LLVM clang-format.
Phabricator shows some kind of `>>` symbol at the beginning of this line, which 
i interpreted as a TAB character, hence my comment.
But indeed no issue after looking at the raw file, sorry about this.
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In D85600#2209545 , @Typz wrote:

> Could this also be causing https://bugs.llvm.org/show_bug.cgi?id=33896 ?
>
> I'll try to see if changes something in that case...

Actually, I can confirm this is unrelated.




Comment at: clang/lib/Format/ContinuationIndenter.cpp:1360
+  if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator)
+NewParenState.UnindentOperator = true;
+  // Mark indentation as alignment if the expression is aligned.

bad indent here : should be space-indented.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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

[PATCH] D85600: [clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Could this also be causing https://bugs.llvm.org/show_bug.cgi?id=33896 ?

I'll try to see if changes something in that case...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85600

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

[PATCH] D82199: [clang-format] restore indent in conditionals when AlignOperands is DontAlign

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:6287
+  Style.BreakBeforeTernaryOperators = false;
+  verifyFormat("int x = aaa ? aa :\n"
+   "? cc :\n"

sammccall wrote:
> aligning the question marks here is a bit weird (given DontAlign) but that's 
> another patch.
> 
> If we disable the question-column behavior with dontalign, this patch will be 
> completely dead, right?
> May want to add a FIXME to remove in that case.
I don't think this is so weird: even with DontAlign, there are other cases when 
some alignment is performed: for exemple when formatting tables in column.

As I see it, ternary operator formatting is a similar case of 2D formatting, 
and while it needs indeed to respect the "general" line wrapping/indent mode 
(as per AlignOperands), it is OK to keep the alignment of the ternary operator 
themselves : otherwise, the whole "mode" for ternary operators need to be 
disabled and makes no sense.

But anyway this is a different patch as you mentioned, and maybe some user of 
DontAlign can come up with a better approach for formatting ternary ops in that 
case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82199



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Regarding `?:`, this is definitely not considered, and should be relatively 
easy to handle.

Regarding case `D`, this looks like a bug... The break in the first operand is 
kind of unexpected (as in "not the usual case"), and I had many such corner 
cases while making and testing the change. I thought I finally had a proper 
solution, seems there was still something, though I am not sure it is related 
to `?:` or could happen in other cases (on second thought, it may indeed be 
related, as there is no operand to align with...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Indeed, I saw the emails, but I didn't have time to check or investigate the 
issues.

As for E, this is more tricky than this. At the moment, the code does not look 
at what is in the first "branch" of the ternary operator : it does not care if 
this is a nested ternary operator as well... (which would probably add *many* 
more cases to handle). So, at the moment,

  return temp[0] > temp[1]   ? temp[0] > temp[2] ? 0 : 2
 : temp[1] > temp[2] ? 1
 : 2;

is equivalent to:

  return temp[0] > temp[1]   ? myFieldWithVeryLongName
 : temp[1] > temp[2] ? 1
 : 2;

which looks fine

At some point, once I had a working change I tried to improve it so that the 
decision would not be "hardcoded" but penalty-based, but I did not succeed in 
this.

In this specific case, once option may be to introduce a special case for 
"balanced ternary operator", e.g. at least in the most simple case (e.g. a ? b 
? c : d : e ? f : g), though there may be a more generalized form... Not sure 
exactly how easy/complicated it would be, though...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In D50078#2055227 , @srj wrote:

> > And I also think it should be on by default instead of modifying many 
> > .clang-format files. So IMHO if we add an option, it should be opt-out.
>
> I can live with it being opt-out. My big concern is that (as it currently 
> stands) our project gets different 'canonical' clang-format results depending 
> on whether people are using LLVM10 vs LLVM11 (and both are common and 
> supported by our project), which is a headache. I just want a way to get 
> consistent, predictable formatting regardless of the clang-format version.


Adding an option would be an issue there, I think: since clang-format is quite 
strict regarding options, the option would need to exist in both LLVM10 and 
LLVM11 if you need to support such use-case...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In D50078#2050623 , @MyDeveloperDay 
wrote:

> I get the point about Opt In, and if we are going to add an option it needs 
> to go in ASAP otherwise too many people will then start complaining it was on 
> by default before.
>
> To be honest I think the question is do we consider this a bug or a feature, 
> because we DO NOT provide Opt In on bug fixes (and there is no Opt Out).
>
> If its a bug, there is no need for us to not have this on by default, its 
> better to drive the improvement than 10,000's of .clang-format files needing 
> to be updated just to get reasonable behavior.


I certainly a bit biased, but I think it is more a bug: it should have been 
there from the start :-)

And I also think it should be on by default instead of modifying many 
.clang-format files. So IMHO if we add an option, it should be opt-out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4db94094b469: clang-format: support aligned nested 
conditionals formatting (authored by Typz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5975,6 +5975,124 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = true;
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa ? \n"
+   "   :  ? \n"
+   "  : ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   "   : cc ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat(
+  "return  ? (a ? bb\n"
+  " : cc)\n"
+  "   : bb ? \n"
+  ": ;",
+  Style);
+  verifyFormat(
+  "return a? (a ? bb\n"
+  " : cc)\n"
+  "   : bb ? \n"
+  ": ;",
+  Style);
+  verifyFormat(
+  "return a? a = (a ? bb\n"
+  " : dd)\n"
+  "   : bb ? \n"
+  ": ;",
+  Style);
+  verifyFormat(
+  "return a? a + (a ? bb\n"
+  " : dd)\n"
+  "   : bb ? \n"
+  ": ;",
+  Style);
+  verifyFormat(
+  "return a? \n"
+  "   : bb ? \n"
+  ": a + (a ? bb\n"
+  " : dd)\n",
+  Style);
+  verifyFormat(
+  "return  ? \n"
+  "   : bb ? \n"
+  ": (a ? bb\n"
+  " : cc);",
+  Style);
+  verifyFormat(
+  "return  ? (a ? bb\n"
+  "   : ccc ? dd\n"
+  " : ee)\n"
+  "   : bb ? \n"
+  ": ;",
+  Style);
+  verifyFormat(
+  "return  ? (aa? bb\n"
+  "   : ccc ? dd\n"
+  " : ee)\n"
+  "   : 

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

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

Fix random crash on windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5950,6 +5950,124 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = true;
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa ? \n"
+   "   :  ? \n"
+   "  : ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   "   : cc ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat(
+  "return  ? (a ? bb\n"
+  " : cc)\n"
+  "   : bb ? \n"
+  ": ;",
+  Style);
+  verifyFormat(
+  "return a? (a ? bb\n"
+  " : cc)\n"
+  "   : bb ? \n"
+  ": ;",
+  Style);
+  verifyFormat(
+  "return a? a = (a ? bb\n"
+  " : dd)\n"
+  "   : bb ? \n"
+  ": ;",
+  Style);
+  verifyFormat(
+  "return a? a + (a ? bb\n"
+  " : dd)\n"
+  "   : bb ? \n"
+  ": ;",
+  Style);
+  verifyFormat(
+  "return a? \n"
+  "   : bb ? \n"
+  ": a + (a ? bb\n"
+  " : dd)\n",
+  Style);
+  verifyFormat(
+  "return  ? \n"
+  "   : bb ? \n"
+  ": (a ? bb\n"
+  " : cc);",
+  Style);
+  verifyFormat(
+  "return  ? (a ? bb\n"
+  "   : ccc ? dd\n"
+  " : ee)\n"
+  "   : bb ? \n"
+  ": ;",
+  Style);
+  verifyFormat(
+  "return  ? (aa? bb\n"
+  "   : ccc ? dd\n"
+  " : ee)\n"
+  "   : bb ? \n"
+  ": ;",
+  Style);

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Still no change with a windows or s390x machine, but i could reproduce the/a 
problem systematically on linux by enabling sanitizer.
In this case, initializing ConditionalsLevel to 0 in constructor does index fix 
this issue (and initializing to 1 ensures the issue happens, though not in 
"regular" linux build).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Thanks for trying the patch, too bad it does not work...
I will see how to get the required setup/env, but it will probably take some 
time (esp. in the current situation).
Thanks again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In D50078#1998578 , @hokein wrote:

> In D50078#1998520 , @Typz wrote:
>
> > In D50078#1998398 , @dyung wrote:
> >
> > > Hi, this change that you submitted in commit 
> > > 5daa25fd7a184524759b6ad065a8bd7e95aa149a 
> > >  
> > > seems to be causing the test "Clang-Unit :: 
> > > Format/./FormatTests.exe/FormatTest.ConfigurableUseOfTab" to randomly 
> > > fail about 50% of the time on Windows.
> > >
> > > Take the most recent 5 runs of the 
> > > llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast buildbot. Builds #31973 
> > > ,
> > >  31975 
> > > 
> > >  and 31976 
> > > 
> > >  all failed because of that test failing.
> > >
> > > On my local Windows computer, if I build your commit and run the 
> > > FormatTest unit test multiple times (the same binary without changing 
> > > anything or rebuilding), that above test fails roughly 50% of the time.
> > >
> > > Can you take a look to figure out why your change might be causing this 
> > > instability?
> >
> >
> > Hi,
> >  That is very strange indeed, nothing is supposed to be random here...
> >  I will try to have a look, but I don't have a windows computer to test, 
> > and never seen this issue on linux/macos :-/
>
>
> The linux (s390x arch) buildbot is also failing, 
> http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/31811/steps/ninja%20check%201/logs/stdio.
>  I reverted it in 47ef09e4848a970c530928496b54085cfdba5a76 
>  to make 
> buildbot happy.


Unfortunately I don't have access to either an s390x or a windows machine; is 
there a way to trigger a "private" build on buildbot, to use the same 
setup/build env ?
Or some other way to quickly get the required build env?

(Otherwise, could you please check if adding initialization of 
ConditionalsLevel to 0 in  constructor of WhitespaceManager::Change class 
(WhitespaceManager.cpp:43).cpp fixes the issue ?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/WhitespaceManager.h:163
+// it does not increase the indent for "chained" conditionals.
+int ConditionalsLevel;
+

This field is not initialised in constructor of WhitespaceManager::Change class 
(WhitespaceManager.cpp:43). Maybe this could be the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In D50078#1998398 , @dyung wrote:

> Hi, this change that you submitted in commit 
> 5daa25fd7a184524759b6ad065a8bd7e95aa149a 
>  seems 
> to be causing the test "Clang-Unit :: 
> Format/./FormatTests.exe/FormatTest.ConfigurableUseOfTab" to randomly fail 
> about 50% of the time on Windows.
>
> Take the most recent 5 runs of the 
> llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast buildbot. Builds #31973 
> ,
>  31975 
> 
>  and 31976 
> 
>  all failed because of that test failing.
>
> On my local Windows computer, if I build your commit and run the FormatTest 
> unit test multiple times (the same binary without changing anything or 
> rebuilding), that above test fails roughly 50% of the time.
>
> Can you take a look to figure out why your change might be causing this 
> instability?


Hi,
That is very strange indeed, nothing is supposed to be random here...
I will try to have a look, but I don't have a windows computer to test, and 
never seen this issue on linux/macos :-/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

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

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Typz marked 2 inline comments as done.
Closed by commit rG3d61b1120e82: clang-format: Introduce stricter AlignOperands 
flag (authored by Typz).

Changed prior to commit:
  https://reviews.llvm.org/D32478?vs=226393=259307#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D32478

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

Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -277,7 +277,7 @@
   verifyFormat("var x = a() in\n"
".aa.aa;");
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
-  Style.AlignOperands = true;
+  Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("var x = a() in\n"
".aa.aa;",
Style);
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4240,6 +4240,9 @@
"  > c) {\n"
"}",
Style);
+  verifyFormat("return a\n"
+   "   && bbb;",
+   Style);
   verifyFormat("return (a)\n"
"   // comment\n"
"   + b;",
@@ -4268,7 +4271,7 @@
 
   Style.ColumnLimit = 60;
   verifyFormat("zz\n"
-   "= b\n"
+   "= \n"
"  >> (aa);",
Style);
 
@@ -4277,7 +4280,7 @@
   Style.TabWidth = 4;
   Style.UseTab = FormatStyle::UT_Always;
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
-  Style.AlignOperands = false;
+  Style.AlignOperands = FormatStyle::OAS_DontAlign;
   EXPECT_EQ("return someVeryVeryLongConditionThatBarelyFitsOnALine\n"
 "\t&& (someOtherLongishConditionPart1\n"
 "\t\t|| someOtherEvenLongerNestedConditionPart2);",
@@ -4287,6 +4290,107 @@
Style));
 }
 
+TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Style.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+
+  verifyFormat("bool value = a\n"
+   "   + a\n"
+   "   + a\n"
+   "  == a\n"
+   " * b\n"
+   " + b\n"
+   "  && a\n"
+   " * a\n"
+   " > c;",
+   Style);
+  verifyFormat("if (a\n"
+   "* \n"
+   "+ aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "+ \n"
+   "  * aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "== \n"
+   "   * aa\n"
+   "   + bbb) {\n}",
+   Style);
+  verifyFormat("if () {\n"
+   "} else if (a\n"
+   "   && b // break\n"
+   "  > c) {\n"
+   "}",
+   Style);
+  verifyFormat("return a\n"
+   "&& 

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In D50147#1648786 , @sammccall wrote:

> > First and forehand, I have a problem to solve in my organization : we have 
> > many projects, and maintaining the config file in some many repositories is 
> > not practical.
> >  In some case (like, LLVM, google or mozilla), this is not an issue because 
> > the formatting rules are hard-coded in clang-format.
>
> I think this option is available to anyone with a public style guide that 
> covers a reasonably large body of open-source code, and which can be 
> reasonably well supported by clang-format.
>  I'd expect QT, KDE etc should be able to use this mechanism.


unfortunately this is not open-source code, so we cannot modify clang-format :-(
hence the will to have the exact same behavior, but without modifying 
clang-format itself

>> In our case, we actually have more than one "standard" style, a combination 
>> of various OS (linux, windows, macos), and not a very strong control on user 
>> computers. So we cannot rely on a specific file or variable being setup by 
>> an administrator.
> 
> In this case my best advice would be in the short term to use .clang-format 
> files. Longer term, some combination of using well-known styles, publicising 
> and teaching clang-format about the styles you use, and gaining the ability 
> to set environment variables would reduce duplication.

what do you mean about "gaining the ability to set environment variables" ?
in our case, we have around 500 repositories, so it really becomes a problem 
maintaining hundreds of copies of each style and verifying projects do not have 
a "variation" on one of the official style.

>> I think many orgs have the same issue, but some of them have found a 
>> solution by hard-coding their style in clang-format...
> 
> I'd like to see evidence that this is a widespread problem.

indeed I have no number at all.
But I think in many case, people would either:

- switch to a built-in style (which is not bad in itself, but can be 
problematic if there is lot of code already),
- not to use clang-format at all (which does not help with ensuring consistent 
formatting),
- or "fork" clang-format (which would we are currently forced to do, but is 
really not efficient)

>>> With that in mind, I'd be very happy to approve the build time config 
>>> and/or an env variable, as long as they're off by default. It's easy to 
>>> turn them on later, but not easy to turn them off.
>>>  If they're going to be on by default, I think we need a strong reason.
>> 
>> I they are going to be off by default, it means we would still need to patch 
>> clang-format to use it, correct ?
> 
> Sorry, by "off by default" I mean that if the environment variable is not 
> set, there would be no default search directory. Relative paths would be an 
> error.
>  So you could install styles centrally on each machine, and they would work 
> if CLANG_FORMAT_STYLE_PATH was set, otherwise you'd get the fallback style. 
> Would that be workable?



- Build option is implemented. This allows turn the feature off if needed, at 
build time (by specifying empty search path). I would prefer to keep thos
- Overriding an environment varialbe to change the search path is fine by me. 
But I would still prefer to have a "working" default, so that it can be used 
out-of-the-box, with no extra env variable to set

>> In D50147#157 , @sammccall 
>> wrote:
>> 
>>> >> - understanding how distro packaging is going to work
>>>
>>> There's a mechanism, but how is it to be used? Will/should projects with a 
>>> style guide provide style packages for distros? Or should these be part of 
>>> the "official" clang-format package? 
>>>  If separate packages exist, how much is it going to confuse users that 
>>> clang-format will silently format the same project with a `.clang-format` 
>>> file different ways depending on what's installed?

The exact same thing happens today with clang-format's integrated styles.

But indeed this is an issue, and handling it would be my next step : once the 
concept of 'external style' is present, I'd like to allow referencing a style 
stored in remote git server or other URL, with clang-format handling the 
retrieval/update/cache.

>> The goal is to actually separate the styles from clang-format : so I don't 
>> see the point to make them part of the official clang-format package.
>>  Usage may be different: the styles may be setup through different packages 
>> (e.g. Qt style in qt-core package), installed manually by user, 
>>  This is surely not perfect, since different packages may indeed provide the 
>> same style : technically this is not an issue (packages must just be marked 
>> as conflicting), but it is indeed the organisation's responsibility to use 
>> different names for the styles...
>>  (to some extent, this may be improved by passing URLs for external styles, 
>> e.g. from git ; but this may 

[PATCH] D70305: clang-format: fix regression in middle pointer alignment

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: MyDeveloperDay, klimek, sammccall.
Herald added a project: clang.

a75f8d98d7ac 
 
introduced a regression with Middle pointer alignment,
which this patch fixes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70305

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
@@ -14843,6 +14843,28 @@
   verifyFormat("Foo::operator&&(void&&);", Style);
   verifyFormat("Foo::operator&&();", Style);
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("Foo::operator*();", Style);
+  verifyFormat("Foo::operator void *();", Style);
+  verifyFormat("Foo::operator()(void *);", Style);
+  verifyFormat("Foo::operator*(void *);", Style);
+  verifyFormat("Foo::operator*();", Style);
+  verifyFormat("operator*(int (*)(), class Foo);", Style);
+
+  verifyFormat("Foo::operator&();", Style);
+  verifyFormat("Foo::operator void &();", Style);
+  verifyFormat("Foo::operator()(void &);", Style);
+  verifyFormat("Foo::operator&(void &);", Style);
+  verifyFormat("Foo::operator&();", Style);
+  verifyFormat("operator&(int (&)(), class Foo);", Style);
+
+  verifyFormat("Foo::operator&&();", Style);
+  verifyFormat("Foo::operator void &&();", Style);
+  verifyFormat("Foo::operator()(void &&);", Style);
+  verifyFormat("Foo::operator&&(void &&);", Style);
+  verifyFormat("Foo::operator&&();", Style);
+  verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 }
 
 } // namespace
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2694,7 +2694,7 @@
 // Space between the type and the *
 // operator void*(), operator char*(), operator Foo*() dependant
 // on PointerAlignment style.
-return (Style.PointerAlignment == FormatStyle::PAS_Right);
+return (Style.PointerAlignment != FormatStyle::PAS_Left);
   const auto SpaceRequiredForArrayInitializerLSquare =
   [](const FormatToken , const FormatStyle ) {
 return Style.SpacesInContainerLiterals ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14843,6 +14843,28 @@
   verifyFormat("Foo::operator&&(void&&);", Style);
   verifyFormat("Foo::operator&&();", Style);
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("Foo::operator*();", Style);
+  verifyFormat("Foo::operator void *();", Style);
+  verifyFormat("Foo::operator()(void *);", Style);
+  verifyFormat("Foo::operator*(void *);", Style);
+  verifyFormat("Foo::operator*();", Style);
+  verifyFormat("operator*(int (*)(), class Foo);", Style);
+
+  verifyFormat("Foo::operator&();", Style);
+  verifyFormat("Foo::operator void &();", Style);
+  verifyFormat("Foo::operator()(void &);", Style);
+  verifyFormat("Foo::operator&(void &);", Style);
+  verifyFormat("Foo::operator&();", Style);
+  verifyFormat("operator&(int (&)(), class Foo);", Style);
+
+  verifyFormat("Foo::operator&&();", Style);
+  verifyFormat("Foo::operator void &&();", Style);
+  verifyFormat("Foo::operator()(void &&);", Style);
+  verifyFormat("Foo::operator&&(void &&);", Style);
+  verifyFormat("Foo::operator&&();", Style);
+  verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 }
 
 } // namespace
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2694,7 +2694,7 @@
 // Space between the type and the *
 // operator void*(), operator char*(), operator Foo*() dependant
 // on PointerAlignment style.
-return (Style.PointerAlignment == FormatStyle::PAS_Right);
+return (Style.PointerAlignment != FormatStyle::PAS_Left);
   const auto SpaceRequiredForArrayInitializerLSquare =
   [](const FormatToken , const FormatStyle ) {
 return Style.SpacesInContainerLiterals ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Francois Ferrand via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Format/Format.h:187
   /// expressions.
-  ///
-  /// Specifically, this aligns operands of a single expression that needs to 
be
-  /// split over multiple lines, e.g.:
-  /// \code
-  ///   int aaa = bbb +
-  /// ccc;
-  /// \endcode
-  bool AlignOperands;
+  OperandAlignmentStyle AlignOperands;
 

MyDeveloperDay wrote:
> I think you are missing an entry in the operator== in Format.h
It is there already, since this is not a new field, but just changing the type 
of an existing field.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D32478



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

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

[Francois Ferrand via Phabricator via cfe-commits]

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

Rebase on top of D50078 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D32478

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

Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -277,7 +277,7 @@
   verifyFormat("var x = a() in\n"
".aa.aa;");
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
-  Style.AlignOperands = true;
+  Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("var x = a() in\n"
".aa.aa;",
Style);
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4111,6 +4111,9 @@
"  > c) {\n"
"}",
Style);
+  verifyFormat("return a\n"
+   "   && bbb;",
+   Style);
   verifyFormat("return (a)\n"
"   // comment\n"
"   + b;",
@@ -4139,7 +4142,7 @@
 
   Style.ColumnLimit = 60;
   verifyFormat("zz\n"
-   "= b\n"
+   "= \n"
"  >> (aa);",
Style);
 
@@ -4148,7 +4151,7 @@
   Style.TabWidth = 4;
   Style.UseTab = FormatStyle::UT_Always;
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
-  Style.AlignOperands = false;
+  Style.AlignOperands = FormatStyle::OAS_DontAlign;
   EXPECT_EQ("return someVeryVeryLongConditionThatBarelyFitsOnALine\n"
 "\t&& (someOtherLongishConditionPart1\n"
 "\t\t|| someOtherEvenLongerNestedConditionPart2);",
@@ -4158,6 +4161,107 @@
Style));
 }
 
+TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Style.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+
+  verifyFormat("bool value = a\n"
+   "   + a\n"
+   "   + a\n"
+   "  == a\n"
+   " * b\n"
+   " + b\n"
+   "  && a\n"
+   " * a\n"
+   " > c;",
+   Style);
+  verifyFormat("if (a\n"
+   "* \n"
+   "+ aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "+ \n"
+   "  * aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "== \n"
+   "   * aa\n"
+   "   + bbb) {\n}",
+   Style);
+  verifyFormat("if () {\n"
+   "} else if (a\n"
+   "   && b // break\n"
+   "  > c) {\n"
+   "}",
+   Style);
+  verifyFormat("return a\n"
+   "&& bbb;",
+   Style);
+  verifyFormat("return (a)\n"
+   " // comment\n"
+   " + b;",
+   Style);
+  

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1125
+  if (Current.is(TT_ConditionalExpr) && Current.is(tok::question) &&
+  ((Current.MustBreakBefore) ||
+   (Current.getNextNonComment() && 
Current.getNextNonComment()->MustBreakBefore)))

Maybe it would be better to go through here whenever this is wrapped (e.g. 
`Newline == true`) : but this implies akso introducing a penalty for "breaking" 
the nested conditional alignment. May ultimately be better, though probably 
somewhat more complex still.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

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

Implement a fallback to regularly indented alignment when the question mark is 
wrapped.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5798,6 +5798,113 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = true;
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa ? \n"
+   "   :  ? \n"
+   "  : ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   "   : cc ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a = (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a + (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? \n"
+   "   : bb ? \n"
+   ": a + (a ? bb\n"
+   " : dd)\n",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (a ? bb\n"
+   " : cc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   "   : ccc ? dd\n"
+   " : ee)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aa? bb\n"
+   "   

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:5739
+   "  ? ccc\n"
+   "  : ddd;");
   verifyFormat("bool aa = a //\n"

MyDeveloperDay wrote:
> Nit: I'm just slightly confused as to what is happening here.. I assume this 
> is the case where they are not aligned in the style. 
Ternary operator do indeed get aligned, thus :
* the "tests" (on first and third lines) get aligned
* the "results" do not fit on the line, thus get further wrapped



Comment at: clang/unittests/Format/FormatTest.cpp:5998
+   "  ddd;",
Style);
   verifyFormat("bool aa = a ? //\n"

MyDeveloperDay wrote:
> as above how does one get back to the original case?
I tried this quite a bit, but could not find a reliable way to "skip" the 
ternary operator alignment when result operand get wrapped.
If you have an idea/hint how to do this, I'd be glad to give a try...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

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

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5734,9 +5734,9 @@
getLLVMStyleWithColumns(60));
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
-   "  : bbb //\n"
-   "? ccc\n"
-   ": ddd;");
+   "  : bbb //\n"
+   "  ? ccc\n"
+   "  : ddd;");
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
"  : (bbb //\n"
@@ -5798,6 +5798,113 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = true;
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa ? \n"
+   "   :  ? \n"
+   "  : ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   "   : cc ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a = (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a + (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? \n"
+   "   : bb ? \n"
+   ": a + (a ? bb\n"
+   " : dd)\n",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (a ? bb\n"
+   "  

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

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

Fix earlier error in patch upload.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5734,9 +5734,9 @@
getLLVMStyleWithColumns(60));
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
-   "  : bbb //\n"
-   "? ccc\n"
-   ": ddd;");
+   "  : bbb //\n"
+   "  ? ccc\n"
+   "  : ddd;");
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
"  : (bbb //\n"
@@ -5798,6 +5798,113 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = true;
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa ? \n"
+   "   :  ? \n"
+   "  : ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   "   : cc ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a = (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a + (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? \n"
+   "   : bb ? \n"
+   ": a + (a ? bb\n"
+   " : dd)\n",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (a ? 

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

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

Fix corner in previous rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5798,6 +5798,113 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = true;
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa ? \n"
+   "   :  ? \n"
+   "  : ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   "   : cc ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a = (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a + (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? \n"
+   "   : bb ? \n"
+   ": a + (a ? bb\n"
+   " : dd)\n",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (a ? bb\n"
+   " : cc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   "   : ccc ? dd\n"
+   " : ee)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aa? bb\n"
+   "   

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

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

Rebased on master, integrating required code from 
https://reviews.llvm.org/D32478


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5734,9 +5734,9 @@
getLLVMStyleWithColumns(60));
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
-   "  : bbb //\n"
-   "? ccc\n"
-   ": ddd;");
+   "  : bbb //\n"
+   "  ? ccc\n"
+   "  : ddd;");
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
"  : (bbb //\n"
@@ -5798,6 +5798,113 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = true;
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa ? \n"
+   "   :  ? \n"
+   "  : ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   "   : cc ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a = (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a + (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? \n"
+   "   : bb ? \n"
+   ": a + (a ? bb\n"
+   " : dd)\n",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   "   

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

This actually depends on another Diff : https://reviews.llvm.org/D32478
That other change introduces the ability to "unindent operator", which is 
required here; however, it also changes AlignOperands to have more than 2 
modes, which does not seem to be acceptable.

Before merge, should I thus merge "unindent operator" ability back in this 
change? Or create another commit for that part only, to be reviewed separately?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50078



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

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50147



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

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

Rebase


Repository:
  rC Clang

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

https://reviews.llvm.org/D50078

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5543,9 +5543,9 @@
getLLVMStyleWithColumns(60));
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
-   "  : bbb //\n"
-   "? ccc\n"
-   ": ddd;");
+   "  : bbb //\n"
+   "  ? ccc\n"
+   "  : ddd;");
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
"  : (bbb //\n"
@@ -5607,6 +5607,113 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+  verifyFormat("return  ? \n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   " : bb   ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa   ? \n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   " :  ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   " :  ? \n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   " :  ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   " : cc)\n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? (a ? bb\n"
+   " : cc)\n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a = (a ? bb\n"
+   " : dd)\n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a + (a ? bb\n"
+   " : dd)\n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? \n"
+   " :  ? \n"
+   ": a + (a ? bb\n"
+   " : dd)\n",
+   Style);
+  verifyFormat("return  ? \n"
+   " :  ? \n"
+   ": (a ? bb\n"
+   "   

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

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 203987.
Typz added a comment.
Herald added a project: clang.

Rebase


Repository:
  rC Clang

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

https://reviews.llvm.org/D32478

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

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -277,7 +277,7 @@
   verifyFormat("var x = a() in\n"
".aa.aa;");
   FormatStyle Style = getGoogleJSStyleWithColumns(80);
-  Style.AlignOperands = true;
+  Style.AlignOperands = FormatStyle::OAS_Align;
   verifyFormat("var x = a() in\n"
".aa.aa;",
Style);
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3837,6 +3837,9 @@
"  > c) {\n"
"}",
Style);
+  verifyFormat("return a\n"
+   "   && bbb;",
+   Style);
   verifyFormat("return (a)\n"
"   // comment\n"
"   + b;",
@@ -3865,7 +3868,7 @@
 
   Style.ColumnLimit = 60;
   verifyFormat("zz\n"
-   "= b\n"
+   "= \n"
"  >> (aa);",
Style);
 
@@ -3874,7 +3877,7 @@
   Style.TabWidth = 4;
   Style.UseTab = FormatStyle::UT_Always;
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
-  Style.AlignOperands = false;
+  Style.AlignOperands = FormatStyle::OAS_DontAlign;
   EXPECT_EQ("return someVeryVeryLongConditionThatBarelyFitsOnALine\n"
 "\t&& (someOtherLongishConditionPart1\n"
 "\t\t|| someOtherEvenLongerNestedConditionPart2);",
@@ -3882,6 +3885,98 @@
Style));
 }
 
+TEST_F(FormatTest, ExpressionIndentationStrictAlign) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+  Style.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+
+  verifyFormat("bool value = a\n"
+   "   + a\n"
+   "   + a\n"
+   "  == a\n"
+   " * b\n"
+   " + b\n"
+   "  && a\n"
+   " * a\n"
+   " > c;",
+   Style);
+  verifyFormat("if (a\n"
+   "* \n"
+   "+ aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "+ \n"
+   "  * aa\n"
+   "== bbb) {\n}",
+   Style);
+  verifyFormat("if (a\n"
+   "== \n"
+   "   * aa\n"
+   "   + bbb) {\n}",
+   Style);
+  verifyFormat("if () {\n"
+   "} else if (a\n"
+   "   && b // break\n"
+   "  > c) {\n"
+   "}",
+   Style);
+  verifyFormat("return a\n"
+   "&& bbb;",
+   Style);
+  verifyFormat("return (a)\n"
+   " // comment\n"
+   " + b;",
+   Style);
+  verifyFormat(
+  "int aa = aa\n"
+  "   * 

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In D50147#157 , @sammccall wrote:

> One thing that's unclear to me is whether your aim is to
>
> 1. solve a concrete problem for your organization
> 2. solve a family of problems for similar organizations
> 3. add a new way of configuring styles for many types of users/projects


First and forehand, I have a problem to solve in my organization : we have many 
projects, and maintaining the config file in some many repositories is not 
practical.
In some case (like, LLVM, google or mozilla), this is not an issue because the 
formatting rules are hard-coded in clang-format.
In other large organisation (like Qt, KDE or many private compagnies), there 
may be coding-rules already in place and it is not practical to store the 
configuration in clang-format.
(This is not so much an issue in small-organisation, with few 
projects/repositories : each project/repo may manage its own clang-format 
config file)

The goal is to support this use-case : organisation-wide styles, without 
patching clang-format.

> If it's 1) I think this is very reasonable and we should focus on finding the 
> simplest mechanism that will work. Is it possible to always set an 
> environment variable, or a build-time parameter, or install the style file in 
> a well-known absolute path (is there really a mix of win/unix users?)

In our case, we actually have more than one "standard" style, a combination of 
various OS (linux, windows, macos), and not a very strong control on user 
computers. So we cannot rely on a specific file or variable being setup by an 
administrator.
Therefore, I think the solution should still be generic enough, and there is no 
reason to restrict it to one use case.

> If it's 2), I think what's missing is evidence that other orgs have similar 
> problems and requirements.

I think many orgs have the same issue, but some of them have found a solution 
by hard-coding their style in clang-format...

> Option 3) is interesting, but much more ambitious and needs to start with a 
> design doc that discusses the use cases, alternatives, and implications. 
> Certainly anything involving fetching styles via git/http falls into this 
> bucket, I think.

git/http/... is indeed much more complicated. It would solve the issue of 
"transparent" setup, but introduces many subtleties : how to cache the result 
(to handle offline use or downtime issues), how to handle updates

> @klimek in case I'm way off base here, or there have been past discussions 
> that shed light on this.
> 
> With that in mind, I'd be very happy to approve the build time config and/or 
> an env variable, as long as they're off by default. It's easy to turn them on 
> later, but not easy to turn them off.
>  If they're going to be on by default, I think we need a strong reason.

I they are going to be off by default, it means we would still need to patch 
clang-format to use it, correct ?
I would like to avoid this, as it is quite time consuming; and it would 
actually be easier in that case to just add yet another hard-coded style in the 
patched binary...

In D50147#157 , @sammccall wrote:

> >> - understanding how distro packaging is going to work
>
> There's a mechanism, but how is it to be used? Will/should projects with a 
> style guide provide style packages for distros? Or should these be part of 
> the "official" clang-format package? 
>  If separate packages exist, how much is it going to confuse users that 
> clang-format will silently format the same project with a `.clang-format` 
> file different ways depending on what's installed?


The goal is to actually separate the styles from clang-format : so I don't see 
the point to make them part of the official clang-format package.
Usage may be different: the styles may be setup through different packages 
(e.g. Qt style in qt-core package), installed manually by user, 
This is surely not perfect, since different packages may indeed provide the 
same style : technically this is not an issue (packages must just be marked as 
conflicting), but it is indeed the organisation's responsibility to use 
different names for the styles...
(to some extent, this may be improved by passing URLs for external styles, e.g. 
from git ; but this may even be an issue if different mirrors must be used...)


Repository:
  rC Clang

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

https://reviews.llvm.org/D50147



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362740: clang-format: better handle namespace macros 
(authored by Typz, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D37813

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/FormatToken.h
  cfe/trunk/lib/Format/FormatTokenLexer.cpp
  cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/lib/Format/TokenAnnotator.h
  cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp
  cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1870,9 +1870,117 @@
Style));
 }
 
+TEST_F(FormatTest, NamespaceMacros) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  verifyFormat("TESTSUITE(A) {\n"
+   "int foo();\n"
+   "} // TESTSUITE(A)",
+   Style);
+
+  verifyFormat("TESTSUITE(A, B) {\n"
+   "int foo();\n"
+   "} // TESTSUITE(A)",
+   Style);
+
+  // Properly indent according to NamespaceIndentation style
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("TESTSUITE(A) {\n"
+   "  int foo();\n"
+   "} // TESTSUITE(A)",
+   Style);
+  verifyFormat("TESTSUITE(A) {\n"
+   "  namespace B {\n"
+   "int foo();\n"
+   "  } // namespace B\n"
+   "} // TESTSUITE(A)",
+   Style);
+  verifyFormat("namespace A {\n"
+   "  TESTSUITE(B) {\n"
+   "int foo();\n"
+   "  } // TESTSUITE(B)\n"
+   "} // namespace A",
+   Style);
+
+  Style.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("TESTSUITE(A) {\n"
+   "TESTSUITE(B) {\n"
+   "  int foo();\n"
+   "} // TESTSUITE(B)\n"
+   "} // TESTSUITE(A)",
+   Style);
+  verifyFormat("TESTSUITE(A) {\n"
+   "namespace B {\n"
+   "  int foo();\n"
+   "} // namespace B\n"
+   "} // TESTSUITE(A)",
+   Style);
+  verifyFormat("namespace A {\n"
+   "TESTSUITE(B) {\n"
+   "  int foo();\n"
+   "} // TESTSUITE(B)\n"
+   "} // namespace A",
+   Style);
+
+  // Properly merge namespace-macros blocks in CompactNamespaces mode
+  Style.NamespaceIndentation = FormatStyle::NI_None;
+  Style.CompactNamespaces = true;
+  verifyFormat("TESTSUITE(A) { TESTSUITE(B) {\n"
+   "}} // TESTSUITE(A::B)",
+   Style);
+
+  EXPECT_EQ("TESTSUITE(out) { TESTSUITE(in) {\n"
+"}} // TESTSUITE(out::in)",
+format("TESTSUITE(out) {\n"
+   "TESTSUITE(in) {\n"
+   "} // TESTSUITE(in)\n"
+   "} // TESTSUITE(out)",
+   Style));
+
+  EXPECT_EQ("TESTSUITE(out) { TESTSUITE(in) {\n"
+"}} // TESTSUITE(out::in)",
+format("TESTSUITE(out) {\n"
+   "TESTSUITE(in) {\n"
+   "} // TESTSUITE(in)\n"
+   "} // TESTSUITE(out)",
+   Style));
+
+  // Do not merge different namespaces/macros
+  EXPECT_EQ("namespace out {\n"
+"TESTSUITE(in) {\n"
+"} // TESTSUITE(in)\n"
+"} // namespace out",
+format("namespace out {\n"
+   "TESTSUITE(in) {\n"
+   "} // TESTSUITE(in)\n"
+   "} // namespace out",
+   Style));
+  EXPECT_EQ("TESTSUITE(out) {\n"
+"namespace in {\n"
+"} // namespace in\n"
+"} // TESTSUITE(out)",
+format("TESTSUITE(out) {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "} // TESTSUITE(out)",
+   Style));
+  Style.NamespaceMacros.push_back("FOOBAR");
+  EXPECT_EQ("TESTSUITE(out) {\n"
+"FOOBAR(in) {\n"
+"} // FOOBAR(in)\n"
+"} // TESTSUITE(out)",
+format("TESTSUITE(out) {\n"
+   "FOOBAR(in) {\n"
+   "} // FOOBAR(in)\n"
+   "} // TESTSUITE(out)",
+   Style));
+}
+
 TEST_F(FormatTest, FormatsCompactNamespaces) {
   FormatStyle Style = getLLVMStyle();
   Style.CompactNamespaces = true;
+  

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

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

Rebase


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813

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

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

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50147



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50078



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813



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

[PATCH] D57184: [clang-format] Allow configuring list of function-like macros that resolve to a type

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361986: [clang-format] Allow configuring list of 
function-like macros that resolve to a… (authored by Typz, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57184?vs=201395=201962#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57184

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

Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -1160,6 +1160,22 @@
   /// For example: BOOST_FOREACH.
   std::vector ForEachMacros;
 
+  /// \brief A vector of macros that should be interpreted as type declarations
+  /// instead of as function calls.
+  ///
+  /// These are expected to be macros of the form:
+  /// \code
+  ///   STACK_OF(...)
+  /// \endcode
+  ///
+  /// In the .clang-format configuration file, this can be configured like:
+  /// \code{.yaml}
+  ///   TypenameMacros: ['STACK_OF', 'LIST']
+  /// \endcode
+  ///
+  /// For example: OpenSSL STACK_OF, BSD LIST_ENTRY.
+  std::vector TypenameMacros;
+
   /// A vector of macros that should be interpreted as complete
   /// statements.
   ///
@@ -1952,7 +1968,8 @@
SpacesInParentheses == R.SpacesInParentheses &&
SpacesInSquareBrackets == R.SpacesInSquareBrackets &&
Standard == R.Standard && TabWidth == R.TabWidth &&
-   StatementMacros == R.StatementMacros && UseTab == R.UseTab;
+   StatementMacros == R.StatementMacros && UseTab == R.UseTab &&
+   TypenameMacros == R.TypenameMacros;
   }
 
   llvm::Optional GetLanguageStyle(LanguageKind Language) const;
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -39,6 +39,8 @@
 Macros.insert({(ForEachMacro), TT_ForEachMacro});
   for (const std::string  : Style.StatementMacros)
 Macros.insert({(StatementMacro), TT_StatementMacro});
+  for (const std::string  : Style.TypenameMacros)
+Macros.insert({(TypenameMacro), TT_TypenameMacro});
 }
 
 ArrayRef FormatTokenLexer::lex() {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -505,6 +505,7 @@
 IO.mapOptional("Standard", Style.Standard);
 IO.mapOptional("StatementMacros", Style.StatementMacros);
 IO.mapOptional("TabWidth", Style.TabWidth);
+IO.mapOptional("TypenameMacros", Style.TypenameMacros);
 IO.mapOptional("UseTab", Style.UseTab);
   }
 };
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1194,11 +1194,12 @@
 
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
-if (!CurrentToken->isOneOf(
-TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
-TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
-TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
-TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
+if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_LambdaLBrace,
+   TT_ForEachMacro, TT_TypenameMacro,
+   TT_FunctionLBrace, TT_ImplicitStringLiteral,
+   TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
+   TT_OverloadedOperator, TT_RegexLiteral,
+   TT_TemplateString, TT_ObjCStringLiteral))
   CurrentToken->Type = TT_Unknown;
 CurrentToken->Role.reset();
 CurrentToken->MatchingParen = nullptr;
@@ -1416,6 +1417,7 @@
   if (AfterParen->Tok.isNot(tok::caret)) {
 if (FormatToken *BeforeParen = Current.MatchingParen->Previous)
   if (BeforeParen->is(tok::identifier) &&
+  !BeforeParen->is(TT_TypenameMacro) &&
   BeforeParen->TokenText == BeforeParen->TokenText.upper() &&
   (!BeforeParen->Previous ||
BeforeParen->Previous->ClosesTemplateDeclaration))
@@ -1667,7 +1669,8 @@
   FormatToken *TokenBeforeMatchingParen =
   PrevToken->MatchingParen->getPreviousNonComment();
   if (TokenBeforeMatchingParen &&
-  TokenBeforeMatchingParen->isOneOf(tok::kw_typeof, tok::kw_decltype))
+  TokenBeforeMatchingParen->isOneOf(tok::kw_typeof, tok::kw_decltype,
+TT_TypenameMacro))
 

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

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



Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:526
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
 "} // namespace A",

klimek wrote:
> Typz wrote:
> > Should I also fix these tests?
> > 
> > They already existed before this patch, but do not follow LLVM namespace 
> > indent style either.
> If you don't mind, that would be useful - doing it in a separate CL is fine, 
> so we keep this one focused (and no review needed).
Done


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

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

- Rebase
- Fix NamespaceEndCommentsFixerTest to match LLVM indent style


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813

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

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

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 201693.
Typz marked 3 inline comments as done.
Typz added a comment.
Herald added subscribers: ormris, mgorny.

- Rebased
- Adapt styles search path to platform conventions
- Allow customizing search path at compile time
- Allow overriding search path at runtime through CLANG_FORMAT_STYLES_PATH env 
variable


Repository:
  rC Clang

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

https://reviews.llvm.org/D50147

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -13185,6 +13185,124 @@
   ASSERT_EQ(*Style1, getGoogleStyle());
 }
 
+namespace {
+class EnvSetter {
+StringRef m_key;
+Optional m_oldValue;
+public:
+EnvSetter(StringRef key, StringRef value = StringRef()) : m_key(key) {
+  if (const char *oldValue = ::getenv(m_key.data()))
+m_oldValue.emplace(oldValue);
+  if (value.empty())
+::unsetenv(m_key.data());
+  else
+::setenv(m_key.data(), value.data(), 1);
+}
+~EnvSetter() {
+  if (m_oldValue)
+::setenv(m_key.data(), m_oldValue.getValue().data(), 1);
+}
+EnvSetter(const EnvSetter &) = delete;
+EnvSetter =(const EnvSetter &) = delete;
+};
+}
+
+TEST(FormatStyle, GetExternalStyle) {
+  // Override HOME environment variable to make tests independant of actual user
+  EnvSetter homeEnv("HOME", "/home");
+
+  // Clear CLANG_FORMAT_STYLES_PATH environment variable
+  EnvSetter stylesPathEnv("CLANG_FORMAT_STYLES_PATH");
+
+  llvm::vfs::InMemoryFileSystem FS;
+
+  // Test 1: format file in /usr/local/share/clang-format/
+  ASSERT_TRUE(
+  FS.addFile("/usr/local/share/clang-format/style1", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  auto Style1 = getStyle("style1", "", "LLVM", "", );
+  ASSERT_TRUE((bool)Style1);
+  ASSERT_EQ(*Style1, getGoogleStyle());
+
+  // Test 2: format file in /usr/share/clang-format/
+  ASSERT_TRUE(
+  FS.addFile("/usr/share/clang-format/style2", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  auto Style2 = getStyle("style2", "", "LLVM", "", );
+  ASSERT_TRUE((bool)Style2);
+  ASSERT_EQ(*Style2, getGoogleStyle());
+
+  // Test 3: format file in ~/.local/share/clang-format/
+  ASSERT_TRUE(
+  FS.addFile("/home/.local/share/clang-format/style3", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  auto Style3 = getStyle("style3", "", "LLVM", "", );
+  ASSERT_TRUE((bool)Style3);
+  ASSERT_EQ(*Style3, getGoogleStyle());
+
+  // Test 4: format file in absolute path
+  ASSERT_TRUE(
+  FS.addFile("/clang-format-styles/style4", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  auto Style4 = getStyle("/clang-format-styles/style4", "", "LLVM", "", );
+  ASSERT_TRUE((bool)Style4);
+  ASSERT_EQ(*Style4, getGoogleStyle());
+
+  // Test 5: format file in relative path
+  ASSERT_TRUE(
+  FS.addFile("/clang-format-styles/style5", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  FS.setCurrentWorkingDirectory("/clang-format-styles");
+  auto Style5 = getStyle("style5", "", "LLVM", "", );
+  ASSERT_TRUE((bool)Style5);
+  ASSERT_EQ(*Style5, getGoogleStyle());
+  FS.setCurrentWorkingDirectory("/");
+
+  // Test 6: file does not exist
+  auto Style6 = getStyle("style6", "", "LLVM", "", );
+  ASSERT_FALSE((bool)Style6);
+  llvm::consumeError(Style6.takeError());
+
+  // Test 7: absolute file does not exist
+  auto Style7 = getStyle("/style7", "", "LLVM", "", );
+  ASSERT_FALSE((bool)Style7);
+  llvm::consumeError(Style7.takeError());
+
+  // Test 8: file is not a format style
+  ASSERT_TRUE(
+  FS.addFile("/usr/local/share/clang-format/nostyle", 0,
+ llvm::MemoryBuffer::getMemBuffer("This is not a style...")));
+  FS.setCurrentWorkingDirectory("/home/clang-format-styles");
+  auto Style8 = getStyle("nostyle", "", "LLVM", "", );
+  ASSERT_FALSE((bool)Style8);
+  llvm::consumeError(Style8.takeError());
+
+  {
+// Override styles path
+EnvSetter customStylesPathEnv("CLANG_FORMAT_STYLES_PATH", "/customPath1:/customPath2");
+
+// Test 8: file in custom styles path
+ASSERT_TRUE(
+FS.addFile("/customPath1/style8_1", 0,
+   llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+auto Style8_1 = getStyle("style8_1", "", "LLVM", "", );
+ASSERT_TRUE((bool)Style8_1);
+ASSERT_EQ(*Style8_1, getGoogleStyle());
+
+ASSERT_TRUE(
+FS.addFile("/customPath2/style8_2", 0,
+   llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Mozilla")));
+auto Style8_2 = getStyle("style8_2", "", "LLVM", "", );
+ASSERT_TRUE((bool)Style8_2);
+

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

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



Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:526
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
 "} // namespace A",

Should I also fix these tests?

They already existed before this patch, but do not follow LLVM namespace indent 
style either.



Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:582
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // TESTSUITE()",

klimek wrote:
> All of the fixNamespaceEndComments tests are indented, but standard llvm 
> style doesn't indent in namespaces at all iiuc.
Ok, understood. I can fix that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

> Ah, wow, I see what confused me now:
>  In the fixNamespaceEndComment tests you use an indent that clang-format 
> would not produce. Can you please fix that, otherwise I find this super 
> confusing :)

Can you point a specific exemple (in code) ?

There are many tests related to fixNamespaceEndComment(), which look fine to me 
and which in many cases are copied from similarly indented test cases with 
standard namespace's...
(Or was it an earlier problem, and I should fix all these existing test cases 
as well?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813



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

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.
Herald added a project: clang.

In D50147#1310146 , @sammccall wrote:

> In D50147#1309880 , @Typz wrote:
>
> > ping?
>
>
> Sorry for losing track of this.
>
> I think `-style=` is a logical extension of the current options. 
> I'm less sure of supporting it in BasedOnStyle, but happy to go either way.


To keep traceability and ensure consistency, storing a clang-format file in 
each repository is still required. So BasedOnStyle is actually the feature we 
need, and from our POV the -style= extension is a side effect :-)

> Referring to styles by names in installed style directories seems like a 
> relatively marginal feature that would need a more careful design, e.g.:
> 
> - respecting platform conventions

I knew this one was coming: the patch is clearly not complete on this aspect, 
but I pushed it already to get an early feedback on the generic goal/approach.
This definitely needs some fixing, and to be honest I was hoping there was 
already something in LLVM code base on this topic (e.g. list of standard path, 
access to base installation path...) : I tried to look. but did not find 
anything yet. Any advice?

> - supporting build-time configuration

I thought of injecting the platform-specific path list through the build sytem 
as well. And this would allow overriding it with any other path list as 
appropriate.

> - understanding how distro packaging is going to work

I don't understand what you mean exactly. With build-time configuration, the 
package can be customized  to look in the appropriate places for the specific 
distribution.

Can you please clarify the issues you see?

> - thinking about addressing the resulting fragmentation as .clang-format 
> files are shared but the referenced files are not Within a tightly controlled 
> organization, these things are less of an issue. We've had luck simply making 
> local changes to support different styles for these scenarios, though that's 
> not ideal.

Our organization is not so controlled, but we want to ease the deployment and 
maintenance of the styles, hence this approach.
(by the way, ideally I would like to eventually further extend this patch to 
support retrieval of external styles through url : e.g. to get style from a 
central server, through http, git)

> One possibility to reduce the scope here: search for unknown names on under 
> `$CLANG_FORMAT_STYLES` if the variable is set. That way it can be set by 
> administrators within an org if appropriate, but clang-format doesn't have to 
> have opinions about the policy here, and all binaries still behave the same.

I don't think having no search path by default (if the env var does not exist 
or is empty) is so nice, but I can definitely add such an environment variable 
override.


Repository:
  rC Clang

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

https://reviews.llvm.org/D50147



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

[PATCH] D57184: [clang-format] Allow configuring list of function-like macros that resolve to a type

[Francois Ferrand via Phabricator via cfe-commits]

Typz accepted this revision.
Typz added a comment.
This revision is now accepted and ready to land.

Actually, looking at all the other options, the same is true for all other 
macro kind of macros (NamespaceMacros...).


Repository:
  rC Clang

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

https://reviews.llvm.org/D57184



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

[PATCH] D57184: [clang-format] Allow configuring list of function-like macros that resolve to a type

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.
Herald added a project: clang.

Regarding variable/setting names, one issue I see is that these macros are not 
really type //names// IMO : the macro name is just part of the type (like a 
template), and its invocation is a type declaration. So maybe renaming to 
`Typedecl` or something like this would be slightly more representative.

Other than this, LGTM.




Comment at: docs/ClangFormatStyleOptions.rst:1237
+**TypenameMacros** (``std::vector``)
+  A vector of macros that should be interpreted as types
+  instead of as function calls.

"type declarations" may be clearer?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57184



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.
Herald added a project: clang.

ping ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50078



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

The patch goal is indeed to indent the content of namespace-macro blocks like 
the content of any 'regular' namespace. So it should look like the content of a 
namespace, possibly depending on the choose style options. To sumarize, here is 
a detailed summary of the observable differences between macros vs normal 
namespace.

**Without this patch**, clang-format does not understand the macro call 
actually defines a namespace, thus:

- the content of the namespace-macro block is indented, even when 
`Style.NamespaceIndentation = FormatStyle::NI_None`

  TESTSUITE(A) {
int i;  // <--- should not be indented
  }
  namespace B {
  int j;
  }



- similarly for nested namespaces, when  `Style.NamespaceIndentation = 
FormatStyle::NI_Inner` :

  TESTSUITE(A) {
  TESTSUITE(B) {
  int i;// <--- should be indented 2-chars only
  }
  }
  namespace C {
  namespace D {
int j;
  }
  }

- There is no automatic fix of namespace end comment when 
`Style.FixNamespaceComments = true`

  TESTSUITE(A) {
int i;
  } // <--- should have a namespace end comment
  namespace B {
int j;
  } // namespace B

- Multiple nested namespaces are not compacted when `Style.CompactNamespaces = 
true`

  TESTSUITE(A) {
  TESTSUITE(B) { //<--- should be merged with previous line
  int i;
  }
  } // <--- should be merged with previous line (and 
have a "combined" namespace end comment)
  namespace A { namespace B {
  int j;
  } // namespace A::B



---

**This patch fixes all these points**, which hopefully leads to the exact same 
formatting when using the namespace keyword or the namespace-macros:

  // Style.NamespaceIndentation = FormatStyle::NI_None
  TESTSUITE(A) {
  int i;
  }
  namespace B {
  int j;
  }
  
  // Style.NamespaceIndentation = FormatStyle::NI_Inner
  TESTSUITE(A) {
  TESTSUITE(B) {
int i;
  }
  }
  namespace C {
  namespace D {
int j;
  }
  }
  
  // Style.FixNamespaceComments = true
  TESTSUITE(A) {
int i;
  } // TESTSUITE(A)
  namespace B {
int j;
  } // namespace B
  
  // Style.CompactNamespaces = true
  TESTSUITE(A) { TESTSUITE(B) {
int i;
  }} // TESTSUITE(A::B)
  namespace A { namespace B {
int j;
  }} // namespace A::B

I did not see any issue in my testing or reviewing the code, but if you see a 
place in the tests where it does not match this, please point it out !


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.
Herald added a project: clang.

ping ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50078



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813



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

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50147



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

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D50147#1272742, @sammccall wrote:

> Being able to discover the right style from the filesystem is powerful, and 
> if I was going to use this flag, I'd consider symlinking the style-file to 
> `subproject/.clang_format` instead. That way the setting is persistent and 
> available to all tools, rather than us needing to get it right on each 
> invocation.


I did not think of this solution. However:

- I am not sure how it works when the .clang-format link is stored in SCM: not 
sure git will allow storing a link which points outside of the repo... But it 
may be worth trying, it could be an alternative
- Making a symlink will hardcode the full path of the style, which may not suit 
different users: maybe in some cases the styles could be installed per-user (as 
users do not have root permissions), which they would be installed system-wide 
(in containers used for CI builds, where we may not know which user will 
actually run the build)
- Making a symlink will definitely not be portable to different OS : esp. on 
Windows, the path would probably not be the same
- I am not sure a symlink can point to a "~"-relative path, and automatically 
perform the user HOME path resolution




Comment at: lib/Format/Format.cpp:938
+  const llvm::SmallVector paths = {
+{"/usr/local/share/clang-format/", Name},
+{"~/.local/share/clang-format/", Name},

sammccall wrote:
> I'm not sure these prefixes are a good idea - can you explain what the 
> purpose is, and why the simpler model of just using the path doesn't work?
> 
> Certainly this needs some more thought about how it would work on windows 
> etc. I'd suggest limiting this patch to filenames.
the goal is actually to store these "base" styles in some global folder, so 
that multiple projects can reference it (through their own .clang-format file), 
without having to make any copy.

The idea is that the project is under SCM, and simply includes a reference to 
the style: in its own .clang-format, which would ideally only contain a single 
//basedOn// tag. The styles are defined globally, then each project only 
indicates which styles in uses.

But indeed, the 'HOME' folder on windows is probably not this one...


Repository:
  rC Clang

https://reviews.llvm.org/D50147



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

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D50147#1272742, @sammccall wrote:

> The idea here does seem to be a natural extension of -style, at least for the 
> case when the arg is a filename directly. I'm not opposed, happy to review 
> this.
>
> I do want to probe the use case a bit though: we have found that configuring 
> via -style= doesn't scale that well to lots of codebases and tools. e.g. if 
> someone's running clang-tidy or clangd via some driver, are they going to 
> have a way to plumb through the right -style=?
>
> Being able to discover the right style from the filesystem is powerful, and 
> if I was going to use this flag, I'd consider symlinking the style-file to 
> `subproject/.clang_format` instead. That way the setting is persistent and 
> available to all tools, rather than us needing to get it right on each 
> invocation.


I totally agree: using '-style' does not scale well. However this patch works 
also when the style is used in the 'basedOn' tag. This is actually the way we 
expect to use this: store a small .clang_format file in each repo, which simply 
references a clang-format file which must be installed (manually, for now) on 
each user's computer.


Repository:
  rC Clang

https://reviews.llvm.org/D50147



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D37813#1254865, @klimek wrote:

> In https://reviews.llvm.org/D37813#1253813, @Typz wrote:
>
> > In https://reviews.llvm.org/D37813#1184051, @klimek wrote:
> >
> > > The problem here is that we have different opinions on how the formatting 
> > > on namespace macros should behave in the first place- I think they should 
> > > behave like namespaces, you want them to be formatted differently.
> > >  Given that you want full control over the formatting of those macros, 
> > > and them not actually be formatted exactly like namespaces or classes, I 
> > > think we need a more generic mechanism for you to express that.
> >
> >
> > Not sure what you mean here. I want them to behave like namespaces as well, 
> > this is actually the use case I have... As implemented, they indeed behave 
> > exactly like namespaces :
> >
> >   TESTSUITE(a) {   namespace a {
> >   } // TESTSUITE(a)} // namespace a
> >   VS
> >   TESTSUITE(a) { TESTSUITE(b) {namespace a { namespace b {
> >   } // TESTSUITE(a::b) }} // namespace a::b
>
>
> I thought we had the discussion upthread that they indent differently from 
> namespaces. I'm working on a patch that allows you to configure macros.


The current behavior is that they indent differently from namespace, because 
clang does not know these macros are conceptually equivalent to namespaces. And 
this patch actually fixes that, letting clang know they are actually macros.


Repository:
  rC Clang

https://reviews.llvm.org/D37813



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D37813#1184051, @klimek wrote:

> The problem here is that we have different opinions on how the formatting on 
> namespace macros should behave in the first place- I think they should behave 
> like namespaces, you want them to be formatted differently.
>  Given that you want full control over the formatting of those macros, and 
> them not actually be formatted exactly like namespaces or classes, I think we 
> need a more generic mechanism for you to express that.


Not sure what you mean here. I want them to behave like namespaces as well, 
this is actually the use case I have... As implemented, they indeed behave 
exactly like namespaces :

  TESTSUITE(a) {   namespace a {
  } // TESTSUITE(a)} // namespace a
  VS
  TESTSUITE(a) { TESTSUITE(b) {namespace a { namespace b {
  } // TESTSUITE(a::b) }} // namespace a::b

(as long as there is a single argument. When multiple arguments are used, I add 
to choose a heuristic...)

As far as I understand, the divergence is that you would want something to 
"match" the implementation of the macro, while I propose a simpler heuristic, 
which should work fine for namespaces...


Repository:
  rC Clang

https://reviews.llvm.org/D37813



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

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

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343602: clang-format: better handle statement macros 
(authored by Typz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D33440?vs=158308=167965#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33440

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/FormatToken.h
  cfe/trunk/lib/Format/FormatTokenLexer.cpp
  cfe/trunk/lib/Format/FormatTokenLexer.h
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.h
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/lib/Format/UnwrappedLineParser.h
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.h
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h
@@ -126,6 +126,7 @@
   void parseObjCInterfaceOrImplementation();
   bool parseObjCProtocol();
   void parseJavaScriptEs6ImportExport();
+  void parseStatementMacro();
   bool tryToParseLambda();
   bool tryToParseLambdaIntroducer();
   void tryToParseJSFunction();
Index: cfe/trunk/lib/Format/FormatTokenLexer.h
===
--- cfe/trunk/lib/Format/FormatTokenLexer.h
+++ cfe/trunk/lib/Format/FormatTokenLexer.h
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/ADT/MapVector.h"
 
 #include 
 
@@ -99,7 +100,8 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  llvm::SmallMapVector Macros;
 
   bool FormattingDisabled;
 
Index: cfe/trunk/lib/Format/FormatToken.h
===
--- cfe/trunk/lib/Format/FormatToken.h
+++ cfe/trunk/lib/Format/FormatToken.h
@@ -86,6 +86,7 @@
   TYPE(RegexLiteral)   \
   TYPE(SelectorName)   \
   TYPE(StartOfName)\
+  TYPE(StatementMacro) \
   TYPE(StructuredBindingLSquare)   \
   TYPE(TemplateCloser) \
   TYPE(TemplateOpener) \
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -480,6 +480,10 @@
   }
   LBraceStack.pop_back();
   break;
+case tok::identifier:
+  if (!Tok->is(TT_StatementMacro))
+  break;
+  LLVM_FALLTHROUGH;
 case tok::at:
 case tok::semi:
 case tok::kw_if:
@@ -1108,6 +1112,10 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  parseStatementMacro();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
@@ -1309,6 +1317,11 @@
 return;
   }
 
+  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+parseStatementMacro();
+return;
+  }
+
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
@@ -2328,6 +2341,16 @@
   }
 }
 
+void UnwrappedLineParser::parseStatementMacro()
+{
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+}
+
 LLVM_ATTRIBUTE_UNUSED static void printDebugInfo(const UnwrappedLine ,
  StringRef Prefix = "") {
   llvm::dbgs() << Prefix << "Line(" << Line.Level
Index: cfe/trunk/lib/Format/FormatTokenLexer.cpp
===
--- cfe/trunk/lib/Format/FormatTokenLexer.cpp
+++ cfe/trunk/lib/Format/FormatTokenLexer.cpp
@@ -37,8 +37,9 @@
   Lex->SetKeepWhitespaceMode(true);
 
   for (const std::string  : Style.ForEachMacros)
-ForEachMacros.push_back((ForEachMacro));
-  llvm::sort(ForEachMacros);
+Macros.insert({(ForEachMacro), TT_ForEachMacro});
+  for (const std::string  : Style.StatementMacros)
+Macros.insert({(StatementMacro), TT_StatementMacro});
 }
 
 ArrayRef FormatTokenLexer::lex() {
@@ -657,12 +658,12 @@
   }
 
   if (Style.isCpp()) {
+auto it = Macros.find(FormatTok->Tok.getIdentifierInfo());
 if (!(Tokens.size() > 0 && Tokens.back()->Tok.getIdentifierInfo() &&
   Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
   tok::pp_define) &&
-

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D50147#1244237, @krasimir wrote:

> I don't understand the use-case this patch is realizing. Suppose I have a 
> bunch of projects that have to share a format style. Then I can checkout them 
> under a common root directory and put an appropriate .clang-format file there.


You can do this manually, but this would not be in source-control and not 
"automatic" : you need to remember to copy the file in the root directory, and 
be sure to copy the correct one.
Also, this manual copy approach does not support updates very well.

We typically use large workspaces with many repositories (using git-repo), 
containing both open-source projects (hopefull with their own `.clang-format`, 
but more often not without any) and in-house projects.
Putting a .clang-format in the root directory would make it the default not 
only for our projects, but also for all OSS projects with no .clang-format, 
where it likely is not correct.
In addition, due to historical or regulatory reasons we unfortunately have 
multiple coding rules for the same language, in different projects : so we 
cannot just put at the root directory.

Allowing to implement "custom" styles outside of clang-format fixes this, by 
allowing to define this style globally; and then simpy reference the style in 
the project.
We can then also version the styles, e.g. create myStyle-1.0 and myStyle-1.1 to 
handle changes and let projects upgrade to the latest version as needed, and 
possibly create links to allow "tracking" the latest revision of a style (e.g. 
myStyle-latest is a symlink which points to myStyle-1.1).


Repository:
  rC Clang

https://reviews.llvm.org/D50147



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

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

clang-format does indeed support .clang-format, which is great for *isolated* 
projects, and which is not affected by this patch.

This patch addresses the issue of *centralizing* the definition of styles, e.g. 
allowing individual projects to reference externally defined styles. This is 
useful when deploying a coding styles compagny-wide, to avoid copying the 
configuration in each project (and later having problem issues to maintain the 
styles or check if the correct style is indeed used). Another way of seeing it 
is that it allows extending the styles which are hard-coded in clang-format 
(llvm, google, ...)


Repository:
  rC Clang

https://reviews.llvm.org/D50147



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

[PATCH] D50147: clang-format: support external styles

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.
Herald added a subscriber: acoomans.

This patch allows defining external styles, which can be used exactly
like the embedded styles (llvm, google, mozilla...).

These styles are clang-format files installed either systemwide in
/usr/local/share/clang-format, or per-user in ~/.local/share/clang-
format. These can be specified by simply using the name of the file,
and clang-format will search the directories for the style:

  clang-format -style=foo-1.0

The patch also allows loading specifying a file name directly, either
relative or absolute:

  clang-format -style=/home/clang-format-styles/foo-1.0
  clang-format -style=styles/foo-1.0

This works also in `BaseOnStyle` field, which allows defining compagny-
wide (and possibly versionned) clang-format styles, without having to
maintain many copies in each repository: each repository will simply
need to store a short .clang-format, which simply references the
compagny-wide style.

The drawback is that these style need to be installed on each computer,
but this may be automated through an OS package. In any case, the error
cannot be ignored, as the user will be presented with an error message
if he does not have the style.

NOTE: this may be further improved by also allowing URL (http://,
git://...) in this field, which would allow clang-format to automatically
download the missing styles.


Repository:
  rC Clang

https://reviews.llvm.org/D50147

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12113,6 +12113,61 @@
   ASSERT_EQ(*Style1, getGoogleStyle());
 }
 
+TEST(FormatStyle, GetExternalStyle) {
+  vfs::InMemoryFileSystem FS;
+  // Test 1: format file in /usr/local/share/clang-format/
+  ASSERT_TRUE(
+  FS.addFile("/usr/local/share/clang-format/style1", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  auto Style1 = getStyle("style1", "", "LLVM", "", );
+  ASSERT_TRUE((bool)Style1);
+  ASSERT_EQ(*Style1, getGoogleStyle());
+
+  // Test 2: format file in ~/.local/share/clang-format/
+  ASSERT_TRUE(
+  FS.addFile("~/.local/share/clang-format/style2", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  auto Style2 = getStyle("style2", "", "LLVM", "", );
+  ASSERT_TRUE((bool)Style2);
+  ASSERT_EQ(*Style2, getGoogleStyle());
+
+  // Test 3: format file in absolute path
+  ASSERT_TRUE(
+  FS.addFile("/clang-format-styles/style3", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  auto Style3 = getStyle("/clang-format-styles/style3", "", "LLVM", "", );
+  ASSERT_TRUE((bool)Style3);
+  ASSERT_EQ(*Style3, getGoogleStyle());
+
+  // Test 4: format file in relative path
+  ASSERT_TRUE(
+  FS.addFile("/home/clang-format-styles/style4", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  FS.setCurrentWorkingDirectory("/home/clang-format-styles");
+  auto Style4 = getStyle("style4", "", "LLVM", "", );
+  ASSERT_TRUE((bool)Style4);
+  ASSERT_EQ(*Style4, getGoogleStyle());
+
+  // Test 5: file does not exist
+  auto Style5 = getStyle("style5", "", "LLVM", "", );
+  ASSERT_FALSE((bool)Style5);
+  llvm::consumeError(Style5.takeError());
+
+  // Test 6: absolute file does not exist
+  auto Style6 = getStyle("/style6", "", "LLVM", "", );
+  ASSERT_FALSE((bool)Style6);
+  llvm::consumeError(Style6.takeError());
+
+  // Test 7: file is not a format style
+  ASSERT_TRUE(
+  FS.addFile("/usr/local/share/clang-format/nostyle", 0,
+ llvm::MemoryBuffer::getMemBuffer("This is not a style...")));
+  FS.setCurrentWorkingDirectory("/home/clang-format-styles");
+  auto Style7 = getStyle("nostyle", "", "LLVM", "", );
+  ASSERT_FALSE((bool)Style7);
+  llvm::consumeError(Style7.takeError());
+}
+
 TEST(FormatStyle, GetStyleOfFile) {
   vfs::InMemoryFileSystem FS;
   // Test 1: format file in the same directory.
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -928,8 +928,42 @@
   return NoStyle;
 }
 
+// Try loading config file from path, in argument to -style and BasedOnStyle.
+bool loadSystemStyle(StringRef Name, FormatStyle *Style,
+ vfs::FileSystem *FS = nullptr) {
+  if (!FS) {
+FS = vfs::getRealFileSystem().get();
+  }
+  const llvm::SmallVector paths = {
+{"/usr/local/share/clang-format/", Name},
+{"~/.local/share/clang-format/", Name},
+Name
+  };
+  for (Twine Path: paths) {
+llvm::SmallVector RealPath;
+Twine ConfigFile{!FS->getRealPath(Path, RealPath) ? RealPath : Path};
+if (FS->exists(ConfigFile)) {
+  llvm::ErrorOr> Text =

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D50078#1184159, @krasimir wrote:

> Could you clarify how each piece is supposed to be aligned in these examples?
>  This is what makes me happy:
>
>   // column  limit V
>   int a = condition1 ? result1
> : conditio2 ? result2
> : loocondition 
>   ? result2
>   : dition3 ? resul3
> : resul4;
>
>
>
>
>   // column  limit V
>   int a = condition1 
>   ? loresult1
>   : conditio2 ? result2
>   : result4;
>


It gives the following:

  // column  limit V
  int a = condition1 ? result1
: conditio2 ? result2
: loocondition
? result2
: dition3 ? resul3
  : resul4;
  
  // column  limit V
  int a = condition1
? loresult1
: conditio2 ? result2
: result4;

i.e. the long result is wrapped and gets an extra indentation.
I have tried quite a bit to "fall back" to the old behavior when there is this 
kind of wrapping, but this always created other situations which got brocken 
because of this: so finally I choose to stay consistent, and apply the same 
behavior whenever there are chained conditionals.

> When BreakBeforeTernaryOperators is false:
> 
>   int a = condition1 ? result1 :
>   conditio2 ? result2 :
>   ditino3 ? resul3 :
> result4;

This ones is indeed aligned like this.


Repository:
  rC Clang

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D50078#1184015, @djasper wrote:

> - I'd be ok with the suggestion for BreakBeforeTernaryOperators=true


Just to be clear, which suggestion would you be ok with?

  int fooo =  ? 0
   :  ? 1
  : ;

or:

  int fooo =    ? 0
 :  ? 1
: ;



> - I don't think the alignment of "?" and ":" (in the WhitespaceManager) 
> should be always on. I think we'd need a flag for that part

No problem, I'll add the option.


Repository:
  rC Clang

https://reviews.llvm.org/D50078



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Ping!

This has been stale for a while now. Last comment indicate this is not the 
right approach, but some prototyping has been done a more generic approach, 
with no timeline.
I understand this, but in the meantime the problem is still there, with no 
solution at the moment...

Can we move this forward in the mean time, possibly marking the API as 'beta', 
'unstable' or another way of saying it is subject to change. It can be removed 
when there is a better solution.
Or maybe there is now a timeline for the better solution? or the prototype is 
available somewhere, with some documentation on limits/works to be done, so 
that work can continue from there?


Repository:
  rC Clang

https://reviews.llvm.org/D37813



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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 158313.
Typz added a comment.
Herald added a subscriber: acoomans.

Regenerate documentation


Repository:
  rC Clang

https://reviews.llvm.org/D43183

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1010,6 +1010,96 @@
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle indentClosingBrace = getLLVMStyle();
+  indentClosingBrace.CaseBlockIndent = FormatStyle::CBIS_ClosingBrace;
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  break;\n"
+   "  }\n"
+   "case 2: {\n"
+   "  break;\n"
+   "  }\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  } break;\n"
+   "case 2: {\n"
+   "  } break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  }\n"
+   "  g();\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  {\n"
+   "g();\n"
+   "h();\n"
+   "  }\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+
+  FormatStyle indentBlock = getLLVMStyle();
+  indentBlock.CaseBlockIndent = FormatStyle::CBIS_Block;
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "break;\n"
+   "  }\n"
+   "case 2: {\n"
+   "break;\n"
+   "  }\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "  } break;\n"
+   "case 2: {\n"
+   "  } break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "  }\n"
+   "  g();\n"
+   "  break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  {\n"
+   "g();\n"
+   "h();\n"
+   "  }\n"
+   "  break;\n"
+   "}",
+   indentBlock);
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -10639,6 +10729,13 @@
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: true",
   AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
 
+  Style.CaseBlockIndent = FormatStyle::CBIS_Block;
+  CHECK_PARSE("CaseBlockIndent: None", CaseBlockIndent, FormatStyle::CBIS_None);
+  CHECK_PARSE("CaseBlockIndent: ClosingBrace", CaseBlockIndent,
+  FormatStyle::CBIS_ClosingBrace);
+  CHECK_PARSE("CaseBlockIndent: Block", CaseBlockIndent,
+  FormatStyle::CBIS_Block);
+
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
   CHECK_PARSE("SpaceBeforeParens: Never", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -88,6 +88,10 @@
   void parseFile();
   void parseLevel(bool HasOpeningBrace);
   void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
+  bool MunchSemi = true) {
+parseBlock(MustBeDeclaration, AddLevel ? 1 : 0, MunchSemi);
+  }
+  void parseBlock(bool MustBeDeclaration, int AddLevel,
   bool MunchSemi = true);
   void parseChildBlock();
   void parsePPDirective();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -515,7 +515,7 @@
   return h;
 }
 
-void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
+void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, int AddLevel,

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 158309.
Typz added a comment.
Herald added a subscriber: acoomans.

Regeneration documentation


Repository:
  rC Clang

https://reviews.llvm.org/D37813

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

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

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

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 158308.
Typz marked an inline comment as done.
Typz added a comment.
Herald added a subscriber: acoomans.

Regenerate documentation


Repository:
  rC Clang

https://reviews.llvm.org/D33440

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2472,6 +2472,45 @@
getLLVMStyleWithColumns(40)));
 
   verifyFormat("MACRO(>)");
+
+  // Some macros contain an implicit semicolon.
+  FormatStyle Style = getLLVMStyle();
+  Style.StatementMacros.push_back("FOO");
+  verifyFormat("FOO(a) int b = 0;");
+  verifyFormat("FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(a);\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(argc, argv, \"4.0.2\")\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO()\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void f() {\n"
+   "  FOO(a)\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("FOO(a)\n"
+   "FOO(b)",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "FOO(b)\n"
+   "int c = 0;",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "int x = FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void foo(int a) { FOO(a) }\n"
+   "uint32_t bar() {}",
+   Style);
 }
 
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
@@ -10728,6 +10767,12 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.StatementMacros.clear();
+  CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros,
+  std::vector{"QUNUSED"});
+  CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros,
+  std::vector({"QUNUSED", "QT_REQUIRE_VERSION"}));
+
   Style.IncludeStyle.IncludeCategories.clear();
   std::vector ExpectedCategories = {
   {"abc/.*", 2}, {".*", 1}};
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -125,6 +125,7 @@
   void parseObjCInterfaceOrImplementation();
   bool parseObjCProtocol();
   void parseJavaScriptEs6ImportExport();
+  void parseStatementMacro();
   bool tryToParseLambda();
   bool tryToParseLambdaIntroducer();
   void tryToParseJSFunction();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -473,6 +473,10 @@
   }
   LBraceStack.pop_back();
   break;
+case tok::identifier:
+  if (!Tok->is(TT_StatementMacro))
+  break;
+  LLVM_FALLTHROUGH;
 case tok::at:
 case tok::semi:
 case tok::kw_if:
@@ -1098,6 +1102,10 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  parseStatementMacro();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
@@ -1297,6 +1305,11 @@
 return;
   }
 
+  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+parseStatementMacro();
+return;
+  }
+
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
@@ -2295,6 +2308,16 @@
   }
 }
 
+void UnwrappedLineParser::parseStatementMacro()
+{
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+}
+
 LLVM_ATTRIBUTE_UNUSED static void printDebugInfo(const UnwrappedLine ,
  StringRef Prefix = "") {
   llvm::dbgs() << Prefix << "Line(" << Line.Level
Index: lib/Format/FormatTokenLexer.h
===
--- lib/Format/FormatTokenLexer.h
+++ lib/Format/FormatTokenLexer.h
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/ADT/MapVector.h"
 
 #include 
 
@@ -99,7 +100,8 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  llvm::SmallMapVector Macros;
 
 

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Notes:

- I choose not to add an option to enable this behavior, as I think of it as 
just another way clang-format can (better) format the code; but I can one if 
needed
- Currently, it relies on another patch (https://reviews.llvm.org/D32478), 
which supports aligning the wrapped operator slightly differently. If/since 
that other patch does not seem to make it, I can change this patch to either do 
the alignment in this specific case (e.g. for wrapped ternary operator only) or 
to keep the 'default' behavior of clang-format (e.g. the wrapped colon would be 
aligned with the first condition):

  // i.e. the better looking option, but which requires specific cases when it 
comes after assignment or return:
  int fooo =  ? 0
   :  ? 1
  : ;
  // or the option more consistent with current clang-format behavior:
  int fooo =    ? 0
 :  ? 1
: ;

The current patch supports both behaviors, as it relies on 
https://reviews.llvm.org/D32478 to specify the expected behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D50078



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.
Herald added a subscriber: acoomans.

When multiple ternary operators are chained, e.g. like an if/else-if/
else-if/.../else sequence, clang-format will keep aligning the colon
with the question mark, which increases the indent for each
conditionals:

  int a = condition1 ? result1
 : condition2 ? result2
  : condition3 ? result3
   : result4;

This patch detects the situation (e.g. conditionals used in false branch
of another conditional), to avoid indenting in that case:

  int a = condition1 ? result1
: condition2 ? result2
: condition3 ? result3
 : result4;

When BreakBeforeTernaryOperators is false, this will format like this:

  int a = condition1 ? result1 :
  condition2 ? result2 :
  conditino3 ? result3 :
   result4;

This formatting style is referenced here:
https://www.fluentcpp.com/2018/02/27/replace-else-if-ternary-operator/
and here:
https://marcmutz.wordpress.com/2010/10/14/top-5-reasons-you-should-love-your-ternary-operator/


Repository:
  rC Clang

https://reviews.llvm.org/D50078

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4801,9 +4801,9 @@
getLLVMStyleWithColumns(60));
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
-   "  : bbb //\n"
-   "? ccc\n"
-   ": ddd;");
+   "  : bbb //\n"
+   "  ? ccc\n"
+   "  : ddd;");
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
"  : (bbb //\n"
@@ -4865,6 +4865,113 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = FormatStyle::OAS_AlignAfterOperator;
+  verifyFormat("return  ? \n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   " : bb   ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa   ? \n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   " :  ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   " :  ? \n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   " :  ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   " : cc)\n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? (a ? bb\n"
+   " : cc)\n"
+   " :  ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a = (a ? bb\n"
+   " : dd)\n"

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

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 158302.
Typz added a comment.
Herald added a subscriber: acoomans.

rebase


Repository:
  rC Clang

https://reviews.llvm.org/D32478

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

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

[PATCH] D48161: clang-format: Fix documentation generation

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL334709: clang-format: Fix documentation generation (authored 
by Typz, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D48161

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h

Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -840,24 +840,24 @@
   enum BreakConstructorInitializersStyle {
 /// Break constructor initializers before the colon and after the commas.
 /// \code
-/// Constructor()
-/// : initializer1(),
-///   initializer2()
+///Constructor()
+///: initializer1(),
+///  initializer2()
 /// \endcode
 BCIS_BeforeColon,
 /// Break constructor initializers before the colon and commas, and align
 /// the commas with the colon.
 /// \code
-/// Constructor()
-/// : initializer1()
-/// , initializer2()
+///Constructor()
+///: initializer1()
+///, initializer2()
 /// \endcode
 BCIS_BeforeComma,
 /// Break constructor initializers after the colon and commas.
 /// \code
-/// Constructor() :
-/// initializer1(),
-/// initializer2()
+///Constructor() :
+///initializer1(),
+///initializer2()
 /// \endcode
 BCIS_AfterColon
   };
@@ -897,25 +897,27 @@
   enum BreakInheritanceListStyle {
 /// Break inheritance list before the colon and after the commas.
 /// \code
-/// class Foo
-/// : Base1,
-///   Base2
-/// {};
+///class Foo
+///: Base1,
+///  Base2
+///{};
 /// \endcode
 BILS_BeforeColon,
 /// Break inheritance list before the colon and commas, and align
 /// the commas with the colon.
 /// \code
-/// Constructor()
-/// : initializer1()
-/// , initializer2()
+///class Foo
+///: Base1
+///, Base2
+///{};
 /// \endcode
 BILS_BeforeComma,
 /// Break inheritance list after the colon and commas.
 /// \code
-/// Constructor() :
-/// initializer1(),
-/// initializer2()
+///class Foo :
+///Base1,
+///Base2
+///{};
 /// \endcode
 BILS_AfterColon
   };
Index: cfe/trunk/docs/ClangFormatStyleOptions.rst
===
--- cfe/trunk/docs/ClangFormatStyleOptions.rst
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst
@@ -1019,28 +1019,28 @@
 
 .. code-block:: c++
 
-Constructor()
-: initializer1(),
-  initializer2()
+   Constructor()
+   : initializer1(),
+ initializer2()
 
   * ``BCIS_BeforeComma`` (in configuration: ``BeforeComma``)
 Break constructor initializers before the colon and commas, and align
 the commas with the colon.
 
 .. code-block:: c++
 
-Constructor()
-: initializer1()
-, initializer2()
+   Constructor()
+   : initializer1()
+   , initializer2()
 
   * ``BCIS_AfterColon`` (in configuration: ``AfterColon``)
 Break constructor initializers after the colon and commas.
 
 .. code-block:: c++
 
-Constructor() :
-initializer1(),
-initializer2()
+   Constructor() :
+   initializer1(),
+   initializer2()
 
 
 
@@ -1054,29 +1054,31 @@
 
 .. code-block:: c++
 
-class Foo
-: Base1,
-  Base2
-{};
+   class Foo
+   : Base1,
+ Base2
+   {};
 
   * ``BILS_BeforeComma`` (in configuration: ``BeforeComma``)
 Break inheritance list before the colon and commas, and align
 the commas with the colon.
 
 .. code-block:: c++
 
-Constructor()
-: initializer1()
-, initializer2()
+   class Foo
+   : Base1
+   , Base2
+   {};
 
   * ``BILS_AfterColon`` (in configuration: ``AfterColon``)
 Break inheritance list after the colon and commas.
 
 .. code-block:: c++
 
-Constructor() :
-initializer1(),
-initializer2()
+   class Foo :
+   Base1,
+   Base2
+   {};
 
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48161: clang-format: Fix documentation generation

[Francois Ferrand via Phabricator via cfe-commits]

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

rebase


Repository:
  rC Clang

https://reviews.llvm.org/D48161

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h

Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -840,24 +840,24 @@
   enum BreakConstructorInitializersStyle {
 /// Break constructor initializers before the colon and after the commas.
 /// \code
-/// Constructor()
-/// : initializer1(),
-///   initializer2()
+///Constructor()
+///: initializer1(),
+///  initializer2()
 /// \endcode
 BCIS_BeforeColon,
 /// Break constructor initializers before the colon and commas, and align
 /// the commas with the colon.
 /// \code
-/// Constructor()
-/// : initializer1()
-/// , initializer2()
+///Constructor()
+///: initializer1()
+///, initializer2()
 /// \endcode
 BCIS_BeforeComma,
 /// Break constructor initializers after the colon and commas.
 /// \code
-/// Constructor() :
-/// initializer1(),
-/// initializer2()
+///Constructor() :
+///initializer1(),
+///initializer2()
 /// \endcode
 BCIS_AfterColon
   };
@@ -897,25 +897,27 @@
   enum BreakInheritanceListStyle {
 /// Break inheritance list before the colon and after the commas.
 /// \code
-/// class Foo
-/// : Base1,
-///   Base2
-/// {};
+///class Foo
+///: Base1,
+///  Base2
+///{};
 /// \endcode
 BILS_BeforeColon,
 /// Break inheritance list before the colon and commas, and align
 /// the commas with the colon.
 /// \code
-/// Constructor()
-/// : initializer1()
-/// , initializer2()
+///class Foo
+///: Base1
+///, Base2
+///{};
 /// \endcode
 BILS_BeforeComma,
 /// Break inheritance list after the colon and commas.
 /// \code
-/// Constructor() :
-/// initializer1(),
-/// initializer2()
+///class Foo :
+///Base1,
+///Base2
+///{};
 /// \endcode
 BILS_AfterColon
   };
Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -1019,28 +1019,28 @@
 
 .. code-block:: c++
 
-Constructor()
-: initializer1(),
-  initializer2()
+   Constructor()
+   : initializer1(),
+ initializer2()
 
   * ``BCIS_BeforeComma`` (in configuration: ``BeforeComma``)
 Break constructor initializers before the colon and commas, and align
 the commas with the colon.
 
 .. code-block:: c++
 
-Constructor()
-: initializer1()
-, initializer2()
+   Constructor()
+   : initializer1()
+   , initializer2()
 
   * ``BCIS_AfterColon`` (in configuration: ``AfterColon``)
 Break constructor initializers after the colon and commas.
 
 .. code-block:: c++
 
-Constructor() :
-initializer1(),
-initializer2()
+   Constructor() :
+   initializer1(),
+   initializer2()
 
 
 
@@ -1054,29 +1054,31 @@
 
 .. code-block:: c++
 
-class Foo
-: Base1,
-  Base2
-{};
+   class Foo
+   : Base1,
+ Base2
+   {};
 
   * ``BILS_BeforeComma`` (in configuration: ``BeforeComma``)
 Break inheritance list before the colon and commas, and align
 the commas with the colon.
 
 .. code-block:: c++
 
-Constructor()
-: initializer1()
-, initializer2()
+   class Foo
+   : Base1
+   , Base2
+   {};
 
   * ``BILS_AfterColon`` (in configuration: ``AfterColon``)
 Break inheritance list after the colon and commas.
 
 .. code-block:: c++
 
-Constructor() :
-initializer1(),
-initializer2()
+   class Foo :
+   Base1,
+   Base2
+   {};
 
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48161: clang-format: Fix documentation generation

[Francois Ferrand via Phabricator via cfe-commits]

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

Add missing block, for consistency


Repository:
  rC Clang

https://reviews.llvm.org/D48161

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h

Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -840,24 +840,24 @@
   enum BreakConstructorInitializersStyle {
 /// Break constructor initializers before the colon and after the commas.
 /// \code
-/// Constructor()
-/// : initializer1(),
-///   initializer2()
+///Constructor()
+///: initializer1(),
+///  initializer2()
 /// \endcode
 BCIS_BeforeColon,
 /// Break constructor initializers before the colon and commas, and align
 /// the commas with the colon.
 /// \code
-/// Constructor()
-/// : initializer1()
-/// , initializer2()
+///Constructor()
+///: initializer1()
+///, initializer2()
 /// \endcode
 BCIS_BeforeComma,
 /// Break constructor initializers after the colon and commas.
 /// \code
-/// Constructor() :
-/// initializer1(),
-/// initializer2()
+///Constructor() :
+///initializer1(),
+///initializer2()
 /// \endcode
 BCIS_AfterColon
   };
@@ -897,25 +897,27 @@
   enum BreakInheritanceListStyle {
 /// Break inheritance list before the colon and after the commas.
 /// \code
-/// class Foo
-/// : Base1,
-///   Base2
-/// {};
+///class Foo
+///: Base1,
+///  Base2
+///{};
 /// \endcode
 BILS_BeforeColon,
 /// Break inheritance list before the colon and commas, and align
 /// the commas with the colon.
 /// \code
-/// Constructor()
-/// : initializer1()
-/// , initializer2()
+///class Foo
+///: Base1
+///, Base2
+///{};
 /// \endcode
 BILS_BeforeComma,
 /// Break inheritance list after the colon and commas.
 /// \code
-/// Constructor() :
-/// initializer1(),
-/// initializer2()
+///class Foo :
+///Base1,
+///Base2
+///{};
 /// \endcode
 BILS_AfterColon
   };
Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -1019,28 +1019,28 @@
 
 .. code-block:: c++
 
-Constructor()
-: initializer1(),
-  initializer2()
+   Constructor()
+   : initializer1(),
+ initializer2()
 
   * ``BCIS_BeforeComma`` (in configuration: ``BeforeComma``)
 Break constructor initializers before the colon and commas, and align
 the commas with the colon.
 
 .. code-block:: c++
 
-Constructor()
-: initializer1()
-, initializer2()
+   Constructor()
+   : initializer1()
+   , initializer2()
 
   * ``BCIS_AfterColon`` (in configuration: ``AfterColon``)
 Break constructor initializers after the colon and commas.
 
 .. code-block:: c++
 
-Constructor() :
-initializer1(),
-initializer2()
+   Constructor() :
+   initializer1(),
+   initializer2()
 
 
 
@@ -1054,29 +1054,31 @@
 
 .. code-block:: c++
 
-class Foo
-: Base1,
-  Base2
-{};
+   class Foo
+   : Base1,
+ Base2
+   {};
 
   * ``BILS_BeforeComma`` (in configuration: ``BeforeComma``)
 Break inheritance list before the colon and commas, and align
 the commas with the colon.
 
 .. code-block:: c++
 
-Constructor()
-: initializer1()
-, initializer2()
+   class Foo
+   : Base1
+   , Base2
+   {};
 
   * ``BILS_AfterColon`` (in configuration: ``AfterColon``)
 Break inheritance list after the colon and commas.
 
 .. code-block:: c++
 
-Constructor() :
-initializer1(),
-initializer2()
+   class Foo :
+   Base1,
+   Base2
+   {};
 
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48161: Fix documentation generation

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.

  It seems that the changes done to `ClangFormatStyleOptions.rst` @334408 are 
causing the generation of the documentation to fail, with the following error:
  
  Warning, treated as error:
/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:1060: WARNING: 
Definition list ends without a blank line; unexpected unindent.
  
  This is due to missing indent in some code block, and fixed by this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D48161

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h

Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -840,24 +840,24 @@
   enum BreakConstructorInitializersStyle {
 /// Break constructor initializers before the colon and after the commas.
 /// \code
-/// Constructor()
-/// : initializer1(),
-///   initializer2()
+///Constructor()
+///: initializer1(),
+///  initializer2()
 /// \endcode
 BCIS_BeforeColon,
 /// Break constructor initializers before the colon and commas, and align
 /// the commas with the colon.
 /// \code
-/// Constructor()
-/// : initializer1()
-/// , initializer2()
+///Constructor()
+///: initializer1()
+///, initializer2()
 /// \endcode
 BCIS_BeforeComma,
 /// Break constructor initializers after the colon and commas.
 /// \code
-/// Constructor() :
-/// initializer1(),
-/// initializer2()
+///Constructor() :
+///initializer1(),
+///initializer2()
 /// \endcode
 BCIS_AfterColon
   };
@@ -897,25 +897,25 @@
   enum BreakInheritanceListStyle {
 /// Break inheritance list before the colon and after the commas.
 /// \code
-/// class Foo
-/// : Base1,
-///   Base2
-/// {};
+///class Foo
+///: Base1,
+///  Base2
+///{};
 /// \endcode
 BILS_BeforeColon,
 /// Break inheritance list before the colon and commas, and align
 /// the commas with the colon.
 /// \code
-/// Constructor()
-/// : initializer1()
-/// , initializer2()
+///class Foo
+///: Base1
+///, Base2
 /// \endcode
 BILS_BeforeComma,
 /// Break inheritance list after the colon and commas.
 /// \code
-/// Constructor() :
-/// initializer1(),
-/// initializer2()
+///class Foo :
+///Base1,
+///Base2
 /// \endcode
 BILS_AfterColon
   };
Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -1019,28 +1019,28 @@
 
 .. code-block:: c++
 
-Constructor()
-: initializer1(),
-  initializer2()
+   Constructor()
+   : initializer1(),
+ initializer2()
 
   * ``BCIS_BeforeComma`` (in configuration: ``BeforeComma``)
 Break constructor initializers before the colon and commas, and align
 the commas with the colon.
 
 .. code-block:: c++
 
-Constructor()
-: initializer1()
-, initializer2()
+   Constructor()
+   : initializer1()
+   , initializer2()
 
   * ``BCIS_AfterColon`` (in configuration: ``AfterColon``)
 Break constructor initializers after the colon and commas.
 
 .. code-block:: c++
 
-Constructor() :
-initializer1(),
-initializer2()
+   Constructor() :
+   initializer1(),
+   initializer2()
 
 
 
@@ -1054,29 +1054,29 @@
 
 .. code-block:: c++
 
-class Foo
-: Base1,
-  Base2
-{};
+   class Foo
+   : Base1,
+ Base2
+   {};
 
   * ``BILS_BeforeComma`` (in configuration: ``BeforeComma``)
 Break inheritance list before the colon and commas, and align
 the commas with the colon.
 
 .. code-block:: c++
 
-Constructor()
-: initializer1()
-, initializer2()
+   class Foo
+   : Base1
+   , Base2
 
   * ``BILS_AfterColon`` (in configuration: ``AfterColon``)
 Break inheritance list after the colon and commas.
 
 .. code-block:: c++
 
-Constructor() :
-initializer1(),
-initializer2()
+   class Foo :
+   Base1,
+   Base2
 
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D42787#1127790, @djasper wrote:

> The normal rule for formatting options apply. If you can dig up a public 
> style guide and a project of reasonable size where it is used, we can add an 
> option.


I don't want to be rude, but it seems to me that in this context this response 
is just a polite way of saying "no" : as discussed already on this patch, this 
is indeed a corner case, and probably not documented anywere, and as far as I 
understand, the current behavior is not referenced in llvm or google coding 
rule either. This is simply the styling that the maintainers find the most 
appropriate.

Hence my question: I know the "rules", but I want to know if you would be open 
to introducing options for tweaking this, in case people do not agree this is 
the most appropriate. Typically, for such corner cases I could imagine a nested 
option, similar to custom brace wrapping, so that the "basic" namespace option 
is not poluted, but further customization can be defined in a nested "advanced" 
option.


Repository:
  rC Clang

https://reviews.llvm.org/D42787



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

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC334408: clang-format: Introduce BreakInheritanceList option 
(authored by Typz, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43015?vs=150755=150756#toc

Repository:
  rC Clang

https://reviews.llvm.org/D43015

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

Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -893,16 +893,35 @@
   /// \endcode
   std::string CommentPragmas;
 
-  /// If ``true``, in the class inheritance expression clang-format will
-  /// break before ``:`` and ``,`` if there is multiple inheritance.
-  /// \code
-  ///true:  false:
-  ///class MyClass  vs. class MyClass : public X, public Y {
-  ///: public X };
-  ///, public Y {
-  ///};
-  /// \endcode
-  bool BreakBeforeInheritanceComma;
+  /// Different ways to break inheritance list.
+  enum BreakInheritanceListStyle {
+/// Break inheritance list before the colon and after the commas.
+/// \code
+/// class Foo
+/// : Base1,
+///   Base2
+/// {};
+/// \endcode
+BILS_BeforeColon,
+/// Break inheritance list before the colon and commas, and align
+/// the commas with the colon.
+/// \code
+/// Constructor()
+/// : initializer1()
+/// , initializer2()
+/// \endcode
+BILS_BeforeComma,
+/// Break inheritance list after the colon and commas.
+/// \code
+/// Constructor() :
+/// initializer1(),
+/// initializer2()
+/// \endcode
+BILS_AfterColon
+  };
+
+  /// The inheritance list style to use.
+  BreakInheritanceListStyle BreakInheritanceList;
 
   /// If ``true``, consecutive namespace declarations will be on the same
   /// line. If ``false``, each namespace is declared on a new line.
@@ -946,7 +965,7 @@
   bool ConstructorInitializerAllOnOneLineOrOnePerLine;
 
   /// The number of characters to use for indentation of constructor
-  /// initializer lists.
+  /// initializer lists as well as inheritance lists.
   unsigned ConstructorInitializerIndentWidth;
 
   /// Indent width for line continuations.
@@ -1673,7 +1692,7 @@
BreakAfterJavaFieldAnnotations == R.BreakAfterJavaFieldAnnotations &&
BreakStringLiterals == R.BreakStringLiterals &&
ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas &&
-   BreakBeforeInheritanceComma == R.BreakBeforeInheritanceComma &&
+   BreakInheritanceList == R.BreakInheritanceList &&
ConstructorInitializerAllOnOneLineOrOnePerLine ==
R.ConstructorInitializerAllOnOneLineOrOnePerLine &&
ConstructorInitializerIndentWidth ==
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -149,6 +149,16 @@
 };
 
 template <>
+struct ScalarEnumerationTraits {
+  static void
+  enumeration(IO , FormatStyle::BreakInheritanceListStyle ) {
+IO.enumCase(Value, "BeforeColon", FormatStyle::BILS_BeforeColon);
+IO.enumCase(Value, "BeforeComma", FormatStyle::BILS_BeforeComma);
+IO.enumCase(Value, "AfterColon", FormatStyle::BILS_AfterColon);
+  }
+};
+
+template <>
 struct ScalarEnumerationTraits {
   static void enumeration(IO , FormatStyle::PPDirectiveIndentStyle ) {
 IO.enumCase(Value, "None", FormatStyle::PPDIS_None);
@@ -350,8 +360,19 @@
 IO.mapOptional("BreakBeforeBinaryOperators",
Style.BreakBeforeBinaryOperators);
 IO.mapOptional("BreakBeforeBraces", Style.BreakBeforeBraces);
+
+bool BreakBeforeInheritanceComma = false;
 IO.mapOptional("BreakBeforeInheritanceComma",
-   Style.BreakBeforeInheritanceComma);
+   BreakBeforeInheritanceComma);
+IO.mapOptional("BreakInheritanceList",
+   Style.BreakInheritanceList);
+// If BreakBeforeInheritanceComma was specified but
+// BreakInheritance was not, initialize the latter from the
+// former for backwards compatibility.
+if (BreakBeforeInheritanceComma &&
+Style.BreakInheritanceList == FormatStyle::BILS_BeforeColon)
+  Style.BreakInheritanceList = FormatStyle::BILS_BeforeComma;
+
 IO.mapOptional("BreakBeforeTernaryOperators",
Style.BreakBeforeTernaryOperators);
 
@@ -624,7 +645,7 @@
  false, false, true,  true,  true};
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
-  LLVMStyle.BreakBeforeInheritanceComma = false;
+  

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

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

rebase


Repository:
  rC Clang

https://reviews.llvm.org/D43015

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1316,15 +1316,40 @@
   verifyFormat("class ::A::B {};");
 }
 
-TEST_F(FormatTest, BreakBeforeInheritanceComma) {
-  FormatStyle StyleWithInheritanceBreak = getLLVMStyle();
-  StyleWithInheritanceBreak.BreakBeforeInheritanceComma = true;
-
-  verifyFormat("class MyClass : public X {};", StyleWithInheritanceBreak);
+TEST_F(FormatTest, BreakInheritanceStyle) {
+  FormatStyle StyleWithInheritanceBreakBeforeComma = getLLVMStyle();
+  StyleWithInheritanceBreakBeforeComma.BreakInheritanceList =
+  FormatStyle::BILS_BeforeComma;
+  verifyFormat("class MyClass : public X {};",
+   StyleWithInheritanceBreakBeforeComma);
   verifyFormat("class MyClass\n"
": public X\n"
", public Y {};",
-   StyleWithInheritanceBreak);
+   StyleWithInheritanceBreakBeforeComma);
+  verifyFormat("class AA\n"
+   ": public BB\n"
+   ", public CC {};",
+   StyleWithInheritanceBreakBeforeComma);
+  verifyFormat("struct a\n"
+   ": public aaa< // break\n"
+   "  > {};",
+   StyleWithInheritanceBreakBeforeComma);
+
+  FormatStyle StyleWithInheritanceBreakAfterColon = getLLVMStyle();
+  StyleWithInheritanceBreakAfterColon.BreakInheritanceList =
+  FormatStyle::BILS_AfterColon;
+  verifyFormat("class MyClass : public X {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("class MyClass : public X, public Y {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("class AA :\n"
+   "public BB,\n"
+   "public CC {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("struct a :\n"
+   "public aaa< // break\n"
+   "> {};",
+   StyleWithInheritanceBreakAfterColon);
 }
 
 TEST_F(FormatTest, FormatsVariableDeclarationsAfterStructOrClass) {
@@ -3726,6 +3751,23 @@
"  aa,\n"
"  bb {}",
Style);
+
+  // `ConstructorInitializerIndentWidth` actually applies to InheritanceList as well
+  Style.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
+  verifyFormat("class SomeClass\n"
+   "  : public aa,\n"
+   "public bb {};",
+   Style);
+  Style.BreakInheritanceList = FormatStyle::BILS_BeforeComma;
+  verifyFormat("class SomeClass\n"
+   "  : public aa\n"
+   "  , public bb {};",
+   Style);
+  Style.BreakInheritanceList = FormatStyle::BILS_AfterColon;
+  verifyFormat("class SomeClass :\n"
+   "  public aa,\n"
+   "  public bb {};",
+   Style);
 }
 
 #ifndef EXPENSIVE_CHECKS
@@ -9164,7 +9206,7 @@
"  (2) {}",
CtorInitializerStyle);
 
-  FormatStyle InheritanceStyle = getLLVMStyle();
+  FormatStyle InheritanceStyle = getLLVMStyleWithColumns(30);
   InheritanceStyle.SpaceBeforeInheritanceColon = false;
   verifyFormat("class Foo: public Bar {};", InheritanceStyle);
   verifyFormat("Foo::Foo() : foo(1) {}", InheritanceStyle);
@@ -9180,6 +9222,29 @@
"default:\n"
"}",
InheritanceStyle);
+  InheritanceStyle.BreakInheritanceList = FormatStyle::BILS_AfterColon;
+  verifyFormat("class Foo:\n"
+   "public aa,\n"
+   "public bb {\n"
+   "}",
+   InheritanceStyle);
+  InheritanceStyle.BreakInheritanceList = FormatStyle::BILS_BeforeComma;
+  verifyFormat("class Foo\n"
+   ": public aa\n"
+   ", public bb {\n"
+   "}",
+   InheritanceStyle);
+  

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

Would it be acceptable to introduce an option to allow enabling this behavior?
I mean would it have a chance of being integrated, or must I keep maintaining a 
fork of clang-format...


Repository:
  rC Clang

https://reviews.llvm.org/D42787



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

https://reviews.llvm.org/D37813



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

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

https://reviews.llvm.org/D43015



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

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

[Francois Ferrand via Phabricator via cfe-commits]

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

Address review comments


Repository:
  rC Clang

https://reviews.llvm.org/D33440

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2472,6 +2472,45 @@
getLLVMStyleWithColumns(40)));
 
   verifyFormat("MACRO(>)");
+
+  // Some macros contain an implicit semicolon.
+  FormatStyle Style = getLLVMStyle();
+  Style.StatementMacros.push_back("FOO");
+  verifyFormat("FOO(a) int b = 0;");
+  verifyFormat("FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(a);\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(argc, argv, \"4.0.2\")\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO()\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void f() {\n"
+   "  FOO(a)\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("FOO(a)\n"
+   "FOO(b)",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "FOO(b)\n"
+   "int c = 0;",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "int x = FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void foo(int a) { FOO(a) }\n"
+   "uint32_t bar() {}",
+   Style);
 }
 
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
@@ -10728,6 +10767,12 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.StatementMacros.clear();
+  CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros,
+  std::vector{"QUNUSED"});
+  CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros,
+  std::vector({"QUNUSED", "QT_REQUIRE_VERSION"}));
+
   Style.IncludeStyle.IncludeCategories.clear();
   std::vector ExpectedCategories = {
   {"abc/.*", 2}, {".*", 1}};
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -125,6 +125,7 @@
   void parseObjCInterfaceOrImplementation();
   bool parseObjCProtocol();
   void parseJavaScriptEs6ImportExport();
+  void parseStatementMacro();
   bool tryToParseLambda();
   bool tryToParseLambdaIntroducer();
   void tryToParseJSFunction();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -473,6 +473,10 @@
   }
   LBraceStack.pop_back();
   break;
+case tok::identifier:
+  if (!Tok->is(TT_StatementMacro))
+  break;
+  LLVM_FALLTHROUGH;
 case tok::at:
 case tok::semi:
 case tok::kw_if:
@@ -1098,6 +1102,10 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  parseStatementMacro();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
@@ -1297,6 +1305,11 @@
 return;
   }
 
+  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+parseStatementMacro();
+return;
+  }
+
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
@@ -2295,6 +2308,16 @@
   }
 }
 
+void UnwrappedLineParser::parseStatementMacro()
+{
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+}
+
 LLVM_ATTRIBUTE_UNUSED static void printDebugInfo(const UnwrappedLine ,
  StringRef Prefix = "") {
   llvm::dbgs() << Prefix << "Line(" << Line.Level
Index: lib/Format/FormatTokenLexer.h
===
--- lib/Format/FormatTokenLexer.h
+++ lib/Format/FormatTokenLexer.h
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/ADT/MapVector.h"
 
 #include 
 
@@ -99,7 +100,8 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  llvm::SmallMapVector Macros;
 
   bool FormattingDisabled;
 
Index: lib/Format/FormatTokenLexer.cpp

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

[Francois Ferrand via Phabricator via cfe-commits]

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



Comment at: lib/Format/Format.cpp:647-648
   LLVMStyle.SortUsingDeclarations = true;
+  LLVMStyle.StatementMacros.push_back("Q_UNUSED");
+  LLVMStyle.StatementMacros.push_back("QT_REQUIRE_VERSION");
 

alexfh wrote:
> What's the reason to have these in the LLVM style? The macros aren't used in 
> LLVM code.
This is similar to the default foreach macros (foreach, Q_FOREACH and 
BOOST_FOREACH) : the macros are added here so that they are the default values 
for any style, since LLVM style is also both the default style and the base 
style for any style.



Repository:
  rC Clang

https://reviews.llvm.org/D33440



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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Francois Ferrand via Phabricator via cfe-commits]

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

rebase


Repository:
  rC Clang

https://reviews.llvm.org/D43183

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1010,6 +1010,96 @@
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle indentClosingBrace = getLLVMStyle();
+  indentClosingBrace.CaseBlockIndent = FormatStyle::CBIS_ClosingBrace;
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  break;\n"
+   "  }\n"
+   "case 2: {\n"
+   "  break;\n"
+   "  }\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  } break;\n"
+   "case 2: {\n"
+   "  } break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "  f();\n"
+   "  }\n"
+   "  g();\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  {\n"
+   "g();\n"
+   "h();\n"
+   "  }\n"
+   "  break;\n"
+   "}",
+   indentClosingBrace);
+
+  FormatStyle indentBlock = getLLVMStyle();
+  indentBlock.CaseBlockIndent = FormatStyle::CBIS_Block;
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "break;\n"
+   "  }\n"
+   "case 2: {\n"
+   "break;\n"
+   "  }\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "  } break;\n"
+   "case 2: {\n"
+   "  } break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1: {\n"
+   "f();\n"
+   "  }\n"
+   "  g();\n"
+   "  break;\n"
+   "}",
+   indentBlock);
+  verifyFormat("switch (x) {\n"
+   "case 1:\n"
+   "  f();\n"
+   "  {\n"
+   "g();\n"
+   "h();\n"
+   "  }\n"
+   "  break;\n"
+   "}",
+   indentBlock);
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -10639,6 +10729,13 @@
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: true",
   AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
 
+  Style.CaseBlockIndent = FormatStyle::CBIS_Block;
+  CHECK_PARSE("CaseBlockIndent: None", CaseBlockIndent, FormatStyle::CBIS_None);
+  CHECK_PARSE("CaseBlockIndent: ClosingBrace", CaseBlockIndent,
+  FormatStyle::CBIS_ClosingBrace);
+  CHECK_PARSE("CaseBlockIndent: Block", CaseBlockIndent,
+  FormatStyle::CBIS_Block);
+
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
   CHECK_PARSE("SpaceBeforeParens: Never", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -88,6 +88,10 @@
   void parseFile();
   void parseLevel(bool HasOpeningBrace);
   void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
+  bool MunchSemi = true) {
+parseBlock(MustBeDeclaration, AddLevel ? 1 : 0, MunchSemi);
+  }
+  void parseBlock(bool MustBeDeclaration, int AddLevel,
   bool MunchSemi = true);
   void parseChildBlock();
   void parsePPDirective();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -515,7 +515,7 @@
   return h;
 }
 
-void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
+void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, int AddLevel,
  bool MunchSemi) {
   

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

[Francois Ferrand via Phabricator via cfe-commits]

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

Rebase


Repository:
  rC Clang

https://reviews.llvm.org/D32478

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

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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

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

Rebase


Repository:
  rC Clang

https://reviews.llvm.org/D37813

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

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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D37813#958222, @klimek wrote:

> Some initial design work has been done, and Krasimir said that he's 
> interested. No timeline though :(


@klimek , @krasimir : any progress on this new preprocessor-like way to 
configure macros ?


Repository:
  rC Clang

https://reviews.llvm.org/D37813



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

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

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 147024.
Typz added a comment.
Herald added a subscriber: mgrang.

Rebase on latest master


Repository:
  rC Clang

https://reviews.llvm.org/D33440

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2472,6 +2472,45 @@
getLLVMStyleWithColumns(40)));
 
   verifyFormat("MACRO(>)");
+
+  // Some macros contain an implicit semicolon
+  FormatStyle Style = getLLVMStyle();
+  Style.StatementMacros.push_back("FOO");
+  verifyFormat("FOO(a) int b = 0;");
+  verifyFormat("FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(a);\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(argc, argv, \"4.0.2\")\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO()\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void f() {\n"
+   "  FOO(a)\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("FOO(a)\n"
+   "FOO(b)",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "FOO(b)\n"
+   "int c = 0;",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "int x = FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void foo(int a) { FOO(a) }\n"
+   "uint32_t bar() {}",
+   Style);
 }
 
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
@@ -10728,6 +10767,12 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.StatementMacros.clear();
+  CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros,
+  std::vector{"QUNUSED"});
+  CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros,
+  std::vector({"QUNUSED", "QT_REQUIRE_VERSION"}));
+
   Style.IncludeStyle.IncludeCategories.clear();
   std::vector ExpectedCategories = {
   {"abc/.*", 2}, {".*", 1}};
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -125,6 +125,7 @@
   void parseObjCInterfaceOrImplementation();
   bool parseObjCProtocol();
   void parseJavaScriptEs6ImportExport();
+  void parseStatementMacro();
   bool tryToParseLambda();
   bool tryToParseLambdaIntroducer();
   void tryToParseJSFunction();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -473,6 +473,10 @@
   }
   LBraceStack.pop_back();
   break;
+case tok::identifier:
+  if (!Tok->is(TT_StatementMacro))
+  break;
+  LLVM_FALLTHROUGH;
 case tok::at:
 case tok::semi:
 case tok::kw_if:
@@ -1098,6 +1102,10 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  parseStatementMacro();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
@@ -1297,6 +1305,11 @@
 return;
   }
 
+  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+parseStatementMacro();
+return;
+  }
+
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
@@ -2295,6 +2308,16 @@
   }
 }
 
+void UnwrappedLineParser::parseStatementMacro()
+{
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+}
+
 LLVM_ATTRIBUTE_UNUSED static void printDebugInfo(const UnwrappedLine ,
  StringRef Prefix = "") {
   llvm::dbgs() << Prefix << "Line(" << Line.Level
Index: lib/Format/FormatTokenLexer.h
===
--- lib/Format/FormatTokenLexer.h
+++ lib/Format/FormatTokenLexer.h
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/ADT/MapVector.h"
 
 #include 
 
@@ -99,7 +100,8 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  llvm::SmallMapVector Macros;
 
   bool FormattingDisabled;
 
Index: lib/Format/FormatTokenLexer.cpp

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

https://reviews.llvm.org/D43015



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

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

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

Rebase on latest master


Repository:
  rC Clang

https://reviews.llvm.org/D43015

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1316,15 +1316,40 @@
   verifyFormat("class ::A::B {};");
 }
 
-TEST_F(FormatTest, BreakBeforeInheritanceComma) {
-  FormatStyle StyleWithInheritanceBreak = getLLVMStyle();
-  StyleWithInheritanceBreak.BreakBeforeInheritanceComma = true;
-
-  verifyFormat("class MyClass : public X {};", StyleWithInheritanceBreak);
+TEST_F(FormatTest, BreakInheritanceStyle) {
+  FormatStyle StyleWithInheritanceBreakBeforeComma = getLLVMStyle();
+  StyleWithInheritanceBreakBeforeComma.BreakInheritanceList =
+  FormatStyle::BILS_BeforeComma;
+  verifyFormat("class MyClass : public X {};",
+   StyleWithInheritanceBreakBeforeComma);
   verifyFormat("class MyClass\n"
": public X\n"
", public Y {};",
-   StyleWithInheritanceBreak);
+   StyleWithInheritanceBreakBeforeComma);
+  verifyFormat("class AA\n"
+   ": public BB\n"
+   ", public CC {};",
+   StyleWithInheritanceBreakBeforeComma);
+  verifyFormat("struct a\n"
+   ": public aaa< // break\n"
+   "  > {};",
+   StyleWithInheritanceBreakBeforeComma);
+
+  FormatStyle StyleWithInheritanceBreakAfterColon = getLLVMStyle();
+  StyleWithInheritanceBreakAfterColon.BreakInheritanceList =
+  FormatStyle::BILS_AfterColon;
+  verifyFormat("class MyClass : public X {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("class MyClass : public X, public Y {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("class AA :\n"
+   "public BB,\n"
+   "public CC {};",
+   StyleWithInheritanceBreakAfterColon);
+  verifyFormat("struct a :\n"
+   "public aaa< // break\n"
+   "> {};",
+   StyleWithInheritanceBreakAfterColon);
 }
 
 TEST_F(FormatTest, FormatsVariableDeclarationsAfterStructOrClass) {
@@ -3726,6 +3751,23 @@
"  aa,\n"
"  bb {}",
Style);
+
+  // `ConstructorInitializerIndentWidth` actually applies to InheritanceList as well
+  Style.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
+  verifyFormat("class SomeClass\n"
+   "  : public aa,\n"
+   "public bb {};",
+   Style);
+  Style.BreakInheritanceList = FormatStyle::BILS_BeforeComma;
+  verifyFormat("class SomeClass\n"
+   "  : public aa\n"
+   "  , public bb {};",
+   Style);
+  Style.BreakInheritanceList = FormatStyle::BILS_AfterColon;
+  verifyFormat("class SomeClass :\n"
+   "  public aa,\n"
+   "  public bb {};",
+   Style);
 }
 
 #ifndef EXPENSIVE_CHECKS
@@ -9101,7 +9143,7 @@
"  (2) {}",
CtorInitializerStyle);
 
-  FormatStyle InheritanceStyle = getLLVMStyle();
+  FormatStyle InheritanceStyle = getLLVMStyleWithColumns(30);
   InheritanceStyle.SpaceBeforeInheritanceColon = false;
   verifyFormat("class Foo: public Bar {};", InheritanceStyle);
   verifyFormat("Foo::Foo() : foo(1) {}", InheritanceStyle);
@@ -9117,6 +9159,29 @@
"default:\n"
"}",
InheritanceStyle);
+  InheritanceStyle.BreakInheritanceList = FormatStyle::BILS_AfterColon;
+  verifyFormat("class Foo:\n"
+   "public aa,\n"
+   "public bb {\n"
+   "}",
+   InheritanceStyle);
+  InheritanceStyle.BreakInheritanceList = FormatStyle::BILS_BeforeComma;
+  verifyFormat("class Foo\n"
+   ": public aa\n"
+   ", public bb {\n"
+   "}",
+   

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL332436: clang-format: Allow optimizer to break template 
declaration. (authored by Typz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42684?vs=147005=147006#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42684

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

Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -351,14 +351,44 @@
   /// \endcode
   bool AlwaysBreakBeforeMultilineStrings;
 
-  /// If ``true``, always break after the ``template<...>`` of a template
-  /// declaration.
-  /// \code
-  ///true:  false:
-  ///template   vs. template  class C {};
-  ///class C {};
-  /// \endcode
-  bool AlwaysBreakTemplateDeclarations;
+  /// Different ways to break after the template declaration.
+  enum BreakTemplateDeclarationsStyle {
+  /// Do not force break before declaration.
+  /// ``PenaltyBreakTemplateDeclaration`` is taken into account.
+  /// \code
+  ///template  T foo() {
+  ///}
+  ///template  T foo(int a,
+  ///int b) {
+  ///}
+  /// \endcode
+  BTDS_No,
+  /// Force break after template declaration only when the following
+  /// declaration spans multiple lines.
+  /// \code
+  ///template  T foo() {
+  ///}
+  ///template 
+  ///T foo(int a,
+  ///  int b) {
+  ///}
+  /// \endcode
+  BTDS_MultiLine,
+  /// Always break after template declaration.
+  /// \code
+  ///template 
+  ///T foo() {
+  ///}
+  ///template 
+  ///T foo(int a,
+  ///  int b) {
+  ///}
+  /// \endcode
+  BTDS_Yes
+  };
+
+  /// The template declaration breaking style to use.
+  BreakTemplateDeclarationsStyle AlwaysBreakTemplateDeclarations;
 
   /// If ``false``, a function call's arguments will either be all on the
   /// same line or will have one line each.
@@ -1295,6 +1325,9 @@
   /// The penalty for each line break introduced inside a string literal.
   unsigned PenaltyBreakString;
 
+  /// The penalty for breaking after template declaration.
+  unsigned PenaltyBreakTemplateDeclaration;
+
   /// The penalty for each character outside of the column limit.
   unsigned PenaltyExcessCharacter;
 
@@ -1679,6 +1712,8 @@
PenaltyBreakString == R.PenaltyBreakString &&
PenaltyExcessCharacter == R.PenaltyExcessCharacter &&
PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
+   PenaltyBreakTemplateDeclaration ==
+   R.PenaltyBreakTemplateDeclaration &&
PointerAlignment == R.PointerAlignment &&
RawStringFormats == R.RawStringFormats &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2338,6 +2338,8 @@
   return 2;
 return 1;
   }
+  if (Left.ClosesTemplateDeclaration)
+return Style.PenaltyBreakTemplateDeclaration;
   if (Left.is(TT_ConditionalExpr))
 return prec::Conditional;
   prec::Level Level = Left.getPrecedence();
@@ -2869,7 +2871,7 @@
   if (Right.Previous->ClosesTemplateDeclaration &&
   Right.Previous->MatchingParen &&
   Right.Previous->MatchingParen->NestingLevel == 0 &&
-  Style.AlwaysBreakTemplateDeclarations)
+  Style.AlwaysBreakTemplateDeclarations == FormatStyle::BTDS_Yes)
 return true;
   if (Right.is(TT_CtorInitializerComma) &&
   Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma &&
Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -402,6 +402,12 @@
 Style.Language == FormatStyle::LK_JavaScript))
 return true;
 
+  // If the template declaration spans multiple lines, force wrap before the
+  // function/class declaration
+  if (Previous.ClosesTemplateDeclaration &&
+  State.Stack.back().BreakBeforeParameter)
+return true;
+
   if (State.Column <= NewLineColumn)
 return false;
 
@@ -453,7 +459,7 @@
 // for cases where 

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

[Francois Ferrand via Phabricator via cfe-commits]

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

Rebase on latest master & address review comments


Repository:
  rC Clang

https://reviews.llvm.org/D42684

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5475,7 +5475,7 @@
"const typename  aaa);");
 
   FormatStyle AlwaysBreak = getLLVMStyle();
-  AlwaysBreak.AlwaysBreakTemplateDeclarations = true;
+  AlwaysBreak.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
   verifyFormat("template \nclass C {};", AlwaysBreak);
   verifyFormat("template \nvoid f();", AlwaysBreak);
   verifyFormat("template \nvoid f() {}", AlwaysBreak);
@@ -5493,6 +5493,32 @@
"public:\n"
"  E *f();\n"
"};");
+
+  FormatStyle NeverBreak = getLLVMStyle();
+  NeverBreak.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_No;
+  verifyFormat("template  class C {};", NeverBreak);
+  verifyFormat("template  void f();", NeverBreak);
+  verifyFormat("template  void f() {}", NeverBreak);
+  verifyFormat("template \nvoid foo(aa ) {}",
+   NeverBreak);
+  verifyFormat("void aaa(\n"
+   "ccc);",
+   NeverBreak);
+  verifyFormat("template  class Fooo,\n"
+   "  template  class Baaar>\n"
+   "struct C {};",
+   NeverBreak);
+  verifyFormat("template  // T can be A, B or C.\n"
+   "struct C {};",
+   NeverBreak);
+  verifyFormat("template  class A {\n"
+   "public:\n"
+   "  E *f();\n"
+   "};", NeverBreak);
+  NeverBreak.PenaltyBreakTemplateDeclaration = 100;
+  verifyFormat("template  void\nfoo(aa ) {}",
+   NeverBreak);
 }
 
 TEST_F(FormatTest, WrapsTemplateParameters) {
@@ -10440,7 +10466,6 @@
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
-  CHECK_PARSE_BOOL(AlwaysBreakTemplateDeclarations);
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
@@ -10506,6 +10531,8 @@
   PenaltyBreakAssignment, 1234u);
   CHECK_PARSE("PenaltyBreakBeforeFirstCallParameter: 1234",
   PenaltyBreakBeforeFirstCallParameter, 1234u);
+  CHECK_PARSE("PenaltyBreakTemplateDeclaration: 1234",
+  PenaltyBreakTemplateDeclaration, 1234u);
   CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u);
   CHECK_PARSE("PenaltyReturnTypeOnItsOwnLine: 1234",
   PenaltyReturnTypeOnItsOwnLine, 1234u);
@@ -10660,6 +10687,18 @@
   AlwaysBreakAfterReturnType,
   FormatStyle::RTBS_TopLevelDefinitions);
 
+  Style.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: No", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_No);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: MultiLine", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_MultiLine);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: Yes", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_Yes);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: false", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_MultiLine);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: true", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_Yes);
+
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
   CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: None",
   AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_None);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2338,6 +2338,8 @@
   return 2;
 return 1;
   }
+  if (Left.ClosesTemplateDeclaration)
+return Style.PenaltyBreakTemplateDeclaration;
   if (Left.is(TT_ConditionalExpr))
 return prec::Conditional;
   prec::Level Level = Left.getPrecedence();
@@ -2869,7 +2871,7 @@
   if (Right.Previous->ClosesTemplateDeclaration &&
   Right.Previous->MatchingParen &&
   Right.Previous->MatchingParen->NestingLevel == 0 &&
-  

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC332434: clang-format: tweak formatting of variable 
initialization blocks (authored by Typz, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43290?vs=147002=147003#toc

Repository:
  rC Clang

https://reviews.llvm.org/D43290

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


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2311,6 +2311,8 @@
   if (Left.opensScope()) {
 if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
   return 0;
+if (Left.is(tok::l_brace) && !Style.Cpp11BracedListStyle)
+  return 19;
 return Left.ParameterCount > 1 ? Style.PenaltyBreakBeforeFirstCallParameter
: 19;
   }
@@ -3048,6 +3050,9 @@
   if (Left.is(tok::equal) && !Right.isOneOf(tok::kw_default, tok::kw_delete) &&
   Line.Type == LT_VirtualFunctionDecl && Left.NestingLevel == 0)
 return false;
+  if (Left.is(tok::equal) && Right.is(tok::l_brace) &&
+  !Style.Cpp11BracedListStyle)
+return false;
   if (Left.is(tok::l_paren) && Left.is(TT_AttributeParen))
 return false;
   if (Left.is(tok::l_paren) && Left.Previous &&
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6708,6 +6708,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyle();
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable =\n"
+   "{{\"a\", 0},\n"
+   " {\"b\", 1},\n"
+   " {\"c\", 2}};",
+   AvoidBreakingFirstArgument);
+
   // Binpacking only if there is no trailing comma
   verifyFormat("const Aa a = {aa, bb,\n"
"  cc, dd};",
@@ -6864,6 +6873,21 @@
   verifyFormat("vector foo = { ::SomeGlobalFunction() };", ExtraSpaces);
   verifyFormat("const struct A a = { .a = 1, .b = 2 };", ExtraSpaces);
   verifyFormat("const struct A a = { [0] = 1, [1] = 2 };", ExtraSpaces);
+
+  // Avoid breaking between initializer/equal sign and opening brace
+  ExtraSpaces.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
+  verifyFormat("const std::unordered_map MyHashTable{\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2311,6 +2311,8 @@
   if (Left.opensScope()) {
 if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
   return 0;
+if (Left.is(tok::l_brace) && !Style.Cpp11BracedListStyle)
+  return 19;
 return Left.ParameterCount > 1 ? Style.PenaltyBreakBeforeFirstCallParameter
: 19;
   }
@@ -3048,6 +3050,9 @@
   if (Left.is(tok::equal) && !Right.isOneOf(tok::kw_default, tok::kw_delete) &&
   Line.Type == LT_VirtualFunctionDecl && Left.NestingLevel == 0)
 return false;
+  if (Left.is(tok::equal) && Right.is(tok::l_brace) &&
+  !Style.Cpp11BracedListStyle)
+return false;
   if (Left.is(tok::l_paren) && Left.is(TT_AttributeParen))
 return false;
   if (Left.is(tok::l_paren) && Left.Previous &&
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6708,6 +6708,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyle();
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable =\n"
+   "{{\"a\", 0},\n"
+   " {\"b\", 1},\n"
+   " {\"c\", 2}};",
+   

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 147002.
Typz marked 6 inline comments as done.
Typz added a comment.

Address review comment & rebase


Repository:
  rC Clang

https://reviews.llvm.org/D43290

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6708,6 +6708,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyle();
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable =\n"
+   "{{\"a\", 0},\n"
+   " {\"b\", 1},\n"
+   " {\"c\", 2}};",
+   AvoidBreakingFirstArgument);
+
   // Binpacking only if there is no trailing comma
   verifyFormat("const Aa a = {aa, bb,\n"
"  cc, dd};",
@@ -6864,6 +6873,21 @@
   verifyFormat("vector foo = { ::SomeGlobalFunction() };", ExtraSpaces);
   verifyFormat("const struct A a = { .a = 1, .b = 2 };", ExtraSpaces);
   verifyFormat("const struct A a = { [0] = 1, [1] = 2 };", ExtraSpaces);
+
+  // Avoid breaking between initializer/equal sign and opening brace
+  ExtraSpaces.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
+  verifyFormat("const std::unordered_map MyHashTable{\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2311,6 +2311,8 @@
   if (Left.opensScope()) {
 if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
   return 0;
+if (Left.is(tok::l_brace) && !Style.Cpp11BracedListStyle)
+  return 19;
 return Left.ParameterCount > 1 ? Style.PenaltyBreakBeforeFirstCallParameter
: 19;
   }
@@ -3048,6 +3050,9 @@
   if (Left.is(tok::equal) && !Right.isOneOf(tok::kw_default, tok::kw_delete) &&
   Line.Type == LT_VirtualFunctionDecl && Left.NestingLevel == 0)
 return false;
+  if (Left.is(tok::equal) && Right.is(tok::l_brace) &&
+  !Style.Cpp11BracedListStyle)
+return false;
   if (Left.is(tok::l_paren) && Left.is(TT_AttributeParen))
 return false;
   if (Left.is(tok::l_paren) && Left.Previous &&


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6708,6 +6708,15 @@
"};");
   verifyFormat("#define A {a, a},");
 
+  // Avoid breaking between equal sign and opening brace
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyle();
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable =\n"
+   "{{\"a\", 0},\n"
+   " {\"b\", 1},\n"
+   " {\"c\", 2}};",
+   AvoidBreakingFirstArgument);
+
   // Binpacking only if there is no trailing comma
   verifyFormat("const Aa a = {aa, bb,\n"
"  cc, dd};",
@@ -6864,6 +6873,21 @@
   verifyFormat("vector foo = { ::SomeGlobalFunction() };", ExtraSpaces);
   verifyFormat("const struct A a = { .a = 1, .b = 2 };", ExtraSpaces);
   verifyFormat("const struct A a = { [0] = 1, [1] = 2 };", ExtraSpaces);
+
+  // Avoid breaking between initializer/equal sign and opening brace
+  ExtraSpaces.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map MyHashTable = {\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   "  { \"c\", 2 }\n"
+   "};",
+   ExtraSpaces);
+  verifyFormat("const std::unordered_map MyHashTable{\n"
+   "  { \"a\", 0 },\n"
+   "  { \"b\", 1 },\n"
+   " 

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

@djasper : ping?


Repository:
  rC Clang

https://reviews.llvm.org/D42684



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

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

@djasper : ping?


Repository:
  rC Clang

https://reviews.llvm.org/D43015



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

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

@djasper : ping?


Repository:
  rC Clang

https://reviews.llvm.org/D43290



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

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

You are right, I did not mean it would help with the handling of these macros; 
but it may be related on the configuration-side of things : adding an option 
for listing these macros may hit the same limitation, and the same mean of 
storing (in the code) the list of these macros may be used.


Repository:
  rC Clang

https://reviews.llvm.org/D44765



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

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

A generic (or at least extandable) approach to specifying macro behaviors was 
introduced here: https://reviews.llvm.org/D33440

But this change seem to be on hold, as a configuration based on providing 
"macro definitions" seem to be preferable and kind-of in the pipe, though with 
no defined timeline...


Repository:
  rC Clang

https://reviews.llvm.org/D44765



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

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

https://reviews.llvm.org/D42684



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

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

https://reviews.llvm.org/D43015



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

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

https://reviews.llvm.org/D43290



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