date:20210103

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits

atirit added a comment.

OK, I'm getting a few more failed unit tests and I'm really not sure what 
correct formatting behaviour is anymore, so I'll just ask.

Assuming the following settings:

  BreakBeforeBraces: Custom
  BraceWrapping: {
AfterEnum: true
  }
  AllowShortEnumsOnASingleLine: true

Which of these is correct? (`FormatTest,AllmanBraceBreaking`)
A (expected in test):

  enum X
  {
Y = 0
  }

B:

  enum X { Y = 0 }

Which of these is correct? (new test)
A:

  enum
  {
 A,
 B
  } Enum1, Enum2

B:

  enum
  {
 A,
 B
  } Enum1,
 Enum2

C:

  enum { A, B } Enum1, Enum2

`true/true` results in option B of the "new test" section. The strange 
behaviour with `Enum1` and `Enum2` occurs when `AllowShortEnumsOnASingleLine` 
isn't allowed to shrink a line for whatever reason, be it a comma on the last 
case, a comment, or too long an `enum`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits

atirit added a comment.

I think accepting a revision that includes only a fix for `AfterEnum` being 
ignored (not the corner case) and the new unit test would be the best way to go 
about this, since they're separate bugs. Then I can fix the corner case (and 
compatibility with the new unit test) in a separate differential.

However, as I've already mentioned, I'm new here, so I defer to the judgment of 
those more experienced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93985: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits

tinloaf created this revision.
tinloaf added reviewers: MyDeveloperDay, djasper.
tinloaf requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently, empty lines and comments break alignment of assignments on 
consecutive
lines. This makes the AlignConsecutiveAssignments option an enum that allows 
controlling
whether empty lines or empty lines and comments should be ignored when aligning
assignments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93985

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  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
@@ -11330,7 +11330,7 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
@@ -11569,7 +11569,7 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
@@ -12271,7 +12271,7 @@
 
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
-  Style.AlignConsecutiveAssignments = true;
+  Style.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Style.AlignConsecutiveDeclarations = true;
   Style.AlignConsecutiveMacros = false;
 
@@ -12364,10 +12364,374 @@
Style);
 }
 
+TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossEmptyLines) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACA_AcrossEmptyLines;
+
+  Alignment.MaxEmptyLinesToKeep = 10;
+  /* Test alignment across empty lines */
+  EXPECT_EQ("int a   = 5;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("int a   = 5;\n"
+   "\n"
+   "int oneTwoThree= 123;",
+   Alignment));
+  EXPECT_EQ("int a   = 5;\n"
+"int one = 1;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;",
+   Alignment));
+  EXPECT_EQ("int a   = 5;\n"
+"int one = 1;\n"
+"\n"
+"int oneTwoThree = 123;\n"
+"int oneTwo  = 12;",
+format("int a = 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n"
+   "int oneTwo = 12;",
+   Alignment));
+
+  /* Test across comments */
+  EXPECT_EQ("int a = 5;\n"
+"/* block comment */\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "/* block comment */\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  EXPECT_EQ("int a = 5;\n"
+"// line comment\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "// line comment\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  /* Test across comments and newlines */
+  EXPECT_EQ("int a = 5;\n"
+"\n"
+"/* block comment */\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "\n"
+   "/* block comment */\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  EXPECT_EQ("int a = 5;\n"
+"\n"
+"// line comment\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "\n"
+   "// line comment\n"
+   "int oneTwoThree=123;",
+   Alignment));
+}
+
+TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossComments) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACA_AcrossComments;
+  verifyFormat("int a   = 5;\n"
+   "int oneTwoThree = 123;",
+   Alignment);
+  verifyFormat("int a   = method();\n"
+   "int oneTwoThree = 133;",
+   Alignment);
+  verifyFormat("a &= 5;\n"
+   "bcd *= 5;\n"
+   "ghtyf += 5;\n"
+   "dvfvdb -= 5;\n"
+   "a /= 5;\n"
+   "vdsvsv %= 5;\n"
+

[PATCH] D93985: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits

tinloaf added a comment.

One remark: I made the change only for AlignConsecutiveAssignments, but I think 
it should be consistent for AlignConsecutiveMacros, AlignConsecutiveBitFields 
and AlignConsecutiveDeclarations, as well. If this change is accepted, I will 
make corresponding changes to the other three.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93985

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

[PATCH] D93985: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits

tinloaf updated this revision to Diff 314276.
tinloaf added a comment.

Fix bogus include of iostream


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93985

Files:
  clang/lib/Format/WhitespaceManager.cpp


Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -13,7 +13,6 @@
 
 #include "WhitespaceManager.h"
 #include "llvm/ADT/STLExtras.h"
-#include 
 
 namespace clang {
 namespace format {


Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -13,7 +13,6 @@
 
 #include "WhitespaceManager.h"
 #include "llvm/ADT/STLExtras.h"
-#include 
 
 namespace clang {
 namespace format {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93985: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits

tinloaf updated this revision to Diff 314277.
tinloaf added a comment.

[format] Add the possibility to align assignments spanning empty lines or 
comments

Currently, empty lines and comments break alignment of assignments on 
consecutive
lines. This makes the AlignConsecutiveAssignments option an enum that allows 
controlling
whether empty lines or empty lines and comments should be ignored when aligning
assignments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93985

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  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
@@ -11330,7 +11330,7 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
@@ -11569,7 +11569,7 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
@@ -12271,7 +12271,7 @@
 
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
-  Style.AlignConsecutiveAssignments = true;
+  Style.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Style.AlignConsecutiveDeclarations = true;
   Style.AlignConsecutiveMacros = false;
 
@@ -12364,10 +12364,374 @@
Style);
 }
 
+TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossEmptyLines) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACA_AcrossEmptyLines;
+
+  Alignment.MaxEmptyLinesToKeep = 10;
+  /* Test alignment across empty lines */
+  EXPECT_EQ("int a   = 5;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("int a   = 5;\n"
+   "\n"
+   "int oneTwoThree= 123;",
+   Alignment));
+  EXPECT_EQ("int a   = 5;\n"
+"int one = 1;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;",
+   Alignment));
+  EXPECT_EQ("int a   = 5;\n"
+"int one = 1;\n"
+"\n"
+"int oneTwoThree = 123;\n"
+"int oneTwo  = 12;",
+format("int a = 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n"
+   "int oneTwo = 12;",
+   Alignment));
+
+  /* Test across comments */
+  EXPECT_EQ("int a = 5;\n"
+"/* block comment */\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "/* block comment */\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  EXPECT_EQ("int a = 5;\n"
+"// line comment\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "// line comment\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  /* Test across comments and newlines */
+  EXPECT_EQ("int a = 5;\n"
+"\n"
+"/* block comment */\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "\n"
+   "/* block comment */\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  EXPECT_EQ("int a = 5;\n"
+"\n"
+"// line comment\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "\n"
+   "// line comment\n"
+   "int oneTwoThree=123;",
+   Alignment));
+}
+
+TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossComments) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACA_AcrossComments;
+  verifyFormat("int a   = 5;\n"
+   "int oneTwoThree = 123;",
+   Alignment);
+  verifyFormat("int a   = method();\n"
+   "int oneTwoThree = 133;",
+   Alignment);
+  verifyFormat("a &= 5;\n"
+   "bcd *= 5;\n"
+   "ghtyf += 5;\n"
+   "dvfvdb -= 5;\n"
+   "a /= 5;\n"
+

[PATCH] D93986: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits

tinloaf created this revision.
tinloaf added reviewers: MyDeveloperDay, djasper.
tinloaf requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently, empty lines and comments break alignment of assignments on 
consecutive
lines. This makes the AlignConsecutiveAssignments option an enum that allows 
controlling
whether empty lines or empty lines and comments should be ignored when aligning
assignments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93986

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  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
@@ -11330,7 +11330,7 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
@@ -11569,7 +11569,7 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
@@ -12271,7 +12271,7 @@
 
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
-  Style.AlignConsecutiveAssignments = true;
+  Style.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Style.AlignConsecutiveDeclarations = true;
   Style.AlignConsecutiveMacros = false;
 
@@ -12364,10 +12364,374 @@
Style);
 }
 
+TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossEmptyLines) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACA_AcrossEmptyLines;
+
+  Alignment.MaxEmptyLinesToKeep = 10;
+  /* Test alignment across empty lines */
+  EXPECT_EQ("int a   = 5;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("int a   = 5;\n"
+   "\n"
+   "int oneTwoThree= 123;",
+   Alignment));
+  EXPECT_EQ("int a   = 5;\n"
+"int one = 1;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;",
+   Alignment));
+  EXPECT_EQ("int a   = 5;\n"
+"int one = 1;\n"
+"\n"
+"int oneTwoThree = 123;\n"
+"int oneTwo  = 12;",
+format("int a = 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n"
+   "int oneTwo = 12;",
+   Alignment));
+
+  /* Test across comments */
+  EXPECT_EQ("int a = 5;\n"
+"/* block comment */\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "/* block comment */\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  EXPECT_EQ("int a = 5;\n"
+"// line comment\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "// line comment\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  /* Test across comments and newlines */
+  EXPECT_EQ("int a = 5;\n"
+"\n"
+"/* block comment */\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "\n"
+   "/* block comment */\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  EXPECT_EQ("int a = 5;\n"
+"\n"
+"// line comment\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "\n"
+   "// line comment\n"
+   "int oneTwoThree=123;",
+   Alignment));
+}
+
+TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossComments) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACA_AcrossComments;
+  verifyFormat("int a   = 5;\n"
+   "int oneTwoThree = 123;",
+   Alignment);
+  verifyFormat("int a   = method();\n"
+   "int oneTwoThree = 133;",
+   Alignment);
+  verifyFormat("a &= 5;\n"
+   "bcd *= 5;\n"
+   "ghtyf += 5;\n"
+   "dvfvdb -= 5;\n"
+   "a /= 5;\n"
+   "vdsvsv %= 5;\n"
+

[PATCH] D93986: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits

tinloaf added a comment.

Two remarks, first: I made the change only for AlignConsecutiveAssignments, but 
I think it should be consistent for AlignConsecutiveMacros, 
AlignConsecutiveBitFields and AlignConsecutiveDeclarations, as well. If this 
change is accepted, I will make corresponding changes to the other three.

Second: Sorry if this change pops up twice. I botched the first revision by not 
understanding arcanist.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

In D93938#2476432 , @atirit wrote:

> I think accepting a revision that includes only a fix for `AfterEnum` being 
> ignored (not the corner case) and the new unit test would be the best way to 
> go about this, since they're separate bugs. Then I can fix the corner case 
> (and compatibility with the new unit test) in a separate differential.
>
> However, as I've already mentioned, I'm new here, so I defer to the judgment 
> of those more experienced.

I'm also quite new, but if that are different issues they should receive their 
own commit (and thus review).

You should always be clear what is not working correctly and reflect that's 
working after your change with tests which illustrate the now working code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93988: [ASTMatchers] Make tests explicit about mode-dependence

2021-01-03 Thread Stephen Kelly via Phabricator via cfe-commits

steveire created this revision.
steveire added a reviewer: aaron.ballman.
steveire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93988

Files:
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -26,13 +26,14 @@
 }
 
 TEST(DeclarationMatcher, ClassDerivedFromDependentTemplateSpecialization) {
-  EXPECT_TRUE(matches(
-"template  struct A {"
-  "  template  struct F {};"
-  "};"
-  "template  struct B : A::template F {};"
-  "B b;",
-cxxRecordDecl(hasName("B"), isDerivedFrom(recordDecl();
+  EXPECT_TRUE(
+  matches("template  struct A {"
+  "  template  struct F {};"
+  "};"
+  "template  struct B : A::template F {};"
+  "B b;",
+  traverse(TK_AsIs, cxxRecordDecl(hasName("B"),
+  isDerivedFrom(recordDecl());
 }
 
 TEST(DeclarationMatcher, hasDeclContext) {
@@ -262,9 +263,9 @@
 }
 
 TEST(HasDeclaration, HasDeclarationOfCXXNewExpr) {
-  EXPECT_TRUE(
-  matches("int *A = new int();",
-  cxxNewExpr(hasDeclaration(functionDecl(parameterCountIs(1));
+  EXPECT_TRUE(matches("int *A = new int();",
+  traverse(TK_AsIs, cxxNewExpr(hasDeclaration(functionDecl(
+parameterCountIs(1)));
 }
 
 TEST(HasDeclaration, HasDeclarationOfTypeAlias) {
@@ -981,60 +982,63 @@
 
 TEST(Matcher, MatchesTypeTemplateArgument) {
   EXPECT_TRUE(matches(
-"template struct B {};"
+  "template struct B {};"
   "B b;",
-classTemplateSpecializationDecl(hasAnyTemplateArgument(refersToType(
-  asString("int"));
+  traverse(TK_AsIs, classTemplateSpecializationDecl(hasAnyTemplateArgument(
+refersToType(asString("int")));
 }
 
 TEST(Matcher, MatchesTemplateTemplateArgument) {
-  EXPECT_TRUE(matches("template class S> class X {};"
-  "template class Y {};"
-  "X xi;",
-  classTemplateSpecializationDecl(hasAnyTemplateArgument(
-  refersToTemplate(templateName());
+  EXPECT_TRUE(matches(
+  "template class S> class X {};"
+  "template class Y {};"
+  "X xi;",
+  traverse(TK_AsIs, classTemplateSpecializationDecl(hasAnyTemplateArgument(
+refersToTemplate(templateName()));
 }
 
 TEST(Matcher, MatchesDeclarationReferenceTemplateArgument) {
-  EXPECT_TRUE(matches(
-"struct B { int next; };"
-  "template struct A {};"
-  "A<&B::next> a;",
-classTemplateSpecializationDecl(hasAnyTemplateArgument(
-  refersToDeclaration(fieldDecl(hasName("next")));
+  EXPECT_TRUE(
+  matches("struct B { int next; };"
+  "template struct A {};"
+  "A<&B::next> a;",
+  traverse(TK_AsIs,
+   classTemplateSpecializationDecl(hasAnyTemplateArgument(
+   refersToDeclaration(fieldDecl(hasName("next";
 
   EXPECT_TRUE(notMatches(
-"template  struct A {};"
+  "template  struct A {};"
   "A a;",
-classTemplateSpecializationDecl(hasAnyTemplateArgument(
-  refersToDeclaration(decl());
+  traverse(TK_AsIs, classTemplateSpecializationDecl(hasAnyTemplateArgument(
+refersToDeclaration(decl()));
 
   EXPECT_TRUE(matches(
-"struct B { int next; };"
+  "struct B { int next; };"
   "template struct A {};"
   "A<&B::next> a;",
-templateSpecializationType(hasAnyTemplateArgument(isExpr(
-  hasDescendant(declRefExpr(to(fieldDecl(hasName("next"));
+  traverse(TK_AsIs, templateSpecializationType(hasAnyTemplateArgument(
+isExpr(hasDescendant(declRefExpr(
+to(fieldDecl(hasName("next")));
 
   EXPECT_TRUE(notMatches(
-"template  struct A {};"
+  "template  struct A {};"
   "A a;",
-templateSpecializationType(hasAnyTemplateArgument(
-  refersToDeclaration(decl());
+  traverse(TK_AsIs, templateSpecializationType(hasAnyTemplateArgument(
+refersToDeclaration(decl()));
 }
 
 
 TEST(Matcher, MatchesSpecificArgument) {
   EXPECT_TRUE(matches(
-"template class A {};"
+  "template class A {};"
   "A a;",
-classTemplateSpecializationDecl(hasTemplateArgument(
-  1, refersToType(asString("int"));
+  traverse(TK_AsIs, classTemplateSpecializationDecl(ha

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

You need to have these conversations by adding new unit tests that prove your 
point, I highly doubt I'll personally be willing to accept any revision without 
more unit tests than this one line change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:1346
   verifyFormat("enum { A, B, C } ShortEnum1, ShortEnum2;", Style);
   Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum {\n"

EXPECT_FALSE(Style.BraceWrapping.AfterEnum)



Comment at: clang/unittests/Format/FormatTest.cpp:1352
"} ShortEnum1, ShortEnum2;",
Style);
 }

please add

```
verifyFormat("enum {\n"
 "A,\n"
 "B,\n"
"} ShortEnum",
Style);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93986: [format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits

tinloaf updated this revision to Diff 314284.
tinloaf added a comment.

Fix formatting issue resulting from ancient clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  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
@@ -11330,7 +11330,7 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
@@ -11569,7 +11569,7 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
@@ -12271,7 +12271,7 @@
 
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
-  Style.AlignConsecutiveAssignments = true;
+  Style.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Style.AlignConsecutiveDeclarations = true;
   Style.AlignConsecutiveMacros = false;
 
@@ -12364,10 +12364,374 @@
Style);
 }
 
+TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossEmptyLines) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACA_AcrossEmptyLines;
+
+  Alignment.MaxEmptyLinesToKeep = 10;
+  /* Test alignment across empty lines */
+  EXPECT_EQ("int a   = 5;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("int a   = 5;\n"
+   "\n"
+   "int oneTwoThree= 123;",
+   Alignment));
+  EXPECT_EQ("int a   = 5;\n"
+"int one = 1;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;",
+   Alignment));
+  EXPECT_EQ("int a   = 5;\n"
+"int one = 1;\n"
+"\n"
+"int oneTwoThree = 123;\n"
+"int oneTwo  = 12;",
+format("int a = 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n"
+   "int oneTwo = 12;",
+   Alignment));
+
+  /* Test across comments */
+  EXPECT_EQ("int a = 5;\n"
+"/* block comment */\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "/* block comment */\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  EXPECT_EQ("int a = 5;\n"
+"// line comment\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "// line comment\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  /* Test across comments and newlines */
+  EXPECT_EQ("int a = 5;\n"
+"\n"
+"/* block comment */\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "\n"
+   "/* block comment */\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  EXPECT_EQ("int a = 5;\n"
+"\n"
+"// line comment\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "\n"
+   "// line comment\n"
+   "int oneTwoThree=123;",
+   Alignment));
+}
+
+TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossComments) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACA_AcrossComments;
+  verifyFormat("int a   = 5;\n"
+   "int oneTwoThree = 123;",
+   Alignment);
+  verifyFormat("int a   = method();\n"
+   "int oneTwoThree = 133;",
+   Alignment);
+  verifyFormat("a &= 5;\n"
+   "bcd *= 5;\n"
+   "ghtyf += 5;\n"
+   "dvfvdb -= 5;\n"
+   "a /= 5;\n"
+   "vdsvsv %= 5;\n"
+   "sfdbddfbdfbb ^= 5;\n"
+   "dvsdsv |= 5;\n"
+   "int dsvvdvsdvvv = 123;",
+   Alignment);
+  verifyFormat("int i = 1, j = 10;\n"
+   "something = 2000;",
+   Alignment);
+  verifyFormat("somethi

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

2021-01-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added a comment.

In D92936#2476356 , @vitalybuka wrote:

> Something is not initialized
> http://lab.llvm.org:8011/#/builders/74/builds/1834/steps/9/logs/stdio

Confirmed; @nullptr.cpp what do you want to do about this? I hypothesize that 
maybe you're not allowed to look at `Seq.getFailedOverloadResult()` (nor 
`Seq.getFailedCandidateSet()`) unless `Seq.getFailureKind()` is one of 
`FK_ConstructorOverloadFailed`, `FK_UserConversionOverloadFailed`, 
`FK_ReferenceInitOverloadFailed`, or `FK_ListConstructorOverloadFailed`.  The 
ctor `InitializationSequence::InitializationSequence` member-initializes 
`FailedCandidateSet` but does not member-initialize `FailedOverloadResult`. 
Perhaps the appropriate fix would be

  --- a/clang/lib/Sema/SemaInit.cpp
  +++ b/clang/lib/Sema/SemaInit.cpp
  @@ -5595,7 +5595,8 @@ InitializationSequence::InitializationSequence(Sema &S,
  MultiExprArg Args,
  bool TopLevelOfInitList,
  bool 
TreatUnavailableAsInvalid)
  -: FailedCandidateSet(Kind.getLocation(), 
OverloadCandidateSet::CSK_Normal) {
  +: FailedOverloadResult(OR_Success),
  +  FailedCandidateSet(Kind.getLocation(), 
OverloadCandidateSet::CSK_Normal) {
 InitializeFrom(S, Entity, Kind, Args, TopLevelOfInitList,
TreatUnavailableAsInvalid);

I've tested this locally and it doesn't cause any new tests to fail (whew!), 
but I haven't rebuilt everything with MSAN to see if this satisfies Vitaly's 
buildbot (and in fact since I'm on OSX I don't think I //can// rebuild with 
MSAN because OSX doesn't support MSAN).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92936

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

[PATCH] D93817: Update transformations to use poison for insertelement/shufflevector's placeholder value

2021-01-03 Thread Juneyoung Lee via Phabricator via cfe-commits

aqjune updated this revision to Diff 314289.
aqjune added a comment.

Rebase
I'll continue splitting after working on lifetime patches


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93817

Files:
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector-constrained.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector2-constrained.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector2.c
  clang/test/CodeGen/X86/avx-shuffle-builtins.c
  clang/test/CodeGen/aarch64-bf16-dotprod-intrinsics.c
  clang/test/CodeGen/aarch64-bf16-getset-intrinsics.c
  clang/test/CodeGen/aarch64-bf16-lane-intrinsics.c
  clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c
  clang/test/CodeGen/aarch64-neon-dot-product.c
  clang/test/CodeGen/arm-bf16-convert-intrinsics.c
  clang/test/CodeGen/arm-bf16-dotprod-intrinsics.c
  clang/test/CodeGen/arm-bf16-getset-intrinsics.c
  clang/test/CodeGen/arm-neon-dot-product.c
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/InterleavedAccessPass.cpp
  llvm/lib/Target/X86/X86InstCombineIntrinsic.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
  llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
  
llvm/test/Transforms/CodeGenPrepare/ARM/sink-add-mul-shufflevector-inseltpoison.ll
  
llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-demanded-vector-elts-inseltpoison.ll
  llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-demanded-vector-elts.ll
  llvm/test/Transforms/InstCombine/X86/x86-avx2-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-avx2.ll
  llvm/test/Transforms/InstCombine/X86/x86-avx512-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-avx512.ll
  llvm/test/Transforms/InstCombine/X86/x86-f16c-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-f16c.ll
  llvm/test/Transforms/InstCombine/X86/x86-vector-shifts-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-vector-shifts.ll
  llvm/test/Transforms/InstCombine/X86/x86-vpermil-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-vpermil.ll
  llvm/test/Transforms/InstCombine/bitcast-vec-canon-inseltpoison.ll
  llvm/test/Transforms/InstCombine/bitcast-vec-canon.ll
  llvm/test/Transforms/InstCombine/broadcast-inseltpoison.ll
  llvm/test/Transforms/InstCombine/broadcast.ll
  llvm/test/Transforms/InstCombine/canonicalize-vector-extract.ll
  llvm/test/Transforms/InstCombine/extractelement-inseltpoison.ll
  llvm/test/Transforms/InstCombine/extractelement.ll
  llvm/test/Transforms/InstCombine/gep-inbounds-null.ll
  llvm/test/Transforms/InstCombine/getelementptr.ll
  llvm/test/Transforms/InstCombine/icmp-vec-inseltpoison.ll
  llvm/test/Transforms/InstCombine/icmp-vec.ll
  llvm/test/Transforms/InstCombine/insert-extract-shuffle-inseltpoison.ll
  llvm/test/Transforms/InstCombine/insert-extract-shuffle.ll
  llvm/test/Transforms/InstCombine/masked_intrinsics-inseltpoison.ll
  llvm/test/Transforms/InstCombine/masked_intrinsics.ll
  llvm/test/Transforms/InstCombine/obfuscated_splat-inseltpoison.ll
  llvm/test/Transforms/InstCombine/obfuscated_splat.ll
  llvm/test/Transforms/InstCombine/select-extractelement-inseltpoison.ll
  llvm/test/Transforms/InstCombine/select-extractelement.ll
  llvm/test/Transforms/InstCombine/shufflevector-div-rem-inseltpoison.ll
  llvm/test/Transforms/InstCombine/shufflevector-div-rem.ll
  llvm/test/Transforms/InstCombine/trunc-extractelement-inseltpoison.ll
  llvm/test/Transforms/InstCombine/trunc-extractelement.ll
  llvm/test/Transforms/InstCombine/trunc-inseltpoison.ll
  llvm/test/Transforms/InstCombine/trunc.ll
  llvm/test/Transforms/InstCombine/vec-binop-select-inseltpoison.ll
  llvm/test/Transforms/InstCombine/vec-binop-select.ll
  llvm/test/Transforms/InstCombine/vec_demanded_elts-inseltpoison.ll
  llvm/test/Transforms/InstCombine/vec_demanded_elts.ll
  llvm/test/Transforms/InstCombine/vec_gep_scalar_arg-inseltpoison.ll
  llvm/test/Transforms/InstCombine/vec_shuffle-inseltpoison.ll
  llvm/test/Transforms/InstCombine/vec_shuffle.ll
  llvm/test/Transforms/InstCombine/vector-casts-inseltpoison.ll
  llvm/test/Transforms/InstCombine/vscale_cmp.ll
  llvm/test/Transforms/InstSimplify/shufflevector-inseltpoison.ll
  
llvm/test/Transforms/LoopVectorize/AArch64/extractvalue-no-scalarization-required.ll
  llvm/test/Transforms/LoopVectorize/SystemZ/addressing.ll
  
llvm/test/Transforms/LoopVectorize/SystemZ/predicated-first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/X86/load-deref-pred.ll
  llvm/test/Transforms/LoopVectorize/X86/metadata-enable.ll
  llvm/test/Transforms/LoopVectorize/X86/strided_load_cost.ll
  llvm/test/Transforms/LoopVectorize/X86/uniform_mem_op.l

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

by and large, this looks pretty good to me..




Comment at: clang/docs/ClangFormatStyleOptions.rst:198
 
-**AlignConsecutiveAssignments** (``bool``)
+**AlignConsecutiveAssignments** (``AlignConsecutiveAssignmentsStyle``)
   If ``true``, aligns consecutive assignments.

Did you regenerate this or make the changes by hand?

you need to run clang/tools/dump_style.py



Comment at: clang/include/clang/Format/Format.h:110
+
+  /// Styles for alignment of consecutive assignments
+  enum AlignConsecutiveAssignmentsStyle {

you need to provide full context diffs

https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface



Comment at: clang/lib/Format/WhitespaceManager.cpp:390
 
+  // Whether the current line consists purely of comments
+  bool LineIsComment = true;

add "." at the end


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:12581
+"\n"
+"/* block comment */\n"
+"\n"

can you add an example with a block comment that spans multiple lines

e.g.

```
int a = 5
/*
 * block comment
*/
int oneTwoThree = 123;
```


```
int a = 5
///
/// block comment
///
int oneTwoThree = 123;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits

tinloaf updated this revision to Diff 314293.
tinloaf marked 3 inline comments as done.
tinloaf added a comment.

Address MyDeveloperDay's review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  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
@@ -11330,7 +11330,7 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
@@ -11569,7 +11569,7 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Tab.AlignConsecutiveDeclarations = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
@@ -12271,7 +12271,7 @@
 
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
-  Style.AlignConsecutiveAssignments = true;
+  Style.AlignConsecutiveAssignments = FormatStyle::ACA_Consecutive;
   Style.AlignConsecutiveDeclarations = true;
   Style.AlignConsecutiveMacros = false;
 
@@ -12364,10 +12364,398 @@
Style);
 }
 
+TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossEmptyLines) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACA_AcrossEmptyLines;
+
+  Alignment.MaxEmptyLinesToKeep = 10;
+  /* Test alignment across empty lines */
+  EXPECT_EQ("int a   = 5;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("int a   = 5;\n"
+   "\n"
+   "int oneTwoThree= 123;",
+   Alignment));
+  EXPECT_EQ("int a   = 5;\n"
+"int one = 1;\n"
+"\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;",
+   Alignment));
+  EXPECT_EQ("int a   = 5;\n"
+"int one = 1;\n"
+"\n"
+"int oneTwoThree = 123;\n"
+"int oneTwo  = 12;",
+format("int a = 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n"
+   "int oneTwo = 12;",
+   Alignment));
+
+  /* Test across comments */
+  EXPECT_EQ("int a = 5;\n"
+"/* block comment */\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "/* block comment */\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  EXPECT_EQ("int a = 5;\n"
+"// line comment\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "// line comment\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  /* Test across comments and newlines */
+  EXPECT_EQ("int a = 5;\n"
+"\n"
+"/* block comment */\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "\n"
+   "/* block comment */\n"
+   "int oneTwoThree=123;",
+   Alignment));
+
+  EXPECT_EQ("int a = 5;\n"
+"\n"
+"// line comment\n"
+"int oneTwoThree = 123;",
+format("int a = 5;\n"
+   "\n"
+   "// line comment\n"
+   "int oneTwoThree=123;",
+   Alignment));
+}
+
+TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossComments) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACA_AcrossComments;
+  verifyFormat("int a   = 5;\n"
+   "int oneTwoThree = 123;",
+   Alignment);
+  verifyFormat("int a   = method();\n"
+   "int oneTwoThree = 133;",
+   Alignment);
+  verifyFormat("a &= 5;\n"
+   "bcd *= 5;\n"
+   "ghtyf += 5;\n"
+   "dvfvdb -= 5;\n"
+   "a /= 5;\n"
+   "vdsvsv %= 5;\n"
+   "sfdbddfbdfbb ^= 5;\n"
+   "dvsdsv |= 5;\n"
+   "int dsvvdvsdvvv = 123;",
+   Alignment);
+  verifyFormat("int i = 1, j = 10;\n"
+   "something = 2000;",
+   Alignment)

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits

atirit added a comment.

In D93938#2476519 , @MyDeveloperDay 
wrote:

> You need to have these conversations by adding new unit tests that prove your 
> point, I highly doubt I'll personally be willing to accept any revision 
> without more unit tests than this one line change

And that's why I said:

> I think accepting a revision that includes only a fix for AfterEnum being 
> ignored (not the corner case) **and the new unit test** would be the best way 
> to go about this.

Obviously the current revision isn't sufficient. I'll be submitting a new one 
shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits

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



Comment at: clang/include/clang/Format/Format.h:110
+
+  /// Styles for alignment of consecutive assignments
+  enum AlignConsecutiveAssignmentsStyle {

MyDeveloperDay wrote:
> you need to provide full context diffs
> 
> https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
Sorry - I messed up my first try using arcanist, now I just used 'git diff' and 
forgot about -U99. Should be fixed in the newest diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits

atirit updated this revision to Diff 314296.
atirit added a comment.

Added unit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

Files:
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1352,6 +1352,49 @@
Style);
 }
 
+TEST_F(FormatTest, AfterEnum) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+
+  Style.AllowShortEnumsOnASingleLine = true;
+  Style.BraceWrapping.AfterEnum = true;
+  verifyFormat("enum { A, B, C } Test1, AfterEnum2;", Style);
+  verifyFormat("enum\n"
+   "{\n"
+   "  A,\n"
+   "  B, // foo\n"
+   "  C\n"
+   "} Test2,\n"
+   "AfterEnum2;",
+   Style);
+  Style.BraceWrapping.AfterEnum = false;
+  verifyFormat("enum { A, B, C } Test3, AfterEnum2;", Style);
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B, // foo\n"
+   "  C\n"
+   "} Test4,\n"
+   "AfterEnum2;",
+   Style);
+
+  Style.AllowShortEnumsOnASingleLine = false;
+  Style.BraceWrapping.AfterEnum = true;
+  verifyFormat("enum\n"
+   "{\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "} Test5, AfterEnum2;",
+   Style);
+  Style.BraceWrapping.AfterEnum = false;
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "} Test6, AfterEnum2;",
+   Style);
+}
+
 TEST_F(FormatTest, ShortCaseLabels) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortCaseLabelsOnASingleLine = true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1352,6 +1352,49 @@
Style);
 }
 
+TEST_F(FormatTest, AfterEnum) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+
+  Style.AllowShortEnumsOnASingleLine = true;
+  Style.BraceWrapping.AfterEnum = true;
+  verifyFormat("enum { A, B, C } Test1, AfterEnum2;", Style);
+  verifyFormat("enum\n"
+   "{\n"
+   "  A,\n"
+   "  B, // foo\n"
+   "  C\n"
+   "} Test2,\n"
+   "AfterEnum2;",
+   Style);
+  Style.BraceWrapping.AfterEnum = false;
+  verifyFormat("enum { A, B, C } Test3, AfterEnum2;", Style);
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B, // foo\n"
+   "  C\n"
+   "} Test4,\n"
+   "AfterEnum2;",
+   Style);
+
+  Style.AllowShortEnumsOnASingleLine = false;
+  Style.BraceWrapping.AfterEnum = true;
+  verifyFormat("enum\n"
+   "{\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "} Test5, AfterEnum2;",
+   Style);
+  Style.BraceWrapping.AfterEnum = false;
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "} Test6, AfterEnum2;",
+   Style);
+}
+
 TEST_F(FormatTest, ShortCaseLabels) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortCaseLabelsOnASingleLine = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits

atirit added a comment.

The first test fails due to the aforementioned corner case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D72281: [Matrix] Add matrix type to Clang.

2021-01-03 Thread Florian Hahn via Phabricator via cfe-commits

fhahn added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:7855
+  return;
+RowsExpr = Columns.get();
+  } else {

RKSimon wrote:
> @fhahn Should this be ColsExpr?
> ```
> ColsExpr = Columns.get();
> ```
> Noticed when looking at 
> https://llvm.org/reports/scan-build/report-SemaType.cpp-BuildMatrixType-11-1.html#EndPath
Thanks for pointing that out! Looks like a copy-paste error. I am just trying 
to come up with a test that actually exercises this code path (or remove it all 
together).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72281

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

we don't commit with failing tests so lets understand why it fails.

Can you add the tests without multiple names for the enum


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits

atirit updated this revision to Diff 314299.
atirit added a comment.

Removed multiple enum names from new test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

Files:
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1358,23 +1358,21 @@
 
   Style.AllowShortEnumsOnASingleLine = true;
   Style.BraceWrapping.AfterEnum = true;
-  verifyFormat("enum { A, B, C } Test1, AfterEnum2;", Style);
+  verifyFormat("enum { A, B, C } Test1;", Style);
   verifyFormat("enum\n"
"{\n"
"  A,\n"
"  B, // foo\n"
"  C\n"
-   "} Test2,\n"
-   "AfterEnum2;",
+   "} Test2;",
Style);
   Style.BraceWrapping.AfterEnum = false;
-  verifyFormat("enum { A, B, C } Test3, AfterEnum2;", Style);
+  verifyFormat("enum { A, B, C } Test3;", Style);
   verifyFormat("enum {\n"
"  A,\n"
"  B, // foo\n"
"  C\n"
-   "} Test4,\n"
-   "AfterEnum2;",
+   "} Test4;",
Style);
 
   Style.AllowShortEnumsOnASingleLine = false;
@@ -1384,14 +1382,14 @@
"  A,\n"
"  B,\n"
"  C\n"
-   "} Test5, AfterEnum2;",
+   "} Test5;",
Style);
   Style.BraceWrapping.AfterEnum = false;
   verifyFormat("enum {\n"
"  A,\n"
"  B,\n"
"  C\n"
-   "} Test6, AfterEnum2;",
+   "} Test6;",
Style);
 }
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1358,23 +1358,21 @@
 
   Style.AllowShortEnumsOnASingleLine = true;
   Style.BraceWrapping.AfterEnum = true;
-  verifyFormat("enum { A, B, C } Test1, AfterEnum2;", Style);
+  verifyFormat("enum { A, B, C } Test1;", Style);
   verifyFormat("enum\n"
"{\n"
"  A,\n"
"  B, // foo\n"
"  C\n"
-   "} Test2,\n"
-   "AfterEnum2;",
+   "} Test2;",
Style);
   Style.BraceWrapping.AfterEnum = false;
-  verifyFormat("enum { A, B, C } Test3, AfterEnum2;", Style);
+  verifyFormat("enum { A, B, C } Test3;", Style);
   verifyFormat("enum {\n"
"  A,\n"
"  B, // foo\n"
"  C\n"
-   "} Test4,\n"
-   "AfterEnum2;",
+   "} Test4;",
Style);
 
   Style.AllowShortEnumsOnASingleLine = false;
@@ -1384,14 +1382,14 @@
"  A,\n"
"  B,\n"
"  C\n"
-   "} Test5, AfterEnum2;",
+   "} Test5;",
Style);
   Style.BraceWrapping.AfterEnum = false;
   verifyFormat("enum {\n"
"  A,\n"
"  B,\n"
"  C\n"
-   "} Test6, AfterEnum2;",
+   "} Test6;",
Style);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits

atirit added a comment.

`AfterEnum: true` currently overrides `AllowShortEnumsOnASingleLine: true`. If 
this is epxected behaviour then I'll modify the test to accomodate that, but 
otherwise, there's a separate issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:130
+/// \endcode
+ACA_AcrossComments
+  };

Is there a case for aligning over empty lines and comments?

```
int a   = 5;

/* comment */
int oneTwoThree = 123;
```



Comment at: clang/lib/Format/Format.cpp:140
+IO.enumCase(Value, "AcrossEmptyLines", FormatStyle::ACA_AcrossEmptyLines);
+IO.enumCase(Value, "AcrossComments", FormatStyle::ACA_AcrossComments);
+  }

move true and false to the bottom and separate with 

```// For backward compatibility.```

comment (see below)





Comment at: clang/lib/Format/WhitespaceManager.cpp:367
+unsigned StartAt, bool AcrossEmpty = false,
+bool AcrossComments = false) {
   unsigned MinColumn = 0;

could this be an enum?

```
enum {
None
AcrossEmptyLines,
AcrossComments,
AcrossEmptyLinesAndComments,
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits

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

I'm sorry, but now you are missing the files from the review, I think you 
diffing against your own commits and not commit that are in the repo. This 
makes the review very difficult to understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:421
   // matching token, the sequence ends here.
-  if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine)
+  if (((Changes[i].NewlinesBefore > 1) && (!AcrossEmpty)) ||
+  (!FoundMatchOnLine && (!(LineIsComment && AcrossComments

Nit: unnecessary parentheses around !AcrossEmpty.
Same around !(LineIsComment && AcrossComments).
Maybe you might factor out a bool variable for this condition?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Lukas Barth via Phabricator via cfe-commits

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



Comment at: clang/include/clang/Format/Format.h:130
+/// \endcode
+ACA_AcrossComments
+  };

MyDeveloperDay wrote:
> Is there a case for aligning over empty lines and comments?
> 
> ```
> int a   = 5;
> 
> /* comment */
> int oneTwoThree = 123;
> ```
Not sure I understand what you mean. Currently, the Option `AcrossComments` 
includes 'across empty lines'. So, there currently is no case for "across 
comments, but not across empty lines". I'm not sure if that is really something 
people want to do. Do you think so? I can add it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits

atirit added a comment.

I don't understand; should every commit I've made be squashed into one and then 
submitted here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits

atirit updated this revision to Diff 314301.
atirit added a comment.

Squashed commits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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
@@ -1344,12 +1344,52 @@
   Style.AllowShortEnumsOnASingleLine = true;
   verifyFormat("enum { A, B, C } ShortEnum1, ShortEnum2;", Style);
   Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "} ShortEnum1, ShortEnum2;",
+   Style);
+}
+
+TEST_F(FormatTest, AfterEnum) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+
+  Style.AllowShortEnumsOnASingleLine = true;
+  Style.BraceWrapping.AfterEnum = true;
+  verifyFormat("enum { A, B, C } Test1;", Style);
+  verifyFormat("enum\n"
+   "{\n"
+   "  A,\n"
+   "  B, // foo\n"
+   "  C\n"
+   "} Test2;",
+   Style);
+  Style.BraceWrapping.AfterEnum = false;
+  verifyFormat("enum { A, B, C } Test3;", Style);
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B, // foo\n"
+   "  C\n"
+   "} Test4;",
+   Style);
+
+  Style.AllowShortEnumsOnASingleLine = false;
+  Style.BraceWrapping.AfterEnum = true;
   verifyFormat("enum\n"
"{\n"
"  A,\n"
"  B,\n"
"  C\n"
-   "} ShortEnum1, ShortEnum2;",
+   "} Test5;",
+   Style);
+  Style.BraceWrapping.AfterEnum = false;
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "} Test6;",
Style);
 }
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -694,6 +694,8 @@
 return Style.BraceWrapping.AfterUnion;
   if (InitialToken.is(tok::kw_struct))
 return Style.BraceWrapping.AfterStruct;
+  if (InitialToken.is(tok::kw_enum))
+return Style.BraceWrapping.AfterEnum;
   return false;
 }
 
@@ -2482,8 +2484,9 @@
 return true;
   }
 
-  if (!Style.AllowShortEnumsOnASingleLine)
+  if (!Style.AllowShortEnumsOnASingleLine && Style.BraceWrapping.AfterEnum)
 addUnwrappedLine();
+
   // Parse enum body.
   nextToken();
   if (!Style.AllowShortEnumsOnASingleLine) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1344,12 +1344,52 @@
   Style.AllowShortEnumsOnASingleLine = true;
   verifyFormat("enum { A, B, C } ShortEnum1, ShortEnum2;", Style);
   Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "} ShortEnum1, ShortEnum2;",
+   Style);
+}
+
+TEST_F(FormatTest, AfterEnum) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+
+  Style.AllowShortEnumsOnASingleLine = true;
+  Style.BraceWrapping.AfterEnum = true;
+  verifyFormat("enum { A, B, C } Test1;", Style);
+  verifyFormat("enum\n"
+   "{\n"
+   "  A,\n"
+   "  B, // foo\n"
+   "  C\n"
+   "} Test2;",
+   Style);
+  Style.BraceWrapping.AfterEnum = false;
+  verifyFormat("enum { A, B, C } Test3;", Style);
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B, // foo\n"
+   "  C\n"
+   "} Test4;",
+   Style);
+
+  Style.AllowShortEnumsOnASingleLine = false;
+  Style.BraceWrapping.AfterEnum = true;
   verifyFormat("enum\n"
"{\n"
"  A,\n"
"  B,\n"
"  C\n"
-   "} ShortEnum1, ShortEnum2;",
+   "} Test5;",
+   Style);
+  Style.BraceWrapping.AfterEnum = false;
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "} Test6;",
Style);
 }
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -694,6 +694,8 @@
 return Style.BraceWrapping.AfterUnion;
   if (InitialToken.is(tok::kw_struct))
 return Style.BraceWrapping.AfterStruct;
+  if (InitialToken.is(tok::kw_enum))
+return Style.Bra

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:1347
   Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "} ShortEnum1, ShortEnum2;",
+   Style);
+}
+
+TEST_F(FormatTest, AfterEnum) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+
+  Style.AllowShortEnumsOnASingleLine = true;
+  Style.BraceWrapping.AfterEnum = true;
+  verifyFormat("enum { A, B, C } Test1;", Style);
+  verifyFormat("enum\n"
+   "{\n"
+   "  A,\n"
+   "  B, // foo\n"
+   "  C\n"
+   "} Test2;",
+   Style);
+  Style.BraceWrapping.AfterEnum = false;
+  verifyFormat("enum { A, B, C } Test3;", Style);
+  verifyFormat("enum {\n"
+   "  A,\n"
+   "  B, // foo\n"
+   "  C\n"
+   "} Test4;",
+   Style);
+
+  Style.AllowShortEnumsOnASingleLine = false;
+  Style.BraceWrapping.AfterEnum = true;
   verifyFormat("enum\n"
"{\n"

keep this test, you should keep one with 1 name and 1 with 2 names


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-03 Thread Ally Tiritoglu via Phabricator via cfe-commits

atirit added a comment.

The test still is there; Git is just showing the diff strangely. I didn't 
modify that test at all. The corner case bug doesn't affect 
`FormatTest.ShortEnums` because that test effectively has `AfterEnum: false`. 
Should I add cases for `AfterEnum: true` in that test too? I had figured the 
new test took care of that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D93999: [clang] Fix message text for `-Wpointer-sign` to account for plain char

2021-01-03 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: aaron.ballman, rsmith.
Herald added subscribers: jfb, jvesely.
hubert.reinterpretcast requested review of this revision.
Herald added a project: clang.

The `-Wpointer-sign` warning text is inappropriate for describing the 
incompatible pointer conversion between plain `char` and explicitly 
`signed`/`unsigned` `char` (whichever plain `char` has the same range as) and 
vice versa.

Specifically, in part, it reads "converts between pointers to integer types 
with different sign". This patch changes that portion to read instead as 
"converts between pointers to integer types that differ by 
signed/unsigned/plain variation".

C17 subclause 6.5.16.1 indicates that the conversions resulting in 
`-Wpointer-sign` warnings in assignment-like contexts are constraint 
violations. This means that strict conformance requires a diagnostic for the 
case where the message text is wrong before this patch. The lack of an even 
more specialized warning group is consistent with GCC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93999

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/Sema/incompatible-sign.c
  clang/test/Sema/incompatible-sign.cpp
  clang/test/Sema/vector-ops.c
  clang/test/SemaObjC/objc-cf-audited-warning.m
  clang/test/SemaOpenCL/builtins-amdgcn-error.cl

Index: clang/test/SemaOpenCL/builtins-amdgcn-error.cl
===
--- clang/test/SemaOpenCL/builtins-amdgcn-error.cl
+++ clang/test/SemaOpenCL/builtins-amdgcn-error.cl
@@ -156,7 +156,7 @@
   const char ptr[] = "workgroup";
   val = __builtin_amdgcn_atomic_inc32(&val, val, __ATOMIC_ACQUIRE, ptr); // expected-error {{expression is not a string literal}}
   int signedVal = 15;
-  signedVal = __builtin_amdgcn_atomic_inc32(&signedVal, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private int *' to parameter of type 'volatile __private unsigned int *' converts between pointers to integer types with different sign}}
+  signedVal = __builtin_amdgcn_atomic_inc32(&signedVal, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private int *' to parameter of type 'volatile __private unsigned int *' converts between pointers to integer types that differ by signed/unsigned/plain variation}}
 }
 
 void test_atomic_inc64() {
@@ -170,7 +170,7 @@
   const char ptr[] = "workgroup";
   val = __builtin_amdgcn_atomic_inc64(&val, val, __ATOMIC_ACQUIRE, ptr); // expected-error {{expression is not a string literal}}
   __INT64_TYPE__ signedVal = 15;
-  signedVal = __builtin_amdgcn_atomic_inc64(&signedVal, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private long *' to parameter of type 'volatile __private unsigned long *' converts between pointers to integer types with different sign}}
+  signedVal = __builtin_amdgcn_atomic_inc64(&signedVal, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private long *' to parameter of type 'volatile __private unsigned long *' converts between pointers to integer types that differ by signed/unsigned/plain variation}}
 }
 
 void test_atomic_dec32() {
@@ -184,7 +184,7 @@
   const char ptr[] = "workgroup";
   val = __builtin_amdgcn_atomic_dec32(&val, val, __ATOMIC_ACQUIRE, ptr); // expected-error {{expression is not a string literal}}
   int signedVal = 15;
-  signedVal = __builtin_amdgcn_atomic_dec32(&signedVal, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private int *' to parameter of type 'volatile __private unsigned int *' converts between pointers to integer types with different sign}}
+  signedVal = __builtin_amdgcn_atomic_dec32(&signedVal, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private int *' to parameter of type 'volatile __private unsigned int *' converts between pointers to integer types that differ by signed/unsigned/plain variation}}
 }
 
 void test_atomic_dec64() {
@@ -198,5 +198,5 @@
   const char ptr[] = "workgroup";
   val = __builtin_amdgcn_atomic_dec64(&val, val, __ATOMIC_ACQUIRE, ptr); // expected-error {{expression is not a string literal}}
   __INT64_TYPE__ signedVal = 15;
-  signedVal = __builtin_amdgcn_atomic_dec64(&signedVal, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private long *' to parameter of type 'volatile __private unsigned long *' converts between pointers to integer types with different sign}}
+  signedVal = __builtin_amdgcn_atomic_dec64(&signedVal, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private long *' to parameter of type 'volatile __private unsigned long *' converts between pointers to integer types that differ by signed/unsigned/plain variation}}
 }
Index: clang/test/SemaObjC/objc-cf-audited-warning.m
===
--- clang/test/SemaObjC/objc-cf-audited-warning.m
+++ clang/test/SemaObjC/objc-cf-audi

[PATCH] D93979: [clang-tidy] Fix windows tests

2021-01-03 Thread Nico Weber via Phabricator via cfe-commits

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

Hey, thanks!

I don't have a win machine at hand either, but let's land this and see what the 
win bot on http://45.33.8.238/ thinks about this. It should take at most 15 min 
to cycle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93979

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

[PATCH] D93901: [NFC] Renaming PackStack to AlignPackStack

2021-01-03 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:65-67
+  // The #pragma align/pack affected a record in an included file,  so Clang
+  // should warn when that pragma was written in a file that included the
+  // included file.

Remove extra space; add missing "the".



Comment at: clang/lib/Sema/SemaAttr.cpp:365-367
+  // FIXME: AlignPackStack may contain both #pragma align and #pragma pack
+  // information, diagnostics below might not be accurate if we have mixed
+  // pragmas.

It may be helpful to pre-commit tests with the incorrect behaviour as part of 
the NFC patch.



Comment at: clang/lib/Sema/SemaAttr.cpp:385-387
+  // FIXME: AlignPackStack may contain both #pragma align and #pragma pack
+  // information, diagnostics below might not be accurate if we have mixed
+  // pragmas.

Same comment re: pre-committing tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93901

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

[clang-tools-extra] 59810c5 - [clang-tidy] Fix windows tests

2021-01-03 Thread Nathan James via cfe-commits


Author: Nathan James
Date: 2021-01-04T00:39:34Z
New Revision: 59810c51e761f241f23f45a120d5c3518983d2d8

URL: 
https://github.com/llvm/llvm-project/commit/59810c51e761f241f23f45a120d5c3518983d2d8
DIFF: 
https://github.com/llvm/llvm-project/commit/59810c51e761f241f23f45a120d5c3518983d2d8.diff

LOG: [clang-tidy] Fix windows tests

Attempt to fix the 2 failing tests identifier in 48646.
Appears that python3 doesn't like nested double quotes in single quoted 
strings, hopefully nested single quotes in double quoted strings is a-ok.

Reviewed By: thakis

Differential Revision: https://reviews.llvm.org/D93979

Added: 


Modified: 

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp
index b3075324efdb..5e70aa00d40e 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp
@@ -1,9 +1,6 @@
-// FIXME: PR48646
-// UNSUPPORTED: system-windows
-
 // RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
-// RUN: -config='{CheckOptions: \
-// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: 
"DEBUG_*|TEST_*"}]}' --
+// RUN: -config="{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: 
'DEBUG_*|TEST_*'}]}" --
 
 #ifndef INCLUDE_GUARD
 #define INCLUDE_GUARD

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
index 625b7dc9134a..22a8c7159a99 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
@@ -1,15 +1,12 @@
-// FIXME: PR48646
-// UNSUPPORTED: system-windows
-
 // RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
-// RUN:   -config='{CheckOptions: [ \
+// RUN:   -config="{CheckOptions: [ \
 // RUN: {key: readability-identifier-naming.ParameterCase, value: 
CamelCase}, \
-// RUN: {key: readability-identifier-naming.ParameterIgnoredRegexp, value: 
"^[a-z]{1,2}$"}, \
+// RUN: {key: readability-identifier-naming.ParameterIgnoredRegexp, value: 
'^[a-z]{1,2}$'}, \
 // RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
-// RUN: {key: readability-identifier-naming.ClassIgnoredRegexp, value: 
"^fo$|^fooo$"}, \
+// RUN: {key: readability-identifier-naming.ClassIgnoredRegexp, value: 
'^fo$|^fooo$'}, \
 // RUN: {key: readability-identifier-naming.StructCase, value: CamelCase}, 
\
-// RUN: {key: readability-identifier-naming.StructIgnoredRegexp, value: 
"sooo|so|soo|$invalidregex["} \
-// RUN:  ]}'
+// RUN: {key: readability-identifier-naming.StructIgnoredRegexp, value: 
'sooo|so|soo|$invalidregex['} \
+// RUN:  ]}"
 
 int testFunc(int a, char **b);
 int testFunc(int ab, char **ba);



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

[PATCH] D93979: [clang-tidy] Fix windows tests

2021-01-03 Thread Nathan James via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG59810c51e761: [clang-tidy] Fix windows tests (authored by 
njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93979

Files:
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
@@ -1,15 +1,12 @@
-// FIXME: PR48646
-// UNSUPPORTED: system-windows
-
 // RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
-// RUN:   -config='{CheckOptions: [ \
+// RUN:   -config="{CheckOptions: [ \
 // RUN: {key: readability-identifier-naming.ParameterCase, value: 
CamelCase}, \
-// RUN: {key: readability-identifier-naming.ParameterIgnoredRegexp, value: 
"^[a-z]{1,2}$"}, \
+// RUN: {key: readability-identifier-naming.ParameterIgnoredRegexp, value: 
'^[a-z]{1,2}$'}, \
 // RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
-// RUN: {key: readability-identifier-naming.ClassIgnoredRegexp, value: 
"^fo$|^fooo$"}, \
+// RUN: {key: readability-identifier-naming.ClassIgnoredRegexp, value: 
'^fo$|^fooo$'}, \
 // RUN: {key: readability-identifier-naming.StructCase, value: CamelCase}, 
\
-// RUN: {key: readability-identifier-naming.StructIgnoredRegexp, value: 
"sooo|so|soo|$invalidregex["} \
-// RUN:  ]}'
+// RUN: {key: readability-identifier-naming.StructIgnoredRegexp, value: 
'sooo|so|soo|$invalidregex['} \
+// RUN:  ]}"
 
 int testFunc(int a, char **b);
 int testFunc(int ab, char **ba);
Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp
@@ -1,9 +1,6 @@
-// FIXME: PR48646
-// UNSUPPORTED: system-windows
-
 // RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
-// RUN: -config='{CheckOptions: \
-// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: 
"DEBUG_*|TEST_*"}]}' --
+// RUN: -config="{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: 
'DEBUG_*|TEST_*'}]}" --
 
 #ifndef INCLUDE_GUARD
 #define INCLUDE_GUARD


Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
@@ -1,15 +1,12 @@
-// FIXME: PR48646
-// UNSUPPORTED: system-windows
-
 // RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
-// RUN:   -config='{CheckOptions: [ \
+// RUN:   -config="{CheckOptions: [ \
 // RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
-// RUN: {key: readability-identifier-naming.ParameterIgnoredRegexp, value: "^[a-z]{1,2}$"}, \
+// RUN: {key: readability-identifier-naming.ParameterIgnoredRegexp, value: '^[a-z]{1,2}$'}, \
 // RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
-// RUN: {key: readability-identifier-naming.ClassIgnoredRegexp, value: "^fo$|^fooo$"}, \
+// RUN: {key: readability-identifier-naming.ClassIgnoredRegexp, value: '^fo$|^fooo$'}, \
 // RUN: {key: readability-identifier-naming.StructCase, value: CamelCase}, \
-// RUN: {key: readability-identifier-naming.StructIgnoredRegexp, value: "sooo|so|soo|$invalidregex["} \
-// RUN:  ]}'
+// RUN: {key: readability-identifier-naming.StructIgnoredRegexp, value: 'sooo|so|soo|$invalidregex['} \
+// RUN:  ]}"
 
 int testFunc(int a, char **b);
 int testFunc(int ab, char **ba);
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-custom.cpp
@@ -1,9 +1,6 @@
-// FIXME: PR48646
-// UNSUPPORTED: system-windows
-
 // RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
-// RUN: -config='{CheckOptions: \
-// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: "DEBUG_*|TEST_*"}]}' --
+// RUN: -config="{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-

[PATCH] D93979: [clang-tidy] Fix windows tests

2021-01-03 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

http://45.33.8.238/win/30678/summary.html It looks like it works, as nothing 
needed to be built it was a v fast build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93979

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

[PATCH] D93979: [clang-tidy] Fix windows tests

2021-01-03 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

Looks like this worked :)

Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93979

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

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2021-01-03 Thread Ella Ma via Phabricator via cfe-commits

OikawaKirie added a comment.

In D92634#2476161 , @danielmarjamaki 
wrote:

>> Besides, the return value should be the exact value computed from the two 
>> integers, even unknown, rather than undefined. As the developers may 
>> overflow an integer on purpose.
>
> I am not sure what you mean. If there is undefined behavior then the value 
> should be undefined and nothing else.. right?

Exactly, it is undefined behavior in the C++ standard. However, the mainstream 
compilers like GCC and Clang implement this as the overflowed value, and some 
programmers also use this feature to do some tricky things. Therefore I suggest 
the computed value should be "the exact value computed from the two integers". 
Or it can be the `Unknown` `SVal`, but rather than the `Undefined` `SVal`, as 
the `Undefined` `SVal` is used to represent what is read from an uninitialized 
variable.

But I do not favour the `Unknown` solution, as it could also trigger other 
problems in the engine, just as what has been mentioned by steakhal. Or maybe 
it would be no longer a problem if you implement this in a checker, but as a 
non-fatal error it is, you can just leave the overflowed value as it is, and 
report the problem only without terminating the symbol execution on this path. 
There is no need to report this problem all the time.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92634

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:130
+/// \endcode
+ACA_AcrossComments
+  };

tinloaf wrote:
> MyDeveloperDay wrote:
> > Is there a case for aligning over empty lines and comments?
> > 
> > ```
> > int a   = 5;
> > 
> > /* comment */
> > int oneTwoThree = 123;
> > ```
> Not sure I understand what you mean. Currently, the Option `AcrossComments` 
> includes 'across empty lines'. So, there currently is no case for "across 
> comments, but not across empty lines". I'm not sure if that is really 
> something people want to do. Do you think so? I can add it. 
I could see a case where you might want to begin a new "alignment group" by 
leaving a blank line.

```
/* align these 3 */
int a = 5;
/* align these 3 */
int b = 6;
/* align these 3 */
int c = 7;

/* align these 2 which are longer */
int d   = 5
/* align these 2 which are longer */
int oneTwoThree = 123;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D94006: [Sema] Fix use-of-uninitialized-value cause by 89b0972aa2f58f927633c63570b36550a17f4e63

2021-01-03 Thread Yang Fan via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94006

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5589,13 +5589,11 @@
   return false;
 }
 
-InitializationSequence::InitializationSequence(Sema &S,
-   const InitializedEntity &Entity,
-   const InitializationKind &Kind,
-   MultiExprArg Args,
-   bool TopLevelOfInitList,
-   bool TreatUnavailableAsInvalid)
-: FailedCandidateSet(Kind.getLocation(), OverloadCandidateSet::CSK_Normal) 
{
+InitializationSequence::InitializationSequence(
+Sema &S, const InitializedEntity &Entity, const InitializationKind &Kind,
+MultiExprArg Args, bool TopLevelOfInitList, bool TreatUnavailableAsInvalid)
+: FailedOverloadResult(OR_Success),
+  FailedCandidateSet(Kind.getLocation(), OverloadCandidateSet::CSK_Normal) 
{
   InitializeFrom(S, Entity, Kind, Args, TopLevelOfInitList,
  TreatUnavailableAsInvalid);
 }


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5589,13 +5589,11 @@
   return false;
 }
 
-InitializationSequence::InitializationSequence(Sema &S,
-   const InitializedEntity &Entity,
-   const InitializationKind &Kind,
-   MultiExprArg Args,
-   bool TopLevelOfInitList,
-   bool TreatUnavailableAsInvalid)
-: FailedCandidateSet(Kind.getLocation(), OverloadCandidateSet::CSK_Normal) {
+InitializationSequence::InitializationSequence(
+Sema &S, const InitializedEntity &Entity, const InitializationKind &Kind,
+MultiExprArg Args, bool TopLevelOfInitList, bool TreatUnavailableAsInvalid)
+: FailedOverloadResult(OR_Success),
+  FailedCandidateSet(Kind.getLocation(), OverloadCandidateSet::CSK_Normal) {
   InitializeFrom(S, Entity, Kind, Args, TopLevelOfInitList,
  TreatUnavailableAsInvalid);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

44 matches

Mail list logo