[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:797
+AlignConsecutiveMacros:
+  Enabled: true
+  AcrossEmptyLines: true

MyDeveloperDay wrote:
> why do we need Enabled?
> 
> isn't it just
> 
> ```
> false:
> AcrossEmptyLines: false
> AcrossComments: false
> 
> true:
> AcrossEmptyLines: true
> AcrossComments: true
> ```
The struct is a bit older. And we need `Enabled`, one might wish to align only 
really consecutive comments, as the option right now does. (Same for macros, 
assignments, etc..)



Comment at: clang/lib/Format/Format.cpp:649
 IO.mapOptional("AlignOperands", Style.AlignOperands);
-IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
 IO.mapOptional("AllowAllArgumentsOnNextLine",

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > you can't remove an option, otherwise you'll break everyones .clang-format
> > That's not correct. We have done it:
> > D108752 -> D108882 -> D127390
> > 
> > You can remove (and in this case should), but you still need to parse it 
> > and act accordingly. Which is done as far as I can see.
> sorry thats what I meant, you can remove it, but you have to make it turn on 
> the correct new style that keeps exactly the old user case, and you can't 
> remove it from the configuration parsing otherwise anyone who has it in their 
> .clang-format is immediately broken with an unknown option.
> 
> to be honest this is an annoyance for introducing new features, which at some 
> point I'd like to drop, you can't have a new option which is not read
> 
> For me, when we introduced new languages, or new features like InsertBraces 
> etc.. I want to put it in my .clang-format even though other people they 
> aren't using a version that uses it. (becuase it won't impact them), i.e. it 
> won't add braces or correct QualifierOrder that doesn't bother me
> 
Do you disagree that it actually is parsed?

But that compatibility parsing has nothing to do with ignoring unknown (new) 
options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:785
+  - Consecutive
+  - AcrossEmptyLines
+  - AcrossComments

yusuke-kadowaki wrote:
> MyDeveloperDay wrote:
> > may be AcrossEmptyLines should be a number (to mean the number of empty 
> > lines)
> We need to introduce a new struct to do that since AlignConsecutiveStyle is 
> shared with some options and not possible to be changed. Plus I think more 
> than two empty lines are formatted to one empty line anyway.
There `MaxEmptyLinesToKeep` which would allow to set it higher.

If we want to change the `bool` to an `unsigned`, then that should be a 
different change.



Comment at: clang/docs/ClangFormatStyleOptions.rst:794
+
+AlignConsecutiveMacros: AcrossEmptyLines
+

yusuke-kadowaki wrote:
> MyDeveloperDay wrote:
> > Should this say `AlignedConsecutuveCommets`
> No. This part is a documentation about AlignConsecutiveStyle type, which is 
> appended automatically after all the options that take AlignConsecutiveStyle 
> type.
Which we could change to reflect that it's used for multiple options.



Comment at: clang/docs/ClangFormatStyleOptions.rst:797
+AlignConsecutiveMacros:
+  Enabled: true
+  AcrossEmptyLines: true

yusuke-kadowaki wrote:
> MyDeveloperDay wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > why do we need Enabled?
> > > > 
> > > > isn't it just
> > > > 
> > > > ```
> > > > false:
> > > > AcrossEmptyLines: false
> > > > AcrossComments: false
> > > > 
> > > > true:
> > > > AcrossEmptyLines: true
> > > > AcrossComments: true
> > > > ```
> > > The struct is a bit older. And we need `Enabled`, one might wish to align 
> > > only really consecutive comments, as the option right now does. (Same for 
> > > macros, assignments, etc..)
> > I'm still not sure I understand, plus are those use cases you describe 
> > tested, I couldn't see that. 
> > 
> > If feels like what you are saying is that AlignConsecutiveStyle is used 
> > elsewhere and it has options that don't pertain to aligning comments? 
> > 
> > I couldn't tell if feel like this documentation is describing something 
> > other than AligningTrailingComments, if I'm confused users could be too? 
> As for the Enabled option,
> Enabled: true
> AcrossEmptyLines: false
> 
> is supposed to work in the same way as the old style AlignTrailingComments. 
> So the tests of AlignTrailingComments are the test cases.
> 
> For the documentation, I also thought it was a bit confusing when I first saw 
> it because the description of the option and the style is kind of connected. 
> But as I also mentioned above, this part is automatically append and it 
> affects all the other options that have AlignConsecutiveStyle if we change. 
> So I think it should be another patch even if we are to modify it.
> I'm still not sure I understand, plus are those use cases you describe 
> tested, I couldn't see that. 
> 
> If feels like what you are saying is that AlignConsecutiveStyle is used 
> elsewhere and it has options that don't pertain to aligning comments? 
> 
> I couldn't tell if feel like this documentation is describing something other 
> than AligningTrailingComments, if I'm confused users could be too? 

It was added in D93986 as an enum, and in D119599 made into a struct which then 
had 2 options only valid for assignments, not the macros or declarations. Both 
accepted by you.

So I see no problem, but think it is the right thing, to port aligning comments 
to the struct, even if the flag `AccrossComments` is a noop in this case. When 
this lands I actually plan to add a flag only used by comments.




Comment at: clang/unittests/Format/FormatTestComments.cpp:2863
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"

Interesting would be a comment which is split, do we continue to align, or not?



Comment at: clang/unittests/Format/FormatTestComments.cpp:2958
+   "   long b;",
+   Style));   
+}

Trailing whitespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Please mark comments as done, when the discussion has ended.




