[PATCH] D148777: [clang-format] Hanlde leading whitespaces for JSON files

2023-04-19 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

(looks like you linked the same issue twice in the summary)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148777

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


[PATCH] D148024: [clang-format] Don't modify template arguments on the LHS of assignment

2023-04-11 Thread Emilia Dreamer 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 rG5dc94b3356bd: [clang-format] Dont modify template 
arguments on the LHS of assignment (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148024

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
@@ -270,6 +270,18 @@
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
   EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("template * = nullptr> void 
f();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1819,7 +1819,7 @@
  Previous && Previous->Previous &&
  !Previous->Previous->isOneOf(tok::comma, tok::semi);
  Previous = Previous->Previous) {
-  if (Previous->isOneOf(tok::r_square, tok::r_paren)) {
+  if (Previous->isOneOf(tok::r_square, tok::r_paren, tok::greater)) {
 Previous = Previous->MatchingParen;
 if (!Previous)
   break;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -270,6 +270,18 @@
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
   EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("template * = nullptr> void f();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1819,7 +1819,7 @@
  Previous && Previous->Previous &&
  !Previous->Previous->isOneOf(tok::comma, tok::semi);
  Previous = Previous->Previous) {
-  if (Previous->isOneOf(tok::r_square, tok::r_paren)) {
+  if (Previous->isOneOf(tok::r_square, tok::r_paren, tok::greater)) {
 Previous = Previous->MatchingParen;
 if (!Previous)
   break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D148024: [clang-format] Don't modify template arguments on the LHS of assignment

2023-04-11 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: owenpan, MyDeveloperDay, HazardyKnusperkeks.
Herald added projects: All, clang.
Herald added a subscriber: cfe-commits.
rymiel requested review of this revision.

After clang-format has determined that an equals sign starts an
expression, it will also go backwards and modify any star/amp/ampamp
binary operators on the left side of the assignment to be
pointers/references instead.

There already exists logic to skip over contents of parentheses and
square brackets, but this patch also expands that logic to apply to
angle brackets. This is so that binary operators inside of template
arguments would not be touched, primary arguments to non-type template
parameters.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148024

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
@@ -270,6 +270,18 @@
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
   EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("template * = nullptr> void 
f();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1819,7 +1819,7 @@
  Previous && Previous->Previous &&
  !Previous->Previous->isOneOf(tok::comma, tok::semi);
  Previous = Previous->Previous) {
-  if (Previous->isOneOf(tok::r_square, tok::r_paren)) {
+  if (Previous->isOneOf(tok::r_square, tok::r_paren, tok::greater)) {
 Previous = Previous->MatchingParen;
 if (!Previous)
   break;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -270,6 +270,18 @@
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
   EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("Foo a = {};");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("template * = nullptr> void f();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1819,7 +1819,7 @@
  Previous && Previous->Previous &&
  !Previous->Previous->isOneOf(tok::comma, tok::semi);
  Previous = Previous->Previous) {
-  if (Previous->isOneOf(tok::r_square, tok::r_paren)) {
+  if (Previous->isOneOf(tok::r_square, tok::r_paren, tok::greater)) {
 Previous = Previous->MatchingParen;
 if (!Previous)
   break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147377: [clang-format] Don't allow variable decls to have trailing return arrows

2023-04-03 Thread Emilia Dreamer 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 rGfd8678996296: [clang-format] Dont allow variable decls 
to have trailing return arrows (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147377

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
@@ -1477,6 +1477,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator++(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto foo() -> auto { return Val; }");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("struct S { auto bar() const -> int; };");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::arrow, TT_TrailingReturnArrow);
+
+  // Not trailing return arrows
+  Tokens = annotate("auto a = b->c;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = (b)->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b->c();");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("decltype(auto) a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b->c(); }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::arrow, TT_Unknown);
+
+  // Mixed
+  Tokens = annotate("auto f() -> int { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_TOKEN(Tokens[13], tok::arrow, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1922,7 +1922,7 @@
Style.Language == FormatStyle::LK_Java) {
   Current.setType(TT_LambdaArrow);
 } else if (Current.is(tok::arrow) && AutoFound &&
-   (Line.MustBeDeclaration || Line.InPPDirective) &&
+   (Line.MightBeFunctionDecl || Line.InPPDirective) &&
Current.NestingLevel == 0 &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1477,6 +1477,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto 

[PATCH] D147377: [clang-format] Don't allow variable decls to have trailing return arrows

2023-04-02 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 510317.
rymiel marked an inline comment as done.
rymiel edited the summary of this revision.
rymiel added a comment.

update commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147377

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
@@ -1468,6 +1468,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator++(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto foo() -> auto { return Val; }");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("struct S { auto bar() const -> int; };");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::arrow, TT_TrailingReturnArrow);
+
+  // Not trailing return arrows
+  Tokens = annotate("auto a = b->c;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = (b)->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b->c();");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("decltype(auto) a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b->c(); }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::arrow, TT_Unknown);
+
+  // Mixed
+  Tokens = annotate("auto f() -> int { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_TOKEN(Tokens[13], tok::arrow, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1919,7 +1919,7 @@
Style.Language == FormatStyle::LK_Java) {
   Current.setType(TT_LambdaArrow);
 } else if (Current.is(tok::arrow) && AutoFound &&
-   (Line.MustBeDeclaration || Line.InPPDirective) &&
+   (Line.MightBeFunctionDecl || Line.InPPDirective) &&
Current.NestingLevel == 0 &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1468,6 +1468,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator++(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  

[PATCH] D147377: [clang-format] Don't allow variable decls to have trailing return arrows

2023-04-02 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel marked an inline comment as done.
rymiel added inline comments.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:1493
+
+  Tokens = annotate("auto foo() -> auto { return Val; }");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;

owenpan wrote:
> Can this be valid C++?
Yes! (https://godbolt.org/z/bnYahK4cz) (also I copied it from FormatTest)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147377

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


[PATCH] D147377: [clang-format] Don't allow variable decls to have trailing return arrows

2023-04-02 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 510315.
rymiel marked an inline comment as done.
rymiel added a comment.

Use MightBeFunctionDecl instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147377

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
@@ -1468,6 +1468,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator++(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto foo() -> auto { return Val; }");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("struct S { auto bar() const -> int; };");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::arrow, TT_TrailingReturnArrow);
+
+  // Not trailing return arrows
+  Tokens = annotate("auto a = b->c;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = (b)->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b->c();");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("decltype(auto) a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b->c(); }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::arrow, TT_Unknown);
+
+  // Mixed
+  Tokens = annotate("auto f() -> int { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_TOKEN(Tokens[13], tok::arrow, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1919,7 +1919,7 @@
Style.Language == FormatStyle::LK_Java) {
   Current.setType(TT_LambdaArrow);
 } else if (Current.is(tok::arrow) && AutoFound &&
-   (Line.MustBeDeclaration || Line.InPPDirective) &&
+   (Line.MightBeFunctionDecl || Line.InPPDirective) &&
Current.NestingLevel == 0 &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1468,6 +1468,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator++(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, 

[PATCH] D147377: [clang-format] Don't allow variable decls to have trailing return arrows

2023-04-02 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1922
 } else if (Current.is(tok::arrow) && AutoFound &&
(Line.MustBeDeclaration || Line.InPPDirective) &&
Current.NestingLevel == 0 &&

owenpan wrote:
> It seems we can simply check if the line might be a function declaration here 
> instead of resetting `AutoFound` below.
I could swear I specifically tried using that flag and it didn't work in all 
cases, but I guess I did something wrong back then...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147377

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


[PATCH] D147318: [clang-format] Don't format typename template parameters as expression

2023-04-01 Thread Emilia Dreamer 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 rG50acd67018a5: [clang-format] Dont format typename 
template parameters as expression (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147318

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
@@ -261,6 +261,15 @@
   Tokens = annotate("template  struct S {};");
   ASSERT_EQ(Tokens.size(), 18u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1723,10 +1723,13 @@
   return false;
 }
 
-// This is the default value of a non-template type parameter, so treat
-// it as an expression.
-if (Contexts.back().ContextKind == tok::less)
-  return true;
+// This is the default value of a template parameter, determine if it's
+// type or non-type.
+if (Contexts.back().ContextKind == tok::less) {
+  assert(Current.Previous->Previous);
+  return !Current.Previous->Previous->isOneOf(tok::kw_typename,
+  tok::kw_class);
+}
 
 Tok = Tok->MatchingParen;
 if (!Tok)


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -261,6 +261,15 @@
   Tokens = annotate("template  struct S {};");
   ASSERT_EQ(Tokens.size(), 18u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1723,10 +1723,13 @@
   return false;
 }
 
-// This is the default value of a non-template type parameter, so treat
-// it as an expression.
-if (Contexts.back().ContextKind == tok::less)
-  return true;
+// This is the default value of a template parameter, determine if it's
+// type or non-type.
+if (Contexts.back().ContextKind == tok::less) {
+  assert(Current.Previous->Previous);
+  return !Current.Previous->Previous->isOneOf(tok::kw_typename,
+  tok::kw_class);
+}
 
 Tok = Tok->MatchingParen;
 if (!Tok)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147377: [clang-format] Don't allow variable decls to have trailing return arrows

2023-04-01 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added projects: All, clang.
Herald added a subscriber: cfe-commits.
rymiel requested review of this revision.

The heuristic for determining if an arrow is a trailing return arrow
looks for the auto keyword, along with parentheses. This isn't
sufficient, since it also triggers on variable declarations with an auto
type, and with an arrow operator.

This patch unsets the `auto` state when the annotator encounters the
equals sign of a variable declaration.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147377

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
@@ -1468,6 +1468,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator++(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto foo() -> auto { return Val; }");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("struct S { auto bar() const -> int; };");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::arrow, TT_TrailingReturnArrow);
+
+  // Not trailing return arrows
+  Tokens = annotate("auto a = b->c;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = (b)->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b->c();");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("decltype(auto) a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b->c(); }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::arrow, TT_Unknown);
+
+  // Mixed
+  Tokens = annotate("auto f() -> int { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_TOKEN(Tokens[13], tok::arrow, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
 return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1970,6 +1970,12 @@
 if (Current.getPrecedence() == prec::Assignment)
   Contexts.back().VerilogAssignmentFound = true;
   }
+  if (AutoFound && Current.is(tok::equal) && Current.Previous &&
+  Current.Previous->isNot(tok::kw_operator)) {
+// AutoFound is only used for function declarations with `auto`. This
+// looks like a variable declaration, so unset it.
+AutoFound = false;
+  }
   Current.setType(TT_BinaryOperator);
 } else if (Current.is(tok::comment)) {
   if (Current.TokenText.startswith("/*")) {


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1468,6 +1468,72 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 

[PATCH] D146101: [clang-format] Add BracedInitializerIndentWidth option.

2023-03-31 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:72
 
-  subtype, napplied = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
-  if napplied == 1:
-return 'List of ' + pluralize(to_yaml_type(subtype))
+  match = re.match(r'std::vector<(.*)>$', typestr)
+  if match:

jp4a50 wrote:
> I changed this from `subn` to `match` here since it's just a simpler way of 
> expressing the same thing.
(Just FYI, those pythons sources are pretty ancient and untouched, I planned on 
refactoring the whole thing using more modern, idiomatic Python but then 
concluded that it's not really necessary)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101

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


[PATCH] D147318: [clang-format] Don't format typename template parameters as expression

2023-03-31 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added projects: All, clang.
Herald added a subscriber: cfe-commits.
rymiel requested review of this revision.

bb4f6c4dca98a47054117708015bb2724256ee83 
 made it 
so that template
parameter defaults are seen akin to assignments and formatted as
expressions, however, the patch did this for all template parameters,
even for `typename` template parameters.

This patch formats `typename` and `class` template parameters as types.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147318

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
@@ -261,6 +261,15 @@
   Tokens = annotate("template  struct S {};");
   ASSERT_EQ(Tokens.size(), 18u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1723,10 +1723,13 @@
   return false;
 }
 
-// This is the default value of a non-template type parameter, so treat
-// it as an expression.
-if (Contexts.back().ContextKind == tok::less)
-  return true;
+// This is the default value of a template parameter, determine if it's
+// type or non-type.
+if (Contexts.back().ContextKind == tok::less) {
+  assert(Current.Previous->Previous);
+  return !Current.Previous->Previous->isOneOf(tok::kw_typename,
+  tok::kw_class);
+}
 
 Tok = Tok->MatchingParen;
 if (!Tok)


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -261,6 +261,15 @@
   Tokens = annotate("template  struct S {};");
   ASSERT_EQ(Tokens.size(), 18u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1723,10 +1723,13 @@
   return false;
 }
 
-// This is the default value of a non-template type parameter, so treat
-// it as an expression.
-if (Contexts.back().ContextKind == tok::less)
-  return true;
+// This is the default value of a template parameter, determine if it's
+// type or non-type.
+if (Contexts.back().ContextKind == tok::less) {
+  assert(Current.Previous->Previous);
+  return !Current.Previous->Previous->isOneOf(tok::kw_typename,
+  tok::kw_class);
+}
 
 Tok = Tok->MatchingParen;
 if (!Tok)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147295: [clang-format] Don't misannotate left squares as lambda introducers

2023-03-30 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel accepted this revision.
rymiel added a comment.
This revision is now accepted and ready to land.

This is great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147295

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


[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

2023-03-29 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669
+  const auto DesignatedInitializerIndentWidth =
+  Style.DesignatedInitializerIndentWidth < 0
+  ? Style.ContinuationIndentWidth
+  : Style.DesignatedInitializerIndentWidth;
+  NewIndent = CurrentState.LastSpace + DesignatedInitializerIndentWidth;

owenpan wrote:
> jp4a50 wrote:
> > HazardyKnusperkeks wrote:
> > > owenpan wrote:
> > > > jp4a50 wrote:
> > > > > HazardyKnusperkeks wrote:
> > > > > > owenpan wrote:
> > > > > > > jp4a50 wrote:
> > > > > > > > owenpan wrote:
> > > > > > > > > owenpan wrote:
> > > > > > > > > > Using -1 to mean `ContinuationIndentWidth` is unnecessary 
> > > > > > > > > > and somewhat confusing IMO.
> > > > > > > > > Please disregard my comment above.
> > > > > > > > Just to make sure we are on the same page, does this mean that 
> > > > > > > > you are happy with the approach of using `-1` as a default 
> > > > > > > > value to indicate that `ContinuationIndentWidth` should be used?
> > > > > > > > 
> > > > > > > > I did initially consider using `std::optional` and 
> > > > > > > > using empty optional to indicate that `ContinuationIndentWidth` 
> > > > > > > > should be used but I saw that `PPIndentWidth` was using `-1` to 
> > > > > > > > default to using `IndentWidth` so I followed that precedent.
> > > > > > > Yep! I would prefer the `optional`, but as you pointed out, we 
> > > > > > > already got `PPIndentWidth`using `-1`.
> > > > > > From the C++ side I totally agree. One could use `value_or()`, 
> > > > > > which would make the code much more readable.
> > > > > > And just because `PPIndentWidth` is using -1 is no reason to repeat 
> > > > > > that, we could just as easily change `PPIndentWidht` to an optional.
> > > > > > 
> > > > > > But how would it look in yaml?
> > > > > In YAML we wouldn't need to support empty optional being *explicitly* 
> > > > > specified - it would just be the default.
> > > > > 
> > > > > So specifying `DesignatedInitializerIndentWidth: 4` would set the 
> > > > > `std::optional` to `4` but if 
> > > > > `DesignatedInitializerIndentWidth` was omitted from the YAML then the 
> > > > > optional would simply not be set during parsing.
> > > > > 
> > > > > Of course, if we were to change `PPIndentWidth` to work that way too, 
> > > > > it would technically be a breaking change because users may have 
> > > > > *explicitly* specified `-1` in their YAML.
> > > > > And just because `PPIndentWidth` is using -1 is no reason to repeat 
> > > > > that, we could just as easily change `PPIndentWidht` to an optional.
> > > > 
> > > > We would have to deal with backward compatibility to avoid regressions 
> > > > though.
> > > > In YAML we wouldn't need to support empty optional being *explicitly* 
> > > > specified - it would just be the default.
> > > > 
> > > > So specifying `DesignatedInitializerIndentWidth: 4` would set the 
> > > > `std::optional` to `4` but if 
> > > > `DesignatedInitializerIndentWidth` was omitted from the YAML then the 
> > > > optional would simply not be set during parsing.
> > > > 
> > > > Of course, if we were to change `PPIndentWidth` to work that way too, 
> > > > it would technically be a breaking change because users may have 
> > > > *explicitly* specified `-1` in their YAML.
> > > 
> > > You need an explicit entry, because you need to be able to write the 
> > > empty optional on `--dump-config`.
> > > > In YAML we wouldn't need to support empty optional being *explicitly* 
> > > > specified - it would just be the default.
> > > > 
> > > > So specifying `DesignatedInitializerIndentWidth: 4` would set the 
> > > > `std::optional` to `4` but if 
> > > > `DesignatedInitializerIndentWidth` was omitted from the YAML then the 
> > > > optional would simply not be set during parsing.
> > > > 
> > > > Of course, if we were to change `PPIndentWidth` to work that way too, 
> > > > it would technically be a breaking change because users may have 
> > > > *explicitly* specified `-1` in their YAML.
> > > 
> > > You need an explicit entry, because you need to be able to write the 
> > > empty optional on `--dump-config`.
> > 
> > It looks like the YAML IO logic just does the right thing (TM) with 
> > `std::optional`s. When calling `IO.mapOptional()` on output, it simply 
> > doesn't write the key out if the value is an empty optional. So I don't 
> > think this is an issue.
> > 
> > As @owenpan says, though, there is still the issue of backward 
> > compatibility with `PPIndentWidth`.
> > 
> > I don't feel strongly about which way to go so I'll leave it to you two to 
> > decide!
> > As @owenpan says, though, there is still the issue of backward 
> > compatibility with `PPIndentWidth`.
> > 
> > I don't feel strongly about which way to go so I'll leave it to you two to 
> > decide!
> 
> @MyDeveloperDay @rymiel can you weigh in?

> can you weigh in?

Well, as someone 

[PATCH] D147176: [clang-format] NFC ensure Style operator== remains sorted for ease of editing

2023-03-29 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

> didn't go via the normal clang-format reviewers

Is there a reason for this? This isn't the only case either, for example:
https://reviews.llvm.org/D143070 https://reviews.llvm.org/D144170 
https://reviews.llvm.org/D88299 https://reviews.llvm.org/D135356

I don't have a lot of experience here, so maybe I'm missing "historical" 
context, but this confuses me a lot...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147176

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


[PATCH] D146760: [clang-format] Treat NTTP default values as expressions

2023-03-25 Thread Emilia Dreamer 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 rGbb4f6c4dca98: [clang-format] Treat NTTP default values as 
expressions (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146760

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
@@ -253,6 +253,14 @@
 "};");
   ASSERT_EQ(Tokens.size(), 30u) << Tokens;
   EXPECT_TOKEN(Tokens[14], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1722,6 +1722,11 @@
   return false;
 }
 
+// This is the default value of a non-template type parameter, so treat
+// it as an expression.
+if (Contexts.back().ContextKind == tok::less)
+  return true;
+
 Tok = Tok->MatchingParen;
 if (!Tok)
   return false;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -253,6 +253,14 @@
 "};");
   ASSERT_EQ(Tokens.size(), 30u) << Tokens;
   EXPECT_TOKEN(Tokens[14], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1722,6 +1722,11 @@
   return false;
 }
 
+// This is the default value of a non-template type parameter, so treat
+// it as an expression.
+if (Contexts.back().ContextKind == tok::less)
+  return true;
+
 Tok = Tok->MatchingParen;
 if (!Tok)
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D146281: [clang-format] Don't wrap struct return types as structs

2023-03-25 Thread Emilia Dreamer 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 rGa8d2bff290e1: [clang-format] Dont wrap struct return 
types as structs (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146281

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3205,10 +3205,13 @@
 format("try{foo();}catch(...){baz();}", Style));
 
   Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.AfterStruct = false;
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_MultiLine;
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   Style.ColumnLimit = 80;
   verifyFormat("void shortfunction() { bar(); }", Style);
+  verifyFormat("struct T shortfunction() { return bar(); }", Style);
+  verifyFormat("struct T {};", Style);
 
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
   verifyFormat("void shortfunction()\n"
@@ -3216,6 +3219,36 @@
"  bar();\n"
"}",
Style);
+  verifyFormat("struct T shortfunction()\n"
+   "{\n"
+   "  return bar();\n"
+   "}",
+   Style);
+  verifyFormat("struct T {};", Style);
+
+  Style.BraceWrapping.AfterFunction = false;
+  Style.BraceWrapping.AfterStruct = true;
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("void shortfunction() { bar(); }", Style);
+  verifyFormat("struct T shortfunction() { return bar(); }", Style);
+  verifyFormat("struct T\n"
+   "{\n"
+   "};",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  verifyFormat("void shortfunction() {\n"
+   "  bar();\n"
+   "}",
+   Style);
+  verifyFormat("struct T shortfunction() {\n"
+   "  return bar();\n"
+   "}",
+   Style);
+  verifyFormat("struct T\n"
+   "{\n"
+   "};",
+   Style);
 }
 
 TEST_F(FormatTest, BeforeWhile) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4916,8 +4916,13 @@
   return true;
 }
 
-return (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) 
||
-   (Line.startsWith(tok::kw_struct) && 
Style.BraceWrapping.AfterStruct);
+// Don't attempt to interpret struct return types as structs.
+if (Right.isNot(TT_FunctionLBrace)) {
+  return (Line.startsWith(tok::kw_class) &&
+  Style.BraceWrapping.AfterClass) ||
+ (Line.startsWith(tok::kw_struct) &&
+  Style.BraceWrapping.AfterStruct);
+}
   }
 
   if (Left.is(TT_ObjCBlockLBrace) &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3205,10 +3205,13 @@
 format("try{foo();}catch(...){baz();}", Style));
 
   Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.AfterStruct = false;
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_MultiLine;
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   Style.ColumnLimit = 80;
   verifyFormat("void shortfunction() { bar(); }", Style);
+  verifyFormat("struct T shortfunction() { return bar(); }", Style);
+  verifyFormat("struct T {};", Style);
 
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
   verifyFormat("void shortfunction()\n"
@@ -3216,6 +3219,36 @@
"  bar();\n"
"}",
Style);
+  verifyFormat("struct T shortfunction()\n"
+   "{\n"
+   "  return bar();\n"
+   "}",
+   Style);
+  verifyFormat("struct T {};", Style);
+
+  Style.BraceWrapping.AfterFunction = false;
+  Style.BraceWrapping.AfterStruct = true;
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("void shortfunction() { bar(); }", Style);
+  verifyFormat("struct T shortfunction() { return bar(); }", Style);
+  verifyFormat("struct T\n"
+   "{\n"
+   "};",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  verifyFormat("void shortfunction() {\n"
+   "  bar();\n"
+   "}",
+   Style);
+  verifyFormat("struct T shortfunction() {\n"
+   "  return bar();\n"
+   "}",
+   Style);
+  

[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-25 Thread Emilia Dreamer 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 rG5409fb38372d: [clang-format] Annotate lambdas with requires 
clauses. (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145642

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1279,6 +1279,110 @@
   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);
+
+  // Lambdas with a requires-clause
+  Tokens = annotate("[]  (T t) requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[14]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T &) requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[8], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[11], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[15]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[16], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T t) requires Foo || Bar {}");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[19]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T t) -> T requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::arrow, TT_LambdaArrow);
+  EXPECT_TOKEN(Tokens[12], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[16]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[17], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar (T t) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar (T &) {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[16], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Foo || Bar (T t) {}");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[15]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires true (T&& t) {}");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[7]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar noexcept {}");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, 

[PATCH] D146844: [clang-format] Handle '_' in ud-suffix for IntegerLiteralSeparator

2023-03-24 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel accepted this revision.
rymiel added a comment.
This revision is now accepted and ready to land.

Could this possibly be an issue for more esoteric underscore-less UDLs, like 
`i`?
Does the code need to search for a suffix, could it not detect the absence of a 
digit? Sorry if the questions are silly, I haven't really looked at this part 
of the formatter in depth


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146844

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


[PATCH] D146760: [clang-format] Treat NTTP default values as expressions

2023-03-23 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

clang-format already has logic to threat the right-hand side of an
equals sign. This patch applies that logic to template defaults,
which are likely to be non-template type parameters in which case the
default value should be annotated as an expression.
This should mostly only ever apply to bool and &&.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146760

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
@@ -253,6 +253,14 @@
 "};");
   ASSERT_EQ(Tokens.size(), 30u) << Tokens;
   EXPECT_TOKEN(Tokens[14], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1719,6 +1719,11 @@
   return false;
 }
 
+// This is the default value of a non-template type parameter, so treat
+// it as an expression.
+if (Contexts.back().ContextKind == tok::less)
+  return true;
+
 Tok = Tok->MatchingParen;
 if (!Tok)
   return false;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -253,6 +253,14 @@
 "};");
   ASSERT_EQ(Tokens.size(), 30u) << Tokens;
   EXPECT_TOKEN(Tokens[14], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {};");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1719,6 +1719,11 @@
   return false;
 }
 
+// This is the default value of a non-template type parameter, so treat
+// it as an expression.
+if (Contexts.back().ContextKind == tok::less)
+  return true;
+
 Tok = Tok->MatchingParen;
 if (!Tok)
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D141811: [clang-format] Allow trailing return types in macros

2023-03-23 Thread Emilia Dreamer 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 rGc70e360b355a: [clang-format] Allow trailing return types in 
macros (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141811

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8010,6 +8010,11 @@
"auto aa(T t)\n"
"-> decltype(eaaa(t.a).());");
 
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  verifyFormat("#define MAKE_DEF(NAME) 
\\\n"
+   "  auto NAME() -> int { return 42; }",
+   Style);
+
   // Not trailing return types.
   verifyFormat("void f() { auto a = b->c(); }");
   verifyFormat("auto a = p->foo();");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1909,7 +1909,8 @@
 } else if (Current.is(tok::arrow) &&
Style.Language == FormatStyle::LK_Java) {
   Current.setType(TT_LambdaArrow);
-} else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
+} else if (Current.is(tok::arrow) && AutoFound &&
+   (Line.MustBeDeclaration || Line.InPPDirective) &&
Current.NestingLevel == 0 &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8010,6 +8010,11 @@
"auto aa(T t)\n"
"-> decltype(eaaa(t.a).());");
 
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  verifyFormat("#define MAKE_DEF(NAME) \\\n"
+   "  auto NAME() -> int { return 42; }",
+   Style);
+
   // Not trailing return types.
   verifyFormat("void f() { auto a = b->c(); }");
   verifyFormat("auto a = p->foo();");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1909,7 +1909,8 @@
 } else if (Current.is(tok::arrow) &&
Style.Language == FormatStyle::LK_Java) {
   Current.setType(TT_LambdaArrow);
-} else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
+} else if (Current.is(tok::arrow) && AutoFound &&
+   (Line.MustBeDeclaration || Line.InPPDirective) &&
Current.NestingLevel == 0 &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-23 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

So, it took me a while but I finally found where the logic is that makes the 
lambda braces stay on one line, but, now I'm not so sure if I should change it:

The thing I wanted to avoid was cases like

  [&](T&& t)
  requires T
  { t; };

Simply because "those braces don't really look nice", but it turns out there's 
a sort of "prior precedent" for this:

For example, FormatTest.cpp line 22046, LambdaWithLineComments:

  verifyFormat("auto k = []() // X\n"
   "{ return; }",
   LLVMWithBeforeLambdaBody);
  verifyFormat(
  "auto k = []() // \n"
  "{ return; }",
  LLVMWithBeforeLambdaBody);

(this is with combination of BraceWrapping.BeforeLambdaBody = true and 
AllowShortLambdaOnASingleLine)
(+ a change in C# which I would like to avoid touching since I exclusively do 
C++)

So, I guess I have the following choices:

1. do nothing! (allow those braces to stay on one line)
2. change the behavior of those existing tests, with whatever impact that has
3. make a //very specific// exception for just requires-clauses

(+ maybe other compromise options, i.e. making it configurable or something)

@owenpan do you perhaps have insight for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145642

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


[PATCH] D141811: [clang-format] Allow trailing return types in macros

2023-03-22 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 507451.
rymiel added a comment.

Reduce column limit for macro test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141811

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8010,6 +8010,11 @@
"auto aa(T t)\n"
"-> decltype(eaaa(t.a).());");
 
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  verifyFormat("#define MAKE_DEF(NAME) 
\\\n"
+   "  auto NAME() -> int { return 42; }",
+   Style);
+
   // Not trailing return types.
   verifyFormat("void f() { auto a = b->c(); }");
   verifyFormat("auto a = p->foo();");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1909,7 +1909,8 @@
 } else if (Current.is(tok::arrow) &&
Style.Language == FormatStyle::LK_Java) {
   Current.setType(TT_LambdaArrow);
-} else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
+} else if (Current.is(tok::arrow) && AutoFound &&
+   (Line.MustBeDeclaration || Line.InPPDirective) &&
Current.NestingLevel == 0 &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8010,6 +8010,11 @@
"auto aa(T t)\n"
"-> decltype(eaaa(t.a).());");
 
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  verifyFormat("#define MAKE_DEF(NAME) \\\n"
+   "  auto NAME() -> int { return 42; }",
+   Style);
+
   // Not trailing return types.
   verifyFormat("void f() { auto a = b->c(); }");
   verifyFormat("auto a = p->foo();");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1909,7 +1909,8 @@
 } else if (Current.is(tok::arrow) &&
Style.Language == FormatStyle::LK_Java) {
   Current.setType(TT_LambdaArrow);
-} else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
+} else if (Current.is(tok::arrow) && AutoFound &&
+   (Line.MustBeDeclaration || Line.InPPDirective) &&
Current.NestingLevel == 0 &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D146284: [clang-format] Annotate noexcept, explicit specifiers as containing expressions

2023-03-22 Thread Emilia Dreamer 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 rGead9644684e8: [clang-format] Annotate noexcept, explicit 
specifiers as containing expressions (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146284

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
@@ -242,6 +242,17 @@
 "}");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("template  void swap() noexcept(Bar && 
Foo);");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[15], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {\n"
+"  explicit(Bar && Foo) S(const S &);\n"
+"};");
+  ASSERT_EQ(Tokens.size(), 30u) << Tokens;
+  EXPECT_TOKEN(Tokens[14], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11592,6 +11592,10 @@
   verifyFormat("template  class A {\n"
"  static_assert(B && C, \"Something is wrong\");\n"
"};");
+  verifyFormat("template  void swap() noexcept(Bar && 
Foo);");
+  verifyFormat("template  struct S {\n"
+   "  explicit(Bar && Foo) S(const S &);\n"
+   "};");
   verifyGoogleFormat("#define IF(a, b, c) if (a && (b == c))");
   verifyGoogleFormat("#define WHILE(a, b, c) while (a && (b == c))");
   verifyFormat("#define A(a, b) (a && b)");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -318,9 +318,10 @@
   // export type X = (...);
   Contexts.back().IsExpression = false;
 } else if (OpeningParen.Previous &&
-   (OpeningParen.Previous->isOneOf(tok::kw_static_assert,
-   tok::kw_while, tok::l_paren,
-   tok::comma, TT_BinaryOperator) 
||
+   (OpeningParen.Previous->isOneOf(
+tok::kw_static_assert, tok::kw_noexcept, tok::kw_explicit,
+tok::kw_while, tok::l_paren, tok::comma,
+TT_BinaryOperator) ||
 OpeningParen.Previous->isIf())) {
   // static_assert, if and while usually contain expressions.
   Contexts.back().IsExpression = true;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -242,6 +242,17 @@
 "}");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("template  void swap() noexcept(Bar && Foo);");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[15], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {\n"
+"  explicit(Bar && Foo) S(const S &);\n"
+"};");
+  ASSERT_EQ(Tokens.size(), 30u) << Tokens;
+  EXPECT_TOKEN(Tokens[14], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11592,6 +11592,10 @@
   verifyFormat("template  class A {\n"
"  static_assert(B && C, \"Something is wrong\");\n"
"};");
+  verifyFormat("template  void swap() noexcept(Bar && Foo);");
+  verifyFormat("template  struct S {\n"
+   "  explicit(Bar && Foo) S(const S &);\n"
+   "};");
   verifyGoogleFormat("#define IF(a, b, c) if (a && (b == c))");
   verifyGoogleFormat("#define WHILE(a, b, c) while (a && (b == c))");
   verifyFormat("#define A(a, b) (a && b)");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -318,9 +318,10 @@
   // export type X = (...);
   Contexts.back().IsExpression = false;
 } else if (OpeningParen.Previous &&

[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-22 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel marked an inline comment as done.
rymiel added inline comments.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:1352
+
+  // Both at once? Probably not even valid.
+  Tokens = annotate("[]  requires Foo (T t) requires Bar 
{}");

usaxena95 wrote:
> This is valid and is accepted by the compilers https://godbolt.org/z/EPbrWbrsv
Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145642

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


[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-22 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

In D145642#4213256 , @usaxena95 wrote:

> Another point:
> While testing this patch, the following still fails to recognise. Might be 
> something special with `true`.
>
>   auto y = [&]
>   requires true(Callable && callable)
> { static_cast(callable); };

Thank you for finding that edge case, I've fixed it and added a test for it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145642

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


[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-22 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 507440.
rymiel added a comment.

Remove unnecessary musing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145642

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1263,6 +1263,110 @@
   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);
+
+  // Lambdas with a requires-clause
+  Tokens = annotate("[]  (T t) requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[14]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T &) requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[8], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[11], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[15]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[16], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T t) requires Foo || Bar {}");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[19]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T t) -> T requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::arrow, TT_LambdaArrow);
+  EXPECT_TOKEN(Tokens[12], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[16]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[17], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar (T t) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar (T &) {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[16], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Foo || Bar (T t) {}");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[15]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires true (T&& t) {}");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[7]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar noexcept {}");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = 

[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-22 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 507439.
rymiel added a comment.

Fix `true` edge case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145642

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1263,6 +1263,111 @@
   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);
+
+  // Lambdas with a requires-clause
+  Tokens = annotate("[]  (T t) requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[14]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T &) requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[8], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[11], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[15]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[16], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T t) requires Foo || Bar {}");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[19]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T t) -> T requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::arrow, TT_LambdaArrow);
+  EXPECT_TOKEN(Tokens[12], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[16]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[17], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar (T t) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar (T &) {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[16], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Foo || Bar (T t) {}");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[15]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires true (T&& t) {}");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[7]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar noexcept {}");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = 

[PATCH] D146284: [clang-format] Annotate noexcept, explicit specifiers as containing expressions

2023-03-19 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 506370.
rymiel added a comment.

Annotator tests (copied from format tests)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146284

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
@@ -242,6 +242,17 @@
 "}");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("template  void swap() noexcept(Bar && 
Foo);");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[15], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {\n"
+"  explicit(Bar && Foo) S(const S &);\n"
+"};");
+  ASSERT_EQ(Tokens.size(), 30u) << Tokens;
+  EXPECT_TOKEN(Tokens[14], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11592,6 +11592,10 @@
   verifyFormat("template  class A {\n"
"  static_assert(B && C, \"Something is wrong\");\n"
"};");
+  verifyFormat("template  void swap() noexcept(Bar && 
Foo);");
+  verifyFormat("template  struct S {\n"
+   "  explicit(Bar && Foo) S(const S &);\n"
+   "};");
   verifyGoogleFormat("#define IF(a, b, c) if (a && (b == c))");
   verifyGoogleFormat("#define WHILE(a, b, c) while (a && (b == c))");
   verifyFormat("#define A(a, b) (a && b)");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -318,9 +318,10 @@
   // export type X = (...);
   Contexts.back().IsExpression = false;
 } else if (OpeningParen.Previous &&
-   (OpeningParen.Previous->isOneOf(tok::kw_static_assert,
-   tok::kw_while, tok::l_paren,
-   tok::comma, TT_BinaryOperator) 
||
+   (OpeningParen.Previous->isOneOf(
+tok::kw_static_assert, tok::kw_noexcept, tok::kw_explicit,
+tok::kw_while, tok::l_paren, tok::comma,
+TT_BinaryOperator) ||
 OpeningParen.Previous->isIf())) {
   // static_assert, if and while usually contain expressions.
   Contexts.back().IsExpression = true;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -242,6 +242,17 @@
 "}");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("template  void swap() noexcept(Bar && Foo);");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[15], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template  struct S {\n"
+"  explicit(Bar && Foo) S(const S &);\n"
+"};");
+  ASSERT_EQ(Tokens.size(), 30u) << Tokens;
+  EXPECT_TOKEN(Tokens[14], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11592,6 +11592,10 @@
   verifyFormat("template  class A {\n"
"  static_assert(B && C, \"Something is wrong\");\n"
"};");
+  verifyFormat("template  void swap() noexcept(Bar && Foo);");
+  verifyFormat("template  struct S {\n"
+   "  explicit(Bar && Foo) S(const S &);\n"
+   "};");
   verifyGoogleFormat("#define IF(a, b, c) if (a && (b == c))");
   verifyGoogleFormat("#define WHILE(a, b, c) while (a && (b == c))");
   verifyFormat("#define A(a, b) (a && b)");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -318,9 +318,10 @@
   // export type X = (...);
   Contexts.back().IsExpression = false;
 } else if (OpeningParen.Previous &&
-   (OpeningParen.Previous->isOneOf(tok::kw_static_assert,
-   tok::kw_while, tok::l_paren,
-   

[PATCH] D146284: [clang-format] Annotate noexcept, explicit specifiers as containing expressions

2023-03-19 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

Yes, good point, I will do that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146284

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


[PATCH] D146140: [clang] Properly parse variable template requires clause in lambda

2023-03-17 Thread Emilia Dreamer 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 rG6acdf58919d5: [clang] Properly parse variable template 
requires clause in lambda (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146140

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaConcept.cpp
  clang/test/SemaTemplate/concepts.cpp


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -55,10 +55,15 @@
 }
 
 namespace P0857R0 {
+  template  static constexpr bool V = true;
+
   void f() {
 auto x = [] requires B {}; // expected-note {{constraints not 
satisfied}} expected-note {{false}}
 x.operator()();
 x.operator()(); // expected-error {{no matching member function}}
+
+auto y = [] requires V () {};
+y.operator()(); // OK
   }
 
   template concept C = true;
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -105,27 +105,35 @@
   QualType Type = ConstraintExpression->getType();
 
   auto CheckForNonPrimary = [&] {
-if (PossibleNonPrimary)
-  *PossibleNonPrimary =
-  // We have the following case:
-  // template requires func(0) struct S { };
-  // The user probably isn't aware of the parentheses required around
-  // the function call, and we're only going to parse 'func' as the
-  // primary-expression, and complain that it is of non-bool type.
-  (NextToken.is(tok::l_paren) &&
-   (IsTrailingRequiresClause ||
-(Type->isDependentType() &&
- isa(ConstraintExpression)) ||
-Type->isFunctionType() ||
-Type->isSpecificBuiltinType(BuiltinType::Overload))) ||
-  // We have the following case:
-  // template requires size_ == 0 struct S { };
-  // The user probably isn't aware of the parentheses required around
-  // the binary operator, and we're only going to parse 'func' as the
-  // first operand, and complain that it is of non-bool type.
-  getBinOpPrecedence(NextToken.getKind(),
- /*GreaterThanIsOperator=*/true,
- getLangOpts().CPlusPlus11) > prec::LogicalAnd;
+if (!PossibleNonPrimary)
+  return;
+
+*PossibleNonPrimary =
+// We have the following case:
+// template requires func(0) struct S { };
+// The user probably isn't aware of the parentheses required around
+// the function call, and we're only going to parse 'func' as the
+// primary-expression, and complain that it is of non-bool type.
+//
+// However, if we're in a lambda, this might also be:
+// [] requires var () {};
+// Which also looks like a function call due to the lambda parentheses,
+// but unlike the first case, isn't an error, so this check is skipped.
+(NextToken.is(tok::l_paren) &&
+ (IsTrailingRequiresClause ||
+  (Type->isDependentType() &&
+   isa(ConstraintExpression) &&
+   !dyn_cast_if_present(getCurFunction())) ||
+  Type->isFunctionType() ||
+  Type->isSpecificBuiltinType(BuiltinType::Overload))) ||
+// We have the following case:
+// template requires size_ == 0 struct S { };
+// The user probably isn't aware of the parentheses required around
+// the binary operator, and we're only going to parse 'func' as the
+// first operand, and complain that it is of non-bool type.
+getBinOpPrecedence(NextToken.getKind(),
+   /*GreaterThanIsOperator=*/true,
+   getLangOpts().CPlusPlus11) > prec::LogicalAnd;
   };
 
   // An atomic constraint!
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -80,6 +80,9 @@
 ^
 - Support for out-of-line definitions of constrained templates has been 
improved.
   This partially fixes `#49620 
`_.
+- Lambda templates with a requires clause directly after the template 
parameters now parse
+  correctly if the requires clause consists of a variable with a dependent 
type.
+  (`#61278 `_)
 
 C++2b Feature Support
 ^


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -55,10 +55,15 @@
 }
 
 namespace 

[PATCH] D146140: [clang] Properly parse variable template requires clause in lambda

2023-03-17 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 506121.
rymiel added a comment.

Add release note


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146140

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaConcept.cpp
  clang/test/SemaTemplate/concepts.cpp


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -55,10 +55,15 @@
 }
 
 namespace P0857R0 {
+  template  static constexpr bool V = true;
+
   void f() {
 auto x = [] requires B {}; // expected-note {{constraints not 
satisfied}} expected-note {{false}}
 x.operator()();
 x.operator()(); // expected-error {{no matching member function}}
+
+auto y = [] requires V () {};
+y.operator()(); // OK
   }
 
   template concept C = true;
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -105,27 +105,35 @@
   QualType Type = ConstraintExpression->getType();
 
   auto CheckForNonPrimary = [&] {
-if (PossibleNonPrimary)
-  *PossibleNonPrimary =
-  // We have the following case:
-  // template requires func(0) struct S { };
-  // The user probably isn't aware of the parentheses required around
-  // the function call, and we're only going to parse 'func' as the
-  // primary-expression, and complain that it is of non-bool type.
-  (NextToken.is(tok::l_paren) &&
-   (IsTrailingRequiresClause ||
-(Type->isDependentType() &&
- isa(ConstraintExpression)) ||
-Type->isFunctionType() ||
-Type->isSpecificBuiltinType(BuiltinType::Overload))) ||
-  // We have the following case:
-  // template requires size_ == 0 struct S { };
-  // The user probably isn't aware of the parentheses required around
-  // the binary operator, and we're only going to parse 'func' as the
-  // first operand, and complain that it is of non-bool type.
-  getBinOpPrecedence(NextToken.getKind(),
- /*GreaterThanIsOperator=*/true,
- getLangOpts().CPlusPlus11) > prec::LogicalAnd;
+if (!PossibleNonPrimary)
+  return;
+
+*PossibleNonPrimary =
+// We have the following case:
+// template requires func(0) struct S { };
+// The user probably isn't aware of the parentheses required around
+// the function call, and we're only going to parse 'func' as the
+// primary-expression, and complain that it is of non-bool type.
+//
+// However, if we're in a lambda, this might also be:
+// [] requires var () {};
+// Which also looks like a function call due to the lambda parentheses,
+// but unlike the first case, isn't an error, so this check is skipped.
+(NextToken.is(tok::l_paren) &&
+ (IsTrailingRequiresClause ||
+  (Type->isDependentType() &&
+   isa(ConstraintExpression) &&
+   !dyn_cast_if_present(getCurFunction())) ||
+  Type->isFunctionType() ||
+  Type->isSpecificBuiltinType(BuiltinType::Overload))) ||
+// We have the following case:
+// template requires size_ == 0 struct S { };
+// The user probably isn't aware of the parentheses required around
+// the binary operator, and we're only going to parse 'func' as the
+// first operand, and complain that it is of non-bool type.
+getBinOpPrecedence(NextToken.getKind(),
+   /*GreaterThanIsOperator=*/true,
+   getLangOpts().CPlusPlus11) > prec::LogicalAnd;
   };
 
   // An atomic constraint!
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -80,6 +80,9 @@
 ^
 - Support for out-of-line definitions of constrained templates has been 
improved.
   This partially fixes `#49620 
`_.
+- Lambda templates with a requires clause directly after the template 
parameters now parse
+  correctly if the requires clause consists of a variable with a dependent 
type.
+  (`#61278 `_)
 
 C++2b Feature Support
 ^


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -55,10 +55,15 @@
 }
 
 namespace P0857R0 {
+  template  static constexpr bool V = true;
+
   void f() {
 auto x = [] requires B {}; // expected-note {{constraints not satisfied}} expected-note 

[PATCH] D146284: [clang-format] Annotate noexcept, explicit specifiers as containing expressions

2023-03-17 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The noexcept specifier and explicit specifier can optionally include a
boolean expression to make these specifiers apply conditionally,
however, clang-format didn't set the context for the parenthesized
content of these specifiers, meaning they inherited the parent context,
which usually isn't an expressions, leading to misannotated binary
operators.

This patch applies expression context to the content of these
specifiers, making them similar to the static_assert keyword.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146284

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11592,6 +11592,10 @@
   verifyFormat("template  class A {\n"
"  static_assert(B && C, \"Something is wrong\");\n"
"};");
+  verifyFormat("template  void swap() noexcept(Bar && 
Foo);");
+  verifyFormat("template  struct S {\n"
+   "  explicit(Bar && Foo) S(const S &);\n"
+   "};");
   verifyGoogleFormat("#define IF(a, b, c) if (a && (b == c))");
   verifyGoogleFormat("#define WHILE(a, b, c) while (a && (b == c))");
   verifyFormat("#define A(a, b) (a && b)");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -318,9 +318,10 @@
   // export type X = (...);
   Contexts.back().IsExpression = false;
 } else if (OpeningParen.Previous &&
-   (OpeningParen.Previous->isOneOf(tok::kw_static_assert,
-   tok::kw_while, tok::l_paren,
-   tok::comma, TT_BinaryOperator) 
||
+   (OpeningParen.Previous->isOneOf(
+tok::kw_static_assert, tok::kw_noexcept, tok::kw_explicit,
+tok::kw_while, tok::l_paren, tok::comma,
+TT_BinaryOperator) ||
 OpeningParen.Previous->isIf())) {
   // static_assert, if and while usually contain expressions.
   Contexts.back().IsExpression = true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11592,6 +11592,10 @@
   verifyFormat("template  class A {\n"
"  static_assert(B && C, \"Something is wrong\");\n"
"};");
+  verifyFormat("template  void swap() noexcept(Bar && Foo);");
+  verifyFormat("template  struct S {\n"
+   "  explicit(Bar && Foo) S(const S &);\n"
+   "};");
   verifyGoogleFormat("#define IF(a, b, c) if (a && (b == c))");
   verifyGoogleFormat("#define WHILE(a, b, c) while (a && (b == c))");
   verifyFormat("#define A(a, b) (a && b)");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -318,9 +318,10 @@
   // export type X = (...);
   Contexts.back().IsExpression = false;
 } else if (OpeningParen.Previous &&
-   (OpeningParen.Previous->isOneOf(tok::kw_static_assert,
-   tok::kw_while, tok::l_paren,
-   tok::comma, TT_BinaryOperator) ||
+   (OpeningParen.Previous->isOneOf(
+tok::kw_static_assert, tok::kw_noexcept, tok::kw_explicit,
+tok::kw_while, tok::l_paren, tok::comma,
+TT_BinaryOperator) ||
 OpeningParen.Previous->isIf())) {
   // static_assert, if and while usually contain expressions.
   Contexts.back().IsExpression = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D146281: [clang-format] Don't wrap struct return types as structs

2023-03-17 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When using BraceWrapping.AfterClass or BraceWrapping.AfterStruct, the
token annotator relies on the first token of the line to determine if
we're dealing with a struct or class, however, this check is faulty if
it's actually a function with an elaborated struct/class return type, as
is common in C.

This patch skips the check if the brace is already annotated as
FunctionLBrace, in which case we already know it's a function and should
be treated as such.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146281

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3205,10 +3205,13 @@
 format("try{foo();}catch(...){baz();}", Style));
 
   Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.AfterStruct = false;
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_MultiLine;
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   Style.ColumnLimit = 80;
   verifyFormat("void shortfunction() { bar(); }", Style);
+  verifyFormat("struct T shortfunction() { return bar(); }", Style);
+  verifyFormat("struct T {};", Style);
 
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
   verifyFormat("void shortfunction()\n"
@@ -3216,6 +3219,36 @@
"  bar();\n"
"}",
Style);
+  verifyFormat("struct T shortfunction()\n"
+   "{\n"
+   "  return bar();\n"
+   "}",
+   Style);
+  verifyFormat("struct T {};", Style);
+
+  Style.BraceWrapping.AfterFunction = false;
+  Style.BraceWrapping.AfterStruct = true;
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("void shortfunction() { bar(); }", Style);
+  verifyFormat("struct T shortfunction() { return bar(); }", Style);
+  verifyFormat("struct T\n"
+   "{\n"
+   "};",
+   Style);
+
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  verifyFormat("void shortfunction() {\n"
+   "  bar();\n"
+   "}",
+   Style);
+  verifyFormat("struct T shortfunction() {\n"
+   "  return bar();\n"
+   "}",
+   Style);
+  verifyFormat("struct T\n"
+   "{\n"
+   "};",
+   Style);
 }
 
 TEST_F(FormatTest, BeforeWhile) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4881,8 +4881,13 @@
   return true;
 }
 
-return (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) 
||
-   (Line.startsWith(tok::kw_struct) && 
Style.BraceWrapping.AfterStruct);
+// Don't attempt to interpret struct return types as structs.
+if (Right.isNot(TT_FunctionLBrace)) {
+  return (Line.startsWith(tok::kw_class) &&
+  Style.BraceWrapping.AfterClass) ||
+ (Line.startsWith(tok::kw_struct) &&
+  Style.BraceWrapping.AfterStruct);
+}
   }
 
   if (Left.is(TT_ObjCBlockLBrace) &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3205,10 +3205,13 @@
 format("try{foo();}catch(...){baz();}", Style));
 
   Style.BraceWrapping.AfterFunction = true;
+  Style.BraceWrapping.AfterStruct = false;
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_MultiLine;
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   Style.ColumnLimit = 80;
   verifyFormat("void shortfunction() { bar(); }", Style);
+  verifyFormat("struct T shortfunction() { return bar(); }", Style);
+  verifyFormat("struct T {};", Style);
 
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
   verifyFormat("void shortfunction()\n"
@@ -3216,6 +3219,36 @@
"  bar();\n"
"}",
Style);
+  verifyFormat("struct T shortfunction()\n"
+   "{\n"
+   "  return bar();\n"
+   "}",
+   Style);
+  verifyFormat("struct T {};", Style);
+
+  Style.BraceWrapping.AfterFunction = false;
+  Style.BraceWrapping.AfterStruct = true;
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+  verifyFormat("void shortfunction() { bar(); }", Style);
+  

[PATCH] D141811: [clang-format] Allow trailing return types in macros

2023-03-17 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel requested review of this revision.
rymiel added a comment.
This revision is now accepted and ready to land.

Okay, I planned changes because I had more ambitious plans for fixing this, but 
those didn't work out, so instead I made a separate issue 
(https://github.com/llvm/llvm-project/issues/61469).
This patch in its current state is a simple fix, so it's probably fine as is


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141811

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


[PATCH] D146140: [clang] Properly parse variable template requires clause in lambda

2023-03-15 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

In D146140#4196886 , @erichkeane 
wrote:

> Ah, also, before commit, ensure that pre-commit CI passes.

Yep, found an issue already ;;

In D146140#4196866 , @erichkeane 
wrote:

> I presume you don't have commit access

I do! I just haven't done anything in this part of the monorepo, which is why 
i'm so lost.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146140

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


[PATCH] D146140: [clang] Properly parse variable template requires clause in lambda

2023-03-15 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 505534.
rymiel added a comment.

Use dyn_cast_if_present, otherwise we segfault in some tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146140

Files:
  clang/lib/Sema/SemaConcept.cpp
  clang/test/SemaTemplate/concepts.cpp


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -55,10 +55,15 @@
 }
 
 namespace P0857R0 {
+  template  static constexpr bool V = true;
+
   void f() {
 auto x = [] requires B {}; // expected-note {{constraints not 
satisfied}} expected-note {{false}}
 x.operator()();
 x.operator()(); // expected-error {{no matching member function}}
+
+auto y = [] requires V () {};
+y.operator()(); // OK
   }
 
   template concept C = true;
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -105,27 +105,35 @@
   QualType Type = ConstraintExpression->getType();
 
   auto CheckForNonPrimary = [&] {
-if (PossibleNonPrimary)
-  *PossibleNonPrimary =
-  // We have the following case:
-  // template requires func(0) struct S { };
-  // The user probably isn't aware of the parentheses required around
-  // the function call, and we're only going to parse 'func' as the
-  // primary-expression, and complain that it is of non-bool type.
-  (NextToken.is(tok::l_paren) &&
-   (IsTrailingRequiresClause ||
-(Type->isDependentType() &&
- isa(ConstraintExpression)) ||
-Type->isFunctionType() ||
-Type->isSpecificBuiltinType(BuiltinType::Overload))) ||
-  // We have the following case:
-  // template requires size_ == 0 struct S { };
-  // The user probably isn't aware of the parentheses required around
-  // the binary operator, and we're only going to parse 'func' as the
-  // first operand, and complain that it is of non-bool type.
-  getBinOpPrecedence(NextToken.getKind(),
- /*GreaterThanIsOperator=*/true,
- getLangOpts().CPlusPlus11) > prec::LogicalAnd;
+if (!PossibleNonPrimary)
+  return;
+
+*PossibleNonPrimary =
+// We have the following case:
+// template requires func(0) struct S { };
+// The user probably isn't aware of the parentheses required around
+// the function call, and we're only going to parse 'func' as the
+// primary-expression, and complain that it is of non-bool type.
+//
+// However, if we're in a lambda, this might also be:
+// [] requires var () {};
+// Which also looks like a function call due to the lambda parentheses,
+// but unlike the first case, isn't an error, so this check is skipped.
+(NextToken.is(tok::l_paren) &&
+ (IsTrailingRequiresClause ||
+  (Type->isDependentType() &&
+   isa(ConstraintExpression) &&
+   !dyn_cast_if_present(getCurFunction())) ||
+  Type->isFunctionType() ||
+  Type->isSpecificBuiltinType(BuiltinType::Overload))) ||
+// We have the following case:
+// template requires size_ == 0 struct S { };
+// The user probably isn't aware of the parentheses required around
+// the binary operator, and we're only going to parse 'func' as the
+// first operand, and complain that it is of non-bool type.
+getBinOpPrecedence(NextToken.getKind(),
+   /*GreaterThanIsOperator=*/true,
+   getLangOpts().CPlusPlus11) > prec::LogicalAnd;
   };
 
   // An atomic constraint!


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -55,10 +55,15 @@
 }
 
 namespace P0857R0 {
+  template  static constexpr bool V = true;
+
   void f() {
 auto x = [] requires B {}; // expected-note {{constraints not satisfied}} expected-note {{false}}
 x.operator()();
 x.operator()(); // expected-error {{no matching member function}}
+
+auto y = [] requires V () {};
+y.operator()(); // OK
   }
 
   template concept C = true;
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -105,27 +105,35 @@
   QualType Type = ConstraintExpression->getType();
 
   auto CheckForNonPrimary = [&] {
-if (PossibleNonPrimary)
-  *PossibleNonPrimary =
-  // We have the following case:
-  // template requires func(0) struct S { };
-  // The 

[PATCH] D146140: [clang] Properly parse variable template requires clause in lambda

2023-03-15 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel marked an inline comment as done.
rymiel added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:1283
 
   ParseScope LambdaScope(this, Scope::LambdaScope | Scope::DeclScope |
Scope::FunctionDeclarationScope |

erichkeane wrote:
> Ok, last bit, I promise :)  I see that we set up the lambda scope here and 
> push the lambda scope on 1287.  Instead of passing a bool, could we just 
> figure out the `IsLambdaRequiresClause` based on that instead?  See 
> definition of `PushLambdaScope` here: 
> https://clang.llvm.org/doxygen/Sema_8cpp_source.html#l02141
> 
> A test to see if that would work would be the one you have below, PLUS an 
> example of a requires clause INSIDE of a lambda that itself isn't a lambda 
> that would reproduce the warning(though I'm not convinced ATM that is 
> possible, I don't think we allow a template inside block scope, unless there 
> is some trick I'm too uncreative enough to come up with).  If none exists, 
> just checking the current scope would work and perhaps be more preferential.  
> 
> We do this rarely, but we do it at least in SemaStmt.cpp in 
> ActOnCapScopeReturnStmt.  Just a 
> `dyn_cast(getCurFunction())` might be able to replace the 
> bool all over the place.
Thank you, I knew there was probably some much cleaner way to do this that I 
simply had no clue about due to my unfamiliarity. I've gone ahead and used 
`dyn_cast(getCurFunction())`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146140

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


[PATCH] D146140: [clang] Properly parse variable template requires clause in lambda

2023-03-15 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 505525.
rymiel added a comment.

utilize getCurFunction()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146140

Files:
  clang/lib/Sema/SemaConcept.cpp
  clang/test/SemaTemplate/concepts.cpp


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -55,10 +55,15 @@
 }
 
 namespace P0857R0 {
+  template  static constexpr bool V = true;
+
   void f() {
 auto x = [] requires B {}; // expected-note {{constraints not 
satisfied}} expected-note {{false}}
 x.operator()();
 x.operator()(); // expected-error {{no matching member function}}
+
+auto y = [] requires V () {};
+y.operator()(); // OK
   }
 
   template concept C = true;
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -105,27 +105,35 @@
   QualType Type = ConstraintExpression->getType();
 
   auto CheckForNonPrimary = [&] {
-if (PossibleNonPrimary)
-  *PossibleNonPrimary =
-  // We have the following case:
-  // template requires func(0) struct S { };
-  // The user probably isn't aware of the parentheses required around
-  // the function call, and we're only going to parse 'func' as the
-  // primary-expression, and complain that it is of non-bool type.
-  (NextToken.is(tok::l_paren) &&
-   (IsTrailingRequiresClause ||
-(Type->isDependentType() &&
- isa(ConstraintExpression)) ||
-Type->isFunctionType() ||
-Type->isSpecificBuiltinType(BuiltinType::Overload))) ||
-  // We have the following case:
-  // template requires size_ == 0 struct S { };
-  // The user probably isn't aware of the parentheses required around
-  // the binary operator, and we're only going to parse 'func' as the
-  // first operand, and complain that it is of non-bool type.
-  getBinOpPrecedence(NextToken.getKind(),
- /*GreaterThanIsOperator=*/true,
- getLangOpts().CPlusPlus11) > prec::LogicalAnd;
+if (!PossibleNonPrimary)
+  return;
+
+*PossibleNonPrimary =
+// We have the following case:
+// template requires func(0) struct S { };
+// The user probably isn't aware of the parentheses required around
+// the function call, and we're only going to parse 'func' as the
+// primary-expression, and complain that it is of non-bool type.
+//
+// However, if we're in a lambda, this might also be:
+// [] requires var () {};
+// Which also looks like a function call due to the lambda parentheses,
+// but unlike the first case, isn't an error, so this check is skipped.
+(NextToken.is(tok::l_paren) &&
+ (IsTrailingRequiresClause ||
+  (Type->isDependentType() &&
+   isa(ConstraintExpression) &&
+   !dyn_cast(getCurFunction())) ||
+  Type->isFunctionType() ||
+  Type->isSpecificBuiltinType(BuiltinType::Overload))) ||
+// We have the following case:
+// template requires size_ == 0 struct S { };
+// The user probably isn't aware of the parentheses required around
+// the binary operator, and we're only going to parse 'func' as the
+// first operand, and complain that it is of non-bool type.
+getBinOpPrecedence(NextToken.getKind(),
+   /*GreaterThanIsOperator=*/true,
+   getLangOpts().CPlusPlus11) > prec::LogicalAnd;
   };
 
   // An atomic constraint!


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -55,10 +55,15 @@
 }
 
 namespace P0857R0 {
+  template  static constexpr bool V = true;
+
   void f() {
 auto x = [] requires B {}; // expected-note {{constraints not satisfied}} expected-note {{false}}
 x.operator()();
 x.operator()(); // expected-error {{no matching member function}}
+
+auto y = [] requires V () {};
+y.operator()(); // OK
   }
 
   template concept C = true;
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -105,27 +105,35 @@
   QualType Type = ConstraintExpression->getType();
 
   auto CheckForNonPrimary = [&] {
-if (PossibleNonPrimary)
-  *PossibleNonPrimary =
-  // We have the following case:
-  // template requires func(0) struct S { };
-  // The user probably isn't aware of the parentheses 

[PATCH] D146140: [clang] Properly parse variable template requires clause in lambda

2023-03-15 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel marked 2 inline comments as done.
rymiel added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:116
   // primary-expression, and complain that it is of non-bool type.
-  (NextToken.is(tok::l_paren) &&
+  (NextToken.is(tok::l_paren) && !IsLambdaRequiresClause &&
(IsTrailingRequiresClause ||

erichkeane wrote:
> rymiel wrote:
> > erichkeane wrote:
> > > I'd like to expand on the comment above in this case.  Also, since we 
> > > don't decide that this is a trailing requires clause in the lambda 
> > > parsing, we should probably make this more specific in this condition.  I 
> > > THINK we still want to do the bin-op precedence condition in this case, 
> > > right?
> > > I'd like to expand on the comment above in this case.
> > 
> > Yes, that's a very good call, doing that now.
> > 
> > > Also, since we don't decide that this is a trailing requires clause in 
> > > the lambda parsing, we should probably make this more specific in this 
> > > condition.
> > 
> > I'm not 100% sure what you mean here...
> > 
> > > I THINK we still want to do the bin-op precedence condition in this case, 
> > > right?
> > 
> > I think it's still being done, but it's not very clear from the mess of a 
> > logic expression
> So my concern is that this is a 'top level' condition here, rather than being 
> 'more specific', but you're right, this is a little confusing logic, and I'm 
> afraid I perhaps got confused as well.  So here is the logic as it sits in 
> your patch.
> ```
> (
>   NextToken.is(tok::l_paren) 
>   && 
>   !IsLambdaRequiresClause 
>   &&
>   (
> IsTrailingRequiresClause
> ||
> (
>Type->isDependentType() //#1
>&&
>isa(ConstraintExpression) 
> ) 
> ||
> Type->isFunctionType()
> ||
> Type->isSpecificBuiltinType(BuiltinType::Overload)
>   )
> ) 
> ||
> getBinOpPrecedence(NextToken.getKind(), /*GreaterThanIsOperator=*/true, 
> getLangOpts().CPlusPlus11) > prec::LogicalAnd;
> ```
> 
> I suspect we don't want to have this skip the `getBinOpPrecedence`, which I 
> think you're right, works correctly.  I'm a bit concerned about your patch 
> skippinjg the `isFunctionType` and `isSpecificBuiltinType` branches. 
> 
> The one in your reproducer hits only the `isDependentType() && 
> isa(ConstraintExpr)`, correct?  So unless you have a more specific 
> example where it should also apply when the type is a function/overload set, 
> I suspect the `!IsLambdaRequiresClause` would be best placed in with the ULE 
> check (~#1).
> 
> Does that make sense?
Yes, that makes a lot of sense, thank you for pointing that out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146140

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


[PATCH] D146140: [clang] Properly parse variable template requires clause in lambda

2023-03-15 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 505499.
rymiel added a comment.

Slightly rewrite CheckForNonPrimary for slightly better clarity


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146140

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -55,10 +55,15 @@
 }
 
 namespace P0857R0 {
+  template  static constexpr bool V = true;
+
   void f() {
 auto x = [] requires B {}; // expected-note {{constraints not satisfied}} expected-note {{false}}
 x.operator()();
 x.operator()(); // expected-error {{no matching member function}}
+
+auto y = [] requires V () {};
+y.operator()(); // OK
   }
 
   template concept C = true;
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -87,7 +87,8 @@
 
 bool Sema::CheckConstraintExpression(const Expr *ConstraintExpression,
  Token NextToken, bool *PossibleNonPrimary,
- bool IsTrailingRequiresClause) {
+ bool IsTrailingRequiresClause,
+ bool IsLambdaRequiresClause) {
   // C++2a [temp.constr.atomic]p1
   // ..E shall be a constant expression of type bool.
 
@@ -105,27 +106,35 @@
   QualType Type = ConstraintExpression->getType();
 
   auto CheckForNonPrimary = [&] {
-if (PossibleNonPrimary)
-  *PossibleNonPrimary =
-  // We have the following case:
-  // template requires func(0) struct S { };
-  // The user probably isn't aware of the parentheses required around
-  // the function call, and we're only going to parse 'func' as the
-  // primary-expression, and complain that it is of non-bool type.
-  (NextToken.is(tok::l_paren) &&
-   (IsTrailingRequiresClause ||
-(Type->isDependentType() &&
- isa(ConstraintExpression)) ||
-Type->isFunctionType() ||
-Type->isSpecificBuiltinType(BuiltinType::Overload))) ||
-  // We have the following case:
-  // template requires size_ == 0 struct S { };
-  // The user probably isn't aware of the parentheses required around
-  // the binary operator, and we're only going to parse 'func' as the
-  // first operand, and complain that it is of non-bool type.
-  getBinOpPrecedence(NextToken.getKind(),
- /*GreaterThanIsOperator=*/true,
- getLangOpts().CPlusPlus11) > prec::LogicalAnd;
+if (!PossibleNonPrimary)
+  return;
+
+*PossibleNonPrimary =
+// We have the following case:
+// template requires func(0) struct S { };
+// The user probably isn't aware of the parentheses required around
+// the function call, and we're only going to parse 'func' as the
+// primary-expression, and complain that it is of non-bool type.
+//
+// However, if we're in a lambda, this might also be:
+// [] requires var () {};
+// Which also looks like a function call due to the lambda parentheses,
+// but unlike the first case, isn't an error, so this check is skipped.
+(NextToken.is(tok::l_paren) &&
+ (IsTrailingRequiresClause ||
+  (Type->isDependentType() &&
+   isa(ConstraintExpression) &&
+   !IsLambdaRequiresClause) ||
+  Type->isFunctionType() ||
+  Type->isSpecificBuiltinType(BuiltinType::Overload))) ||
+// We have the following case:
+// template requires size_ == 0 struct S { };
+// The user probably isn't aware of the parentheses required around
+// the binary operator, and we're only going to parse 'func' as the
+// first operand, and complain that it is of non-bool type.
+getBinOpPrecedence(NextToken.getKind(),
+   /*GreaterThanIsOperator=*/true,
+   getLangOpts().CPlusPlus11) > prec::LogicalAnd;
   };
 
   // An atomic constraint!
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -1345,7 +1345,8 @@
   if (TryConsumeToken(tok::kw_requires)) {
 RequiresClause =
 Actions.ActOnRequiresClause(ParseConstraintLogicalOrExpression(
-/*IsTrailingRequiresClause=*/false));
+

[PATCH] D146140: [clang] Properly parse variable template requires clause in lambda

2023-03-15 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:116
   // primary-expression, and complain that it is of non-bool type.
-  (NextToken.is(tok::l_paren) &&
+  (NextToken.is(tok::l_paren) && !IsLambdaRequiresClause &&
(IsTrailingRequiresClause ||

erichkeane wrote:
> I'd like to expand on the comment above in this case.  Also, since we don't 
> decide that this is a trailing requires clause in the lambda parsing, we 
> should probably make this more specific in this condition.  I THINK we still 
> want to do the bin-op precedence condition in this case, right?
> I'd like to expand on the comment above in this case.

Yes, that's a very good call, doing that now.

> Also, since we don't decide that this is a trailing requires clause in the 
> lambda parsing, we should probably make this more specific in this condition.

I'm not 100% sure what you mean here...

> I THINK we still want to do the bin-op precedence condition in this case, 
> right?

I think it's still being done, but it's not very clear from the mess of a logic 
expression


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146140

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


[PATCH] D146140: [clang] Properly parse variable template requires clause in lambda

2023-03-15 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added a project: clang.
rymiel added reviewers: aaron.ballman, erichkeane.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a subscriber: cfe-commits.

Since P0857, part of C++20, a *lambda-expression* can contain a
*requires-clause* after its *template-parameter-list*.

While support for this was added as part of
eccc734a69c0c012ae3160887b65a535b35ead3e 
, one 
specific case isn't
handled properly, where the *requires-clause* consists of an
instantiation of a boolean variable template. This is due to a
diagnostic check which was written with the assumption that a
*requires-clause* can never be followed by a left parenthesis. This
assumption no longer holds for lambdas.

This diagnostic check would then attempt to perform a "recovery", but it
does so in a valid parse state, resulting in an invalid parse state
instead!

This patch adds a special case when parsing requires clauses of lambda
templates, to skip this diagnostic check.

Fixes https://github.com/llvm/llvm-project/issues/61278
Fixes https://github.com/llvm/llvm-project/issues/61387


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146140

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -55,10 +55,15 @@
 }
 
 namespace P0857R0 {
+  template  static constexpr bool V = true;
+
   void f() {
 auto x = [] requires B {}; // expected-note {{constraints not satisfied}} expected-note {{false}}
 x.operator()();
 x.operator()(); // expected-error {{no matching member function}}
+
+auto y = [] requires V () {};
+y.operator()(); // OK
   }
 
   template concept C = true;
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -87,7 +87,8 @@
 
 bool Sema::CheckConstraintExpression(const Expr *ConstraintExpression,
  Token NextToken, bool *PossibleNonPrimary,
- bool IsTrailingRequiresClause) {
+ bool IsTrailingRequiresClause,
+ bool IsLambdaRequiresClause) {
   // C++2a [temp.constr.atomic]p1
   // ..E shall be a constant expression of type bool.
 
@@ -112,7 +113,7 @@
   // The user probably isn't aware of the parentheses required around
   // the function call, and we're only going to parse 'func' as the
   // primary-expression, and complain that it is of non-bool type.
-  (NextToken.is(tok::l_paren) &&
+  (NextToken.is(tok::l_paren) && !IsLambdaRequiresClause &&
(IsTrailingRequiresClause ||
 (Type->isDependentType() &&
  isa(ConstraintExpression)) ||
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -1345,7 +1345,8 @@
   if (TryConsumeToken(tok::kw_requires)) {
 RequiresClause =
 Actions.ActOnRequiresClause(ParseConstraintLogicalOrExpression(
-/*IsTrailingRequiresClause=*/false));
+/*IsTrailingRequiresClause=*/false,
+/*IsLambdaRequiresClause=*/true));
 if (RequiresClause.isInvalid())
   SkipUntil({tok::l_brace, tok::l_paren}, StopAtSemi | StopBeforeMatch);
   }
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -255,7 +255,8 @@
 ///
 /// \endverbatim
 ExprResult
-Parser::ParseConstraintLogicalAndExpression(bool IsTrailingRequiresClause) {
+Parser::ParseConstraintLogicalAndExpression(bool IsTrailingRequiresClause,
+bool IsLambdaRequiresClause) {
   EnterExpressionEvaluationContext ConstantEvaluated(
   Actions, Sema::ExpressionEvaluationContext::Unevaluated);
   bool NotPrimaryExpression = false;
@@ -299,9 +300,9 @@
   NotPrimaryExpression = false;
 }
 bool PossibleNonPrimary;
-bool IsConstraintExpr =
-Actions.CheckConstraintExpression(E.get(), Tok, ,
-  IsTrailingRequiresClause);
+bool IsConstraintExpr = Actions.CheckConstraintExpression(
+E.get(), Tok, , IsTrailingRequiresClause,
+IsLambdaRequiresClause);
 if (!IsConstraintExpr || PossibleNonPrimary) {
   // Atomic constraint might be an unparenthesized 

[PATCH] D145656: [clang-format] Treat &/&& as reference when followed by requires clause

2023-03-14 Thread Emilia Dreamer 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 rGc9163901c8e1: [clang-format] Treat / as 
reference when followed by requires clause (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145656

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
@@ -744,10 +744,15 @@
 "void S::bar() const & requires Baz { }");
   ASSERT_EQ(Tokens.size(), 85u) << Tokens;
   EXPECT_TOKEN(Tokens[13], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[24], tok::amp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[25], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[35], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[36], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[47], tok::amp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[49], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[59], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[61], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[76], tok::amp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[77], tok::kw_requires, TT_RequiresClause);
 
   Tokens = annotate("void Class::member() && requires(Constant) {}");
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24148,6 +24148,14 @@
"}",
Style);
 
+  verifyFormat("template \n"
+   "int S::bar(T t) &&\n"
+   "  requires F\n"
+   "{\n"
+   "  return 5;\n"
+   "}",
+   Style);
+
   verifyFormat("template \n"
"int bar(T t)\n"
"  requires F;",
@@ -24161,6 +24169,14 @@
"}",
Style);
 
+  verifyFormat("template \n"
+   "int S::bar(T t) &&\n"
+   "requires F\n"
+   "{\n"
+   "  return 5;\n"
+   "}",
+   Style);
+
   verifyFormat("template \n"
"int bar(T t)\n"
"requires F\n"
@@ -24174,6 +24190,7 @@
"template  requires Foo void bar() {}\n"
"template  void bar() requires Foo {}\n"
"template  void bar() requires Foo;\n"
+   "template  void S::bar() && requires Foo {}\n"
"template  requires Foo Bar(T) -> Bar;",
Style);
 
@@ -24185,6 +24202,8 @@
"requires Foo void bar() {}\n"
"template \n"
"void bar() requires Foo {}\n"
+   "template \n"
+   "void S::bar() && requires Foo {}\n"
"template \n"
"requires Foo Baz(T) -> Baz;",
ColumnStyle);
@@ -24229,6 +24248,9 @@
"void bar()\n"
"requires Foo;\n"
"template \n"
+   "void S::bar() &&\n"
+   "requires Foo {}\n"
+   "template \n"
"requires Foo Bar(T) -> Bar;",
Style);
 
@@ -24259,6 +24281,9 @@
"void bar()\n"
"  requires Foo {}\n"
"template \n"
+   "void S::bar() &&\n"
+   "  requires Foo {}\n"
+   "template \n"
"  requires Foo Bar(T) -> Bar;",
Style);
 
@@ -24289,6 +24314,9 @@
"void bar() requires Foo\n"
"{}\n"
"template  void bar() requires Foo;\n"
+   "template \n"
+   "void S::bar() && requires Foo\n"
+   "{}\n"
"template  requires Foo\n"
"Bar(T) -> Bar;",
Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2400,7 +2400,7 @@
 
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept, tok::comma,
-   tok::r_paren) ||
+   tok::r_paren, TT_RequiresClause) ||
 NextToken->canBePointerOrReferenceQualifier() ||
 (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment())) {
   return TT_PointerOrReference;
@@ -3758,8 +3758,9 @@
 if (Right.is(TT_BlockComment))
   return true;
 // foo() -> const Bar * override/final
-if (Right.isOneOf(Keywords.kw_override, 

[PATCH] D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine

2023-03-11 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/lib/Format/Format.cpp:809
Style.AllowShortCaseLabelsOnASingleLine);
+IO.mapOptional("AllowShortCompoundRequirementOnASingleLine",
+   Style.AllowShortCompoundRequirementOnASingleLine);

Backl1ght wrote:
> MyDeveloperDay wrote:
> > haven't we use "Requires" in other options? What is the definition of 
> > Compound?
> > 
> > I might be tempted for this to be AllShortRequiresOnASingleLine but then it 
> > be an enum with the following options
> > 
> > ```
> > Leave
> > Never
> > Always
> > Compound
> > ```
> There are some options related to requires like IndentRequiresClause, 
> RequiresClausePosition, etc. But as for single line and brace wrapping, 
> requires is not yet considered.
> 
> "Compound" is from Compound Requirements section in this page 
> https://en.cppreference.com/w/cpp/language/requires
> 
> I think this is a better way, I would try implementing it.
A //compound-requirement// is what the standard calls it, so I think the name 
is fine.

I'm not sure if collating it with other options as suggested is a good idea, 
since a //requires-expression// can contain multiple //compound-requirement//s, 
such as:

```
requires {
  { E1 } -> C;
  { E2 } -> D;
};
```

This //requires-expression// has two //compound-requirement//s, and two 
formatting of those two individually is what's being controlled by the option.

Although I will agree it should probably still be an enum, but in that case, it 
probably shouldn't start with "Allow"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139834

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


[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-10 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

Could you please clarify what you mean by "regressions" here? Isn't the 
behaviour of this syntax broken to begin with? It doesn't change anything about 
lambdas without //requires-clause//s


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145642

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


[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-10 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

In D145642#4183539 , @owenpan wrote:

> Would this patch make the end result look worse without "making the requires 
> clause formatting of lambdas
> match the formatting for functions"? If yes, then we should not land it just 
> yet.

Depends on your definition of "worse" :)
I outlined this on the github issue 
(https://github.com/llvm/llvm-project/issues/61269), essentially:

The result for the example provided on the github issue something like this:

  [&](Callable&& callable)
requires std::is_invocable_v
  { static_cast(callable); };

Unlike function declarations, lambdas with a requires clause still have the 
body on one line. I don't yet know how to go about changing this, and **if** I 
should at all. This of course isn't an issue if the lambda body has multiple 
lines.

There is also the case of requires clauses before the parameters, which results 
in this:

  [&]
requires std::is_invocable_v
  (Callable &) { static_cast(callable); };

This poses a whole new set of problems (like, starting a line with an opening 
paren is pretty weird!)
But, this sort of syntax seems fairly obscure: I only found out about it 
reading the C++ grammar, and the clang frontend itself doesn't actually parse 
it at all in some cases (I filed 
https://github.com/llvm/llvm-project/issues/61278 for this)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145642

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


[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-09 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel marked an inline comment as done.
rymiel added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3393
+break;
+  } else {
+return;

HazardyKnusperkeks wrote:
> don't need `else` after `break`.
> In fact I would negate the condition and return, and keep the remainder out 
> of any `if` or `else`.
Thank you! I clearly wasn't thinking clearly there


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145642

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


[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-09 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 503753.
rymiel added a comment.

Improve code flow in parseConstraintExpression


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145642

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1263,6 +1263,102 @@
   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);
+
+  // Lambdas with a requires-clause
+  Tokens = annotate("[]  (T t) requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[14]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T &) requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[8], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[11], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[15]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[16], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T t) requires Foo || Bar {}");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[19]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T t) -> T requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::arrow, TT_LambdaArrow);
+  EXPECT_TOKEN(Tokens[12], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[16]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[17], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar (T t) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar (T &) {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[16], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Foo || Bar (T t) {}");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[15]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar noexcept {}");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar -> T {}");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[11], tok::arrow, TT_LambdaArrow);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_LambdaLBrace);
+
+  // 

[PATCH] D145656: [clang-format] Treat &/&& as reference when followed by requires clause

2023-03-08 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: HazardyKnusperkeks, MyDeveloperDay, owenpan.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Previously, the token annotator would get confused and annotate a member
function's ref qualifier as a binary operator, if said function also had
a requires clause after it.

This patch accounts for that, treating requires clauses more similarly
to `noexcept`, which also comes after the ref qualifier.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145656

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
@@ -744,10 +744,15 @@
 "void S::bar() const & requires Baz { }");
   ASSERT_EQ(Tokens.size(), 85u) << Tokens;
   EXPECT_TOKEN(Tokens[13], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[24], tok::amp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[25], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[35], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[36], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[47], tok::amp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[49], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[59], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[61], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[76], tok::amp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[77], tok::kw_requires, TT_RequiresClause);
 
   Tokens = annotate("void Class::member() && requires(Constant) {}");
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24148,6 +24148,14 @@
"}",
Style);
 
+  verifyFormat("template \n"
+   "int S::bar(T t) &&\n"
+   "  requires F\n"
+   "{\n"
+   "  return 5;\n"
+   "}",
+   Style);
+
   verifyFormat("template \n"
"int bar(T t)\n"
"  requires F;",
@@ -24161,6 +24169,14 @@
"}",
Style);
 
+  verifyFormat("template \n"
+   "int S::bar(T t) &&\n"
+   "requires F\n"
+   "{\n"
+   "  return 5;\n"
+   "}",
+   Style);
+
   verifyFormat("template \n"
"int bar(T t)\n"
"requires F\n"
@@ -24174,6 +24190,7 @@
"template  requires Foo void bar() {}\n"
"template  void bar() requires Foo {}\n"
"template  void bar() requires Foo;\n"
+   "template  void S::bar() && requires Foo {}\n"
"template  requires Foo Bar(T) -> Bar;",
Style);
 
@@ -24185,6 +24202,8 @@
"requires Foo void bar() {}\n"
"template \n"
"void bar() requires Foo {}\n"
+   "template \n"
+   "void S::bar() && requires Foo {}\n"
"template \n"
"requires Foo Baz(T) -> Baz;",
ColumnStyle);
@@ -24229,6 +24248,9 @@
"void bar()\n"
"requires Foo;\n"
"template \n"
+   "void S::bar() &&\n"
+   "requires Foo {}\n"
+   "template \n"
"requires Foo Bar(T) -> Bar;",
Style);
 
@@ -24259,6 +24281,9 @@
"void bar()\n"
"  requires Foo {}\n"
"template \n"
+   "void S::bar() &&\n"
+   "  requires Foo {}\n"
+   "template \n"
"  requires Foo Bar(T) -> Bar;",
Style);
 
@@ -24289,6 +24314,9 @@
"void bar() requires Foo\n"
"{}\n"
"template  void bar() requires Foo;\n"
+   "template \n"
+   "void S::bar() && requires Foo\n"
+   "{}\n"
"template  requires Foo\n"
"Bar(T) -> Bar;",
Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2400,7 +2400,7 @@
 
 if (!NextToken ||
 NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept, tok::comma,
-   tok::r_paren) ||
+   tok::r_paren, TT_RequiresClause) ||
 

[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-08 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: HazardyKnusperkeks, MyDeveloperDay, owenpan.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The C++ grammar allows lambdas to have a *requires-clause* in two
places, either directly after the *template-parameter-list*, such as:

`[]  requires foo (T t) { ... };`

Or, at the end of the *lambda-declarator* (before the lambda's body):

`[]  (T t) requires foo { ... };`

Previously, these cases weren't handled at all, resulting in weird
results.

Note that this commit only handles token annotation, so the actual
formatting still ends up suboptimal. This is mostly because I do not yet
know how to approach making the requires clause formatting of lambdas
match the formatting for functions.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145642

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1263,6 +1263,102 @@
   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);
+
+  // Lambdas with a requires-clause
+  Tokens = annotate("[]  (T t) requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[14]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T &) requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[8], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[11], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[15]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[16], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T t) requires Foo || Bar {}");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[19]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  (T t) -> T requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[10], tok::arrow, TT_LambdaArrow);
+  EXPECT_TOKEN(Tokens[12], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[16]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[17], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar (T t) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar (T &) {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[16], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Foo || Bar (T t) {}");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[15]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar {}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[10]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  requires Bar noexcept {}");
+  ASSERT_EQ(Tokens.size(), 15u) << 

[PATCH] D144709: [clang-format] Improve QualifierAlignment

2023-03-04 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/unittests/Format/QualifierFixerTest.cpp:733
+  // TODO: Something strange is going on with this formating.
+  verifyFormat("Bar < Foo, const Foo >> ;", "Bar < Foo, Foo const >> ;", 
Style);
+

AlexanderHederstaf wrote:
> rymiel wrote:
> > This... isn't valid syntax? an opening < is missing.
> > Perhaps you could adapt https://github.com/llvm/llvm-project/issues/56111 
> > into a test, it seems to be fixed by the patch, but there might still be 
> > corner-cases
> Right, I must've focused too much on the missing right >. What is the 
> procedure now that the review is accepted. Can I still upload a fix to this 
> and to include that issue in the tests?
> What is the procedure now that the review is accepted
No different to the usual! The patch will stay accepted, and reviewers can just 
re-approve anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144709

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


[PATCH] D144709: [clang-format] Improve QualifierAlignment

2023-03-04 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/unittests/Format/QualifierFixerTest.cpp:523
+  // TODO: Something strange is going on with this formating.
+  verifyFormat("Bar < Foo, Foo const >> ;", "Bar < Foo, const Foo >> ;", 
Style);
+

(see below)



Comment at: clang/unittests/Format/QualifierFixerTest.cpp:733
+  // TODO: Something strange is going on with this formating.
+  verifyFormat("Bar < Foo, const Foo >> ;", "Bar < Foo, Foo const >> ;", 
Style);
+

This... isn't valid syntax? an opening < is missing.
Perhaps you could adapt https://github.com/llvm/llvm-project/issues/56111 into 
a test, it seems to be fixed by the patch, but there might still be corner-cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144709

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


[PATCH] D144709: [clang-format] Improve QualifierAlignment

2023-03-04 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

This patch also fixes: https://github.com/llvm/llvm-project/issues/56111, 
https://github.com/llvm/llvm-project/issues/58696, and 
https://github.com/llvm/llvm-project/issues/54730 (some of those could probably 
be made duplicates but it seems they're all going down in one blow anyway)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144709

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


[PATCH] D144709: [clang-format] Improve QualifierAlignment

2023-02-28 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

In D144709#4158555 , 
@AlexanderHederstaf wrote:

> Discovered incorrect code generation for  `llvm/include/llvm/ADT/StringMap.h`.
>
>   using base = StringMapIterBase,
>  const StringMapEntry>;
>
> was converted to, removing two characters. I checked the Token length in 
> InsertQualifierAfter and found that it was not 1. How should I proceed with 
> this particular issue @MyDeveloperDay
>
>   using base = StringMapIterBase,
>  StringMapEntry const
>
> I commited the change to SetLength(1) but that looks a bit much like a hack, 
> perhaps anyone more familiar with the codebase can have a look at this issue.

Looks like https://github.com/llvm/llvm-project/issues/56111


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144709

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


[PATCH] D144537: [clang-format] Don't move qualifiers past pointers-to-member

2023-02-25 Thread Emilia Dreamer 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 rG393e197cd6eb: [clang-format] Dont move qualifiers past 
pointers-to-member (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144537

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -420,6 +420,16 @@
 
   // don't adjust macros
   verifyFormat("const INTPTR a;", "const INTPTR a;", Style);
+
+  // Pointers to members
+  verifyFormat("int S::*a;", Style);
+  verifyFormat("int const S::*a;", "const int S:: *a;", Style);
+  verifyFormat("int const S::*const a;", "const int S::* const a;", Style);
+  verifyFormat("int A::*const A::*p1;", Style);
+  verifyFormat("float (C::*p)(int);", Style);
+  verifyFormat("float (C::*const p)(int);", Style);
+  verifyFormat("float (C::*p)(int) const;", Style);
+  verifyFormat("float const (C::*p)(int);", "const float (C::*p)(int);", 
Style);
 }
 
 TEST_F(QualifierFixerTest, LeftQualifier) {
@@ -565,6 +575,16 @@
 
   // don't adjust macros
   verifyFormat("INTPTR const a;", "INTPTR const a;", Style);
+
+  // Pointers to members
+  verifyFormat("int S::*a;", Style);
+  verifyFormat("const int S::*a;", "int const S:: *a;", Style);
+  verifyFormat("const int S::*const a;", "int const S::* const a;", Style);
+  verifyFormat("int A::*const A::*p1;", Style);
+  verifyFormat("float (C::*p)(int);", Style);
+  verifyFormat("float (C::*const p)(int);", Style);
+  verifyFormat("float (C::*p)(int) const;", Style);
+  verifyFormat("const float (C::*p)(int);", "float const (C::*p)(int);", 
Style);
 }
 
 TEST_F(QualifierFixerTest, ConstVolatileQualifiersOrder) {
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -280,8 +280,11 @@
 // The case  `const Foo &&` -> `Foo const &&`
 // The case  `const std::Foo &&` -> `std::Foo const &&`
 // The case  `const std::Foo &&` -> `std::Foo const &&`
-while (Next && Next->isOneOf(tok::identifier, tok::coloncolon))
+// However,  `const Bar::*` remains the same.
+while (Next && Next->isOneOf(tok::identifier, tok::coloncolon) &&
+   !Next->startsSequence(tok::coloncolon, tok::star)) {
   Next = Next->Next;
+}
 if (Next && Next->is(TT_TemplateOpener)) {
   Next = Next->MatchingParen;
   // Move to the end of any template class members e.g.


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -420,6 +420,16 @@
 
   // don't adjust macros
   verifyFormat("const INTPTR a;", "const INTPTR a;", Style);
+
+  // Pointers to members
+  verifyFormat("int S::*a;", Style);
+  verifyFormat("int const S::*a;", "const int S:: *a;", Style);
+  verifyFormat("int const S::*const a;", "const int S::* const a;", Style);
+  verifyFormat("int A::*const A::*p1;", Style);
+  verifyFormat("float (C::*p)(int);", Style);
+  verifyFormat("float (C::*const p)(int);", Style);
+  verifyFormat("float (C::*p)(int) const;", Style);
+  verifyFormat("float const (C::*p)(int);", "const float (C::*p)(int);", Style);
 }
 
 TEST_F(QualifierFixerTest, LeftQualifier) {
@@ -565,6 +575,16 @@
 
   // don't adjust macros
   verifyFormat("INTPTR const a;", "INTPTR const a;", Style);
+
+  // Pointers to members
+  verifyFormat("int S::*a;", Style);
+  verifyFormat("const int S::*a;", "int const S:: *a;", Style);
+  verifyFormat("const int S::*const a;", "int const S::* const a;", Style);
+  verifyFormat("int A::*const A::*p1;", Style);
+  verifyFormat("float (C::*p)(int);", Style);
+  verifyFormat("float (C::*const p)(int);", Style);
+  verifyFormat("float (C::*p)(int) const;", Style);
+  verifyFormat("const float (C::*p)(int);", "float const (C::*p)(int);", Style);
 }
 
 TEST_F(QualifierFixerTest, ConstVolatileQualifiersOrder) {
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -280,8 +280,11 @@
 // The case  `const Foo &&` -> `Foo const &&`
 // The case  `const std::Foo &&` -> `std::Foo const &&`
 // The case  `const std::Foo &&` -> `std::Foo const &&`
-while (Next && Next->isOneOf(tok::identifier, tok::coloncolon))
+// However,  `const Bar::*` remains the same.
+while (Next && 

[PATCH] D144296: [clang-format] Rewrite how indent is reduced for compacted namespaces

2023-02-25 Thread Emilia Dreamer 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 rG7ba91016c6ad: [clang-format] Rewrite how indent is reduced 
for compacted namespaces (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144296

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4431,6 +4431,46 @@
"int k; }} // namespace out::mid",
Style));
 
+  verifyFormat("namespace A { namespace B { namespace C {\n"
+   "  int i;\n"
+   "}}} // namespace A::B::C\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   "namespace A { namespace B {\n"
+   "namespace C {\n"
+   "  int i;\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   Style);
+
+  verifyFormat("namespace A { namespace B { namespace C {\n"
+   "#ifdef FOO\n"
+   "  int i;\n"
+   "#endif\n"
+   "}}} // namespace A::B::C\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   "namespace A { namespace B {\n"
+   "namespace C {\n"
+   "#ifdef FOO\n"
+   "  int i;\n"
+   "#endif\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   Style);
+
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
   EXPECT_EQ("namespace out { namespace in {\n"
 "  int i;\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -60,7 +60,8 @@
 Offset = getIndentOffset(*Line.First);
 // Update the indent level cache size so that we can rely on it
 // having the right size in adjustToUnmodifiedline.
-skipLine(Line, /*UnknownIndent=*/true);
+if (Line.Level >= IndentForLevel.size())
+  IndentForLevel.resize(Line.Level + 1, -1);
 if (Style.IndentPPDirectives != FormatStyle::PPDIS_None &&
 (Line.InPPDirective ||
  (Style.IndentPPDirectives == FormatStyle::PPDIS_BeforeHash &&
@@ -81,13 +82,6 @@
   Indent = Line.Level * Style.IndentWidth + Style.ContinuationIndentWidth;
   }
 
-  /// Update the indent state given that \p Line indent should be
-  /// skipped.
-  void skipLine(const AnnotatedLine , bool UnknownIndent = false) {
-if (Line.Level >= IndentForLevel.size())
-  IndentForLevel.resize(Line.Level + 1, UnknownIndent ? -1 : Indent);
-  }
-
   /// Update the level indent to adapt to the given \p Line.
   ///
   /// When a line is not formatted, we move the subsequent lines on the same
@@ -367,20 +361,27 @@
 // instead of TheLine->First.
 
 if (Style.CompactNamespaces) {
-  if (auto nsToken = TheLine->First->getNamespaceToken()) {
-int i = 0;
-unsigned closingLine = TheLine->MatchingClosingBlockLineIndex - 1;
-for (; I + 1 + i != E &&
-   nsToken->TokenText == getNamespaceTokenText(I[i + 1]) &&
-   closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
-   I[i + 1]->Last->TotalLength < Limit;
- i++, --closingLine) {
-  // No extra indent for compacted namespaces.
-  IndentTracker.skipLine(*I[i + 1]);
-
-  Limit -= I[i + 1]->Last->TotalLength;
+  if (const auto *NSToken = TheLine->First->getNamespaceToken()) {
+int J = 1;
+assert(TheLine->MatchingClosingBlockLineIndex > 0);
+for (auto ClosingLineIndex = TheLine->MatchingClosingBlockLineIndex - 1;
+ I + J != E && NSToken->TokenText == getNamespaceTokenText(I[J]) &&
+ ClosingLineIndex == I[J]->MatchingClosingBlockLineIndex &&
+ I[J]->Last->TotalLength < Limit;
+ ++J, --ClosingLineIndex) {
+  Limit -= I[J]->Last->TotalLength;
+
+  // Reduce indent level for bodies of namespaces which were compacted,
+  // but only if their content was indented in the first place.
+  auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
+  auto OutdentBy = I[J]->Level - TheLine->Level;
+  for (auto *CompactedLine 

[PATCH] D144537: [clang-format] Don't move qualifiers past pointers-to-member

2023-02-25 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

In D144537#4145545 , @MyDeveloperDay 
wrote:

> maybe we should cherry pick into 16?

It's up to one of you (mostly because I don't know how to do backports and I 
don't want to mess it up)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144537

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


[PATCH] D144709: [clang-format] Improve west to east const

2023-02-25 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

just as a heads up, I'm applying https://reviews.llvm.org/D144537 since it's a 
quick bug fix; this patch *would* replace that one, but may need more time. 
(also conflicts may appear)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144709

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


[PATCH] D144537: [clang-format] Don't move qualifiers past pointers-to-member

2023-02-21 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: owenpan, MyDeveloperDay, HazardyKnusperkeks.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Previously, given a pointer-to-member type such as `Foo const Bar::*`,
clang-format would see the `const Bar::` part as a regular type name
with scope resolution operators, and with `QualifierAlignment: Right` it
would attempt to "fix" it, resulting in `Foo Bar::const *`, a syntax
error.

This patch no longer allows qualifiers to be moved across `::*`.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144537

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -420,6 +420,16 @@
 
   // don't adjust macros
   verifyFormat("const INTPTR a;", "const INTPTR a;", Style);
+
+  // Pointers to members
+  verifyFormat("int S::*a;", Style);
+  verifyFormat("int const S::*a;", "const int S:: *a;", Style);
+  verifyFormat("int const S::*const a;", "const int S::* const a;", Style);
+  verifyFormat("int A::*const A::*p1;", Style);
+  verifyFormat("float (C::*p)(int);", Style);
+  verifyFormat("float (C::*const p)(int);", Style);
+  verifyFormat("float (C::*p)(int) const;", Style);
+  verifyFormat("float const (C::*p)(int);", "const float (C::*p)(int);", 
Style);
 }
 
 TEST_F(QualifierFixerTest, LeftQualifier) {
@@ -565,6 +575,16 @@
 
   // don't adjust macros
   verifyFormat("INTPTR const a;", "INTPTR const a;", Style);
+
+  // Pointers to members
+  verifyFormat("int S::*a;", Style);
+  verifyFormat("const int S::*a;", "int const S:: *a;", Style);
+  verifyFormat("const int S::*const a;", "int const S::* const a;", Style);
+  verifyFormat("int A::*const A::*p1;", Style);
+  verifyFormat("float (C::*p)(int);", Style);
+  verifyFormat("float (C::*const p)(int);", Style);
+  verifyFormat("float (C::*p)(int) const;", Style);
+  verifyFormat("const float (C::*p)(int);", "float const (C::*p)(int);", 
Style);
 }
 
 TEST_F(QualifierFixerTest, ConstVolatileQualifiersOrder) {
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -280,8 +280,11 @@
 // The case  `const Foo &&` -> `Foo const &&`
 // The case  `const std::Foo &&` -> `std::Foo const &&`
 // The case  `const std::Foo &&` -> `std::Foo const &&`
-while (Next && Next->isOneOf(tok::identifier, tok::coloncolon))
+// However,  `const Bar::*` remains the same.
+while (Next && Next->isOneOf(tok::identifier, tok::coloncolon) &&
+   !Next->startsSequence(tok::coloncolon, tok::star)) {
   Next = Next->Next;
+}
 if (Next && Next->is(TT_TemplateOpener)) {
   Next = Next->MatchingParen;
   // Move to the end of any template class members e.g.


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -420,6 +420,16 @@
 
   // don't adjust macros
   verifyFormat("const INTPTR a;", "const INTPTR a;", Style);
+
+  // Pointers to members
+  verifyFormat("int S::*a;", Style);
+  verifyFormat("int const S::*a;", "const int S:: *a;", Style);
+  verifyFormat("int const S::*const a;", "const int S::* const a;", Style);
+  verifyFormat("int A::*const A::*p1;", Style);
+  verifyFormat("float (C::*p)(int);", Style);
+  verifyFormat("float (C::*const p)(int);", Style);
+  verifyFormat("float (C::*p)(int) const;", Style);
+  verifyFormat("float const (C::*p)(int);", "const float (C::*p)(int);", Style);
 }
 
 TEST_F(QualifierFixerTest, LeftQualifier) {
@@ -565,6 +575,16 @@
 
   // don't adjust macros
   verifyFormat("INTPTR const a;", "INTPTR const a;", Style);
+
+  // Pointers to members
+  verifyFormat("int S::*a;", Style);
+  verifyFormat("const int S::*a;", "int const S:: *a;", Style);
+  verifyFormat("const int S::*const a;", "int const S::* const a;", Style);
+  verifyFormat("int A::*const A::*p1;", Style);
+  verifyFormat("float (C::*p)(int);", Style);
+  verifyFormat("float (C::*const p)(int);", Style);
+  verifyFormat("float (C::*p)(int) const;", Style);
+  verifyFormat("const float (C::*p)(int);", "float const (C::*p)(int);", Style);
 }
 
 TEST_F(QualifierFixerTest, ConstVolatileQualifiersOrder) {
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ 

[PATCH] D144296: [clang-format] Rewrite how indent is reduced for compacted namespaces

2023-02-21 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 499330.
rymiel added a comment.

Inline and remove LevelIndentTracker::skipLine


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144296

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4431,6 +4431,46 @@
"int k; }} // namespace out::mid",
Style));
 
+  verifyFormat("namespace A { namespace B { namespace C {\n"
+   "  int i;\n"
+   "}}} // namespace A::B::C\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   "namespace A { namespace B {\n"
+   "namespace C {\n"
+   "  int i;\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   Style);
+
+  verifyFormat("namespace A { namespace B { namespace C {\n"
+   "#ifdef FOO\n"
+   "  int i;\n"
+   "#endif\n"
+   "}}} // namespace A::B::C\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   "namespace A { namespace B {\n"
+   "namespace C {\n"
+   "#ifdef FOO\n"
+   "  int i;\n"
+   "#endif\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   Style);
+
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
   EXPECT_EQ("namespace out { namespace in {\n"
 "  int i;\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -59,7 +59,8 @@
 Offset = getIndentOffset(*Line.First);
 // Update the indent level cache size so that we can rely on it
 // having the right size in adjustToUnmodifiedline.
-skipLine(Line, /*UnknownIndent=*/true);
+if (Line.Level >= IndentForLevel.size())
+  IndentForLevel.resize(Line.Level + 1, -1);
 if (Style.IndentPPDirectives != FormatStyle::PPDIS_None &&
 (Line.InPPDirective ||
  (Style.IndentPPDirectives == FormatStyle::PPDIS_BeforeHash &&
@@ -80,13 +81,6 @@
   Indent = Line.Level * Style.IndentWidth + Style.ContinuationIndentWidth;
   }
 
-  /// Update the indent state given that \p Line indent should be
-  /// skipped.
-  void skipLine(const AnnotatedLine , bool UnknownIndent = false) {
-if (Line.Level >= IndentForLevel.size())
-  IndentForLevel.resize(Line.Level + 1, UnknownIndent ? -1 : Indent);
-  }
-
   /// Update the level indent to adapt to the given \p Line.
   ///
   /// When a line is not formatted, we move the subsequent lines on the same
@@ -366,20 +360,27 @@
 // instead of TheLine->First.
 
 if (Style.CompactNamespaces) {
-  if (auto nsToken = TheLine->First->getNamespaceToken()) {
-int i = 0;
-unsigned closingLine = TheLine->MatchingClosingBlockLineIndex - 1;
-for (; I + 1 + i != E &&
-   nsToken->TokenText == getNamespaceTokenText(I[i + 1]) &&
-   closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
-   I[i + 1]->Last->TotalLength < Limit;
- i++, --closingLine) {
-  // No extra indent for compacted namespaces.
-  IndentTracker.skipLine(*I[i + 1]);
+  if (const auto *NSToken = TheLine->First->getNamespaceToken()) {
+int J = 1;
+assert(TheLine->MatchingClosingBlockLineIndex > 0);
+for (auto ClosingLineIndex = TheLine->MatchingClosingBlockLineIndex - 1;
+ I + J != E && NSToken->TokenText == getNamespaceTokenText(I[J]) &&
+ ClosingLineIndex == I[J]->MatchingClosingBlockLineIndex &&
+ I[J]->Last->TotalLength < Limit;
+ ++J, --ClosingLineIndex) {
+  Limit -= I[J]->Last->TotalLength;
 
-  Limit -= I[i + 1]->Last->TotalLength;
+  // Reduce indent level for bodies of namespaces which were compacted,
+  // but only if their content was indented in the first place.
+  auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
+  auto OutdentBy = I[J]->Level - TheLine->Level;
+  for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
+   ++CompactedLine) {
+if (!(*CompactedLine)->InPPDirective)
+  

[PATCH] D144296: [clang-format] Rewrite how indent is reduced for compacted namespaces

2023-02-21 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:387
+if (!(*compactedLine)->InPPDirective)
+  (*compactedLine)->Level-= dedentBy;
+  }

owenpan wrote:
> Did git-clang-format miss this?
Nope, I forgot to amend the change!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144296

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


[PATCH] D144296: [clang-format] Rewrite how indent is reduced for compacted namespaces

2023-02-21 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 499328.
rymiel marked 7 inline comments as done.
rymiel added a comment.

Apply suggestions and add extra test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144296

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4431,6 +4431,46 @@
"int k; }} // namespace out::mid",
Style));
 
+  verifyFormat("namespace A { namespace B { namespace C {\n"
+   "  int i;\n"
+   "}}} // namespace A::B::C\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   "namespace A { namespace B {\n"
+   "namespace C {\n"
+   "  int i;\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   Style);
+
+  verifyFormat("namespace A { namespace B { namespace C {\n"
+   "#ifdef FOO\n"
+   "  int i;\n"
+   "#endif\n"
+   "}}} // namespace A::B::C\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   "namespace A { namespace B {\n"
+   "namespace C {\n"
+   "#ifdef FOO\n"
+   "  int i;\n"
+   "#endif\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   Style);
+
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
   EXPECT_EQ("namespace out { namespace in {\n"
 "  int i;\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -366,20 +366,27 @@
 // instead of TheLine->First.
 
 if (Style.CompactNamespaces) {
-  if (auto nsToken = TheLine->First->getNamespaceToken()) {
-int i = 0;
-unsigned closingLine = TheLine->MatchingClosingBlockLineIndex - 1;
-for (; I + 1 + i != E &&
-   nsToken->TokenText == getNamespaceTokenText(I[i + 1]) &&
-   closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
-   I[i + 1]->Last->TotalLength < Limit;
- i++, --closingLine) {
-  // No extra indent for compacted namespaces.
-  IndentTracker.skipLine(*I[i + 1]);
+  if (const auto *NSToken = TheLine->First->getNamespaceToken()) {
+int J = 1;
+assert(TheLine->MatchingClosingBlockLineIndex > 0);
+for (auto ClosingLineIndex = TheLine->MatchingClosingBlockLineIndex - 
1;
+ I + J != E && NSToken->TokenText == getNamespaceTokenText(I[J]) &&
+ ClosingLineIndex == I[J]->MatchingClosingBlockLineIndex &&
+ I[J]->Last->TotalLength < Limit;
+ ++J, --ClosingLineIndex) {
+  Limit -= I[J]->Last->TotalLength;
 
-  Limit -= I[i + 1]->Last->TotalLength;
+  // Reduce indent level for bodies of namespaces which were compacted,
+  // but only if their content was indented in the first place.
+  auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
+  auto OutdentBy = I[J]->Level - TheLine->Level;
+  for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
+   ++CompactedLine) {
+if (!(*CompactedLine)->InPPDirective)
+  (*CompactedLine)->Level -= OutdentBy;
+  }
 }
-return i;
+return J - 1;
   }
 
   if (auto nsToken = getMatchingNamespaceToken(TheLine, AnnotatedLines)) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4431,6 +4431,46 @@
"int k; }} // namespace out::mid",
Style));
 
+  verifyFormat("namespace A { namespace B { namespace C {\n"
+   "  int i;\n"
+   "}}} // namespace A::B::C\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   "namespace A { namespace B {\n"
+   "namespace C {\n"
+   "  int i;\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n"
+   "int main() {\n"
+   

[PATCH] D144296: [clang-format] Rewrite how indent is reduced for compacted namespaces

2023-02-17 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

Note: this patch also makes `LevelIndentTracker::skipLine` obsolete. I suppose 
I could clean that up in a following patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144296

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


[PATCH] D144296: [clang-format] Rewrite how indent is reduced for compacted namespaces

2023-02-17 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added a project: clang-format.
rymiel added reviewers: owenpan, MyDeveloperDay, HazardyKnusperkeks.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The previous version set the indentation directly using IndentForLevel,
however, this has a few caveats, namely:

- IndentForLevel applies to all scopes of the entire program being

formatted, but this indentation should only be adjusted for scopes of
namespaces.

- The method it used only set the correct indent amount if one wasn't

already set for a given level, meaning it didn't work correctly if
anything with indentation preceded a namespace keyword. This includes
preprocessing directives if using IndentPPDirectives.

This patch instead reduces the Level of all lines within namespaces
which are compacted by CompactNamespaces. This means they will get
correct indentation using the normal process.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144296

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4431,6 +4431,24 @@
"int k; }} // namespace out::mid",
Style));
 
+  verifyFormat("namespace A { namespace B { namespace C {\n"
+   "  int i;\n"
+   "}}} // namespace A::B::C\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   "namespace A { namespace B {\n"
+   "namespace C {\n"
+   "  int i;\n"
+   "}} // namespace B::C\n"
+   "} // namespace A\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   Style);
+
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
   EXPECT_EQ("namespace out { namespace in {\n"
 "  int i;\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -366,20 +366,28 @@
 // instead of TheLine->First.
 
 if (Style.CompactNamespaces) {
-  if (auto nsToken = TheLine->First->getNamespaceToken()) {
-int i = 0;
-unsigned closingLine = TheLine->MatchingClosingBlockLineIndex - 1;
-for (; I + 1 + i != E &&
-   nsToken->TokenText == getNamespaceTokenText(I[i + 1]) &&
-   closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
-   I[i + 1]->Last->TotalLength < Limit;
- i++, --closingLine) {
-  // No extra indent for compacted namespaces.
-  IndentTracker.skipLine(*I[i + 1]);
+  if (auto *nsToken = TheLine->First->getNamespaceToken()) {
+int j = 1;
+unsigned closingLineIndex = TheLine->MatchingClosingBlockLineIndex - 1;
 
-  Limit -= I[i + 1]->Last->TotalLength;
+for (; I + j != E &&
+   nsToken->TokenText == getNamespaceTokenText(I[j]) &&
+   closingLineIndex == I[j]->MatchingClosingBlockLineIndex &&
+   I[j]->Last->TotalLength < Limit;
+ j++, --closingLineIndex) {
+  Limit -= I[j]->Last->TotalLength;
+
+  // Reduce indent level for bodies of namespaces which were compacted,
+  // but only if their content was indented in the first place
+  auto *closingLine = AnnotatedLines.begin() + closingLineIndex + 1;
+  auto dedentBy = I[j]->Level - TheLine->Level;
+  for (auto *compactedLine = I + j; compactedLine <= closingLine;
+   compactedLine++) {
+if (!(*compactedLine)->InPPDirective)
+  (*compactedLine)->Level-= dedentBy;
+  }
 }
-return i;
+return j - 1;
   }
 
   if (auto nsToken = getMatchingNamespaceToken(TheLine, AnnotatedLines)) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4431,6 +4431,24 @@
"int k; }} // namespace out::mid",
Style));
 
+  verifyFormat("namespace A { namespace B { namespace C {\n"
+   "  int i;\n"
+   "}}} // namespace A::B::C\n"
+   "int main() {\n"
+   "  if (true)\n"
+   "return 0;\n"
+   "}",
+   "namespace A { namespace B {\n"
+   "namespace C {\n"
+   "  int i;\n"
+   "}} // namespace B::C\n"
+   

[PATCH] D143546: [clang-format] Insert a space between a numeric UDL and a dot

2023-02-08 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

In D143546#4112077 , @owenpan wrote:

> As this one is an invalid-code-generation bug, I wanted it fixed ASAP.

Do you intend to backport it to the 16 release branch then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143546

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


[PATCH] D143546: [clang-format] Insert a space between a numeric UDL and a dot

2023-02-07 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel accepted this revision.
rymiel added a comment.
This revision is now accepted and ready to land.

I've been sniped! (but only because i spent hours discussing with people if the 
error is pedantically valid, which it turns out it is)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143546

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


[PATCH] D142804: [clang-format] Support clang-format on/off line comments as prefixes

2023-01-28 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/lib/Format/Format.cpp:3901
+  return Comment.startswith(Prefix) &&
+ (Comment.size() == Size || isblank(Comment[Size]));
+}

Should the space be required? What about `// clang-format off: reasoning` or 
similar?

Also, should this be documented?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142804

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


[PATCH] D142139: [clang-format] Disallow templates to be followed by literal

2023-01-26 Thread Emilia Dreamer 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 rG8a3de13573bd: [clang-format] Disallow templates to be 
followed by literal (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142139

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10365,6 +10365,7 @@
 
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
+  verifyFormat("a < 0 ? b : a > 0 ? c : d;");
   verifyFormat("void f() {\n"
"  while (a < b && c > d) {\n"
"  }\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -185,6 +185,8 @@
 } else {
   CurrentToken->setType(TT_TemplateCloser);
 }
+if (CurrentToken->Next && CurrentToken->Next->Tok.isLiteral())
+  return false;
 next();
 return true;
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10365,6 +10365,7 @@
 
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
+  verifyFormat("a < 0 ? b : a > 0 ? c : d;");
   verifyFormat("void f() {\n"
"  while (a < b && c > d) {\n"
"  }\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -185,6 +185,8 @@
 } else {
   CurrentToken->setType(TT_TemplateCloser);
 }
+if (CurrentToken->Next && CurrentToken->Next->Tok.isLiteral())
+  return false;
 next();
 return true;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D142578: [Clang][Doc] Edit the Clang release notes

2023-01-26 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

I think it's worthwhile to mark comments with the "Done" checkmark in 
Phrabricator if they have been addressed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142578

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


[PATCH] D142578: [Clang][Doc] Edit the Clang release notes

2023-01-25 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

I don't know sphinx enough to know if the trailing underscore is needed in 
links, but there's about 5 links that don't have the underscore (Search for

  >`)

)




Comment at: clang/docs/ReleaseNotes.rst:577
+  potential false positives, this diagnostic will not diagnose use of the
+  ``true`` macro (from ``>``) in C language mode despite the macro
+  being defined to expand to ``1``. (`#53253 
`_)





Comment at: clang/docs/ReleaseNotes.rst:587
 
-C2x Feature Support

-- Implemented `WG14 N2662 
`_,
-  so the [[maybe_unused]] attribute may be applied to a label to silence an
-  ``-Wunused-label`` warning.
-- Implemented `WG14 N2508 
`_,
-  so labels can placed everywhere inside a compound statement.
-- Implemented `WG14 N2927 
`_,
-  the Not-so-magic ``typeof`` operator. Also implemented
-  `WG14 N2930 `_,
-  renaming ``remove_quals``, so the ``typeof_unqual`` operator is also
-  supported. Both of these operators are supported only in C2x mode. The
-  ``typeof`` operator specifies the type of the given parenthesized expression
-  operand or type name, including all qualifiers. The ``typeof_unqual``
-  operator is similar to ``typeof`` except that all qualifiers are removed,
-  including atomic type qualification and type attributes which behave like a
-  qualifier, such as an address space attribute.
+- Correctly diagnose a future keyword if it exist as a keyword in the higher
+  language version and specifies in which version it will be a keyword. This





Comment at: clang/docs/ReleaseNotes.rst:589
+  language version and specifies in which version it will be a keyword. This
+  supports both c and c++ language.
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142578

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


[PATCH] D142412: [clang-format] Put peekNextToken(/*SkipComment=*/true) to good use

2023-01-24 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3592-3601
 case tok::kw_bool:
   // bool is only allowed if it is directly followed by a paren for a cast:
   // concept C = bool(...);
   // and bool is the only type, all other types as cast must be inside a
   // cast to bool an thus are handled by the other cases.
   if (Tokens->peekNextToken()->isNot(tok::l_paren))
 return;

owenpan wrote:
> owenpan wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > rymiel wrote:
> > > > > owenpan wrote:
> > > > > > rymiel wrote:
> > > > > > > rymiel wrote:
> > > > > > > > owenpan wrote:
> > > > > > > > > We should pass `true` to `peekNextToken()` on Line 3597 here, 
> > > > > > > > > but removing this entire case would have no effect on the 
> > > > > > > > > existing unit test (Line 23693 in FormatTest.cpp). 
> > > > > > > > > @HazardyKnusperkeks any idea?
> > > > > > > > The peeking is there because of 
> > > > > > > > https://reviews.llvm.org/D134325, it has no format tests, only 
> > > > > > > > an annotator test
> > > > > > > Also, I don't think that function is involved in concept 
> > > > > > > declarations since https://reviews.llvm.org/D140339
> > > > > > So which tests were meant to check the `kw_bool` case here?
> > > > > Well, originally the one you mentioned on line 23693, but since the 
> > > > > code paths have been changed, I think it's really only the one added 
> > > > > in https://reviews.llvm.org/D134325
> > > > I love the new annotator tests, but we should probably always try to 
> > > > add a verifyFormat test as well even if its trivial in the future, just 
> > > > so we have some context as to what type of code warranted the change.
> > > As far as I can see it does not only can be removed, it should be removed.
> > > 
> > > `bool` can not appear on the top level of a requires clause. As @rymiel 
> > > said it is there for the concepts, which are now handled differently.
> > @rymiel @HazardyKnusperkeks thanks for the explanations. Can we remove the 
> > `kw_bool` case then?
> > As far as I can see it does not only can be removed, it should be removed.
> 
> Okay, I will leave it to you or @rymiel then. :)
> As far as I can see it does not only can be removed, it should be removed.

You are right! I forgot the fact that this only ever applied for concepts, and 
the bug fix only ever worked around that. I had it backwards in my head because 
the bug affected requires clauses too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142412

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


[PATCH] D142412: [clang-format] Put peekNextToken(/*SkipComment=*/true) to good use

2023-01-23 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3592-3601
 case tok::kw_bool:
   // bool is only allowed if it is directly followed by a paren for a cast:
   // concept C = bool(...);
   // and bool is the only type, all other types as cast must be inside a
   // cast to bool an thus are handled by the other cases.
   if (Tokens->peekNextToken()->isNot(tok::l_paren))
 return;

owenpan wrote:
> rymiel wrote:
> > rymiel wrote:
> > > owenpan wrote:
> > > > We should pass `true` to `peekNextToken()` on Line 3597 here, but 
> > > > removing this entire case would have no effect on the existing unit 
> > > > test (Line 23693 in FormatTest.cpp). @HazardyKnusperkeks any idea?
> > > The peeking is there because of https://reviews.llvm.org/D134325, it has 
> > > no format tests, only an annotator test
> > Also, I don't think that function is involved in concept declarations since 
> > https://reviews.llvm.org/D140339
> So which tests were meant to check the `kw_bool` case here?
Well, originally the one you mentioned on line 23693, but since the code paths 
have been changed, I think it's really only the one added in 
https://reviews.llvm.org/D134325


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142412

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


[PATCH] D142412: [clang-format] Put peekNextToken(/*SkipComment=*/true) to good use

2023-01-23 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3592-3601
 case tok::kw_bool:
   // bool is only allowed if it is directly followed by a paren for a cast:
   // concept C = bool(...);
   // and bool is the only type, all other types as cast must be inside a
   // cast to bool an thus are handled by the other cases.
   if (Tokens->peekNextToken()->isNot(tok::l_paren))
 return;

rymiel wrote:
> owenpan wrote:
> > We should pass `true` to `peekNextToken()` on Line 3597 here, but removing 
> > this entire case would have no effect on the existing unit test (Line 23693 
> > in FormatTest.cpp). @HazardyKnusperkeks any idea?
> The peeking is there because of https://reviews.llvm.org/D134325, it has no 
> format tests, only an annotator test
Also, I don't think that function is involved in concept declarations since 
https://reviews.llvm.org/D140339


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142412

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


[PATCH] D142412: [clang-format] Put peekNextToken(/*SkipComment=*/true) to good use

2023-01-23 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3592-3601
 case tok::kw_bool:
   // bool is only allowed if it is directly followed by a paren for a cast:
   // concept C = bool(...);
   // and bool is the only type, all other types as cast must be inside a
   // cast to bool an thus are handled by the other cases.
   if (Tokens->peekNextToken()->isNot(tok::l_paren))
 return;

owenpan wrote:
> We should pass `true` to `peekNextToken()` on Line 3597 here, but removing 
> this entire case would have no effect on the existing unit test (Line 23693 
> in FormatTest.cpp). @HazardyKnusperkeks any idea?
The peeking is there because of https://reviews.llvm.org/D134325, it has no 
format tests, only an annotator test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142412

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


[PATCH] D141811: [clang-format] Allow trailing return types in macros

2023-01-20 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

In D141811#4068180 , 
@HazardyKnusperkeks wrote:

> I thought about `auto NAME() -> int { return 42; }`.
>
> `decltype(auto) a = (b) -> c;` is something else...

I'm not sure how that affects this? Using `decltype(auto)` as in 
`decltype(auto) NAME() -> int { return 42; }` will recognize the arrow as a 
trailing return type, but it doesn't really matter as its invalid syntax. I 
tried to find valid syntax where the arrow would be seen as a trailing return 
type, and I did with that second example


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141811

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


[PATCH] D142139: [clang-format] Disallow templates to be followed by literal

2023-01-19 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 490584.
rymiel added a comment.

Drop unrelated change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142139

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10345,6 +10345,7 @@
 
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
+  verifyFormat("a < 0 ? b : a > 0 ? c : d;");
   verifyFormat("void f() {\n"
"  while (a < b && c > d) {\n"
"  }\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -185,6 +185,8 @@
 } else {
   CurrentToken->setType(TT_TemplateCloser);
 }
+if (CurrentToken->Next && CurrentToken->Next->Tok.isLiteral())
+  return false;
 next();
 return true;
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10345,6 +10345,7 @@
 
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
+  verifyFormat("a < 0 ? b : a > 0 ? c : d;");
   verifyFormat("void f() {\n"
"  while (a < b && c > d) {\n"
"  }\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -185,6 +185,8 @@
 } else {
   CurrentToken->setType(TT_TemplateCloser);
 }
+if (CurrentToken->Next && CurrentToken->Next->Tok.isLiteral())
+  return false;
 next();
 return true;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D142139: [clang-format] Disallow templates to be followed by literal

2023-01-19 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
rymiel added a project: clang-format.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There should not be any cases where the angle brackets of template
parameters are directly followed by a literal. It is more likely that a
comparison is taking place instead.

This patch makes the TokenAnnotator prefer to annotate < and > as
operators when directly followed by a literal. A similar check already
exists for literals directly *before* potential template args.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142139

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10345,6 +10345,7 @@
 
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
+  verifyFormat("a < 0 ? b : a > 0 ? c : d;");
   verifyFormat("void f() {\n"
"  while (a < b && c > d) {\n"
"  }\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -125,19 +125,18 @@
 if (NonTemplateLess.count(CurrentToken->Previous))
   return false;
 
-const FormatToken  = *CurrentToken->Previous; // The '<'.
-if (Previous.Previous) {
-  if (Previous.Previous->Tok.isLiteral())
+FormatToken *Left = CurrentToken->Previous; // The '<'.
+if (Left->Previous) {
+  if (Left->Previous->Tok.isLiteral())
 return false;
-  if (Previous.Previous->is(tok::r_paren) && Contexts.size() > 1 &&
-  (!Previous.Previous->MatchingParen ||
-   !Previous.Previous->MatchingParen->is(
+  if (Left->Previous->is(tok::r_paren) && Contexts.size() > 1 &&
+  (!Left->Previous->MatchingParen ||
+   !Left->Previous->MatchingParen->is(
TT_OverloadedOperatorLParen))) {
 return false;
   }
 }
 
-FormatToken *Left = CurrentToken->Previous;
 Left->ParentBracket = Contexts.back().ContextKind;
 ScopedContextCreator ContextCreator(*this, tok::less, 12);
 
@@ -185,6 +184,8 @@
 } else {
   CurrentToken->setType(TT_TemplateCloser);
 }
+if (CurrentToken->Next && CurrentToken->Next->Tok.isLiteral())
+  return false;
 next();
 return true;
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10345,6 +10345,7 @@
 
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
+  verifyFormat("a < 0 ? b : a > 0 ? c : d;");
   verifyFormat("void f() {\n"
"  while (a < b && c > d) {\n"
"  }\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -125,19 +125,18 @@
 if (NonTemplateLess.count(CurrentToken->Previous))
   return false;
 
-const FormatToken  = *CurrentToken->Previous; // The '<'.
-if (Previous.Previous) {
-  if (Previous.Previous->Tok.isLiteral())
+FormatToken *Left = CurrentToken->Previous; // The '<'.
+if (Left->Previous) {
+  if (Left->Previous->Tok.isLiteral())
 return false;
-  if (Previous.Previous->is(tok::r_paren) && Contexts.size() > 1 &&
-  (!Previous.Previous->MatchingParen ||
-   !Previous.Previous->MatchingParen->is(
+  if (Left->Previous->is(tok::r_paren) && Contexts.size() > 1 &&
+  (!Left->Previous->MatchingParen ||
+   !Left->Previous->MatchingParen->is(
TT_OverloadedOperatorLParen))) {
 return false;
   }
 }
 
-FormatToken *Left = CurrentToken->Previous;
 Left->ParentBracket = Contexts.back().ContextKind;
 ScopedContextCreator ContextCreator(*this, tok::less, 12);
 
@@ -185,6 +184,8 @@
 } else {
   CurrentToken->setType(TT_TemplateCloser);
 }
+if (CurrentToken->Next && CurrentToken->Next->Tok.isLiteral())
+  return false;
 next();
 return true;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D141811: [clang-format] Allow trailing return types in macros

2023-01-18 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

I suppose this means `auto a = (b) -> c;` is also technically broken, actually, 
and there's no easy fix for that.

I did have an idea for adding an additional pass taking place somewhere in 
`TokenAnnotator::calculateFormattingInformation`, which would detect trailing 
return types using a bit more context, as a potential fix for 
https://github.com/llvm/llvm-project/issues/41495. I suppose I could make that 
theoretically replace all of this AutoFound logic. I should actually try to see 
if my idea would work at all though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141811

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


[PATCH] D141811: [clang-format] Allow trailing return types in macros

2023-01-18 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel planned changes to this revision.
rymiel added a comment.

In D141811#4055485 , 
@HazardyKnusperkeks wrote:

> What about `decltype(auto)`?

Turns out this is a problem even without this patch:

`decltype(auto) a = (b) -> c;`

I'm sure this could somehow be valid c++ syntax, but it does have to be at the 
top scope. I suppose I could fix it in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141811

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


[PATCH] D141811: [clang-format] Allow trailing return types in macros

2023-01-15 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
rymiel added a project: clang-format.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The trailing return type arrow checker verifies that a declaration is
being parsed, however, this isn't true when inside of macros.

It turns out the existence of the auto keyword is enough to make
sure that we're dealing with a trailing return type, and whether we're
in a declaration doesn't matter.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141811

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7986,6 +7986,10 @@
"auto aa(T t)\n"
"-> decltype(eaaa(t.a).());");
 
+  verifyFormat("#define MAKE_DEF(NAME) 
"
+   "\\\n"
+   "  auto NAME() -> int { return 42; }");
+
   // Not trailing return types.
   verifyFormat("void f() { auto a = b->c(); }");
   verifyFormat("auto a = p->foo();");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1880,7 +1880,8 @@
 } else if (Current.is(tok::arrow) &&
Style.Language == FormatStyle::LK_Java) {
   Current.setType(TT_LambdaArrow);
-} else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
+} else if (Current.is(tok::arrow) && AutoFound &&
+   (Line.MustBeDeclaration || Line.InPPDirective) &&
Current.NestingLevel == 0 &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7986,6 +7986,10 @@
"auto aa(T t)\n"
"-> decltype(eaaa(t.a).());");
 
+  verifyFormat("#define MAKE_DEF(NAME) "
+   "\\\n"
+   "  auto NAME() -> int { return 42; }");
+
   // Not trailing return types.
   verifyFormat("void f() { auto a = b->c(); }");
   verifyFormat("auto a = p->foo();");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1880,7 +1880,8 @@
 } else if (Current.is(tok::arrow) &&
Style.Language == FormatStyle::LK_Java) {
   Current.setType(TT_LambdaArrow);
-} else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
+} else if (Current.is(tok::arrow) && AutoFound &&
+   (Line.MustBeDeclaration || Line.InPPDirective) &&
Current.NestingLevel == 0 &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D141654: [clang-format] Replace DeriveLineEnding and UseCRLF with LineEnding

2023-01-13 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

This also made me wonder, is there an actual policy on deprecated options? As 
in, when they are actually removed. Is there even prior precedent for doing 
this? (I wouldn't know because I'm a very recent user)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141654

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


[PATCH] D141654: [clang-format] Replace DeriveLineEnding and UseCRLF with LineEnding

2023-01-13 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel accepted this revision.
rymiel added a comment.
This revision is now accepted and ready to land.

(This needs to run the updated dump script from D138446 
)

I'm not sure what the strict //benefit// of squishing the two options together 
is, but I support it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141654

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


[PATCH] D138446: [clang-format][docs] Add ability to link to specific config options

2023-01-12 Thread Emilia Dreamer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9cddd7a2a1d2: [clang-format][docs] Add ability to link to 
specific config options (authored by rymiel).

Changed prior to commit:
  https://reviews.llvm.org/D138446?vs=476929=488720#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138446

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/tools/dump_format_style.py

Index: clang/docs/tools/dump_format_style.py
===
--- clang/docs/tools/dump_format_style.py
+++ clang/docs/tools/dump_format_style.py
@@ -98,12 +98,10 @@
 self.version = version
 
   def __str__(self):
+s = ".. _%s:\n\n**%s** (``%s``) " % (self.name, self.name, to_yaml_type(self.type))
 if self.version:
-  s = '**%s** (``%s``) :versionbadge:`clang-format %s`\n%s' % (self.name, to_yaml_type(self.type), self.version,
- doxygen2rst(indent(self.comment, 2)))
-else:
-  s = '**%s** (``%s``)\n%s' % (self.name, to_yaml_type(self.type),
- doxygen2rst(indent(self.comment, 2)))
+  s += ':versionbadge:`clang-format %s` ' % self.version
+s += ':ref:`¶ <%s>`\n%s' % (self.name, doxygen2rst(indent(self.comment, 2)))
 if self.enum and self.enum.values:
   s += indent('\n\nPossible values:\n\n%s\n' % self.enum, 2)
 if self.nested_struct:
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -142,8 +142,9 @@
 enumeration member (with a prefix, e.g. ``LS_Auto``), and as a value usable in
 the configuration (without a prefix: ``Auto``).
 
+.. _BasedOnStyle:
 
-**BasedOnStyle** (``String``)
+**BasedOnStyle** (``String``) :ref:`¶ `
   The style used for all options not specifically set in the configuration.
 
   This option is supported only in the :program:`clang-format` configuration
@@ -184,10 +185,14 @@
 
 .. START_FORMAT_STYLE_OPTIONS
 
-**AccessModifierOffset** (``Integer``) :versionbadge:`clang-format 3.3`
+.. _AccessModifierOffset:
+
+**AccessModifierOffset** (``Integer``) :versionbadge:`clang-format 3.3` :ref:`¶ `
   The extra indent or outdent of access modifiers, e.g. ``public:``.
 
-**AlignAfterOpenBracket** (``BracketAlignmentStyle``) :versionbadge:`clang-format 3.8`
+.. _AlignAfterOpenBracket:
+
+**AlignAfterOpenBracket** (``BracketAlignmentStyle``) :versionbadge:`clang-format 3.8` :ref:`¶ `
   If ``true``, horizontally aligns arguments after an open bracket.
 
   This applies to round brackets (parentheses), angle brackets and square
@@ -238,7 +243,9 @@
 
 
 
-**AlignArrayOfStructures** (``ArrayInitializerAlignmentStyle``) :versionbadge:`clang-format 13`
+.. _AlignArrayOfStructures:
+
+**AlignArrayOfStructures** (``ArrayInitializerAlignmentStyle``) :versionbadge:`clang-format 13` :ref:`¶ `
   if not ``None``, when using initialization for an array of structs
   aligns the fields into columns.
 
@@ -276,7 +283,9 @@
 
 
 
-**AlignConsecutiveAssignments** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 3.8`
+.. _AlignConsecutiveAssignments:
+
+**AlignConsecutiveAssignments** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 3.8` :ref:`¶ `
   Style of aligning consecutive assignments.
 
   ``Consecutive`` will result in formattings like:
@@ -398,7 +407,9 @@
   bbb >>= 2;
 
 
-**AlignConsecutiveBitFields** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 11`
+.. _AlignConsecutiveBitFields:
+
+**AlignConsecutiveBitFields** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 11` :ref:`¶ `
   Style of aligning consecutive bit fields.
 
   ``Consecutive`` will align the bitfield separators of consecutive lines.
@@ -521,7 +532,9 @@
   bbb >>= 2;
 
 
-**AlignConsecutiveDeclarations** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 3.8`
+.. _AlignConsecutiveDeclarations:
+
+**AlignConsecutiveDeclarations** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 3.8` :ref:`¶ `
   Style of aligning consecutive declarations.
 
   ``Consecutive`` will align the declaration names of consecutive lines.
@@ -644,7 +657,9 @@
   bbb >>= 2;
 
 
-**AlignConsecutiveMacros** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 9`
+.. _AlignConsecutiveMacros:
+
+**AlignConsecutiveMacros** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 9` :ref:`¶ `
   Style of aligning consecutive macro definitions.
 
   ``Consecutive`` will result in formattings like:
@@ -768,7 +783,9 @@
   bbb >>= 2;
 
 
-**AlignEscapedNewlines** (``EscapedNewlineAlignmentStyle``) :versionbadge:`clang-format 5`
+.. _AlignEscapedNewlines:
+
+**AlignEscapedNewlines** (``EscapedNewlineAlignmentStyle``) :versionbadge:`clang-format 5` :ref:`¶ `
   Options for aligning backslashes in escaped 

[PATCH] D141288: [clang-format] Inherit RightAlign options across scopes

2023-01-10 Thread Emilia Dreamer 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 rG51ba660a0700: [clang-format] Inherit RightAlign options 
across scopes (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141288

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
@@ -17693,6 +17693,20 @@
"dvsdsv<<= 5;\n"
"int dsvvdvsdvvv = 123;",
Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx = 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy = 6;\n"
+   "}",
+   Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx+= 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy+= 6;\n"
+   "}",
+   Alignment);
   // Test that `<=` is not treated as a compound assignment.
   verifyFormat("aa &= 5;\n"
"b <= 10;\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -609,7 +609,8 @@
   ++CommasBeforeMatch;
 } else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
   // Call AlignTokens recursively, skipping over this scope block.
-  unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i, ACS);
+  unsigned StoppedAt =
+  AlignTokens(Style, Matches, Changes, i, ACS, RightJustify);
   i = StoppedAt - 1;
   continue;
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17693,6 +17693,20 @@
"dvsdsv<<= 5;\n"
"int dsvvdvsdvvv = 123;",
Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx = 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy = 6;\n"
+   "}",
+   Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx+= 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy+= 6;\n"
+   "}",
+   Alignment);
   // Test that `<=` is not treated as a compound assignment.
   verifyFormat("aa &= 5;\n"
"b <= 10;\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -609,7 +609,8 @@
   ++CommasBeforeMatch;
 } else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
   // Call AlignTokens recursively, skipping over this scope block.
-  unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i, ACS);
+  unsigned StoppedAt =
+  AlignTokens(Style, Matches, Changes, i, ACS, RightJustify);
   i = StoppedAt - 1;
   continue;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D139211: [clang-format] Properly handle the C11 _Generic keyword.

2023-01-10 Thread Emilia Dreamer 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 rG0904e0bac831: [clang-format] Properly handle the C11 
_Generic keyword. (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139211

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/FormatToken.h
  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
@@ -1152,6 +1152,14 @@
   EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
+  auto Tokens = annotate("_Generic(x, int: 1, default: 0)");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw__Generic, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::colon, TT_GenericSelectionColon);
+  EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
+}
+
 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
@@ -23012,6 +23012,41 @@
   verifyFormat("x = (_Atomic( uint64_t ))", Style);
 }
 
+TEST_F(FormatTest, C11Generic) {
+  verifyFormat("_Generic(x, int: 1, default: 0)");
+  verifyFormat("#define cbrt(X) _Generic((X), float: cbrtf, default: cbrt)(X)");
+  verifyFormat("_Generic(x, const char *: 1, char *const: 16, int: 8);");
+  verifyFormat("_Generic(x, int: f1, const int: f2)();");
+  verifyFormat("_Generic(x, struct A: 1, void (*)(void): 2);");
+
+  verifyFormat("_Generic(x,\n"
+   "float: f,\n"
+   "default: d,\n"
+   "long double: ld,\n"
+   "float _Complex: fc,\n"
+   "double _Complex: dc,\n"
+   "long double _Complex: ldc)");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 40;
+  verifyFormat("#define LIMIT_MAX(T)   \\\n"
+   "  _Generic(((T)0), \\\n"
+   "  unsigned int: UINT_MAX,  \\\n"
+   "  unsigned long: ULONG_MAX,\\\n"
+   "  unsigned long long: ULLONG_MAX)",
+   Style);
+  verifyFormat("_Generic(x,\n"
+   "struct A: 1,\n"
+   "void (*)(void): 2);",
+   Style);
+
+  Style.ContinuationIndentWidth = 2;
+  verifyFormat("_Generic(x,\n"
+   "  struct A: 1,\n"
+   "  void (*)(void): 2);",
+   Style);
+}
+
 TEST_F(FormatTest, AmbersandInLamda) {
   // Test case reported in https://bugs.llvm.org/show_bug.cgi?id=41899
   FormatStyle AlignStyle = getLLVMStyle();
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -322,6 +322,10 @@
   Contexts.back().IsExpression = false;
 } else if (OpeningParen.is(TT_RequiresExpressionLParen)) {
   Contexts.back().IsExpression = false;
+} else if (OpeningParen.Previous &&
+   OpeningParen.Previous->is(tok::kw__Generic)) {
+  Contexts.back().ContextType = Context::C11GenericSelection;
+  Contexts.back().IsExpression = true;
 } else if (Line.InPPDirective &&
(!OpeningParen.Previous ||
 !OpeningParen.Previous->is(tok::identifier))) {
@@ -1027,6 +1031,8 @@
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->setType(TT_RangeBasedForLoopColon);
+  } else if (Contexts.back().ContextType == Context::C11GenericSelection) {
+Tok->setType(TT_GenericSelectionColon);
   } else if (CurrentToken && CurrentToken->is(tok::numeric_constant)) {
 Tok->setType(TT_BitFieldColon);
   } else if (Contexts.size() == 1 &&
@@ -1626,6 +1632,8 @@
   StructArrayInitializer,
   // Like in `static_cast`.
   TemplateArgument,
+  // C11 _Generic selection.
+  C11GenericSelection,
 } ContextType = Unknown;
   };
 
@@ -3254,7 +3262,8 @@
 return 100;
   }
   if (Left.is(tok::l_paren) && Left.Previous &&
-  (Left.Previous->is(tok::kw_for) || Left.Previous->isIf())) {
+  (Left.Previous->isOneOf(tok::kw_for, tok::kw__Generic) ||
+   Left.Previous->isIf())) {
 return 1000;
   }
   if (Left.is(tok::equal) && InFunctionDecl)
@@ -4163,6 +4172,8 @@
   return false;
 

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/test/SemaCXX/cxx20-decomposition.cpp:160
+return a;
+}() ; }(0);
+(void)[&](auto c) { return b + [](auto) {

aaron.ballman wrote:
> Same edit elsewhere. Did clang-format get confused?
It can't be clang-format because it would figuratively explode due to 
https://github.com/llvm/llvm-project/issues/40694! (but it did remind me to 
investigate that issue)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

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


[PATCH] D141288: [clang-format] Inherit RightAlign options across scopes

2023-01-09 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
rymiel added a project: clang-format.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D119599  added the ability to align compound 
assignments, right aligning
them in order to line up at the equals sign.
However, that patch didn't account for AlignTokens being called
recursively across scopes, which reset the right justification to be
false in any scope besides the top scope. This meant the compound
assignments were aligned, just not at the right place.
(No tests also ever introduced any scopes)

This patch makes sure to inherit the right justification value, just as
every other parameter is passed on.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141288

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
@@ -17693,6 +17693,20 @@
"dvsdsv<<= 5;\n"
"int dsvvdvsdvvv = 123;",
Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx = 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy = 6;\n"
+   "}",
+   Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx+= 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy+= 6;\n"
+   "}",
+   Alignment);
   // Test that `<=` is not treated as a compound assignment.
   verifyFormat("aa &= 5;\n"
"b <= 10;\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -609,7 +609,8 @@
   ++CommasBeforeMatch;
 } else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
   // Call AlignTokens recursively, skipping over this scope block.
-  unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i, ACS);
+  unsigned StoppedAt =
+  AlignTokens(Style, Matches, Changes, i, ACS, RightJustify);
   i = StoppedAt - 1;
   continue;
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -17693,6 +17693,20 @@
"dvsdsv<<= 5;\n"
"int dsvvdvsdvvv = 123;",
Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx = 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy = 6;\n"
+   "}",
+   Alignment);
+  verifyFormat("int xxx = 5;\n"
+   "xxx+= 5;\n"
+   "{\n"
+   "  int yyy = 6;\n"
+   "  yyy+= 6;\n"
+   "}",
+   Alignment);
   // Test that `<=` is not treated as a compound assignment.
   verifyFormat("aa &= 5;\n"
"b <= 10;\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -609,7 +609,8 @@
   ++CommasBeforeMatch;
 } else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
   // Call AlignTokens recursively, skipping over this scope block.
-  unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i, ACS);
+  unsigned StoppedAt =
+  AlignTokens(Style, Matches, Changes, i, ACS, RightJustify);
   i = StoppedAt - 1;
   continue;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D141287: [clang-format] Inherit RightAlign option across scopes

2023-01-09 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel abandoned this revision.
rymiel added a comment.

I apologize, I managed to really mess up with git somehow


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141287

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


[PATCH] D141287: [clang-format] Inherit RightAlign option across scopes

2023-01-09 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
rymiel added a project: clang-format.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D119599  added the ability to align compound 
assignments, right aligning
them in order to line up at the equals sign.
However, that patch didn't account for AlignTokens being called
recursively across scopes, which reset the right justification to be
false in any scope besides the top scope. This meant the compound
assignments were aligned, just not at the right place.
(No tests also ever introduced any scopes)

This patch makes sure to inherit the right justification value, just as
every other parameter is passed on.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141287

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/FormatToken.h
  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
@@ -1136,6 +1136,14 @@
   EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
+  auto Tokens = annotate("_Generic(x, int: 1, default: 0)");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw__Generic, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::colon, TT_GenericSelectionColon);
+  EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
+}
+
 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
@@ -22996,6 +22996,41 @@
   verifyFormat("x = (_Atomic( uint64_t ))", Style);
 }
 
+TEST_F(FormatTest, C11Generic) {
+  verifyFormat("_Generic(x, int: 1, default: 0)");
+  verifyFormat("#define cbrt(X) _Generic((X), float: cbrtf, default: cbrt)(X)");
+  verifyFormat("_Generic(x, const char *: 1, char *const: 16, int: 8);");
+  verifyFormat("_Generic(x, int: f1, const int: f2)();");
+  verifyFormat("_Generic(x, struct A: 1, void (*)(void): 2);");
+
+  verifyFormat("_Generic(x,\n"
+   "float: f,\n"
+   "default: d,\n"
+   "long double: ld,\n"
+   "float _Complex: fc,\n"
+   "double _Complex: dc,\n"
+   "long double _Complex: ldc)");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 40;
+  verifyFormat("#define LIMIT_MAX(T)   \\\n"
+   "  _Generic(((T)0), \\\n"
+   "  unsigned int: UINT_MAX,  \\\n"
+   "  unsigned long: ULONG_MAX,\\\n"
+   "  unsigned long long: ULLONG_MAX)",
+   Style);
+  verifyFormat("_Generic(x,\n"
+   "struct A: 1,\n"
+   "void (*)(void): 2);",
+   Style);
+
+  Style.ContinuationIndentWidth = 2;
+  verifyFormat("_Generic(x,\n"
+   "  struct A: 1,\n"
+   "  void (*)(void): 2);",
+   Style);
+}
+
 TEST_F(FormatTest, AmbersandInLamda) {
   // Test case reported in https://bugs.llvm.org/show_bug.cgi?id=41899
   FormatStyle AlignStyle = getLLVMStyle();
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -323,6 +323,10 @@
   Contexts.back().IsExpression = false;
 } else if (OpeningParen.is(TT_RequiresExpressionLParen)) {
   Contexts.back().IsExpression = false;
+} else if (OpeningParen.Previous &&
+   OpeningParen.Previous->is(tok::kw__Generic)) {
+  Contexts.back().ContextType = Context::C11GenericSelection;
+  Contexts.back().IsExpression = true;
 } else if (Line.InPPDirective &&
(!OpeningParen.Previous ||
 !OpeningParen.Previous->is(tok::identifier))) {
@@ -1028,6 +1032,8 @@
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->setType(TT_RangeBasedForLoopColon);
+  } else if (Contexts.back().ContextType == Context::C11GenericSelection) {
+Tok->setType(TT_GenericSelectionColon);
   } else if (CurrentToken && CurrentToken->is(tok::numeric_constant)) {
 Tok->setType(TT_BitFieldColon);
   } else if (Contexts.size() == 1 &&

[PATCH] D141098: [clang-format][NFC] Set DeriveLineEnding to false in config files

2023-01-06 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

> I think we should combine `DeriveLineEnding` and `UseCRLF`

Do you mean a single `LineEnding` options which is an enum with `LF`, `CRLF` 
and `Derive`, or similar?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141098

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


[PATCH] D141098: [clang-format][NFC] Set DeriveLineEnding to false in config files

2023-01-05 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added a comment.

The LLVM Coding Standard apparently doesn't mention line endings..? A quick 
grep does show a bunch of \r\n results, primarily in tests. I'd be in favor of 
forcing \n, but I think this is a wider matter..?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141098

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


[PATCH] D140312: [clang-format] Disallow decltype in the middle of constraints

2023-01-05 Thread Emilia Dreamer 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 rGd9899501576e: [clang-format] Disallow decltype in the middle 
of constraints (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140312

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
@@ -580,6 +580,14 @@
   EXPECT_TRUE(Tokens[9]->ClosesRequiresClause);
   EXPECT_TOKEN(Tokens[11], tok::identifier, TT_FunctionDeclarationName);
 
+  Tokens = annotate("template \n"
+"requires Bar\n"
+"decltype(auto) foo(T) { return false; }");
+  ASSERT_EQ(Tokens.size(), 24u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[9]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[14], tok::identifier, TT_FunctionDeclarationName);
+
   Tokens = annotate("template \n"
 "struct S {\n"
 "  void foo() const requires Bar;\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24199,6 +24199,10 @@
"  }\n"
"};");
 
+  verifyFormat("template \n"
+   "  requires(std::same_as)\n"
+   "decltype(auto) fun() {}");
+
   auto Style = getLLVMStyle();
 
   verifyFormat(
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3543,7 +3543,6 @@
 case tok::minus:
 case tok::star:
 case tok::slash:
-case tok::kw_decltype:
   LambdaNextTimeAllowed = true;
   // Just eat them.
   nextToken();


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -580,6 +580,14 @@
   EXPECT_TRUE(Tokens[9]->ClosesRequiresClause);
   EXPECT_TOKEN(Tokens[11], tok::identifier, TT_FunctionDeclarationName);
 
+  Tokens = annotate("template \n"
+"requires Bar\n"
+"decltype(auto) foo(T) { return false; }");
+  ASSERT_EQ(Tokens.size(), 24u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TRUE(Tokens[9]->ClosesRequiresClause);
+  EXPECT_TOKEN(Tokens[14], tok::identifier, TT_FunctionDeclarationName);
+
   Tokens = annotate("template \n"
 "struct S {\n"
 "  void foo() const requires Bar;\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24199,6 +24199,10 @@
"  }\n"
"};");
 
+  verifyFormat("template \n"
+   "  requires(std::same_as)\n"
+   "decltype(auto) fun() {}");
+
   auto Style = getLLVMStyle();
 
   verifyFormat(
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3543,7 +3543,6 @@
 case tok::minus:
 case tok::star:
 case tok::slash:
-case tok::kw_decltype:
   LambdaNextTimeAllowed = true;
   // Just eat them.
   nextToken();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D140339: [clang-format] Remove special logic for parsing concept definitions.

2023-01-05 Thread Emilia Dreamer 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 rGb1eeec6177fa: [clang-format] Remove special logic for 
parsing concept definitions. (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140339

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23588,10 +23588,10 @@
"concept DelayedCheck = !!false || requires(T t) { t.bar(); } "
"&& sizeof(T) <= 8;");
 
-  verifyFormat(
-  "template \n"
-  "concept DelayedCheck = static_cast(0) ||\n"
-  "   requires(T t) { t.bar(); } && sizeof(T) <= 8;");
+  verifyFormat("template \n"
+   "concept DelayedCheck =\n"
+   "static_cast(0) || requires(T t) { t.bar(); } && "
+   "sizeof(T) <= 8;");
 
   verifyFormat("template \n"
"concept DelayedCheck = bool(0) || requires(T t) { t.bar(); } "
@@ -23599,8 +23599,8 @@
 
   verifyFormat(
   "template \n"
-  "concept DelayedCheck = (bool)(0) ||\n"
-  "   requires(T t) { t.bar(); } && sizeof(T) <= 8;");
+  "concept DelayedCheck =\n"
+  "(bool)(0) || requires(T t) { t.bar(); } && sizeof(T) <= 8;");
 
   verifyFormat("template \n"
"concept DelayedCheck = (bool)0 || requires(T t) { t.bar(); } "
@@ -23636,6 +23636,9 @@
   verifyFormat("template \n"
"concept True = S::Value;");
 
+  verifyFormat("template \n"
+   "concept True = T.field;");
+
   verifyFormat(
   "template \n"
   "concept C = []() { return true; }() && requires(T t) { t.bar(); } &&\n"
@@ -23914,11 +23917,9 @@
   verifyFormat("template  concept True = true;", Style);
 
   verifyFormat(
-  "template  concept C = decltype([]() -> std::true_type {\n"
-  "return {};\n"
-  "  }())::value &&\n"
-  "  requires(T t) { t.bar(); } && "
-  "sizeof(T) <= 8;",
+  "template  concept C =\n"
+  "decltype([]() -> std::true_type { return {}; }())::value &&\n"
+  "requires(T t) { t.bar(); } && sizeof(T) <= 8;",
   Style);
 
   verifyFormat("template  concept Semiregular =\n"
@@ -23936,21 +23937,20 @@
 
   // The following tests are invalid C++, we just want to make sure we don't
   // assert.
-  verifyFormat("template \n"
-   "concept C = requires C2;");
+  verifyNoCrash("template \n"
+"concept C = requires C2;");
 
-  verifyFormat("template \n"
-   "concept C = 5 + 4;");
+  verifyNoCrash("template \n"
+"concept C = 5 + 4;");
 
-  verifyFormat("template \n"
-   "concept C =\n"
-   "class X;");
+  verifyNoCrash("template \n"
+"concept C = class X;");
 
-  verifyFormat("template \n"
-   "concept C = [] && true;");
+  verifyNoCrash("template \n"
+"concept C = [] && true;");
 
-  verifyFormat("template \n"
-   "concept C = [] && requires(T t) { typename T::size_type; };");
+  verifyNoCrash("template \n"
+"concept C = [] && requires(T t) { typename T::size_type; };");
 }
 
 TEST_F(FormatTest, RequiresClausesPositions) {
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -158,7 +158,6 @@
   void parseAccessSpecifier();
   bool parseEnum();
   bool parseStructLike();
-  void parseConcept();
   bool parseRequires();
   void parseRequiresClause(FormatToken *RequiresToken);
   void parseRequiresExpression(FormatToken *RequiresToken);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1818,9 +1818,6 @@
 break;
   }
   break;
-case tok::kw_concept:
-  parseConcept();
-  return;
 case tok::kw_requires: {
   if (Style.isCpp()) {
 bool ParsedClause = parseRequires();
@@ -3277,26 +3274,6 @@
   }
 }
 
-/// \brief Parses a concept definition.
-/// \pre The current token has to be the concept keyword.
-///
-/// Returns if either the concept has been completely parsed, or if it detects
-/// that the concept definition is incorrect.
-void UnwrappedLineParser::parseConcept() {
-  assert(FormatTok->is(tok::kw_concept) && "'concept' expected");
-  

[PATCH] D140339: [clang-format] Remove special logic for parsing concept definitions.

2023-01-05 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel marked an inline comment as done.
rymiel added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:23593-23594
-  "template \n"
-  "concept DelayedCheck = static_cast(0) ||\n"
-  "   requires(T t) { t.bar(); } && sizeof(T) <= 8;");
 

owenpan wrote:
> What would happen with parentheses added?
> ```
> concept DelayedCheck = (static_cast(0) || requires(T t) { t.bar(); }) 
> && sizeof(T) <= 8;
> concept DelayedCheck = static_cast(0) || (requires(T t) { t.bar(); } && 
> sizeof(T) <= 8);
> ```
No difference:
```
concept DelayedCheck =
(static_cast(0) || requires(T t) { t.bar(); }) && sizeof(T) <= 8;
concept DelayedCheck =
static_cast(0) || (requires(T t) { t.bar(); } && sizeof(T) <= 8);
concept DelayedCheck =
static_cast(0) || requires(T t) { t.bar(); } && sizeof(T) <= 8;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140339

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


[PATCH] D140767: [clang-format] Require space before noexcept qualifier

2023-01-05 Thread Emilia Dreamer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG54fab18cedac: [clang-format] Require space before noexcept 
qualifier (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140767

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10566,10 +10566,10 @@
 
 TEST_F(FormatTest, UnderstandsFunctionRefQualification) {
   verifyFormat("void A::b() && {}");
-  verifyFormat("void A::b() & {}");
+  verifyFormat("void A::b() && noexcept {}");
   verifyFormat("Deleted =(const Deleted &) & = default;");
   verifyFormat("Deleted =(const Deleted &) && = delete;");
-  verifyFormat("Deleted =(const Deleted &)  = default;");
+  verifyFormat("Deleted =(const Deleted &) & noexcept = default;");
   verifyFormat("SomeType MemberFunction(const Deleted &) & = delete;");
   verifyFormat("SomeType MemberFunction(const Deleted &) && = delete;");
   verifyFormat("Deleted =(const Deleted &) &;");
@@ -10579,16 +10579,16 @@
   verifyFormat("SomeType MemberFunction(const Deleted &) && {}");
   verifyFormat("SomeType MemberFunction(const Deleted &) && final {}");
   verifyFormat("SomeType MemberFunction(const Deleted &) && override {}");
-  verifyFormat("SomeType MemberFunction(const Deleted &) & {}");
+  verifyFormat("SomeType MemberFunction(const Deleted &) && noexcept {}");
   verifyFormat("void Fn(T const &) const &;");
   verifyFormat("void Fn(T const volatile &&) const volatile &&;");
-  verifyFormat("void Fn(T const volatile &&) const volatile &");
+  verifyFormat("void Fn(T const volatile &&) const volatile && noexcept;");
   verifyFormat("template \n"
"void F(T) && = delete;",
getGoogleStyle());
   verifyFormat("template  void operator=(T) &;");
   verifyFormat("template  void operator=(T) const &;");
-  verifyFormat("template  void operator=(T) ");
+  verifyFormat("template  void operator=(T) & noexcept;");
   verifyFormat("template  void operator=(T) & = default;");
   verifyFormat("template  void operator=(T) &&;");
   verifyFormat("template  void operator=(T) && = delete;");
@@ -10678,31 +10678,31 @@
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int (const std::string )  {}\n"
+   "  int (const std::string ) & noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int (const std::string ) & {}\n"
+   "  int (const std::string ) && noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int (const std::string ) const  {}\n"
+   "  int (const std::string ) const & noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int (const std::string ) const  {}\n"
+   "  int (const std::string ) const & noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  auto foo(const std::string ) & -> int & {}\n"
+   "  auto foo(const std::string ) && noexcept -> int & {}\n"
"};",
BreakTemplate);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3530,7 +3530,8 @@
 if (Right.is(TT_BlockComment))
   return true;
 // foo() -> const Bar * override/final
-if (Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
+if (Right.isOneOf(Keywords.kw_override, Keywords.kw_final,
+  tok::kw_noexcept) &&
 !Right.is(TT_StartOfName)) {
   return true;
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10566,10 +10566,10 @@
 
 TEST_F(FormatTest, UnderstandsFunctionRefQualification) {
   verifyFormat("void A::b() && {}");
-  verifyFormat("void A::b() & {}");
+  verifyFormat("void A::b() && noexcept {}");
   verifyFormat("Deleted =(const Deleted &) & = default;");
   verifyFormat("Deleted =(const Deleted &) && = delete;");
-  verifyFormat("Deleted =(const Deleted &)  = default;");
+  verifyFormat("Deleted =(const Deleted &) & noexcept = default;");
   verifyFormat("SomeType MemberFunction(const Deleted &) & = delete;");
   verifyFormat("SomeType MemberFunction(const Deleted &) && = delete;");
 

[PATCH] D140339: [clang-format] Remove special logic for parsing concept definitions.

2023-01-02 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:23946-23947
-  verifyFormat("template \n"
-   "concept C =\n"
-   "class X;");
 

owenpan wrote:
> Just want to make sure that the line break didn't matter.
Sorry, that line break was removed when verifyFormat was still used


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140339

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


[PATCH] D140767: [clang-format] Require space before noexcept qualifier

2022-12-29 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel created this revision.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This brings the noexcept qualifier more visually in line with the other
keyword qualifiers, such as "final" and "override".

Originally reported as https://github.com/llvm/llvm-project/issues/44542,
it was closed as "working by design" and reinforcing tests were added
as part of a218706cba90248be0c60bd6a8f10dbcf0270955 
. The 
exact spacing
depended on the `PointerAlignment` option, where the default value of
`Right` would leave no space.

This patch seeks to change this behaviour, regardless of the configured
`PointerAlignment` option (matching the previous behaviour of the `Left`
option).

Closes https://github.com/llvm/llvm-project/issues/59729


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140767

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10566,10 +10566,10 @@
 
 TEST_F(FormatTest, UnderstandsFunctionRefQualification) {
   verifyFormat("void A::b() && {}");
-  verifyFormat("void A::b() & {}");
+  verifyFormat("void A::b() && noexcept {}");
   verifyFormat("Deleted =(const Deleted &) & = default;");
   verifyFormat("Deleted =(const Deleted &) && = delete;");
-  verifyFormat("Deleted =(const Deleted &)  = default;");
+  verifyFormat("Deleted =(const Deleted &) & noexcept = default;");
   verifyFormat("SomeType MemberFunction(const Deleted &) & = delete;");
   verifyFormat("SomeType MemberFunction(const Deleted &) && = delete;");
   verifyFormat("Deleted =(const Deleted &) &;");
@@ -10579,16 +10579,16 @@
   verifyFormat("SomeType MemberFunction(const Deleted &) && {}");
   verifyFormat("SomeType MemberFunction(const Deleted &) && final {}");
   verifyFormat("SomeType MemberFunction(const Deleted &) && override {}");
-  verifyFormat("SomeType MemberFunction(const Deleted &) & {}");
+  verifyFormat("SomeType MemberFunction(const Deleted &) && noexcept {}");
   verifyFormat("void Fn(T const &) const &;");
   verifyFormat("void Fn(T const volatile &&) const volatile &&;");
-  verifyFormat("void Fn(T const volatile &&) const volatile &");
+  verifyFormat("void Fn(T const volatile &&) const volatile && noexcept;");
   verifyFormat("template \n"
"void F(T) && = delete;",
getGoogleStyle());
   verifyFormat("template  void operator=(T) &;");
   verifyFormat("template  void operator=(T) const &;");
-  verifyFormat("template  void operator=(T) ");
+  verifyFormat("template  void operator=(T) & noexcept;");
   verifyFormat("template  void operator=(T) & = default;");
   verifyFormat("template  void operator=(T) &&;");
   verifyFormat("template  void operator=(T) && = delete;");
@@ -10678,31 +10678,31 @@
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int (const std::string )  {}\n"
+   "  int (const std::string ) & noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int (const std::string ) & {}\n"
+   "  int (const std::string ) && noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int (const std::string ) const  {}\n"
+   "  int (const std::string ) const & noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  int (const std::string ) const  {}\n"
+   "  int (const std::string ) const & noexcept {}\n"
"};",
BreakTemplate);
 
   verifyFormat("struct f {\n"
"  template \n"
-   "  auto foo(const std::string ) & -> int & {}\n"
+   "  auto foo(const std::string ) && noexcept -> int & {}\n"
"};",
BreakTemplate);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3495,7 +3495,8 @@
 if (Right.is(TT_BlockComment))
   return true;
 // foo() -> const Bar * override/final
-if (Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
+if (Right.isOneOf(Keywords.kw_override, Keywords.kw_final,
+  tok::kw_noexcept) &&
 !Right.is(TT_StartOfName)) {
   return true;
 }


Index: clang/unittests/Format/FormatTest.cpp

[PATCH] D139211: [clang-format] Properly handle the C11 _Generic keyword.

2022-12-23 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 485062.
rymiel marked an inline comment as done.
rymiel retitled this revision from "[WIP][clang-format] Properly handle the C11 
_Generic keyword." to "[clang-format] Properly handle the C11 _Generic 
keyword.".
rymiel added a comment.
This revision is now accepted and ready to land.

Change indenting behaviour

I'm still not sure what I'm doing regarding ContinuationIndenter, since I don't 
fully understand its mechanisms, so I just followed my usual method of "stick a 
very specific check in a random spot that makes the correct fields have the 
correct value", which feels like a hack here. There is probably a much better 
way to do this, perhaps something with fake parens or something, to actually 
treat the "controlling expression" and the actual selectors differently, but 
right now I've just put in a specific check to dedent the selectors to be 
relative to the _Generic keyword, and not aligned to the opening paren.

It is also no surprise that I don't know how to write tests; I included organic 
examples of cases that are semi-realistic use cases of _Generic, but there are 
definitely edge cases I don't have listed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139211

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/FormatToken.h
  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
@@ -1136,6 +1136,14 @@
   EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
+  auto Tokens = annotate("_Generic(x, int: 1, default: 0)");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw__Generic, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::colon, TT_GenericSelectionColon);
+  EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
+}
+
 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
@@ -22996,6 +22996,41 @@
   verifyFormat("x = (_Atomic( uint64_t ))", Style);
 }
 
+TEST_F(FormatTest, C11Generic) {
+  verifyFormat("_Generic(x, int: 1, default: 0)");
+  verifyFormat("#define cbrt(X) _Generic((X), float: cbrtf, default: cbrt)(X)");
+  verifyFormat("_Generic(x, const char *: 1, char *const: 16, int: 8);");
+  verifyFormat("_Generic(x, int: f1, const int: f2)();");
+  verifyFormat("_Generic(x, struct A: 1, void (*)(void): 2);");
+
+  verifyFormat("_Generic(x,\n"
+   "float: f,\n"
+   "default: d,\n"
+   "long double: ld,\n"
+   "float _Complex: fc,\n"
+   "double _Complex: dc,\n"
+   "long double _Complex: ldc)");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 40;
+  verifyFormat("#define LIMIT_MAX(T)   \\\n"
+   "  _Generic(((T)0), \\\n"
+   "  unsigned int: UINT_MAX,  \\\n"
+   "  unsigned long: ULONG_MAX,\\\n"
+   "  unsigned long long: ULLONG_MAX)",
+   Style);
+  verifyFormat("_Generic(x,\n"
+   "struct A: 1,\n"
+   "void (*)(void): 2);",
+   Style);
+
+  Style.ContinuationIndentWidth = 2;
+  verifyFormat("_Generic(x,\n"
+   "  struct A: 1,\n"
+   "  void (*)(void): 2);",
+   Style);
+}
+
 TEST_F(FormatTest, AmbersandInLamda) {
   // Test case reported in https://bugs.llvm.org/show_bug.cgi?id=41899
   FormatStyle AlignStyle = getLLVMStyle();
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -323,6 +323,10 @@
   Contexts.back().IsExpression = false;
 } else if (OpeningParen.is(TT_RequiresExpressionLParen)) {
   Contexts.back().IsExpression = false;
+} else if (OpeningParen.Previous &&
+   OpeningParen.Previous->is(tok::kw__Generic)) {
+  Contexts.back().ContextType = Context::C11GenericSelection;
+  Contexts.back().IsExpression = true;
 } else if (Line.InPPDirective &&
(!OpeningParen.Previous ||
 !OpeningParen.Previous->is(tok::identifier))) {
@@ -1028,6 +1032,8 @@
 }
   } else if 

[PATCH] D139211: [WIP][clang-format] Properly handle the C11 _Generic keyword.

2022-12-22 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel planned changes to this revision.
rymiel added a comment.

Going to try to make the indentation of the cases following the controlling 
expression not continuations, since that results in weird results such as:

  #define LIMIT_MAX(T) \
_Generic(  \
((T)0),\
unsigned int: UINT_MAX,\
unsigned long: ULONG_MAX,  \
unsigned long long: ULLONG_MAX)

My desired output is instead:

  #define LIMIT_MAX(T) \
_Generic(((T)0),   \
unsigned int: UINT_MAX,\
unsigned long: ULONG_MAX,  \
unsigned long long: ULLONG_MAX)

or something similar, but I'm struggling with unfamiliar parts of the codebase 
and taking my time getting familiar with them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139211

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


  1   2   3   >