Comment at: clang/lib/Format/TokenAnnotator.cpp:2376
+const FormatToken *Prev = PrevToken;
+if (Prev->is(tok::r_square) && (Prev = Prev->getPreviousNonComment()) &&
+Prev->is(tok::l_square) && (Prev = Prev->getPreviousNonComment()) &&

I don't know if this is nice. I think I would prefer 
`PrevToken->getPreviousNonComment()` and subsequently 
`PrevToken->getPreviousNonComment()->getPreviousNonComment()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:141
 
-  /// Alignment options.
-  ///
-  /// They can also be read as a whole for compatibility. The choices are:
+  /// Alignment styles of ``AlignConsecutiveStyle`` are:
   /// - None

That I wouldn't change.



Comment at: clang/include/clang/Format/Format.h:154
   /// \code
   ///   AlignConsecutiveMacros: AcrossEmptyLines
   ///

The change/addition has to be here, since here it directly states 
`AlignConsecutiveMacros`.



Comment at: clang/unittests/Format/FormatTestComments.cpp:2863
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"

yusuke-kadowaki wrote:
> HazardyKnusperkeks wrote:
> > Interesting would be a comment which is split, do we continue to align, or 
> > not?
> Could you give me a specific example?
Something like
```
int foo = 2323234; // Comment
int bar = 52323;   // This is a very long comment, ...
   // which is wrapped around.

int x = 2; // Is this still aligned?
```
You may need to make the comment longer or reduce the column limit, as often 
used for testing the wrapping behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D133087: [clang-format][NFC][Docs] fix wrong example of warping class definitions

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

You have to make the change in the `Format.h` and run 
`clang/docs/tools/dump_format_style.py` which generates the rst.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133087

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

[PATCH] D133093: [clang-format] Fix a bug in merging blocks with a wrapped l_brace

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:716
+auto IsCtrlStmt = [](const auto &Line) {
+  return Line.First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
+ tok::kw_do, tok::kw_for, TT_ForEachMacro);

switch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133093

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

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In my opinion we are nearly done.




Comment at: clang/docs/ClangFormatStyleOptions.rst:3698
 
-  QualifierOrder: ['inline', 'static', 'type', 'const']
+  QualifierOrder: ['inline', 'static' , 'type', 'const']
 

Anyone knows where this comes from?
You are using the script to generate the rst, right?



Comment at: clang/include/clang/Format/Format.h:154
   /// \code
   ///   AlignConsecutiveMacros: AcrossEmptyLines
   ///

yusuke-kadowaki wrote:
> HazardyKnusperkeks wrote:
> > The change/addition has to be here, since here it directly states 
> > `AlignConsecutiveMacros`.
> What are you meaning to say here? I thought saying `AlignConsecutiveStyle` is 
> used for multiple options is what we are trying to do.
> 
> Should we say something like, 
> > Here AlignConsecutiveMacros is used as an example, but in practice 
> > AlignConsecutiveStyle is also used with other options.
> in this place?
The example mentions explicitly only `AlignConsecutiveMacros` which is a bit 
misleading if you are only looking at the documentation of 
`AlignConsecutiveAssignments` for example.

This is not your fault, and I'm fine with ignoring it here. A separate patch to 
fix that is welcome. :)



Comment at: clang/lib/Format/WhitespaceManager.cpp:930
   unsigned Newlines = 0;
+  unsigned int NewLineThreshold = 
Style.AlignConsecutiveTrailingComments.AcrossEmptyLines ? 2 : 1;
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {

And accompanied by a short test. That should be everything to support the 
mixture of the options, right?



Comment at: clang/lib/Format/WhitespaceManager.cpp:984
+}
+else if (BreakBeforeNext || Newlines > NewLineThreshold ||
(ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||

Is this clang-format formatted?



Comment at: clang/unittests/Format/FormatTestComments.cpp:4262
 /\
-/ 
+/
   )",

Please revert this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D133087: [clang-format][NFC][Docs] fix wrong example of warping class definitions

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

Thanks.


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

https://reviews.llvm.org/D133087

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

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D132131#3766590 , @MyDeveloperDay 
wrote:

> Is there a technical reason for reusing the struct rather than introducing a 
> new one?

I see, good point. Really only if one would port the implementation to 
`AlignTokens`. If that's feasible (or sensible) I don't know. Otherwise one 
will only reuse the parsing code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D131750: [clang-format] Distinguish logical and after bracket from reference

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a subscriber: tstellar.
HazardyKnusperkeks added a comment.

You have to ask @tstellar to cherry-pick it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131750

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

[PATCH] D133087: [clang-format][NFC][Docs] fix wrong example of warping class definitions

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D133087#3768928 , @Passw wrote:

> I do not have commit access, please help to commit this change. 
> Thanks.

Please state name and email for the commit.


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

https://reviews.llvm.org/D133087

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

[PATCH] D129934: [clang-format][docs] Fix incorrect 'clang-format 4' option markers

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG961fd77687d2: [clang-format][docs] Fix incorrect 
'clang-format 4' option markers (authored by kuzkry, committed by 
HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129934

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


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2980,7 +2980,7 @@
   ////* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment 
with plenty of
   /// * information */
   /// \endcode
-  /// \version 4
+  /// \version 3.8
   bool ReflowComments;
   // clang-format on
 
@@ -3236,7 +3236,7 @@
   /// insensitive fashion.
   /// If ``CaseSensitive``, includes are sorted in an alphabetical or case
   /// sensitive fashion.
-  /// \version 4
+  /// \version 3.8
   SortIncludesOptions SortIncludes;
 
   /// Position for Java Static imports.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -3670,7 +3670,7 @@
 
 
 
-**ReflowComments** (``Boolean``) :versionbadge:`clang-format 4`
+**ReflowComments** (``Boolean``) :versionbadge:`clang-format 3.8`
   If ``true``, clang-format will attempt to re-flow comments.
 
   .. code-block:: c++
@@ -3910,7 +3910,7 @@
int bar;   int bar;
  } // namespace b   } // namespace b
 
-**SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 4`
+**SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 3.8`
   Controls if and how clang-format will sort ``#includes``.
   If ``Never``, includes are never sorted.
   If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2980,7 +2980,7 @@
   ////* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
   /// * information */
   /// \endcode
-  /// \version 4
+  /// \version 3.8
   bool ReflowComments;
   // clang-format on
 
@@ -3236,7 +3236,7 @@
   /// insensitive fashion.
   /// If ``CaseSensitive``, includes are sorted in an alphabetical or case
   /// sensitive fashion.
-  /// \version 4
+  /// \version 3.8
   SortIncludesOptions SortIncludes;
 
   /// Position for Java Static imports.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -3670,7 +3670,7 @@
 
 
 
-**ReflowComments** (``Boolean``) :versionbadge:`clang-format 4`
+**ReflowComments** (``Boolean``) :versionbadge:`clang-format 3.8`
   If ``true``, clang-format will attempt to re-flow comments.
 
   .. code-block:: c++
@@ -3910,7 +3910,7 @@
int bar;   int bar;
  } // namespace b   } // namespace b
 
-**SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 4`
+**SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 3.8`
   Controls if and how clang-format will sort ``#includes``.
   If ``Never``, includes are never sorted.
   If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126365: [git-clang-format] Stop ignoring changes for files with space in path

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D126365#3736864 , @Eitot wrote:

> In D126365#3565891 , @curdeius 
> wrote:
>
>> @Eitot, do you need help landing this?
>
> I do need help. Could someone land this for me?

Please state a name and email for the commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126365

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

[PATCH] D133087: [clang-format][NFC][Docs] fix wrong example of warping class definitions

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D133087#3768928 , @Passw wrote:

> I do not have commit access, please help to commit this change. 
> Thanks.

Please state a name and email for the commit.


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

https://reviews.llvm.org/D133087

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

[PATCH] D131978: [clang-format] Concepts: allow identifiers after negation

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Please state a name and email for the commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131978

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

[PATCH] D132295: [clang-format] Change heuristic for locating lambda template arguments

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Can you please rebase the patch, it doesn't apply anymore.
Sorry for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132295

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

[PATCH] D132189: [clang-format] Don't put `noexcept` on empty line following constructor

[Björn Schäpers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf54d42abcf82: [clang-format] Don't put `noexcept` on 
empty line following constructor (authored by rymiel, committed by 
HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132189

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -788,6 +788,19 @@
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsFunctionAnnotations) {
+  auto Tokens = annotate("template \n"
+ "DEPRECATED(\"Use NewClass::NewFunction instead.\")\n"
+ "string OldFunction(const string ¶meter) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_FunctionAnnotationRParen);
+
+  Tokens = annotate("template \n"
+"A(T) noexcept;");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9607,6 +9607,15 @@
   verifyFormat("template  // T can be A, B or C.\n"
"struct C {};",
AlwaysBreak);
+  verifyFormat("template \n"
+   "C(T) noexcept;",
+   AlwaysBreak);
+  verifyFormat("template \n"
+   "ClassName(T) noexcept;",
+   AlwaysBreak);
+  verifyFormat("template \n"
+   "POOR_NAME(T) noexcept;",
+   AlwaysBreak);
   verifyFormat("template  class A {\n"
"public:\n"
"  E *f();\n"
@@ -9617,6 +9626,9 @@
   verifyFormat("template  class C {};", NeverBreak);
   verifyFormat("template  void f();", NeverBreak);
   verifyFormat("template  void f() {}", NeverBreak);
+  verifyFormat("template  C(T) noexcept;", NeverBreak);
+  verifyFormat("template  ClassName(T) noexcept;", NeverBreak);
+  verifyFormat("template  POOR_NAME(T) noexcept;", NeverBreak);
   verifyFormat("template \nvoid foo(aa "
") {}",
NeverBreak);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1924,7 +1924,7 @@
   !Current.Next->isBinaryOperator() &&
   !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
  tok::comma, tok::period, tok::arrow,
- tok::coloncolon)) {
+ tok::coloncolon, tok::kw_noexcept)) {
 if (FormatToken *AfterParen = Current.MatchingParen->Next) {
   // Make sure this isn't the return type of an Obj-C block declaration
   if (AfterParen->isNot(tok::caret)) {


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -788,6 +788,19 @@
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsFunctionAnnotations) {
+  auto Tokens = annotate("template \n"
+ "DEPRECATED(\"Use NewClass::NewFunction instead.\")\n"
+ "string OldFunction(const string ¶meter) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_FunctionAnnotationRParen);
+
+  Tokens = annotate("template \n"
+"A(T) noexcept;");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9607,6 +9607,15 @@
   verifyFormat("template  // T can be A, B or C.\n"
"struct C {};",
AlwaysBreak);
+  verifyFormat("template \n"
+   "C(T) noexcept;

[PATCH] D132762: [clang-format] Allow `throw` to be a keyword in front of casts

[Björn Schäpers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc6e7752f8e14: [clang-format] Allow `throw` to be a keyword 
in front of casts (authored by rymiel, committed by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132762

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -319,6 +319,34 @@
   EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_CastRParen);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsCasts) {
+  auto Tokens = annotate("(void)p;");
+  EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("auto x = (Foo)p;");
+  EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("(std::vector)p;");
+  EXPECT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("return (Foo)p;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_CastRParen);
+
+  Tokens = annotate("throw (Foo)p;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_CastRParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsDynamicExceptionSpecifier) {
+  auto Tokens = annotate("void f() throw(int);");
+  EXPECT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_throw, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsFunctionRefQualifiers) {
   auto Tokens = annotate("void f() &;");
   EXPECT_EQ(Tokens.size(), 7u) << Tokens;
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11038,6 +11038,7 @@
   verifyFormat("my_int a = (my_int)2.0f;");
   verifyFormat("my_int a = (my_int)sizeof(int);");
   verifyFormat("return (my_int)aaa;");
+  verifyFormat("throw (my_int)aaa;");
   verifyFormat("#define x ((int)-1)");
   verifyFormat("#define LENGTH(x, y) (x) - (y) + 1");
   verifyFormat("#define p(q) ((int *)&q)");
@@ -15730,6 +15731,7 @@
   verifyFormat("Type *A = ( Type * )P;", Spaces);
   verifyFormat("Type *A = ( vector )P;", Spaces);
   verifyFormat("x = ( int32 )y;", Spaces);
+  verifyFormat("throw ( int32 )x;", Spaces);
   verifyFormat("int a = ( int )(2.0f);", Spaces);
   verifyFormat("#define AA(X) sizeof((( X * )NULL)->a)", Spaces);
   verifyFormat("my_int a = ( my_int )sizeof(int);", Spaces);
@@ -15793,6 +15795,7 @@
   verifyFormat("#define CONF_BOOL(x) ( bool ) (x)", Spaces);
   verifyFormat("bool *y = ( bool * ) ( void * ) (x);", Spaces);
   verifyFormat("bool *y = ( bool * ) (x);", Spaces);
+  verifyFormat("throw ( int32 ) x;", Spaces);
 
   // Run subset of tests again with:
   Spaces.SpacesInCStyleCastParentheses = false;
@@ -15817,6 +15820,8 @@
   verifyFormat("bool *y = (bool *) (void *) (x);", Spaces);
   verifyFormat("bool *y = (bool *) (void *) (int) (x);", Spaces);
   verifyFormat("bool *y = (bool *) (void *) (int) foo(x);", Spaces);
+  verifyFormat("throw (int32) x;", Spaces);
+
   Spaces.ColumnLimit = 80;
   Spaces.IndentWidth = 4;
   Spaces.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2153,7 +2153,7 @@
   // before the parentheses, this is unlikely to be a cast.
   if (LeftOfParens->Tok.getIdentifierInfo() &&
   !LeftOfParens->isOneOf(Keywords.kw_in, tok::kw_return, tok::kw_case,
- tok::kw_delete)) {
+ tok::kw_delete, tok::kw_throw)) {
 return false;
   }
 
@@ -3315,6 +3315,10 @@
   !Right.isOneOf(tok::semi, tok::r_paren, tok::hashhash)) {
 return true;
   }
+  if (Left.is(tok::kw_throw) && Right.is(tok::l_paren) && Right.MatchingParen 
&&
+  Right.MatchingParen->is(TT_CastRParen)) {
+return true;
+  }
   if (Style.isJson() && Left.is(tok::string_literal) && Right.is(tok::colon))
 return false;
   if (Left.is(Keywords.kw_assert) && Style.Language == FormatStyle::LK_Java)


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -319,6 +319,34 @@
   EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_CastRParen);
 }
 
+TEST_F(TokenAnnotatorTest,

[PATCH] D131978: [clang-format] Concepts: allow identifiers after negation

[Björn Schäpers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd3dd10a8b48: [clang-format] Concepts: allow identifiers 
after negation (authored by rymiel, committed by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131978

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -374,6 +374,13 @@
   EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
   EXPECT_TOKEN(Tokens[16], tok::ampamp, TT_BinaryOperator);
 
+  Tokens = annotate("template \n"
+"concept C = Foo && !Bar;");
+
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[10], tok::exclaim, TT_UnaryOperator);
+
   Tokens = annotate("template \n"
 "concept C = requires(T t) {\n"
 "  { t.foo() };\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24217,6 +24217,15 @@
"concept DelayedCheck = false || requires(T t) { t.bar(); } && "
"sizeof(T) <= 8;");
 
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !DerivedUnit;");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !(DerivedUnit);");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !!DerivedUnit;");
+
   verifyFormat("template \n"
"concept DelayedCheck = !!false || requires(T t) { t.bar(); } "
"&& sizeof(T) <= 8;");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3544,7 +3544,8 @@
   switch (FormatTok->Previous->Tok.getKind()) {
   case tok::coloncolon:  // Nested identifier.
   case tok::ampamp:  // Start of a function or variable for the
-  case tok::pipepipe:// constraint expression.
+  case tok::pipepipe:// constraint expression. (binary)
+  case tok::exclaim: // The same as above, but unary.
   case tok::kw_requires: // Initial identifier of a requires clause.
   case tok::equal:   // Initial identifier of a concept declaration.
 break;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -374,6 +374,13 @@
   EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
   EXPECT_TOKEN(Tokens[16], tok::ampamp, TT_BinaryOperator);
 
+  Tokens = annotate("template \n"
+"concept C = Foo && !Bar;");
+
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[10], tok::exclaim, TT_UnaryOperator);
+
   Tokens = annotate("template \n"
 "concept C = requires(T t) {\n"
 "  { t.foo() };\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24217,6 +24217,15 @@
"concept DelayedCheck = false || requires(T t) { t.bar(); } && "
"sizeof(T) <= 8;");
 
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !DerivedUnit;");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !(DerivedUnit);");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !!DerivedUnit;");
+
   verifyFormat("template \n"
"concept DelayedCheck = !!false || requires(T t) { t.bar(); } "
"&& sizeof(T) <= 8;");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3544,7 +3544,8 @@
   switch (FormatTok->Previous->Tok.getKind()) {
   case tok::coloncolon:  // Nested identifier.
   case tok::ampamp:  // Start of a function or variable for the
-  case tok::pipepipe:// constraint expression.
+  case tok::pipepipe:// constraint expression. (binary)
+  case tok::exclaim: // The same as above, but unary.
   case tok::kw_require

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D132131#3770170 , @MyDeveloperDay 
wrote:

> My personal preference is a new struct without Enabled

We go for a new struct. But why without enabled?
Currently we have a boolean on/off switch `AlignTrailingComments`. This change 
wanted to add a second boolean to enable aligning ignoring empty lines, 
naturally its value only matters if `AlignTrailingComments` is `true`.
My request was to use the existing struct (and I'm still in favor of that, but 
not strong enough to fight anything or anybody over it), but a struct seems to 
be better than 2 booleans. But an on/off switch still needs to be there, and at 
least using the same name as other options `Enabled` seems to be the most 
consistent solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D133087: [clang-format] [doc] Fix example of wrapping class definitions

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2adf241592b3: [clang-format] [doc] Fix example of wrapping 
class definitions (authored by Passw, committed by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133087

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


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1423,11 +1423,11 @@
 /// Wrap class definitions.
 /// \code
 ///   true:
-///   class foo {};
-///
-///   false:
 ///   class foo
 ///   {};
+///
+///   false:
+///   class foo {};
 /// \endcode
 bool AfterClass;
 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1439,12 +1439,12 @@
 .. code-block:: c++
 
   true:
-  class foo {};
-
-  false:
   class foo
   {};
 
+  false:
+  class foo {};
+
   * ``BraceWrappingAfterControlStatementStyle AfterControlStatement``
 Wrap control statements (``if``/``for``/``while``/``switch``/..).
 


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1423,11 +1423,11 @@
 /// Wrap class definitions.
 /// \code
 ///   true:
-///   class foo {};
-///
-///   false:
 ///   class foo
 ///   {};
+///
+///   false:
+///   class foo {};
 /// \endcode
 bool AfterClass;
 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1439,12 +1439,12 @@
 .. code-block:: c++
 
   true:
-  class foo {};
-
-  false:
   class foo
   {};
 
+  false:
+  class foo {};
+
   * ``BraceWrappingAfterControlStatementStyle AfterControlStatement``
 Wrap control statements (``if``/``for``/``while``/``switch``/..).
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132295: [clang-format] Change heuristic for locating lambda template arguments

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG247613548bac: [clang-format] Change heuristic for locating 
lambda template arguments (authored by rymiel, committed by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132295

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -809,18 +809,85 @@
 
   Tokens = annotate("[]() -> auto {}");
   ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_LambdaArrow);
   EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_LambdaLBrace);
 
   Tokens = annotate("[]() -> auto & {}");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_LambdaArrow);
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
 
   Tokens = annotate("[]() -> auto * {}");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_LambdaArrow);
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[] {}");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[] noexcept {}");
+  ASSERT_EQ(Tokens.size(), 6u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[3], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[] -> auto {}");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_LambdaArrow);
+  EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsFunctionAnnotations) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -21730,6 +21730,18 @@
"g();\n"
"  }\n"
"};\n");
+  verifyFormat("auto L = [](T...) {\n"
+   "  {\n"
+   "f();\n"
+   "g();\n"
+   "  }\n"
+   "};");
+  verifyFormat("auto L = [](T...) {\n"
+   "  {\n"
+   "f();\n"
+   "g();\n"
+   "  }\n"
+   "};");
 
   // Multiple lambdas 

[PATCH] D133087: [clang-format] [doc] Fix example of wrapping class definitions

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Just a note, I had to edit your patch. Did you create it with git diff?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133087

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

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:865
+
+  * ``TCAS_DontAlign`` (in configuration: ``DontAlign``)
+Don't align trailing comments.

MyDeveloperDay wrote:
> yusuke-kadowaki wrote:
> > MyDeveloperDay wrote:
> > > Is Don'tAlign the same as Leave that other options support  (for options 
> > > it best if we try and use the same terminiology
> > > 
> > > Always,Never,Leave (if that fits)
> > IMHO, `Leave` probably fits but `DontAlign`  is more straightforward. Also 
> > `DontAlign` is used for other alignment styles like `BracketAlignmentStyle` 
> > so it would be more natural.
> Leave should mean do nothing.. I'm personally not a fan of DontAlign because 
> obvious there should be a ' between the n and the t but I see we use it 
> elsewhere
> 
> But actually now I see your DontAlign is effectively the as before (i.e. it 
> will not add extra spaces) 
> 
> The point of Leave is what people have been asking for when we introduce a 
> new option, that is we if possible add an option which means "Don't touch it 
> at all"  i.e.  if I want to have
> 
> ```
> int a;   //  abc
> int b;   /// bcd
> int c;// cde
> ```
> 
> Then so be it
> 
> 
Leave is a nice option, yes.
I think it is complementary to `Dont`.

But maybe if the option is called `AlignTrailingComments` one may interpret 
`Leave` not as "don't touch the position of a comment at all" (what do we do, 
if the comment is outside of the column limit?), but as "just don't touch them, 
when they are somewhat aligned". Just a thought.



Comment at: clang/include/clang/Format/Format.h:373
+  /// Different styles for aligning trailing comments.
+  enum TrailingCommentsAlignmentStyle : int8_t {
+/// Don't align trailing comments.

MyDeveloperDay wrote:
> yusuke-kadowaki wrote:
> > MyDeveloperDay wrote:
> > > all options have a life cycle
> > > 
> > > bool -> enum -> struct
> > > 
> > > One of the thing I ask you before, would we want to align across N empty 
> > > lines, if ever so they I think
> > > we should go straight to a struct
> > > all options have a life cycle
> > 
> > I see your point. But we are checking `Style.MaxEmptyLinesToKeep` to 
> > determine how many consecutive lines to align. So currently no need to 
> > specify it from the option. You'll find the implementation below.
> I think its a bad idea to hijack a different option to do this..I don't think 
> it means the same thing and I think what whilst it might work there will be 
> someone somewhere who doesn't want it to behave like this and will call it 
> out as a bug.
> 
> 
Since you are strictly against `Enabled` what to put into a struct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D132131#3776956 , @MyDeveloperDay 
wrote:

> an example of the exact reason why we should not reuse the same struct...   
> https://github.com/llvm/llvm-project/issues/57464

Just a bit rephrasing.

In D132131#3777004 , @MyDeveloperDay 
wrote:

> another reason for using a struct, is that we might want to say something 
> like "AlignNamespaceTrainingComments:false" 
> https://github.com/llvm/llvm-project/issues/57504

Something like that we really want, and that is something I'd like to put on 
top of this change. Also for all comments following a `}`. That's the only 
reason I've disabled aligning comments for now.




Comment at: clang/docs/ClangFormatStyleOptions.rst:865
+
+  * ``TCAS_DontAlign`` (in configuration: ``DontAlign``)
+Don't align trailing comments.

yusuke-kadowaki wrote:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > yusuke-kadowaki wrote:
> > > > MyDeveloperDay wrote:
> > > > > Is Don'tAlign the same as Leave that other options support  (for 
> > > > > options it best if we try and use the same terminiology
> > > > > 
> > > > > Always,Never,Leave (if that fits)
> > > > IMHO, `Leave` probably fits but `DontAlign`  is more straightforward. 
> > > > Also `DontAlign` is used for other alignment styles like 
> > > > `BracketAlignmentStyle` so it would be more natural.
> > > Leave should mean do nothing.. I'm personally not a fan of DontAlign 
> > > because obvious there should be a ' between the n and the t but I see we 
> > > use it elsewhere
> > > 
> > > But actually now I see your DontAlign is effectively the as before (i.e. 
> > > it will not add extra spaces) 
> > > 
> > > The point of Leave is what people have been asking for when we introduce 
> > > a new option, that is we if possible add an option which means "Don't 
> > > touch it at all"  i.e.  if I want to have
> > > 
> > > ```
> > > int a;   //  abc
> > > int b;   /// bcd
> > > int c;// cde
> > > ```
> > > 
> > > Then so be it
> > > 
> > > 
> > Leave is a nice option, yes.
> > I think it is complementary to `Dont`.
> > 
> > But maybe if the option is called `AlignTrailingComments` one may interpret 
> > `Leave` not as "don't touch the position of a comment at all" (what do we 
> > do, if the comment is outside of the column limit?), but as "just don't 
> > touch them, when they are somewhat aligned". Just a thought.
> > Leave should mean do nothing
> 
> Ok now I see what `Leave` means. 
> But that should be another piece of work no? 
> 
> Of course me or someone can add the feature later (I don't really know how to 
> implement that though at least for now) 
> 
> 
Fine by me.



Comment at: clang/include/clang/Format/Format.h:373
+  /// Different styles for aligning trailing comments.
+  enum TrailingCommentsAlignmentStyle : int8_t {
+/// Don't align trailing comments.

yusuke-kadowaki wrote:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > yusuke-kadowaki wrote:
> > > > MyDeveloperDay wrote:
> > > > > all options have a life cycle
> > > > > 
> > > > > bool -> enum -> struct
> > > > > 
> > > > > One of the thing I ask you before, would we want to align across N 
> > > > > empty lines, if ever so they I think
> > > > > we should go straight to a struct
> > > > > all options have a life cycle
> > > > 
> > > > I see your point. But we are checking `Style.MaxEmptyLinesToKeep` to 
> > > > determine how many consecutive lines to align. So currently no need to 
> > > > specify it from the option. You'll find the implementation below.
> > > I think its a bad idea to hijack a different option to do this..I don't 
> > > think it means the same thing and I think what whilst it might work there 
> > > will be someone somewhere who doesn't want it to behave like this and 
> > > will call it out as a bug.
> > > 
> > > 
> > Since you are strictly against `Enabled` what to put into a struct?
> > Since you are strictly against Enabled what to put into a struct?
> 
> @MyDeveloperDay 
> Can you answer this? Plus it would be helpful if you just write down what 
> your ideal struct be like.
> 
> > I don't think it means the same thing
> 
> How so? There's no empty lines more than `MaxEmptyLinesToKeep` all the time 
> no?
> 
> > I don't think it means the same thing
> 
> How so? There's no empty lines more than `MaxEmptyLinesToKeep` all the time 
> no?
> 

But you could set `MaxEmptyLinesToKeep` to 3 and aligning comments to over 2 
empty lines.
```
int thisIsAVariable = 5; // It Really is


int stillAligned = 2;// Yep



int notSoMuch = 2; // Nope
```
It certainly doesn't hurt to have a value there.



Comment at: clang/include/clang/Format/Format.h:373
+  /// Different styles for aligning trailing comments.
+  enum TrailingCommentsAlignmentStyle : int8_t {
+/// Don't 

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:469
+  /// Different styles for merging short case labels.
+  enum ShortCaseLabelStyle : int8_t {
+/// Never merge case code

While we're at it, shouldn't there be a `Leave`? ;)


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

https://reviews.llvm.org/D133571

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

[PATCH] D133589: [clang-format] JSON formatting add new option for controlling newlines in json arrays

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/.clang-format:2
 BasedOnStyle: LLVM
+RemoveBracesLLVM: true

Unrelated.



Comment at: clang/include/clang/Format/Format.h:2063
+  /// otherwise it will scan until the closing `]` to determine if it should 
add
+  /// newlines between elements (prettier compatible)
+  ///





Comment at: clang/include/clang/Format/Format.h:2065
+  ///
+  /// NOTE: This is currently only for formatting JSON
+  /// \code





Comment at: clang/include/clang/Format/Format.h:2076
+  /// \version 16
+  bool BreakArrays;
+

Please sort.



Comment at: clang/include/clang/Format/Format.h:2076
+  /// \version 16
+  bool BreakArrays;
+

HazardyKnusperkeks wrote:
> Please sort.
What about `Leave`?



Comment at: clang/include/clang/Format/Format.h:3917
EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier &&
+   BreakArrays == R.BreakArrays &&
ExperimentalAutoDetectBinPacking ==

Please sort.



Comment at: clang/lib/Format/TokenAnnotator.cpp:4402
   return true;
-// Always break after a JSON array opener.
-// [
-// ]
+// Always break after a JSON array opener based on BreakArrays
 if (Left.is(TT_ArrayInitializerLSquare) && Left.is(tok::l_square) &&




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

https://reviews.llvm.org/D133589

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

[PATCH] D133589: [clang-format] JSON formatting add new option for controlling newlines in json arrays

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:1965
 (Style.JavaScriptQuotes == FormatStyle::JSQS_Double &&
- !Input.startswith("\'"))) {
+ !Input.startswith("\'")))
   continue;

These braces should stay, shouldn't they?



Comment at: clang/lib/Format/Format.cpp:2190
   Matching->is(TT_ArrayInitializerLSquare)) &&
-!(FormatTok->is(tok::r_brace) && Matching->is(TT_DictLiteral))) {
+!(FormatTok->is(tok::r_brace) && Matching->is(TT_DictLiteral)))
   continue;

It seems like there are a ton of unrelated brace removals.


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

https://reviews.llvm.org/D133589

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

[PATCH] D133571: [clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:469
+  /// Different styles for merging short case labels.
+  enum ShortCaseLabelStyle : int8_t {
+/// Never merge case code

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > While we're at it, shouldn't there be a `Leave`? ;)
> I think Leave is Never
No. `Leave` would leave me with this:
```
switch (a) {
  case 1: x = 1; break;
  case 2:
return;
}
```
If I'd start with it. `Leave` allows to mix the Yes and No options.


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

https://reviews.llvm.org/D133571

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

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:865
+
+  * ``TCAS_DontAlign`` (in configuration: ``DontAlign``)
+Don't align trailing comments.

yusuke-kadowaki wrote:
> HazardyKnusperkeks wrote:
> > yusuke-kadowaki wrote:
> > > HazardyKnusperkeks wrote:
> > > > MyDeveloperDay wrote:
> > > > > yusuke-kadowaki wrote:
> > > > > > MyDeveloperDay wrote:
> > > > > > > Is Don'tAlign the same as Leave that other options support  (for 
> > > > > > > options it best if we try and use the same terminiology
> > > > > > > 
> > > > > > > Always,Never,Leave (if that fits)
> > > > > > IMHO, `Leave` probably fits but `DontAlign`  is more 
> > > > > > straightforward. Also `DontAlign` is used for other alignment 
> > > > > > styles like `BracketAlignmentStyle` so it would be more natural.
> > > > > Leave should mean do nothing.. I'm personally not a fan of DontAlign 
> > > > > because obvious there should be a ' between the n and the t but I see 
> > > > > we use it elsewhere
> > > > > 
> > > > > But actually now I see your DontAlign is effectively the as before 
> > > > > (i.e. it will not add extra spaces) 
> > > > > 
> > > > > The point of Leave is what people have been asking for when we 
> > > > > introduce a new option, that is we if possible add an option which 
> > > > > means "Don't touch it at all"  i.e.  if I want to have
> > > > > 
> > > > > ```
> > > > > int a;   //  abc
> > > > > int b;   /// bcd
> > > > > int c;// cde
> > > > > ```
> > > > > 
> > > > > Then so be it
> > > > > 
> > > > > 
> > > > Leave is a nice option, yes.
> > > > I think it is complementary to `Dont`.
> > > > 
> > > > But maybe if the option is called `AlignTrailingComments` one may 
> > > > interpret `Leave` not as "don't touch the position of a comment at all" 
> > > > (what do we do, if the comment is outside of the column limit?), but as 
> > > > "just don't touch them, when they are somewhat aligned". Just a thought.
> > > > Leave should mean do nothing
> > > 
> > > Ok now I see what `Leave` means. 
> > > But that should be another piece of work no? 
> > > 
> > > Of course me or someone can add the feature later (I don't really know 
> > > how to implement that though at least for now) 
> > > 
> > > 
> > Fine by me.
> @HazardyKnusperkeks 
> Is this complicated? If it's not I might as well do this.
> 
> Also it would be helpful if you could provide some implementation guidance. 
> Sorry to ask this even though I haven't tried it myself yet.
> @HazardyKnusperkeks 
> Is this complicated? If it's not I might as well do this.
> 
> Also it would be helpful if you could provide some implementation guidance. 
> Sorry to ask this even though I haven't tried it myself yet.

I think you refer to the non aligning of comments following r_braces. I don't 
think so, because at least the cases I think about the r_brace should be the 
first token on a unwrapped line, so you just have to check for Line->First.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D127873: [clang-format] Fix misplacement of `*` in declaration of pointer to struct

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D127873#3614559 , @jackhong12 
wrote:

> Sorry, I don't have commit access. @HazardyKnusperkeks, could you help me 
> commit it?
>
> If I want to contribute to LLVM in the future, how do I get the commit 
> permission? Does it depend on the number of patches I submit?

https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

I can push it for you, but if you want to get the commit access, you can wait 
and do it yourself. Just ask for it, I didn't have to wait long.


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

https://reviews.llvm.org/D127873

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

[PATCH] D128709: [clang-format] Handle Verilog attributes

[Björn Schäpers via Phabricator via cfe-commits]

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

Seems straight forward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128709

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

[PATCH] D128712: [clang-format] Handle Verilog modules

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/FormatToken.h:1742
+  /// Returns whether \p Tok is a Verilog keyword that opens a module, etc.
+  bool isVerilogHier(const FormatToken &Tok) const {
+if (Tok.endsSequence(kw_function, kw_with))

Please write it out.

For a german //hier// has a different meaning. ;)



Comment at: clang/lib/Format/TokenAnnotator.cpp:241
 bool StartsObjCMethodExpr = false;
-if (FormatToken *MaybeSel = OpeningParen.Previous) {
-  // @selector( starts a selector.
-  if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous 
&&
-  MaybeSel->Previous->is(tok::at)) {
-StartsObjCMethodExpr = true;
+if (Style.isCpp()) {
+  if (FormatToken *MaybeSel = OpeningParen.Previous) {

?

I don't know where this is also used.

Let's see what the others say.



Comment at: clang/lib/Format/TokenAnnotator.cpp:766
   CurrentToken->setType(TT_AttributeColon);
-} else if (Left->isOneOf(TT_ArraySubscriptLSquare,
+} else if (Style.isCpp() &&
+   Left->isOneOf(TT_ArraySubscriptLSquare,

See above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128712

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

[PATCH] D128714: [clang-format] Handle Verilog case statements

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:1265
   LLVMStyle.IndentAccessModifiers = false;
-  LLVMStyle.IndentCaseLabels = false;
+  LLVMStyle.IndentCaseLabels = LLVMStyle.isVerilog() ? true : false;
   LLVMStyle.IndentCaseBlocks = false;

You should put that below.



Comment at: clang/lib/Format/Format.cpp:1349
 
   // Defaults that differ when not C++.
   if (Language == FormatStyle::LK_TableGen)

Here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128714

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

[PATCH] D129064: [clang-format] Avoid crash in LevelIndentTracker.

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:62-70
 while (IndentForLevel.size() <= Line.Level)
   IndentForLevel.push_back(-1);
 if (Line.InPPDirective) {
   unsigned IndentWidth =
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {

So how about that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129064

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

[PATCH] D129105: [clang-format][NFC] Clean up IndentForLevel in LevelIndentTracker

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:82-83
+if (Size <= Line.Level)
+  IndentForLevel.insert(IndentForLevel.end(), Line.Level - Size + 1,
+UnknownIndent ? -1 : Indent);
   }

Could we do this with with `resize` similar to my proposal in D129064?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129105

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

[PATCH] D121756: [clang-format] Clean up code looking for if statements

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D121756#3407320 , @owenpan wrote:

> In D121756#3398165 , @sstwcw wrote:
>
>> It turned out this patch does change behavior.
>>
>>   -  while (
>>   -  FormatTok->isOneOf(tok::identifier, tok::kw_requires, 
>> tok::coloncolon)) {
>>   +  while (FormatTok->isOneOf(tok::identifier, tok::kw_requires,
>>   +tok::coloncolon)) {
>>
>> So what do I do?
>
> IMO, refactoring and cleaning up code should be NFC. You can first add 
> `while`, `switch`, etc (if it makes sense) in one patch and then refactor in 
> another patch (or vice versa). It would be much easier to review.

+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121756

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

[PATCH] D121754: [clang-format] Refactor determineStarAmpUsage NFC

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

I cannot view the diff between the last two revisions:

> Unhandled Exception ("Exception")
> Found unknown intradiff source line, expected a line beginning with "+", "-", 
> or " " (space): \ No newline at end of file
> .




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121754

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

[PATCH] D126845: [clang-format] Handle Verilog numbers and operators

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D126845#3675721 , @sstwcw wrote:

> Are there any problems with this revision?  The ones that depend on it are 
> approved.

Sorry, last time you updated I had no time, and then I forgot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126845

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

[PATCH] D88299: [clang-format] Add MacroUnexpander.

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

A bit surprising such a big change...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88299

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

[PATCH] D106112: [clang-format] Break an unwrapped line at a K&R C parameter decl

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D106112#2883301 , @curdeius wrote:

> Looks okay, but I was wondering if we don't want to guard all K&R-related 
> changes behind e.g. ```Standard: Cpp78``, so as not to possibly introduce 
> strange bugs in newer modes.
> It may be an overkill if there are 2 patches like this, but if there are 
> more, that might become sensible to do so.
> WDYT?

I think this would be reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106112

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

[PATCH] D106349: [clang-format] respect AfterEnum for enums

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D106349#2893672 , @MyDeveloperDay 
wrote:

> This feels like there is some overlap with D93938: [clang-format] Fixed 
> AfterEnum handling 

Yeah that may be, but it is stale for over a month, with I think open 
questions. So if this here fixes at least one bug with no obvious new ones, I 
think we should go for it.

Regarding the patch: Please upload it with the context available. 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106349

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

[PATCH] D106349: [clang-format] respect AfterEnum for enums

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Looks good, could you also add a note in the relasenotes.rst?


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

https://reviews.llvm.org/D106349

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

[PATCH] D106349: [clang-format] respect AfterEnum for enums

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D106349#2900770 , @m1cha wrote:

> In D106349#2897400 , 
> @HazardyKnusperkeks wrote:
>
>> Looks good, could you also add a note in the relasenotes.rst?
>
> Sure
>
> Can I do something about the failing unittests? I don't know how they are 
> related to the changes I made.

They (most likely) aren't. That's something which set me off at the beginning 
too. If the Format Tests pass on your machine most probably everything is fine 
for you and for clang-format.
When you update the patch with release notes you can rebase on current main, 
maybe it is already fixed, maybe not.


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

https://reviews.llvm.org/D106349

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

[PATCH] D98214: [clang-format] Fix aligning with linebreaks

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D98214#2900803 , @baramin wrote:

> You are right.
>
> The problem is in
>
>   FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>
> line indentation. It is 6 instead of 4.

What you call a regression I call a fix. :)
I can not see the .clang-format on you linked issue.

If we agree on that it is a bug, I will try to fix it, but that decision have 
others to do. (And then I will need the .clang-format.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98214

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

[PATCH] D98214: [clang-format] Fix aligning with linebreaks

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D98214#2901304 , @baramin wrote:

> This is true.
>  .clang-format:
>
>   Language:Cpp
>   AlignConsecutiveAssignments: Consecutive
>   BinPackArguments: false
>   BinPackParameters: false
>   ColumnLimit: 120
>   ConstructorInitializerIndentWidth: 4
>   ContinuationIndentWidth: 4
>   IndentWidth: 4
>   TabWidth:4
>   UseCRLF: false
>   UseTab:  Never
>
> Format, that looks like a regression for me:
> Now:
>
>   void SomeFunc() {
>   newWatcher.maxAgeUsec = ToLegacyTimestamp(GetMaxAge(
>   FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   newWatcher.maxAge = ToLegacyTimestamp(GetMaxAge(
>   FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   newWatcher.max= ToLegacyTimestamp(GetMaxAge(
>  FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   }
>
> Before:
>
>   void SomeFunc() {
>   newWatcher.maxAgeUsec = ToLegacyTimestamp(GetMaxAge(
>   FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   newWatcher.maxAge = ToLegacyTimestamp(GetMaxAge(
>   FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   newWatcher.max= ToLegacyTimestamp(GetMaxAge(
>   FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   }

Yeah okay, that looks wrong. I will take a look at it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98214

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

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:3800
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeInlineASMColon = true;
   verifyFormat(

I already gave my go in the past, but now I have to wonder, why are you setting 
this to `true` here? Does this mean before your change clang-format did behave 
as if the option is `true`? If that's the case please set the default also to 
`true`. If the old behavior is neither `true` nor `false` we have to find 
something different.


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

https://reviews.llvm.org/D91950

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

[PATCH] D106773: [clang-format] Fix aligning with linebreaks #2

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius, baramin.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This amends c5243c63cda3c740d6e9c7e501f6518c21688da3 
 to fix 
formatting continued function calls with BinPacking = false.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106773

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16411,6 +16411,37 @@
"}",
Style);
   // clang-format on
+
+  Style = getLLVMStyleWithColumns(120);
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.ContinuationIndentWidth = 4;
+  Style.IndentWidth = 4;
+
+  // clang-format off
+  verifyFormat("void SomeFunc() {\n"
+   "newWatcher.maxAgeUsec = 
ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.maxAge = 
ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.max= 
ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "}",
+   Style);
+  // clang-format on
+
+  Style.BinPackArguments = false;
+
+  // clang-format off
+  verifyFormat("void SomeFunc() {\n"
+   "newWatcher.maxAgeUsec = ToLegacyTimestamp(GetMaxAge(\n"
+   "
FromLegacyTimestamp(monitorFrequencyUsec), 
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.maxAge = ToLegacyTimestamp(GetMaxAge(\n"
+   "
FromLegacyTimestamp(monitorFrequencyUsec), 
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.max= ToLegacyTimestamp(GetMaxAge(\n"
+   "
FromLegacyTimestamp(monitorFrequencyUsec), 
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "}",
+   Style);
+  // clang-format on
 }
 
 TEST_F(FormatTest, AlignWithInitializerPeriods) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -347,7 +347,7 @@
 if (ScopeStart > Start + 1 &&
 Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
 Changes[ScopeStart - 1].Tok->is(tok::l_paren))
-  return true;
+  return Style.BinPackArguments;
 
 // Ternary operator
 if (Changes[i].Tok->is(TT_ConditionalExpr))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16411,6 +16411,37 @@
"}",
Style);
   // clang-format on
+
+  Style = getLLVMStyleWithColumns(120);
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.ContinuationIndentWidth = 4;
+  Style.IndentWidth = 4;
+
+  // clang-format off
+  verifyFormat("void SomeFunc() {\n"
+   "newWatcher.maxAgeUsec = ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.maxAge = ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.max= ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "}",
+   Style);
+  // clang-format on
+
+  Style.BinPackArguments = false;
+
+  // clang-format off
+  verifyFormat("void SomeFunc() {\n"
+   "newWatcher.maxAgeUsec = ToLegacyTimestamp(GetMaxAge(\n"
+   "FromLegacyTimestamp(monitorFrequencyUsec), seconds(std::uint64_t(maxSam

[PATCH] D98214: [clang-format] Fix aligning with linebreaks

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D98214#2901304 , @baramin wrote:

> This is true.
>  .clang-format:
>
>   Language:Cpp
>   AlignConsecutiveAssignments: Consecutive
>   BinPackArguments: false
>   BinPackParameters: false
>   ColumnLimit: 120
>   ConstructorInitializerIndentWidth: 4
>   ContinuationIndentWidth: 4
>   IndentWidth: 4
>   TabWidth:4
>   UseCRLF: false
>   UseTab:  Never
>
> Format, that looks like a regression for me:
> Now:
>
>   void SomeFunc() {
>   newWatcher.maxAgeUsec = ToLegacyTimestamp(GetMaxAge(
>   FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   newWatcher.maxAge = ToLegacyTimestamp(GetMaxAge(
>   FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   newWatcher.max= ToLegacyTimestamp(GetMaxAge(
>  FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   }
>
> Before:
>
>   void SomeFunc() {
>   newWatcher.maxAgeUsec = ToLegacyTimestamp(GetMaxAge(
>   FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   newWatcher.maxAge = ToLegacyTimestamp(GetMaxAge(
>   FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   newWatcher.max= ToLegacyTimestamp(GetMaxAge(
>   FromLegacyTimestamp(monitorFrequencyUsec), 
> seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));
>   }

The fix is in D106773 , please have a look at 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98214

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

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added a comment.
This revision now requires changes to proceed.

Then I am sorry that I've missed that before. But you need to change your 
`bool` to an `enum` and also model the current behavior, so that there is no 
change in formatting without adapting the `.clang-format`. A option names maybe 
`Always`, `Never`, and `OnlyMultiline`?


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

https://reviews.llvm.org/D91950

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

[PATCH] D106773: [clang-format] Fix aligning with linebreaks #2

[Björn Schäpers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG75f6a795ee0f: [clang-format] Fix aligning with linebreaks #2 
(authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106773

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16419,6 +16419,37 @@
"}",
Style);
   // clang-format on
+
+  Style = getLLVMStyleWithColumns(120);
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.ContinuationIndentWidth = 4;
+  Style.IndentWidth = 4;
+
+  // clang-format off
+  verifyFormat("void SomeFunc() {\n"
+   "newWatcher.maxAgeUsec = 
ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.maxAge = 
ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.max= 
ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "}",
+   Style);
+  // clang-format on
+
+  Style.BinPackArguments = false;
+
+  // clang-format off
+  verifyFormat("void SomeFunc() {\n"
+   "newWatcher.maxAgeUsec = ToLegacyTimestamp(GetMaxAge(\n"
+   "
FromLegacyTimestamp(monitorFrequencyUsec), 
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.maxAge = ToLegacyTimestamp(GetMaxAge(\n"
+   "
FromLegacyTimestamp(monitorFrequencyUsec), 
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.max= ToLegacyTimestamp(GetMaxAge(\n"
+   "
FromLegacyTimestamp(monitorFrequencyUsec), 
seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "}",
+   Style);
+  // clang-format on
 }
 
 TEST_F(FormatTest, AlignWithInitializerPeriods) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -347,7 +347,7 @@
 if (ScopeStart > Start + 1 &&
 Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
 Changes[ScopeStart - 1].Tok->is(tok::l_paren))
-  return true;
+  return Style.BinPackArguments;
 
 // Ternary operator
 if (Changes[i].Tok->is(TT_ConditionalExpr))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16419,6 +16419,37 @@
"}",
Style);
   // clang-format on
+
+  Style = getLLVMStyleWithColumns(120);
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.ContinuationIndentWidth = 4;
+  Style.IndentWidth = 4;
+
+  // clang-format off
+  verifyFormat("void SomeFunc() {\n"
+   "newWatcher.maxAgeUsec = ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.maxAge = ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.max= ToLegacyTimestamp(GetMaxAge(FromLegacyTimestamp(monitorFrequencyUsec),\n"
+   "seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "}",
+   Style);
+  // clang-format on
+
+  Style.BinPackArguments = false;
+
+  // clang-format off
+  verifyFormat("void SomeFunc() {\n"
+   "newWatcher.maxAgeUsec = ToLegacyTimestamp(GetMaxAge(\n"
+   "FromLegacyTimestamp(monitorFrequencyUsec), seconds(std::uint64_t(maxSampleAge)), maxKeepSamples));\n"
+   "newWatcher.maxAge = ToLegacyTimestamp(GetMaxAge(\n"
+   "FromLegacyTimestamp(monitorFrequencyUs

[PATCH] D106773: [clang-format] Fix aligning with linebreaks #2

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D106773#2912732 , @curdeius wrote:

> This bug fix should probably be cherry-picked into 13.x branch. WDYT?

Yeah, I wrote already an email about it. Or can (and should) I push it myself?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106773

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

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

And format what clang-format says. Then it looks good to me.




Comment at: clang/unittests/Format/FormatTest.cpp:6250
+  verifyFormat("asm volatile(\"lng\",\n"
+   " : : val);",
+   Style);

Could you add for Never and Always the code from Above?

```
  "asm(\"movq\\t%%rbx, %%rsi\\n\\t\"\n"
  "\"cpuid\\n\\t\"\n"
  "\"xchgq\\t%%rbx, %%rsi\\n\\t\"\n"
  ": \"=a\"(*rEAX), \"=S\"(*rEBX), \"=c\"(*rECX), \"=d\"(*rEDX)\n"
  ": \"a\"(value));"
```


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

https://reviews.llvm.org/D91950

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

[PATCH] D106773: [clang-format] Fix aligning with linebreaks #2

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

It's picked: 0e3777bb0ad94ecd1429dc96409177cdccf39bdd 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106773

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

[PATCH] D106349: [clang-format] respect AfterEnum for enums

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

No one objects, you can push it. Or if you don't have commit access (and don't 
want to, or don't want to wait for it) we can push it, then please state name 
and email for the commit.


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

https://reviews.llvm.org/D106349

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

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

Thanks for your patience. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:2759
+  IncludeName =
+  StringRef(Twine("<", IncludeName).concat(Twine(">")).str());
+}

How does that not go out of scope and I have a dangling reference?



Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:177
 const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ 
\\]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 

I don't see why this is needed. It should match with the previous stuff.



Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:177
 const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ 
\\]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 

HazardyKnusperkeks wrote:
> I don't see why this is needed. It should match with the previous stuff.
Do we want that optional?



Comment at: clang/unittests/Format/SortIncludesTest.cpp:471
+ "@import Foundation;\n"
+ "#import \"a.h\"\n"));
+}

I've no idea about obj-c, is it feasible to mix `#` and `@`? If so please add a 
test where the `@` is between `#`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D120631: [clang-format][docs] Fix incorrect 'clang-format 12' option markers

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Your patches are not directly applicable.
You have

  --- clang/include/clang/Format/Format.h
  +++ clang/include/clang/Format/Format.h

instead of

  --- a/clang/include/clang/Format/Format.h
  +++ b/clang/include/clang/Format/Format.h

which results in

  $ git apply diff.patch
  error: docs/ClangFormatStyleOptions.rst: No such file or directory
  error: include/clang/Format/Format.h: No such file or directory


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120631

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

[PATCH] D120774: [clang-format] Handle builtins in constraint expression

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8b4d68bf65ef: [clang-format] Handle builtins in constraint 
expression (authored by HazardyKnusperkeks).

Changed prior to commit:
  https://reviews.llvm.org/D120774?vs=413430&id=414881#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120774

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23810,6 +23810,18 @@
   verifyFormat("template \n"
"concept Node = std::is_object_v;");
 
+  verifyFormat("template \n"
+   "concept integral = __is_integral(T);");
+
+  verifyFormat("template \n"
+   "concept is2D = __array_extent(T, 1) == 2;");
+
+  verifyFormat("template \n"
+   "concept isRhs = __is_rvalue_expr(std::declval() + 2)");
+
+  verifyFormat("template \n"
+   "concept Same = __is_same_as;");
+
   auto Style = getLLVMStyle();
   Style.BreakBeforeConceptDeclarations = FormatStyle::BBCDS_Allowed;
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3132,30 +3132,6 @@
 return;
   break;
 
-case tok::identifier:
-  // We need to differentiate identifiers for a template deduction guide,
-  // variables, or function return types (the constraint expression has
-  // ended before that), and basically all other cases. But it's easier to
-  // check the other way around.
-  assert(FormatTok->Previous);
-  switch (FormatTok->Previous->Tok.getKind()) {
-  case tok::coloncolon:  // Nested identifier.
-  case tok::ampamp:  // Start of a function or variable for the
-  case tok::pipepipe:// constraint expression.
-  case tok::kw_requires: // Initial identifier of a requires clause.
-  case tok::equal:   // Initial identifier of a concept declaration.
-break;
-  default:
-return;
-  }
-
-  // Read identifier with optional template declaration.
-  nextToken();
-  if (FormatTok->is(tok::less))
-parseBracedList(/*ContinueOnSemicolons=*/false, /*IsEnum=*/false,
-/*ClosingBraceKind=*/tok::greater);
-  break;
-
 case tok::kw_const:
 case tok::semi:
 case tok::kw_class:
@@ -3232,7 +3208,34 @@
   break;
 
 default:
-  return;
+  if (!FormatTok->Tok.getIdentifierInfo()) {
+// Identifiers are part of the default case, we check for more then
+// tok::identifier to handle builtin type traits.
+return;
+  }
+
+  // We need to differentiate identifiers for a template deduction guide,
+  // variables, or function return types (the constraint expression has
+  // ended before that), and basically all other cases. But it's easier to
+  // check the other way around.
+  assert(FormatTok->Previous);
+  switch (FormatTok->Previous->Tok.getKind()) {
+  case tok::coloncolon:  // Nested identifier.
+  case tok::ampamp:  // Start of a function or variable for the
+  case tok::pipepipe:// constraint expression.
+  case tok::kw_requires: // Initial identifier of a requires clause.
+  case tok::equal:   // Initial identifier of a concept declaration.
+break;
+  default:
+return;
+  }
+
+  // Read identifier with optional template declaration.
+  nextToken();
+  if (FormatTok->is(tok::less))
+parseBracedList(/*ContinueOnSemicolons=*/false, /*IsEnum=*/false,
+/*ClosingBraceKind=*/tok::greater);
+  break;
 }
   } while (!eof());
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23810,6 +23810,18 @@
   verifyFormat("template \n"
"concept Node = std::is_object_v;");
 
+  verifyFormat("template \n"
+   "concept integral = __is_integral(T);");
+
+  verifyFormat("template \n"
+   "concept is2D = __array_extent(T, 1) == 2;");
+
+  verifyFormat("template \n"
+   "concept isRhs = __is_rvalue_expr(std::declval() + 2)");
+
+  verifyFormat("template \n"
+   "concept Same = __is_same_as;");
+
   auto Style = getLLVMStyle();
   Style.BreakBeforeConceptDeclarations = FormatStyle::BBCDS_Allowed;
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineP

[PATCH] D120631: [clang-format][docs] Fix incorrect 'clang-format 12' option markers

[Björn Schäpers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9106a07f1fcb: [clang-format][docs] Fix incorrect 
'clang-format 12' option markers (authored by kuzkry, committed by 
HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120631

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

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -503,7 +503,7 @@
 
   /// If ``true``, horizontally align operands of binary and ternary
   /// expressions.
-  /// \version 12
+  /// \version 3.5
   OperandAlignmentStyle AlignOperands;
 
   /// If ``true``, aligns trailing comments.
@@ -566,7 +566,7 @@
   /// B
   ///   } myEnum;
   /// \endcode
-  /// \version 12
+  /// \version 11
   bool AllowShortEnumsOnASingleLine;
 
   /// Different styles for merging short blocks containing at most one
@@ -991,7 +991,7 @@
   ///   //^ inserted
   ///   ]
   /// \endcode
-  /// \version 12
+  /// \version 11
   TrailingCommaStyle InsertTrailingCommas;
 
   /// If ``false``, a function declaration's or function definition's
@@ -2359,7 +2359,7 @@
   /// \endcode
   ///
   /// For example: BOOST_PP_STRINGIZE
-  /// \version 12
+  /// \version 11
   std::vector WhitespaceSensitiveMacros;
 
   tooling::IncludeStyle IncludeStyle;
@@ -2522,7 +2522,7 @@
   };
 
   /// IndentExternBlockStyle is the type of indenting of extern blocks.
-  /// \version 12
+  /// \version 11
   IndentExternBlockStyle IndentExternBlock;
 
   /// Indent the requires clause in a template. This only applies when
@@ -2922,7 +2922,7 @@
   ///}]
   ///}
   /// \endcode
-  /// \version 12
+  /// \version 11
   bool ObjCBreakBeforeNestedBlockParam;
 
   /// Add a space in front of an Objective-C protocol list, i.e. use
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -606,7 +606,7 @@
 
 
 
-**AlignOperands** (``OperandAlignmentStyle``) :versionbadge:`clang-format 12`
+**AlignOperands** (``OperandAlignmentStyle``) :versionbadge:`clang-format 3.5`
   If ``true``, horizontally align operands of binary and ternary
   expressions.
 
@@ -749,7 +749,7 @@
   return;
 }
 
-**AllowShortEnumsOnASingleLine** (``Boolean``) :versionbadge:`clang-format 12`
+**AllowShortEnumsOnASingleLine** (``Boolean``) :versionbadge:`clang-format 11`
   Allow short enums on a single line.
 
   .. code-block:: c++
@@ -2606,7 +2606,7 @@
plop();  plop();
  }  }
 
-**IndentExternBlock** (``IndentExternBlockStyle``) :versionbadge:`clang-format 12`
+**IndentExternBlock** (``IndentExternBlockStyle``) :versionbadge:`clang-format 11`
   IndentExternBlockStyle is the type of indenting of extern blocks.
 
   Possible values:
@@ -2792,7 +2792,7 @@
   --i;  --i;
 while (i);} while (i);
 
-**InsertTrailingCommas** (``TrailingCommaStyle``) :versionbadge:`clang-format 12`
+**InsertTrailingCommas** (``TrailingCommaStyle``) :versionbadge:`clang-format 11`
   If set to ``TCS_Wrapped`` will insert trailing commas in container
   literals (arrays and objects) that wrap across multiple lines.
   It is currently only available for JavaScript
@@ -3150,7 +3150,7 @@
  [self onOperationDone];
  }];
 
-**ObjCBreakBeforeNestedBlockParam** (``Boolean``) :versionbadge:`clang-format 12`
+**ObjCBreakBeforeNestedBlockParam** (``Boolean``) :versionbadge:`clang-format 11`
   Break parameters list into lines when there is nested block
   parameters in a function call.
 
@@ -4358,7 +4358,7 @@
 
 
 
-**WhitespaceSensitiveMacros** (``List of Strings``) :versionbadge:`clang-format 12`
+**WhitespaceSensitiveMacros** (``List of Strings``) :versionbadge:`clang-format 11`
   A vector of macros which are whitespace-sensitive and should not
   be touched.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

You either get commit access 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access or ask 
someone of us to push it for you. In the latter case we need a name and mail 
for the commit. I recommend the former, especially if you want to contribute 
more, it's not that hard, just ask. :)

Then you can push to the main branch on Github.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, curdeius, MyDeveloperDay.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: All.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/54350

As driveby rename `Left` to `OpeningParen`, this gives a better overview what 
it is, and we have  a loop in the function, where `Left` is not adjusted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121550

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -331,6 +331,14 @@
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[29], tok::kw_requires,
TT_RequiresClauseInARequiresExpression);
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "
+"requires true; };");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23891,9 +23891,9 @@
   verifyFormat(
   "template  concept C = decltype([]() -> std::true_type {\n"
   "return {};\n"
-  "  }())::value\n"
-  "  && requires(T t) { t.bar(); } &&\n"
-  "  sizeof(T) <= 8;",
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",
   Style);
 
   verifyFormat("template  concept Semiregular =\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -211,32 +211,34 @@
   bool parseParens(bool LookForDecls = false) {
 if (!CurrentToken)
   return false;
-FormatToken *Left = CurrentToken->Previous;
-assert(Left && "Unknown previous token");
-FormatToken *PrevNonComment = Left->getPreviousNonComment();
-Left->ParentBracket = Contexts.back().ContextKind;
+assert(CurrentToken->Previous && "Unknown previous token");
+FormatToken &OpeningParen = *CurrentToken->Previous;
+assert(OpeningParen.is(tok::l_paren));
+FormatToken *PrevNonComment = OpeningParen.getPreviousNonComment();
+OpeningParen.ParentBracket = Contexts.back().ContextKind;
 ScopedContextCreator ContextCreator(*this, tok::l_paren, 1);
 
 // FIXME: This is a bit of a hack. Do better.
 Contexts.back().ColonIsForRangeExpr =
 Contexts.size() == 2 && Contexts[0].ColonIsForRangeExpr;
 
-if (Left->Previous && Left->Previous->is(TT_UntouchableMacroFunc)) {
-  Left->Finalized = true;
+if (OpeningParen.Previous &&
+OpeningParen.Previous->is(TT_UntouchableMacroFunc)) {
+  OpeningParen.Finalized = true;
   return parseUntouchableParens();
 }
 
 bool StartsObjCMethodExpr = false;
-if (FormatToken *MaybeSel = Left->Previous) {
+if (FormatToken *MaybeSel = OpeningParen.Previous) {
   // @selector( starts a selector.
   if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous &&
   MaybeSel->Previous->is(tok::at))
 StartsObjCMethodExpr = true;
 }
 
-if (Left->is(TT_OverloadedOperatorLParen)) {
+if (OpeningParen.is(TT_OverloadedOperatorLParen)) {
   // Find the previous kw_operator token.
-  FormatToken *Prev = Left;
+  FormatToken *Prev = &OpeningParen;
   while (!Prev->is(tok::kw_operator)) {
 Prev = Prev->Previous;
 assert(Prev && "Expect a kw_operator prior to the OperatorLParen!");
@@ -255,54 +257,58 @@
   // type X = (...);
   // export type X = (...);
   Contexts.back().IsExpression = false;
-} else if (Left->Previous &&
-   (Left->Previous->isOneOf(tok::kw_static_assert, tok::kw_while,
-tok::l_paren, tok::comma) ||
-Left->Previous->isIf() ||
-Left->Previous->is(TT_BinaryOperator))) {
+} else if (Opening

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:424-425
 
-  if (CurrentToken->is(tok::l_brace))
-Left->setType(TT_Unknown); // Not TT_ObjCBlockLParen
+  if (CurrentToken->is(tok::l_brace) && 
OpeningParen.is(TT_ObjCBlockLParen))
+OpeningParen.setType(TT_Unknown);
   if (CurrentToken->is(tok::comma) && CurrentToken->Next &&

This is the actual fix. It reset to many types, although the comment said what 
it should reset.



Comment at: clang/unittests/Format/FormatTest.cpp:23894-23896
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",

I know we don't like changing tests, but I think there was a bug before which 
were never hit in our tests but here.

Before
```
 M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=130 Name=decltype L=42 PPK=2 
FakeLParens=5/ FakeRParens=0 II=0xf136c40 Text='decltype'
 M=0 C=0 T=Unknown S=0 F=0 B=1 BK=0 P=23 Name=l_paren L=43 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=1 T=LambdaLSquare S=0 F=0 B=0 BK=0 P=140 Name=l_square L=44 PPK=2 
FakeLParens=1/ FakeRParens=0 II=0x0 Text='['
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=79 Name=r_square L=45 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=']'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=46 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=47 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=')'
 M=0 C=1 T=LambdaArrow S=1 F=0 B=0 BK=0 P=150 Name=arrow L=50 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='->'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=43 Name=identifier L=54 PPK=2 
FakeLParens= FakeRParens=0 II=0xf130c60 Text='std'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=coloncolon L=56 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=540 Name=identifier L=65 PPK=2 
FakeLParens= FakeRParens=0 II=0xf130c88 Text='true_type'
 M=0 C=0 T=LambdaLBrace S=1 F=0 B=0 BK=1 P=43 Name=l_brace L=67 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='{'
 M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=61 Name=r_brace L=80 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='}'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=81 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=82 PPK=2 FakeLParens= 
FakeRParens=1 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=83 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=coloncolon L=85 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=TrailingAnnotation S=0 F=0 B=0 BK=0 P=520 Name=identifier L=90 PPK=2 
FakeLParens= FakeRParens=0 II=0xf130ca8 Text='value'
 M=0 C=1 T=BinaryOperator S=1 F=0 B=0 BK=0 P=25 Name=ampamp L=93 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='&&'
```
after
```
 M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=130 Name=decltype L=42 PPK=2 
FakeLParens=5/ FakeRParens=0 II=0xee66c40 Text='decltype'
 M=0 C=0 T=TypeDeclarationParen S=0 F=0 B=1 BK=0 P=23 Name=l_paren L=43 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=LambdaLSquare S=0 F=0 B=0 BK=0 P=140 Name=l_square L=44 PPK=2 
FakeLParens=1/ FakeRParens=0 II=0x0 Text='['
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=79 Name=r_square L=45 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=']'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=46 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=47 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=')'
 M=0 C=1 T=LambdaArrow S=1 F=0 B=0 BK=0 P=150 Name=arrow L=50 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='->'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=43 Name=identifier L=54 PPK=2 
FakeLParens= FakeRParens=0 II=0xee60c60 Text='std'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=coloncolon L=56 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=540 Name=identifier L=65 PPK=2 
FakeLParens= FakeRParens=0 II=0xee60c88 Text='true_type'
 M=0 C=0 T=LambdaLBrace S=1 F=0 B=0 BK=1 P=43 Name=l_brace L=67 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='{'
 M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=61 Name=r_brace L=80 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='}'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=81 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=82 PPK=2 FakeLParens= 
FakeRParens=1 II=0x0 Text=')'
 M=0 C=0 T=TypeDeclarationParen S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=83 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=coloncolon L=85 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=520 Name=identifier L=90 PPK=2 
FakeLParens= FakeRParens=0 II=0xee60ca8 Text='value'
 M=0 C=0 T=BinaryOperator S=1 

[PATCH] D112019: [clang-format] [PR51412] AlignConsecutiveMacros fights with Visual Studio and resource.h

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Why limit to macros, could it be a member of AlignConsecutiveStyle and apply to 
the other stuff as well?




Comment at: clang/include/clang/Format/Format.h:256
+  /// \endcode
+  /// \version 14
+  unsigned AlignConsecutiveMacrosMinWidth;

Version 15



Comment at: clang/lib/Format/Format.cpp:1154
+  LLVMStyle.AlignConsecutiveMacrosMinWidth = 0;
+  LLVMStyle.AlignConsecutiveMacrosIgnoreMax = false;
   LLVMStyle.AllowAllArgumentsOnNextLine = true;

I before M


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

https://reviews.llvm.org/D112019

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

[PATCH] D121557: [clang-format][NFC] Rename Left to OpeningParen...

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, MyDeveloperDay, curdeius.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: All.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

in TokenAnnotator::parseParens(). Left is misleading since we have a loop and 
Left is not adjusted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121557

Files:
  clang/lib/Format/TokenAnnotator.cpp

Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -211,32 +211,34 @@
   bool parseParens(bool LookForDecls = false) {
 if (!CurrentToken)
   return false;
-FormatToken *Left = CurrentToken->Previous;
-assert(Left && "Unknown previous token");
-FormatToken *PrevNonComment = Left->getPreviousNonComment();
-Left->ParentBracket = Contexts.back().ContextKind;
+assert(CurrentToken->Previous && "Unknown previous token");
+FormatToken &OpeningParen = *CurrentToken->Previous;
+assert(OpeningParen.is(tok::l_paren));
+FormatToken *PrevNonComment = OpeningParen.getPreviousNonComment();
+OpeningParen.ParentBracket = Contexts.back().ContextKind;
 ScopedContextCreator ContextCreator(*this, tok::l_paren, 1);
 
 // FIXME: This is a bit of a hack. Do better.
 Contexts.back().ColonIsForRangeExpr =
 Contexts.size() == 2 && Contexts[0].ColonIsForRangeExpr;
 
-if (Left->Previous && Left->Previous->is(TT_UntouchableMacroFunc)) {
-  Left->Finalized = true;
+if (OpeningParen.Previous &&
+OpeningParen.Previous->is(TT_UntouchableMacroFunc)) {
+  OpeningParen.Finalized = true;
   return parseUntouchableParens();
 }
 
 bool StartsObjCMethodExpr = false;
-if (FormatToken *MaybeSel = Left->Previous) {
+if (FormatToken *MaybeSel = OpeningParen.Previous) {
   // @selector( starts a selector.
   if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous &&
   MaybeSel->Previous->is(tok::at))
 StartsObjCMethodExpr = true;
 }
 
-if (Left->is(TT_OverloadedOperatorLParen)) {
+if (OpeningParen.is(TT_OverloadedOperatorLParen)) {
   // Find the previous kw_operator token.
-  FormatToken *Prev = Left;
+  FormatToken *Prev = &OpeningParen;
   while (!Prev->is(tok::kw_operator)) {
 Prev = Prev->Previous;
 assert(Prev && "Expect a kw_operator prior to the OperatorLParen!");
@@ -255,54 +257,58 @@
   // type X = (...);
   // export type X = (...);
   Contexts.back().IsExpression = false;
-} else if (Left->Previous &&
-   (Left->Previous->isOneOf(tok::kw_static_assert, tok::kw_while,
-tok::l_paren, tok::comma) ||
-Left->Previous->isIf() ||
-Left->Previous->is(TT_BinaryOperator))) {
+} else if (OpeningParen.Previous &&
+   (OpeningParen.Previous->isOneOf(tok::kw_static_assert,
+   tok::kw_while, tok::l_paren,
+   tok::comma) ||
+OpeningParen.Previous->isIf() ||
+OpeningParen.Previous->is(TT_BinaryOperator))) {
   // static_assert, if and while usually contain expressions.
   Contexts.back().IsExpression = true;
-} else if (Style.isJavaScript() && Left->Previous &&
-   (Left->Previous->is(Keywords.kw_function) ||
-(Left->Previous->endsSequence(tok::identifier,
-  Keywords.kw_function {
+} else if (Style.isJavaScript() && OpeningParen.Previous &&
+   (OpeningParen.Previous->is(Keywords.kw_function) ||
+(OpeningParen.Previous->endsSequence(tok::identifier,
+ Keywords.kw_function {
   // function(...) or function f(...)
   Contexts.back().IsExpression = false;
-} else if (Style.isJavaScript() && Left->Previous &&
-   Left->Previous->is(TT_JsTypeColon)) {
+} else if (Style.isJavaScript() && OpeningParen.Previous &&
+   OpeningParen.Previous->is(TT_JsTypeColon)) {
   // let x: (SomeType);
   Contexts.back().IsExpression = false;
-} else if (isLambdaParameterList(Left)) {
+} else if (isLambdaParameterList(&OpeningParen)) {
   // This is a parameter list of a lambda expression.
   Contexts.back().IsExpression = false;
 } else if (Line.InPPDirective &&
-   (!Left->Previous || !Left->Previous->is(tok::identifier))) {
+   (!OpeningParen.Previous ||
+!OpeningParen.Previous->is(tok::identifier))) {
   Contexts.back().IsExpression = true;
 } else if (Contexts[Contexts.size() - 2].CaretFound) {

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks updated this revision to Diff 414956.
HazardyKnusperkeks marked 2 inline comments as done.
HazardyKnusperkeks added a comment.

Split rename and bug fix


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

https://reviews.llvm.org/D121550

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -331,6 +331,14 @@
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[29], tok::kw_requires,
TT_RequiresClauseInARequiresExpression);
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "
+"requires true; };");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23891,9 +23891,9 @@
   verifyFormat(
   "template  concept C = decltype([]() -> std::true_type {\n"
   "return {};\n"
-  "  }())::value\n"
-  "  && requires(T t) { t.bar(); } &&\n"
-  "  sizeof(T) <= 8;",
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",
   Style);
 
   verifyFormat("template  concept Semiregular =\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -421,8 +421,8 @@
   if (CurrentToken->isOneOf(tok::r_square, tok::r_brace))
 return false;
 
-  if (CurrentToken->is(tok::l_brace))
-OpeningParen.setType(TT_Unknown); // Not TT_ObjCBlockLParen
+  if (CurrentToken->is(tok::l_brace) && 
OpeningParen.is(TT_ObjCBlockLParen))
+OpeningParen.setType(TT_Unknown);
   if (CurrentToken->is(tok::comma) && CurrentToken->Next &&
   !CurrentToken->Next->HasUnescapedNewline &&
   !CurrentToken->Next->isTrailingComment())


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -331,6 +331,14 @@
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[29], tok::kw_requires,
TT_RequiresClauseInARequiresExpression);
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "
+"requires true; };");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23891,9 +23891,9 @@
   verifyFormat(
   "template  concept C = decltype([]() -> std::true_type {\n"
   "return {};\n"
-  "  }())::value\n"
-  "  && requires(T t) { t.bar(); } &&\n"
-  "  sizeof(T) <= 8;",
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",
   Style);
 
   verifyFormat("template  concept Semiregular =\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -421,8 +421,8 @@
   if (CurrentToken->isOneOf(tok::r_square, tok::r_brace))
 return false;
 
-  if (CurrentToken->is(tok::l_brace))
-OpeningParen.setType(TT_Unknown); // Not TT_ObjCBlockLPa

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:215
+assert(CurrentToken->Previous && "Unknown previous token");
+FormatToken &OpeningParen = *CurrentToken->Previous;
+assert(OpeningParen.is(tok::l_paren));

curdeius wrote:
> Maybe you can commit the rename as a NFC commit and rebase the patch to 
> remove the noise?
Done in D121557



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:335
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "

curdeius wrote:
> I'd prefer a valid C++ code (if it's not too complicated) if possible but 
> that's acceptable.
I thought I'd stick to the bug report. The only thing that comes to my mind 
does compile on gcc, but clang gives a specific error (which I support, without 
having looked into the standard). https://gcc.godbolt.org/z/WvT9861b6


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

https://reviews.llvm.org/D121550

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

[PATCH] D121558: [clang-format][NFC] Left renamed to OpeningBrace...

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, MyDeveloperDay, curdeius.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: All.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

in TokenAnnotator::parseBrace. Left is misleading, because we have a loop and 
Left does not move.

Also return early.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121558

Files:
  clang/lib/Format/TokenAnnotator.cpp

Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -774,63 +774,66 @@
   }
 
   bool parseBrace() {
-if (CurrentToken) {
-  FormatToken *Left = CurrentToken->Previous;
-  Left->ParentBracket = Contexts.back().ContextKind;
+if (!CurrentToken)
+  return true;
 
-  if (Contexts.back().CaretFound)
-Left->setType(TT_ObjCBlockLBrace);
-  Contexts.back().CaretFound = false;
+assert(CurrentToken->Previous);
+FormatToken &OpeningBrace = *CurrentToken->Previous;
+assert(OpeningBrace.is(tok::l_brace));
+OpeningBrace.ParentBracket = Contexts.back().ContextKind;
 
-  ScopedContextCreator ContextCreator(*this, tok::l_brace, 1);
-  Contexts.back().ColonIsDictLiteral = true;
-  if (Left->is(BK_BracedInit))
-Contexts.back().IsExpression = true;
-  if (Style.isJavaScript() && Left->Previous &&
-  Left->Previous->is(TT_JsTypeColon))
-Contexts.back().IsExpression = false;
+if (Contexts.back().CaretFound)
+  OpeningBrace.setType(TT_ObjCBlockLBrace);
+Contexts.back().CaretFound = false;
 
-  unsigned CommaCount = 0;
-  while (CurrentToken) {
-if (CurrentToken->is(tok::r_brace)) {
-  assert(Left->Optional == CurrentToken->Optional);
-  Left->MatchingParen = CurrentToken;
-  CurrentToken->MatchingParen = Left;
-  if (Style.AlignArrayOfStructures != FormatStyle::AIAS_None) {
-if (Left->ParentBracket == tok::l_brace &&
-couldBeInStructArrayInitializer() && CommaCount > 0)
-  Contexts.back().InStructArrayInitializer = true;
-  }
-  next();
-  return true;
-}
-if (CurrentToken->isOneOf(tok::r_paren, tok::r_square))
-  return false;
-updateParameterCount(Left, CurrentToken);
-if (CurrentToken->isOneOf(tok::colon, tok::l_brace, tok::less)) {
-  FormatToken *Previous = CurrentToken->getPreviousNonComment();
-  if (Previous->is(TT_JsTypeOptionalQuestion))
-Previous = Previous->getPreviousNonComment();
-  if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
-  Style.Language == FormatStyle::LK_Proto ||
-  Style.Language == FormatStyle::LK_TextProto) {
-Left->setType(TT_DictLiteral);
-if (Previous->Tok.getIdentifierInfo() ||
-Previous->is(tok::string_literal))
-  Previous->setType(TT_SelectorName);
-  }
-  if (CurrentToken->is(tok::colon) || Style.isJavaScript())
-Left->setType(TT_DictLiteral);
+ScopedContextCreator ContextCreator(*this, tok::l_brace, 1);
+Contexts.back().ColonIsDictLiteral = true;
+if (OpeningBrace.is(BK_BracedInit))
+  Contexts.back().IsExpression = true;
+if (Style.isJavaScript() && OpeningBrace.Previous &&
+OpeningBrace.Previous->is(TT_JsTypeColon))
+  Contexts.back().IsExpression = false;
+
+unsigned CommaCount = 0;
+while (CurrentToken) {
+  if (CurrentToken->is(tok::r_brace)) {
+assert(OpeningBrace.Optional == CurrentToken->Optional);
+OpeningBrace.MatchingParen = CurrentToken;
+CurrentToken->MatchingParen = &OpeningBrace;
+if (Style.AlignArrayOfStructures != FormatStyle::AIAS_None) {
+  if (OpeningBrace.ParentBracket == tok::l_brace &&
+  couldBeInStructArrayInitializer() && CommaCount > 0)
+Contexts.back().InStructArrayInitializer = true;
 }
-if (CurrentToken->is(tok::comma)) {
-  if (Style.isJavaScript())
-Left->overwriteFixedType(TT_DictLiteral);
-  ++CommaCount;
+next();
+return true;
+  }
+  if (CurrentToken->isOneOf(tok::r_paren, tok::r_square))
+return false;
+  updateParameterCount(&OpeningBrace, CurrentToken);
+  if (CurrentToken->isOneOf(tok::colon, tok::l_brace, tok::less)) {
+FormatToken *Previous = CurrentToken->getPreviousNonComment();
+if (Previous->is(TT_JsTypeOptionalQuestion))
+  Previous = Previous->getPreviousNonComment();
+if ((CurrentToken->is(tok::colon) &&
+ (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+   

[PATCH] D121559: [clang-format] Fix crash on asm block with label

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, MyDeveloperDay, curdeius.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: All.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/54349


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121559

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -577,6 +577,15 @@
   << I;
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsAsm) {
+  auto Tokens = annotate("__asm{\n"
+ "a:\n"
+ "};");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw_asm, TT_Unknown);
+  EXPECT_TOKEN(Tokens[1], tok::l_brace, TT_InlineASMBrace);
+  EXPECT_TOKEN(Tokens[4], tok::r_brace, TT_InlineASMBrace);
+}
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -824,8 +824,10 @@
   Previous->is(tok::string_literal))
 Previous->setType(TT_SelectorName);
 }
-if (CurrentToken->is(tok::colon) || Style.isJavaScript())
+if (CurrentToken->is(tok::colon) && OpeningBrace.is(TT_Unknown))
   OpeningBrace.setType(TT_DictLiteral);
+if (Style.isJavaScript())
+  OpeningBrace.overwriteFixedType(TT_DictLiteral);
   }
   if (CurrentToken->is(tok::comma)) {
 if (Style.isJavaScript())


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -577,6 +577,15 @@
   << I;
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsAsm) {
+  auto Tokens = annotate("__asm{\n"
+ "a:\n"
+ "};");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw_asm, TT_Unknown);
+  EXPECT_TOKEN(Tokens[1], tok::l_brace, TT_InlineASMBrace);
+  EXPECT_TOKEN(Tokens[4], tok::r_brace, TT_InlineASMBrace);
+}
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -824,8 +824,10 @@
   Previous->is(tok::string_literal))
 Previous->setType(TT_SelectorName);
 }
-if (CurrentToken->is(tok::colon) || Style.isJavaScript())
+if (CurrentToken->is(tok::colon) && OpeningBrace.is(TT_Unknown))
   OpeningBrace.setType(TT_DictLiteral);
+if (Style.isJavaScript())
+  OpeningBrace.overwriteFixedType(TT_DictLiteral);
   }
   if (CurrentToken->is(tok::comma)) {
 if (Style.isJavaScript())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112019: [clang-format] [PR51412] AlignConsecutiveMacros fights with Visual Studio and resource.h

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

In D112019#3378031 , @MyDeveloperDay 
wrote:

> In D112019#3378024 , 
> @HazardyKnusperkeks wrote:
>
>> Why limit to macros, could it be a member of AlignConsecutiveStyle and apply 
>> to the other stuff as well?
>
> I personally don't have a use case other than the resource.h case that I 
> raise above, do you think this would be useful to be elsewhere in the other 
> AlignConsecutive cases?
>
> Of course, this code alters AlignMacroSequence() and not AlignTokens() what 
> if we unified the options (so they were part of the struct? but kept the 
> functionality change in separate reviews? for now the options would have no 
> effect on other Consecutive options?

I don't know if there will be a use case. The more I think about it, it maybe 
harder to apply it to the other options.
I withdraw my comment. :)


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

https://reviews.llvm.org/D112019

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

[PATCH] D121557: [clang-format][NFC] Rename Left to OpeningParen...

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2d8e907016ef: [clang-format][NFC] Rename Left to 
OpeningParen... (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121557

Files:
  clang/lib/Format/TokenAnnotator.cpp

Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -211,32 +211,34 @@
   bool parseParens(bool LookForDecls = false) {
 if (!CurrentToken)
   return false;
-FormatToken *Left = CurrentToken->Previous;
-assert(Left && "Unknown previous token");
-FormatToken *PrevNonComment = Left->getPreviousNonComment();
-Left->ParentBracket = Contexts.back().ContextKind;
+assert(CurrentToken->Previous && "Unknown previous token");
+FormatToken &OpeningParen = *CurrentToken->Previous;
+assert(OpeningParen.is(tok::l_paren));
+FormatToken *PrevNonComment = OpeningParen.getPreviousNonComment();
+OpeningParen.ParentBracket = Contexts.back().ContextKind;
 ScopedContextCreator ContextCreator(*this, tok::l_paren, 1);
 
 // FIXME: This is a bit of a hack. Do better.
 Contexts.back().ColonIsForRangeExpr =
 Contexts.size() == 2 && Contexts[0].ColonIsForRangeExpr;
 
-if (Left->Previous && Left->Previous->is(TT_UntouchableMacroFunc)) {
-  Left->Finalized = true;
+if (OpeningParen.Previous &&
+OpeningParen.Previous->is(TT_UntouchableMacroFunc)) {
+  OpeningParen.Finalized = true;
   return parseUntouchableParens();
 }
 
 bool StartsObjCMethodExpr = false;
-if (FormatToken *MaybeSel = Left->Previous) {
+if (FormatToken *MaybeSel = OpeningParen.Previous) {
   // @selector( starts a selector.
   if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous &&
   MaybeSel->Previous->is(tok::at))
 StartsObjCMethodExpr = true;
 }
 
-if (Left->is(TT_OverloadedOperatorLParen)) {
+if (OpeningParen.is(TT_OverloadedOperatorLParen)) {
   // Find the previous kw_operator token.
-  FormatToken *Prev = Left;
+  FormatToken *Prev = &OpeningParen;
   while (!Prev->is(tok::kw_operator)) {
 Prev = Prev->Previous;
 assert(Prev && "Expect a kw_operator prior to the OperatorLParen!");
@@ -255,54 +257,58 @@
   // type X = (...);
   // export type X = (...);
   Contexts.back().IsExpression = false;
-} else if (Left->Previous &&
-   (Left->Previous->isOneOf(tok::kw_static_assert, tok::kw_while,
-tok::l_paren, tok::comma) ||
-Left->Previous->isIf() ||
-Left->Previous->is(TT_BinaryOperator))) {
+} else if (OpeningParen.Previous &&
+   (OpeningParen.Previous->isOneOf(tok::kw_static_assert,
+   tok::kw_while, tok::l_paren,
+   tok::comma) ||
+OpeningParen.Previous->isIf() ||
+OpeningParen.Previous->is(TT_BinaryOperator))) {
   // static_assert, if and while usually contain expressions.
   Contexts.back().IsExpression = true;
-} else if (Style.isJavaScript() && Left->Previous &&
-   (Left->Previous->is(Keywords.kw_function) ||
-(Left->Previous->endsSequence(tok::identifier,
-  Keywords.kw_function {
+} else if (Style.isJavaScript() && OpeningParen.Previous &&
+   (OpeningParen.Previous->is(Keywords.kw_function) ||
+(OpeningParen.Previous->endsSequence(tok::identifier,
+ Keywords.kw_function {
   // function(...) or function f(...)
   Contexts.back().IsExpression = false;
-} else if (Style.isJavaScript() && Left->Previous &&
-   Left->Previous->is(TT_JsTypeColon)) {
+} else if (Style.isJavaScript() && OpeningParen.Previous &&
+   OpeningParen.Previous->is(TT_JsTypeColon)) {
   // let x: (SomeType);
   Contexts.back().IsExpression = false;
-} else if (isLambdaParameterList(Left)) {
+} else if (isLambdaParameterList(&OpeningParen)) {
   // This is a parameter list of a lambda expression.
   Contexts.back().IsExpression = false;
 } else if (Line.InPPDirective &&
-   (!Left->Previous || !Left->Previous->is(tok::identifier))) {
+   (!OpeningParen.Previous ||
+!OpeningParen.Previous->is(tok::identifier))) {
   Contexts.back().IsExpression = true;
 } else if (Contexts[Contexts.size() - 2].CaretFound) {
   // This is the parameter list of an ObjC block.
   Contexts.back().IsExpression = false;
-} else if (Left->Previous && Left->Previous->is(TT_ForE

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGacd17a2be81a: [clang-format] Fix crash on invalid requires 
expression (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121550

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -356,6 +356,14 @@
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[29], tok::kw_requires,
TT_RequiresClauseInARequiresExpression);
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "
+"requires true; };");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24041,9 +24041,9 @@
   verifyFormat(
   "template  concept C = decltype([]() -> std::true_type {\n"
   "return {};\n"
-  "  }())::value\n"
-  "  && requires(T t) { t.bar(); } &&\n"
-  "  sizeof(T) <= 8;",
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",
   Style);
 
   verifyFormat("template  concept Semiregular =\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -421,8 +421,8 @@
   if (CurrentToken->isOneOf(tok::r_square, tok::r_brace))
 return false;
 
-  if (CurrentToken->is(tok::l_brace))
-OpeningParen.setType(TT_Unknown); // Not TT_ObjCBlockLParen
+  if (CurrentToken->is(tok::l_brace) && 
OpeningParen.is(TT_ObjCBlockLParen))
+OpeningParen.setType(TT_Unknown);
   if (CurrentToken->is(tok::comma) && CurrentToken->Next &&
   !CurrentToken->Next->HasUnescapedNewline &&
   !CurrentToken->Next->isTrailingComment())


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -356,6 +356,14 @@
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[29], tok::kw_requires,
TT_RequiresClauseInARequiresExpression);
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "
+"requires true; };");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24041,9 +24041,9 @@
   verifyFormat(
   "template  concept C = decltype([]() -> std::true_type {\n"
   "return {};\n"
-  "  }())::value\n"
-  "  && requires(T t) { t.bar(); } &&\n"
-  "  sizeof(T) <= 8;",
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",
   Style);
 
   verifyFormat("template  concept Semiregular =\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -421,8 +421,8 @@
   if (CurrentToken->isOneOf(tok::r_square, tok::r_brace))
 return false;
 
-  if (CurrentToken->is(tok::l_brace))
-

[PATCH] D121558: [clang-format][NFC] Left renamed to OpeningBrace...

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb7494a1d72c1: [clang-format][NFC] Left renamed to 
OpeningBrace... (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121558

Files:
  clang/lib/Format/TokenAnnotator.cpp

Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -774,63 +774,66 @@
   }
 
   bool parseBrace() {
-if (CurrentToken) {
-  FormatToken *Left = CurrentToken->Previous;
-  Left->ParentBracket = Contexts.back().ContextKind;
+if (!CurrentToken)
+  return true;
 
-  if (Contexts.back().CaretFound)
-Left->setType(TT_ObjCBlockLBrace);
-  Contexts.back().CaretFound = false;
+assert(CurrentToken->Previous);
+FormatToken &OpeningBrace = *CurrentToken->Previous;
+assert(OpeningBrace.is(tok::l_brace));
+OpeningBrace.ParentBracket = Contexts.back().ContextKind;
 
-  ScopedContextCreator ContextCreator(*this, tok::l_brace, 1);
-  Contexts.back().ColonIsDictLiteral = true;
-  if (Left->is(BK_BracedInit))
-Contexts.back().IsExpression = true;
-  if (Style.isJavaScript() && Left->Previous &&
-  Left->Previous->is(TT_JsTypeColon))
-Contexts.back().IsExpression = false;
+if (Contexts.back().CaretFound)
+  OpeningBrace.setType(TT_ObjCBlockLBrace);
+Contexts.back().CaretFound = false;
 
-  unsigned CommaCount = 0;
-  while (CurrentToken) {
-if (CurrentToken->is(tok::r_brace)) {
-  assert(Left->Optional == CurrentToken->Optional);
-  Left->MatchingParen = CurrentToken;
-  CurrentToken->MatchingParen = Left;
-  if (Style.AlignArrayOfStructures != FormatStyle::AIAS_None) {
-if (Left->ParentBracket == tok::l_brace &&
-couldBeInStructArrayInitializer() && CommaCount > 0)
-  Contexts.back().InStructArrayInitializer = true;
-  }
-  next();
-  return true;
-}
-if (CurrentToken->isOneOf(tok::r_paren, tok::r_square))
-  return false;
-updateParameterCount(Left, CurrentToken);
-if (CurrentToken->isOneOf(tok::colon, tok::l_brace, tok::less)) {
-  FormatToken *Previous = CurrentToken->getPreviousNonComment();
-  if (Previous->is(TT_JsTypeOptionalQuestion))
-Previous = Previous->getPreviousNonComment();
-  if ((CurrentToken->is(tok::colon) &&
-   (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
-  Style.Language == FormatStyle::LK_Proto ||
-  Style.Language == FormatStyle::LK_TextProto) {
-Left->setType(TT_DictLiteral);
-if (Previous->Tok.getIdentifierInfo() ||
-Previous->is(tok::string_literal))
-  Previous->setType(TT_SelectorName);
-  }
-  if (CurrentToken->is(tok::colon) || Style.isJavaScript())
-Left->setType(TT_DictLiteral);
+ScopedContextCreator ContextCreator(*this, tok::l_brace, 1);
+Contexts.back().ColonIsDictLiteral = true;
+if (OpeningBrace.is(BK_BracedInit))
+  Contexts.back().IsExpression = true;
+if (Style.isJavaScript() && OpeningBrace.Previous &&
+OpeningBrace.Previous->is(TT_JsTypeColon))
+  Contexts.back().IsExpression = false;
+
+unsigned CommaCount = 0;
+while (CurrentToken) {
+  if (CurrentToken->is(tok::r_brace)) {
+assert(OpeningBrace.Optional == CurrentToken->Optional);
+OpeningBrace.MatchingParen = CurrentToken;
+CurrentToken->MatchingParen = &OpeningBrace;
+if (Style.AlignArrayOfStructures != FormatStyle::AIAS_None) {
+  if (OpeningBrace.ParentBracket == tok::l_brace &&
+  couldBeInStructArrayInitializer() && CommaCount > 0)
+Contexts.back().InStructArrayInitializer = true;
 }
-if (CurrentToken->is(tok::comma)) {
-  if (Style.isJavaScript())
-Left->overwriteFixedType(TT_DictLiteral);
-  ++CommaCount;
+next();
+return true;
+  }
+  if (CurrentToken->isOneOf(tok::r_paren, tok::r_square))
+return false;
+  updateParameterCount(&OpeningBrace, CurrentToken);
+  if (CurrentToken->isOneOf(tok::colon, tok::l_brace, tok::less)) {
+FormatToken *Previous = CurrentToken->getPreviousNonComment();
+if (Previous->is(TT_JsTypeOptionalQuestion))
+  Previous = Previous->getPreviousNonComment();
+if ((CurrentToken->is(tok::colon) &&
+ (!Contexts.back().ColonIsDictLiteral || !Style.isCpp())) ||
+Style.Language == FormatStyle::LK_Proto ||
+Style.Language == FormatStyle::LK_TextProto) {
+  OpeningBrace.setType(TT_DictLiteral);
+  if (Previ

[PATCH] D121559: [clang-format] Fix crash on asm block with label

[Björn Schäpers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG35abbf166d4a: [clang-format] Fix crash on asm block with 
label (authored by HazardyKnusperkeks).

Changed prior to commit:
  https://reviews.llvm.org/D121559?vs=414958&id=415068#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121559

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -602,6 +602,16 @@
   << I;
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsAsm) {
+  auto Tokens = annotate("__asm{\n"
+ "a:\n"
+ "};");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw_asm, TT_Unknown);
+  EXPECT_TOKEN(Tokens[1], tok::l_brace, TT_InlineASMBrace);
+  EXPECT_TOKEN(Tokens[4], tok::r_brace, TT_InlineASMBrace);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -824,8 +824,10 @@
   Previous->is(tok::string_literal))
 Previous->setType(TT_SelectorName);
 }
-if (CurrentToken->is(tok::colon) || Style.isJavaScript())
+if (CurrentToken->is(tok::colon) && OpeningBrace.is(TT_Unknown))
   OpeningBrace.setType(TT_DictLiteral);
+else if (Style.isJavaScript())
+  OpeningBrace.overwriteFixedType(TT_DictLiteral);
   }
   if (CurrentToken->is(tok::comma)) {
 if (Style.isJavaScript())


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -602,6 +602,16 @@
   << I;
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsAsm) {
+  auto Tokens = annotate("__asm{\n"
+ "a:\n"
+ "};");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw_asm, TT_Unknown);
+  EXPECT_TOKEN(Tokens[1], tok::l_brace, TT_InlineASMBrace);
+  EXPECT_TOKEN(Tokens[4], tok::r_brace, TT_InlineASMBrace);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -824,8 +824,10 @@
   Previous->is(tok::string_literal))
 Previous->setType(TT_SelectorName);
 }
-if (CurrentToken->is(tok::colon) || Style.isJavaScript())
+if (CurrentToken->is(tok::colon) && OpeningBrace.is(TT_Unknown))
   OpeningBrace.setType(TT_DictLiteral);
+else if (Style.isJavaScript())
+  OpeningBrace.overwriteFixedType(TT_DictLiteral);
   }
   if (CurrentToken->is(tok::comma)) {
 if (Style.isJavaScript())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121596: [clang-format] Fix crash with ObjC Blocks

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, MyDeveloperDay, curdeius.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: All.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/54367
Fixes https://github.com/llvm/llvm-project/issues/54368


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121596

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -612,6 +612,22 @@
   EXPECT_TOKEN(Tokens[4], tok::r_brace, TT_InlineASMBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsObjCBlock) {
+  auto Tokens = annotate("int (^)() = ^ ()\n"
+ "  external_source_symbol() { //\n"
+ "  return 1;\n"
+ "};");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ObjCBlockLParen);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_ObjCBlockLBrace);
+
+  Tokens = annotate("int *p = ^int*(){ //\n"
+"  return nullptr;\n"
+"}();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_ObjCBlockLBrace);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -783,7 +783,7 @@
 OpeningBrace.ParentBracket = Contexts.back().ContextKind;
 
 if (Contexts.back().CaretFound)
-  OpeningBrace.setType(TT_ObjCBlockLBrace);
+  OpeningBrace.overwriteFixedType(TT_ObjCBlockLBrace);
 Contexts.back().CaretFound = false;
 
 ScopedContextCreator ContextCreator(*this, tok::l_brace, 1);


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -612,6 +612,22 @@
   EXPECT_TOKEN(Tokens[4], tok::r_brace, TT_InlineASMBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsObjCBlock) {
+  auto Tokens = annotate("int (^)() = ^ ()\n"
+ "  external_source_symbol() { //\n"
+ "  return 1;\n"
+ "};");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ObjCBlockLParen);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_ObjCBlockLBrace);
+
+  Tokens = annotate("int *p = ^int*(){ //\n"
+"  return nullptr;\n"
+"}();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_ObjCBlockLBrace);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -783,7 +783,7 @@
 OpeningBrace.ParentBracket = Contexts.back().ContextKind;
 
 if (Contexts.back().CaretFound)
-  OpeningBrace.setType(TT_ObjCBlockLBrace);
+  OpeningBrace.overwriteFixedType(TT_ObjCBlockLBrace);
 Contexts.back().CaretFound = false;
 
 ScopedContextCreator ContextCreator(*this, tok::l_brace, 1);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121596: [clang-format] Fix crash with ObjC Blocks

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:615
 
+TEST_F(TokenAnnotatorTest, UnderstandsObjCBlock) {
+  auto Tokens = annotate("int (^)() = ^ ()\n"

I hope the name is appropriate, I don't know shit about objective c.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121596

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

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D121550#3381903 , @owenpan wrote:

> Nvm. See https://github.com/llvm/llvm-project/issues/54384.

Phew. I thought I needed a fix for the fix, for the fix, for the fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121550

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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Some test with a main header and blocks would be nice.




Comment at: clang/lib/Format/Format.cpp:2779
 /*CheckMainHeader=*/!MainIncludeFound && FirstIncludeBlock);
-int Priority = Categories.getSortIncludePriority(
-IncludeName, !MainIncludeFound && FirstIncludeBlock);
+int Priority = WithSemicolon ? INT_MAX
+ : Categories.getSortIncludePriority(

I don't know about the others, but I prefer C++: `std::numeric_limits`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D121596: [clang-format] Fix crash with ObjC Blocks

[Björn Schäpers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1db8112311c7: [clang-format] Fix crash with ObjC Blocks 
(authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121596

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -644,6 +644,22 @@
   EXPECT_TOKEN(Tokens[4], tok::r_brace, TT_InlineASMBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsObjCBlock) {
+  auto Tokens = annotate("int (^)() = ^ ()\n"
+ "  external_source_symbol() { //\n"
+ "  return 1;\n"
+ "};");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ObjCBlockLParen);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_ObjCBlockLBrace);
+
+  Tokens = annotate("int *p = ^int*(){ //\n"
+"  return nullptr;\n"
+"}();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_ObjCBlockLBrace);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -783,7 +783,7 @@
 OpeningBrace.ParentBracket = Contexts.back().ContextKind;
 
 if (Contexts.back().CaretFound)
-  OpeningBrace.setType(TT_ObjCBlockLBrace);
+  OpeningBrace.overwriteFixedType(TT_ObjCBlockLBrace);
 Contexts.back().CaretFound = false;
 
 ScopedContextCreator ContextCreator(*this, tok::l_brace, 1);


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -644,6 +644,22 @@
   EXPECT_TOKEN(Tokens[4], tok::r_brace, TT_InlineASMBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsObjCBlock) {
+  auto Tokens = annotate("int (^)() = ^ ()\n"
+ "  external_source_symbol() { //\n"
+ "  return 1;\n"
+ "};");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ObjCBlockLParen);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_ObjCBlockLBrace);
+
+  Tokens = annotate("int *p = ^int*(){ //\n"
+"  return nullptr;\n"
+"}();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_ObjCBlockLBrace);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -783,7 +783,7 @@
 OpeningBrace.ParentBracket = Contexts.back().ContextKind;
 
 if (Contexts.back().CaretFound)
-  OpeningBrace.setType(TT_ObjCBlockLBrace);
+  OpeningBrace.overwriteFixedType(TT_ObjCBlockLBrace);
 Contexts.back().CaretFound = false;
 
 ScopedContextCreator ContextCreator(*this, tok::l_brace, 1);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D121370#3383574 , 
@HazardyKnusperkeks wrote:

> Some test with a main header and blocks would be nice.

This one please. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D121758: [clang-format] Add support for formatting Verilog code

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

This is an enormous patch. @MyDeveloperDay mentioned different places which 
could be separate patches, but in addition could the adding of the verilog 
language be split up, so that one can comprehend the patch in a reasonable time?




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:534
 continue;
-  parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
- /*MunchSemi=*/true, /*UnindentWhitesmithBraces=*/false,
- CanContainBracedList,
+  parseBlock(/*Flags=*/CanContainBracedList * 
BLOCK_CAN_CONTAIN_BRACED_LIST,
+ /*AddLevels=*/1u,

MyDeveloperDay wrote:
> sstwcw wrote:
> > One of the people in charge said multiplying with a boolean might trigger 
> > warnings.  Here I compiled with gcc.  This version doesn't trigger 
> > warnings.  The other way to do it, `CanContainBracedList ? 
> > BLOCK_CAN_CONTAIN_BRACED_LIST : 0`, triggers a warning that I shouldn't mix 
> > enum and integer.
> this is a hard no from me.
bool should only be used as boolean value, not as integral, even if c++ allows 
it.



Comment at: clang/lib/Format/UnwrappedLineParser.h:110
+BLOCK_VERILOG_HIER = 0x10
+  };
+  IfStmtKind parseBlock(unsigned Flags = 0u, unsigned AddLevels = 1u,

MyDeveloperDay wrote:
> Oh! please no... I can't say how much  of my life has been ruined by flags 
> and the various `|  || & && ~&` bugs,  I'm sorry I'd rather have a structure 
> and it be clear
struct would be better, yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121758

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

[PATCH] D121754: [clang-format] Refactor determineStarAmpUsage

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2161
 
-if (PrevToken->isOneOf(tok::l_paren, tok::l_square, tok::l_brace,
-   tok::comma, tok::semi, tok::kw_return, tok::colon,
-   tok::kw_co_return, tok::kw_co_await,
-   tok::kw_co_yield, tok::equal, tok::kw_delete,
-   tok::kw_sizeof, tok::kw_throw, TT_BinaryOperator,
-   TT_ConditionalExpr, TT_UnaryOperator, 
TT_CastRParen))
+if (determinePlusMinusCaretUsage(Tok) == TT_UnaryOperator)
   return TT_UnaryOperator;

curdeius wrote:
> As below, before, question, kw_return, kw_case, at were not handled here.
> Please add tests.
This needs a comment or better a new common function. Otherwise it's just 
unnecessary hard to understand why we now check for PlusMinusCaret. (I've never 
heard of this function and I've stepped a lot through clang-format. ;))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121754

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

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3069
 
-bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
-  const FormatToken &Left,
-  const FormatToken &Right) {
+bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
+ const FormatToken &Right) {

I'd kept the name `spaceRequiredBetween`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121755

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

[PATCH] D121753: [clang-format] Use a macro for non-C keywords

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/FormatToken.h:907
+  KEYWORD(infer, 0)
\
+  KEYWORD(is, ATTR_JS_KEYWORD | ATTR_CSHARP_KEYWORD | ATTR_CSHARP_KEYWORD) 
\
+  KEYWORD(let, ATTR_JS_KEYWORD | ATTR_CSHARP_KEYWORD)  
\

Doubled



Comment at: clang/lib/Format/FormatToken.h:911
+  KEYWORD(readonly,
\
+  ATTR_JS_KEYWORD | ATTR_CSHARP_KEYWORD | ATTR_CSHARP_KEYWORD) 
\
+  KEYWORD(set, ATTR_JS_KEYWORD | ATTR_CSHARP_KEYWORD)  
\

Doubled



Comment at: clang/lib/Format/FormatToken.h:924
+  KEYWORD(interface,   
\
+  ATTR_JS_KEYWORD | ATTR_CSHARP_KEYWORD | ATTR_CSHARP_KEYWORD) 
\
+  KEYWORD(native, 0)   
\

doubled



Comment at: clang/lib/Format/FormatToken.h:947
+  /* C# */ 
\
+  KEYWORD(dollar, 0)   
\
+  KEYWORD(base, ATTR_CSHARP_KEYWORD)   
\

Why not CSharp?



Comment at: clang/lib/Format/FormatToken.h:992-999
+  enum {
+ATTR_JS_KEYWORD = 0x1,
+ATTR_CSHARP_KEYWORD = 0x2,
+  };
+  unsigned getAttrs(const FormatToken &Tok) const {
+auto At = KeywordAttr.find(Tok.Tok.getIdentifierInfo());
+return At == KeywordAttr.end() ? 0u : At->second;

No fan of DEFINE_LIKE_NAMES. More a fan of scoped::enums. ;)



Comment at: clang/lib/Format/FormatToken.h:1149
-case tok::identifier: {
-  // For identifiers, make sure they are true identifiers, excluding the
-  // JavaScript pseudo-keywords (not lexed by LLVM/clang as keywords).

Keep the comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:115
   // Handle Qt signals.
-  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_Q_SIGNALS) 
&&
 RootToken.Next && RootToken.Next->is(tok::colon)))

I can live with that, but the former was nicer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121753

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

[PATCH] D121756: [clang-format] Clean up code looking for if statements

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Please add tests in TokenAnnotatorTests for `TT_ConditionLParen`.




Comment at: clang/lib/Format/FormatToken.h:521
+  /// statement's condition like if or while.
+  bool isConditionLParen(bool IncludeSpecial) const {
+if (!is(tok::l_paren))

Please document what this means.



Comment at: clang/lib/Format/TokenAnnotator.cpp:133
 Left->ParentBracket != tok::less &&
-(isKeywordWithCondition(*Line.First) ||
- CurrentToken->getStartOfNonWhitespace() ==

Any reason why one doesn't need this check anymore?



Comment at: clang/lib/Format/TokenAnnotator.cpp:1431-1442
+TT_AttributeMacro, TT_BracedListLBrace, TT_ClassLBrace,
+TT_CompoundRequirementLBrace, TT_ConditionLParen, TT_EnumLBrace,
+TT_FatArrow, TT_ForEachMacro, TT_FunctionLBrace,
+TT_FunctionLikeOrFreestandingMacro, TT_IfMacro,
+TT_ImplicitStringLiteral, TT_InlineASMBrace, TT_LambdaArrow,
+TT_LambdaLBrace, TT_LambdaLSquare, TT_NamespaceMacro,
+TT_ObjCStringLiteral, TT_OverloadedOperator, TT_RecordLBrace,

Unrelated Change.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2426
+if (FormatTok->Tok.is(tok::l_paren)) {
+  FormatTok->setType(TT_ConditionLParen);
   parseParens();

Please use `setFixedType`.
Then you don't need to add `TT_ConditionLParen` to the exclude list in the 
continuation indenter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121756

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

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Looks basically okay.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2851
-addUnwrappedLine();
-++Line->Level;
-parseStructuralElement();

This is completely missing. Didn't it affect anything?



Comment at: clang/lib/Format/UnwrappedLineParser.h:128
   void parseTryCatch();
+  void parseIndentedBlock(bool BracesAreOptional = true,
+  bool RBraceOnSeparateLine = true);

I'm no fan of default arguments.
But we have them all around...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121757

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

[PATCH] D121754: [clang-format] Refactor determineStarAmpUsage

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2138
+
+// These keywords are deliberately not included here. They are either
+// included in determineStarAmpUsage or determinePlusMinusCaretUsage.

I'm not sure about this. Why not handle them here too?



Comment at: clang/lib/Format/TokenAnnotator.cpp:2146-2147
+//   know how they can be followed by a star or amp.
+// co_await, delete - It doesn't make sense to have them followed by a 
unary
+//   `+` or `-`.
+if (PrevToken->isOneOf(TT_ConditionalExpr, tok::l_paren, tok::comma,

Especially here, why should a `+` after `delete` be a binary operator?
How much sense it makes doesn't matter, it is valid c++: 
https://gcc.godbolt.org/z/c1x1nn3Ej



Comment at: clang/unittests/Format/FormatTest.cpp:9758
+  verifyFormat("for (x = 0; -10 < x; --x) {\n}");
+  verifyFormat("sizeof -x");
+  verifyFormat("sizeof +x");

A format test is fine, a token annotator test would be better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121754

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

[PATCH] D121753: [clang-format] Use a macro for non-C keywords

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/FormatToken.h:992-999
+  enum {
+ATTR_JS_KEYWORD = 0x1,
+ATTR_CSHARP_KEYWORD = 0x2,
+  };
+  unsigned getAttrs(const FormatToken &Tok) const {
+auto At = KeywordAttr.find(Tok.Tok.getIdentifierInfo());
+return At == KeywordAttr.end() ? 0u : At->second;

sstwcw wrote:
> HazardyKnusperkeks wrote:
> > No fan of DEFINE_LIKE_NAMES. More a fan of scoped::enums. ;)
> It looks like I would have to use the `KeywordLanguage::` prefix throughout 
> the macro list and also define an operator to combine categories for keywords 
> that several languages have.  Do you know of a simpler way?  With the current 
> enum, the DEFINE_LIKE_NAMES can't be used directly outside 
> `AdditionalKeywords`.
You can always drop the `class` and are back at integers.



Comment at: clang/lib/Format/FormatToken.h:992-999
+  enum {
+ATTR_JS_KEYWORD = 0x1,
+ATTR_CSHARP_KEYWORD = 0x2,
+  };
+  unsigned getAttrs(const FormatToken &Tok) const {
+auto At = KeywordAttr.find(Tok.Tok.getIdentifierInfo());
+return At == KeywordAttr.end() ? 0u : At->second;

HazardyKnusperkeks wrote:
> sstwcw wrote:
> > HazardyKnusperkeks wrote:
> > > No fan of DEFINE_LIKE_NAMES. More a fan of scoped::enums. ;)
> > It looks like I would have to use the `KeywordLanguage::` prefix throughout 
> > the macro list and also define an operator to combine categories for 
> > keywords that several languages have.  Do you know of a simpler way?  With 
> > the current enum, the DEFINE_LIKE_NAMES can't be used directly outside 
> > `AdditionalKeywords`.
> You can always drop the `class` and are back at integers.
> With the current enum, the DEFINE_LIKE_NAMES can't be used directly outside 
> `AdditionalKeywords`.

This one I don't understand? They are public and can be used, can't they? And 
the type I can get with `decltype`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121753

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

[PATCH] D121756: [clang-format] Clean up code looking for if statements

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1431-1442
+TT_AttributeMacro, TT_BracedListLBrace, TT_ClassLBrace,
+TT_CompoundRequirementLBrace, TT_ConditionLParen, TT_EnumLBrace,
+TT_FatArrow, TT_ForEachMacro, TT_FunctionLBrace,
+TT_FunctionLikeOrFreestandingMacro, TT_IfMacro,
+TT_ImplicitStringLiteral, TT_InlineASMBrace, TT_LambdaArrow,
+TT_LambdaLBrace, TT_LambdaLSquare, TT_NamespaceMacro,
+TT_ObjCStringLiteral, TT_OverloadedOperator, TT_RecordLBrace,

sstwcw wrote:
> HazardyKnusperkeks wrote:
> > Unrelated Change.
> Sorry.  This part has changed several times in the main line since I started 
> working on this patch.  After a few merge conflicts I got tired and started 
> running sort uniq on this part.
Nothing against that, but in a separate patch.
Although my goal is the get this list smaller again with the finalized type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121756

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

[PATCH] D121907: [clang-format] Use an enum for context types.

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D121907#3390226 , @MyDeveloperDay 
wrote:

> So out of interest, what is the reason? my assumption is that you wanted to 
> add more for Verilog and you felt adding the extra bools was the wrong design 
> and its better an an enum
>
>   bool InCpp11AttributeSpecifier = false;
>   bool InCSharpAttributeSpecifier = false;
>
> Does the fact that some aren't exclusive make you think its ok to split it 
> into enums and bools is ok?  (no real opinion just wondered what you and 
> others think?)

It is really helpful to see that these are exclusive. It helps reasoning about 
the code.




Comment at: clang/lib/Format/TokenAnnotator.cpp:1498
 bool CaretFound = false;
-bool IsForEachMacro = false;
+// These two are not handled like the rest in ContextType because they are
+// not exclusive.

This is a comment for the change, afterwards it's strange to see for newcomers. 
Since it's without `Context`. *scnr*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121907

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

[PATCH] D121916: [clang-format] [doc] Add script to automatically update help output in ClangFormat.rst.

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/tools/clang-format/ClangFormat.cpp:105
+SortIncludes("sort-includes",
+ cl::desc("If set, overrides the include sorting behavior\n"
+  "determined by the SortIncludes style flag"),

Couldn't one split the string in python? I think an arbitrary position to split 
the help isn't nice. I for one have often the terminal over half the monitor 
spread, sometimes even the complete monitor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121916

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

[PATCH] D121754: [clang-format] Refactor determineStarAmpUsage

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2146-2147
+//   know how they can be followed by a star or amp.
+// co_await, delete - It doesn't make sense to have them followed by a 
unary
+//   `+` or `-`.
+if (PrevToken->isOneOf(TT_ConditionalExpr, tok::l_paren, tok::comma,

sstwcw wrote:
> sstwcw wrote:
> > MyDeveloperDay wrote:
> > > HazardyKnusperkeks wrote:
> > > > Especially here, why should a `+` after `delete` be a binary operator?
> > > > How much sense it makes doesn't matter, it is valid c++: 
> > > > https://gcc.godbolt.org/z/c1x1nn3Ej
> > > I'm also trying to understand did you mean you couldn't have
> > > 
> > > case -1:
> > > case +1:
> > > case +0:
> > > case  -0:
> > > 
> > > https://gcc.godbolt.org/z/qvE44d5xz
> > In the new version `+` following `delete` is a unary operator.  Previously 
> > I was under the impression that we only formatted code that is sensible.
> > case -1:
> > case +1:
> 
> I meant we couldn't have things like `case *x` or `case &x`.  Is the comment 
> not clear enough?
> > case -1:
> > case +1:
> 
> I meant we couldn't have things like `case *x` or `case &x`.  Is the comment 
> not clear enough?

But we can: https://gcc.godbolt.org/z/8Mb1xo7oP

> In the new version `+` following `delete` is a unary operator.  Previously I 
> was under the impression that we only formatted code that is sensible.

What is `sensible` this is rather subjective, isn't it? I went a long way to 
get requires clauses and expressions doing to "right thing" and even added 
explicitly the unit tests for stuff I think no sane person would ever write, 
but it's valid code. So in my opinion it should be formatted correctly. I got 
it wrong on a couple of cases, all but one that I'm aware of are fixed now.

And even if it were invalid code, when it makes the functions to reason about 
easier why not? **I** think invalid code can be formatted weirdly, once it's 
valid again we will format it correctly. But all valid code should be formatted 
as best as we can.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2146-2147
+//   know how they can be followed by a star or amp.
+// co_await, delete - It doesn't make sense to have them followed by a 
unary
+//   `+` or `-`.
+if (PrevToken->isOneOf(TT_ConditionalExpr, tok::l_paren, tok::comma,

HazardyKnusperkeks wrote:
> sstwcw wrote:
> > sstwcw wrote:
> > > MyDeveloperDay wrote:
> > > > HazardyKnusperkeks wrote:
> > > > > Especially here, why should a `+` after `delete` be a binary operator?
> > > > > How much sense it makes doesn't matter, it is valid c++: 
> > > > > https://gcc.godbolt.org/z/c1x1nn3Ej
> > > > I'm also trying to understand did you mean you couldn't have
> > > > 
> > > > case -1:
> > > > case +1:
> > > > case +0:
> > > > case  -0:
> > > > 
> > > > https://gcc.godbolt.org/z/qvE44d5xz
> > > In the new version `+` following `delete` is a unary operator.  
> > > Previously I was under the impression that we only formatted code that is 
> > > sensible.
> > > case -1:
> > > case +1:
> > 
> > I meant we couldn't have things like `case *x` or `case &x`.  Is the 
> > comment not clear enough?
> > > case -1:
> > > case +1:
> > 
> > I meant we couldn't have things like `case *x` or `case &x`.  Is the 
> > comment not clear enough?
> 
> But we can: https://gcc.godbolt.org/z/8Mb1xo7oP
> 
> > In the new version `+` following `delete` is a unary operator.  Previously 
> > I was under the impression that we only formatted code that is sensible.
> 
> What is `sensible` this is rather subjective, isn't it? I went a long way to 
> get requires clauses and expressions doing to "right thing" and even added 
> explicitly the unit tests for stuff I think no sane person would ever write, 
> but it's valid code. So in my opinion it should be formatted correctly. I got 
> it wrong on a couple of cases, all but one that I'm aware of are fixed now.
> 
> And even if it were invalid code, when it makes the functions to reason about 
> easier why not? **I** think invalid code can be formatted weirdly, once it's 
> valid again we will format it correctly. But all valid code should be 
> formatted as best as we can.
> In the new version `+` following `delete` is a unary operator.  Previously I 
> was under the impression that we only formatted code that is sensible.





Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:85
+  auto Tokens = annotate("x - 0");
+  EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::minus, TT_BinaryOperator);

I know the other cases also use EXPECT, but the size should be an ASSERT, so 
there would be no seg fault if the number would change (it shouldn't).



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:157
+  EXPECT_TOKEN(Tokens[1], tok::plus, TT_UnaryOperator);
+  Tokens = annotate("(int)-x");
+  EXPECT_EQ(Tok

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

FWIW I proposed to go this way: 
https://discourse.llvm.org/t/clang-format-spacerequiredbefore-vs-spacerequiredbetween/60901/5?u=hazardyknusperkeks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121755

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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

But please fix the format. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D121450: [clang-format] Handle attributes before case label.

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1826
+  if (Style.isJavaScript() && Line->MustBeDeclaration)
+// 'case: string' field declaration.
+break;

Here's the loop. In the switch before that worked, since it was not in a loop. 
Here we need to do something. (nextToken()?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121450

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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:2694-2697
+if (!Matches[i].empty()) {
+  res = Matches[i];
+  break;
+}





Comment at: clang/lib/Format/Format.cpp:2699
+  }
+  assert(!res.empty());
+  return res;

Is there something like LLVM_UNREACHABLE?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D121756: [clang-format] Clean up code looking for if statements

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added a comment.

Just a formality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121756

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

[PATCH] D121754: [clang-format] Refactor determineStarAmpUsage

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Just a side note, I often get this on your changes:
Unhandled Exception ("Exception")
Found unknown intradiff source line, expected a line beginning with "+", "-", 
or " " (space): \ No newline at end of file
.




Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:85
+  auto Tokens = annotate("x - 0");
+  EXPECT_EQ(Tokens.size(), 4u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::minus, TT_BinaryOperator);

sstwcw wrote:
> HazardyKnusperkeks wrote:
> > I know the other cases also use EXPECT, but the size should be an ASSERT, 
> > so there would be no seg fault if the number would change (it shouldn't).
> I tried a wrong number and `EXPECT_EQ` didn't give a segfault.
The potential segfault is not in `EXPECT_EQ`, but in accessing a non existent 
token.
The number of tokens will most likely never change, but than again we don't 
need the `EXPECT_EQ` either.
But if it changes (gets smaller) we want to abort the test after the failed 
check and not accessing `Tokens` with an invalid index, that's why I'm pleading 
(and using) `ASSERT_EQ`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121754

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

[PATCH] D122064: [clang-format][docs] Fix incorrect 'clang-format 11' option markers

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D122064#3430644 , @kuzkry wrote:

> Kind reminder, please deliver this revision. I don't have write access.

It's not forgotten, at least by me, but I currently have no time to update my 
LLVM, so I could push anything.
You just could ask for push rights, especially if you want to change something 
more in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122064

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

[PATCH] D123450: [clang-format] Parse Verilog if statements

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/FormatToken.h:374
+  /// Verilog we want to treat the backtick like a hash.
+  tok::TokenKind AliasToken = tok::unknown;
+

Can't we do that with a type?

I'm not very happy about the alias, because you can still call `Tok.getKind()`.



Comment at: clang/lib/Format/FormatToken.h:1157
+VerilogExtraKeywords = std::unordered_set(
+{kw_always,   kw_always_comb,  kw_always_ff,kw_always_latch,
+ kw_assert,   kw_assign,   kw_assume,   kw_automatic,

sstwcw wrote:
> Does anyone know why this part gets aligned unlike the two lists above?
Have you reformatted the other lines with the same config and revision?
If yes, my guess would be the missing comment.



Comment at: clang/lib/Format/FormatToken.h:1533
+switch (Tok.Tok.getKind()) {
+case tok::kw_case:
+case tok::kw_class:

So you have a blacklist what is not a keyword? Seems a bit non future proof, 
new C++ keywords would have to be added here.



Comment at: clang/lib/Format/FormatToken.h:1593
+
+  std::unordered_set VerilogExtraKeywords;
 };

For consistency reasons add the comment like above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123450

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