[PATCH] D154184: [clang-format] Correctly annotate */&/&& in operator function calls

[Emilia Kond via Phabricator via cfe-commits]

rymiel added a comment.

Shouldn't that regression already be handled by D155358 
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154184

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

[PATCH] D155358: [clang-format] Correctly annotate overloaded operator function name

[Emilia Kond via Phabricator via cfe-commits]

rymiel accepted this revision.
rymiel added a comment.

Was caused by 3f3620e5c9ee0f7b64afc39e5a26c6f4cc5e7b37 
, thank 
you for fixing it up!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155358

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

[PATCH] D150848: [clang-format] Respect ColumnLimit 0 line breaks in inline asm

[Emilia Kond 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 rG7a38b3bfeb56: [clang-format] Respect ColumnLimit 0 line 
breaks in inline asm (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150848

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4612,6 +4612,24 @@
 format("__asm   {\n"
"}\n"
"int   i;"));
+
+  auto Style = getLLVMStyleWithColumns(0);
+  const StringRef Code1{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));"};
+  const StringRef Code2{"asm(\"xyz\"\n"
+": \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));"};
+  const StringRef Code3{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));"};
+
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline;
+  verifyFormat(Code1, Style);
+  EXPECT_EQ(Code2, format(Code2, Style));
+  EXPECT_EQ(Code3, format(Code3, Style));
+
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Always;
+  EXPECT_EQ(Code2, format(Code1, Style));
+  EXPECT_EQ(Code2, format(Code2, Style));
+  EXPECT_EQ(Code2, format(Code3, Style));
 }
 
 TEST_F(FormatTest, FormatTryCatch) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -357,7 +357,8 @@
   if (Current.MustBreakBefore ||
   (Current.is(TT_InlineASMColon) &&
(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always ||
-Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline))) 
{
+(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline &&
+ Style.ColumnLimit > 0 {
 return true;
   }
   if (CurrentState.BreakBeforeClosingBrace &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4612,6 +4612,24 @@
 format("__asm   {\n"
"}\n"
"int   i;"));
+
+  auto Style = getLLVMStyleWithColumns(0);
+  const StringRef Code1{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));"};
+  const StringRef Code2{"asm(\"xyz\"\n"
+": \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));"};
+  const StringRef Code3{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));"};
+
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline;
+  verifyFormat(Code1, Style);
+  EXPECT_EQ(Code2, format(Code2, Style));
+  EXPECT_EQ(Code3, format(Code3, Style));
+
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Always;
+  EXPECT_EQ(Code2, format(Code1, Style));
+  EXPECT_EQ(Code2, format(Code2, Style));
+  EXPECT_EQ(Code2, format(Code3, Style));
 }
 
 TEST_F(FormatTest, FormatTryCatch) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -357,7 +357,8 @@
   if (Current.MustBreakBefore ||
   (Current.is(TT_InlineASMColon) &&
(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always ||
-Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline))) {
+(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline &&
+ Style.ColumnLimit > 0 {
 return true;
   }
   if (CurrentState.BreakBeforeClosingBrace &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153641: [clang-format] Preserve AmpAmpTokenType in nested parentheses

[Emilia Kond 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.

When parsing a requires clause, the UnwrappedLineParser would delegate to
parseParens with an AmpAmpTokenType set to BinaryOperator. However,
parseParens would not carry this over into any nested parens, meaning it
could assign a different token type to an && in a requires clause.

This patch makes sure parseParens inherits its parameter when performing
a recursive call.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153641

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
@@ -906,6 +906,16 @@
   annotate("auto bar() -> Template requires(is_integral_v) {}");
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("void foo() requires((A) && C) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("void foo() requires(((A) && C)) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2431,14 +2431,14 @@
 
 /// \brief Parses a pair of parentheses (and everything between them).
 /// \param AmpAmpTokenType If different than TT_Unknown sets this type for all
-/// double ampersands. This only counts for the current parens scope.
+/// double ampersands. This applies for all nested scopes as well.
 void UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
   assert(FormatTok->is(tok::l_paren) && "'(' expected.");
   nextToken();
   do {
 switch (FormatTok->Tok.getKind()) {
 case tok::l_paren:
-  parseParens();
+  parseParens(AmpAmpTokenType);
   if (Style.Language == FormatStyle::LK_Java && 
FormatTok->is(tok::l_brace))
 parseChildBlock();
   break;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -906,6 +906,16 @@
   annotate("auto bar() -> Template requires(is_integral_v) {}");
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("void foo() requires((A) && C) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("void foo() requires(((A) && C)) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2431,14 +2431,14 @@
 
 /// \brief Parses a pair of parentheses (and everything between them).
 /// \param AmpAmpTokenType If different than TT_Unknown sets this type for all
-/// double ampersands. This only counts for the current parens scope.
+/// double ampersands. This applies for all nested scopes as well.
 void UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
   assert(FormatTok->is(tok::l_paren) && "'(' expected.");
   nextToken();
   do {
 switch (FormatTok->Tok.getKind()) {
 case tok::l_paren:
-  parseParens();
+  parseParens(AmpAmpTokenType);
   if (Style.Language == FormatStyle::LK_Java && FormatTok->is(tok::l_brace))
 parseChildBlock();
   break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153641: [clang-format] Preserve AmpAmpTokenType in nested parentheses

[Emilia Kond via Phabricator via cfe-commits]

rymiel added a comment.

@HazardyKnusperkeks I'm not sure why it didn't recurse already, given that you 
even documented that it doesn't, but I chose to trust in the Beyoncé rule.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153641

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

[PATCH] D153641: [clang-format] Preserve AmpAmpTokenType in nested parentheses

[Emilia Kond via Phabricator via cfe-commits]

rymiel updated this revision to Diff 534220.
rymiel added a comment.

Add annotator test cases involving a lambda


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153641

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
@@ -906,6 +906,26 @@
   annotate("auto bar() -> Template requires(is_integral_v) {}");
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("void foo() requires((A) && C) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("void foo() requires(((A) && C)) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("void foo() requires([](T&&){}(t)) {}");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() requires([](T&& u){}(t)) {}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2431,14 +2431,14 @@
 
 /// \brief Parses a pair of parentheses (and everything between them).
 /// \param AmpAmpTokenType If different than TT_Unknown sets this type for all
-/// double ampersands. This only counts for the current parens scope.
+/// double ampersands. This applies for all nested scopes as well.
 void UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
   assert(FormatTok->is(tok::l_paren) && "'(' expected.");
   nextToken();
   do {
 switch (FormatTok->Tok.getKind()) {
 case tok::l_paren:
-  parseParens();
+  parseParens(AmpAmpTokenType);
   if (Style.Language == FormatStyle::LK_Java && 
FormatTok->is(tok::l_brace))
 parseChildBlock();
   break;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -906,6 +906,26 @@
   annotate("auto bar() -> Template requires(is_integral_v) {}");
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("void foo() requires((A) && C) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("void foo() requires(((A) && C)) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("void foo() requires([](T&&){}(t)) {}");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() requires([](T&& u){}(t)) {}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2431,14 +2431,14 @@
 
 /// \brief Parses a pair of parentheses (and everything between them).
 /// \param AmpAmpTokenType If different than TT_Unknown sets this type for all
-/// double ampersands. This only counts for the current parens scope.
+/// double ampersands. This applies for all nested scopes as well.
 void UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
   assert(FormatTok->is(tok::l_paren) && "'(' expected.");
   nextToken();
   do {
 switch (FormatTok->Tok.getKind()) {
 case tok::l_paren:
-  parseParens();
+  parseParens(AmpAmpTokenType);
   if (Style.Language == FormatStyle::LK_Java && FormatTok->

[PATCH] D150848: [clang-format] Respect ColumnLimit 0 line breaks in inline asm

[Emilia Kond via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Format/FormatTest.cpp:4626-4627
+  verifyFormat(Code1, Style);
+  EXPECT_EQ(Code2, format(Code2, Style));
+  EXPECT_EQ(Code3, format(Code3, Style));
+

owenpan wrote:
> We have `verifyNoChange` now. See D153109.
Sorry, this patch was so old and I forgot that refactor happened, thanks for 
reminding me, i pushed 
https://github.com/llvm/llvm-project/commit/6e39b0ddf08ab5cd44fed2d16e9669d64aa733ef


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150848

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

[PATCH] D153745: [clang-format] Fix bugs in annotating r_paren as C-style cast

[Emilia Kond via Phabricator via cfe-commits]

rymiel accepted this revision.
rymiel added a comment.

It appears other operators aren't affected, after D153641 
 I would have not been surprised if && had 
become unary :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153745

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

[PATCH] D153641: [clang-format] Preserve AmpAmpTokenType in nested parentheses

[Emilia Kond via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG15e14f129fe2: [clang-format] Preserve AmpAmpTokenType in 
nested parentheses (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153641

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
@@ -906,6 +906,26 @@
   annotate("auto bar() -> Template requires(is_integral_v) {}");
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("void foo() requires((A) && C) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("void foo() requires(((A) && C)) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("void foo() requires([](T&&){}(t)) {}");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() requires([](T&& u){}(t)) {}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2431,14 +2431,14 @@
 
 /// \brief Parses a pair of parentheses (and everything between them).
 /// \param AmpAmpTokenType If different than TT_Unknown sets this type for all
-/// double ampersands. This only counts for the current parens scope.
+/// double ampersands. This applies for all nested scopes as well.
 void UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
   assert(FormatTok->is(tok::l_paren) && "'(' expected.");
   nextToken();
   do {
 switch (FormatTok->Tok.getKind()) {
 case tok::l_paren:
-  parseParens();
+  parseParens(AmpAmpTokenType);
   if (Style.Language == FormatStyle::LK_Java && 
FormatTok->is(tok::l_brace))
 parseChildBlock();
   break;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -906,6 +906,26 @@
   annotate("auto bar() -> Template requires(is_integral_v) {}");
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("void foo() requires((A) && C) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("void foo() requires(((A) && C)) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("void foo() requires([](T&&){}(t)) {}");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() requires([](T&& u){}(t)) {}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2431,14 +2431,14 @@
 
 /// \brief Parses a pair of parentheses (and everything between them).
 /// \param AmpAmpTokenType If different than TT_Unknown sets this type for all
-/// double ampersands. This only counts for the current parens scope.
+/// double ampersands. This applies for all nested scopes as well.
 void UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
   assert(FormatTok->is(tok::l_paren) && "'(' expected.");
   nextToken();
   do {
 switch (FormatTok->Tok.getKind()) {
 case tok::l_paren:
-  parseParens();
+  parseParens(AmpAm

[PATCH] D153798: [clang-format] Correctly annotate operator free function call

[Emilia Kond via Phabricator via cfe-commits]

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

The annotator correctly annotates an overloaded operator call when
called as a member function, like `x.operator+(y)`, however, when called
as a free function, like `operator+(x, y)`, the annotator assumed it was
an overloaded operator function *declaration*, instead of a call.

This patch allows for a free function call to correctly be annotated as
a call, but only if the current like cannot be a declaration, usually
within the bodies of a function.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153798

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
@@ -657,6 +657,33 @@
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+
+  Tokens = annotate("class Foo {\n"
+"  int operator+(a* b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[8], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() {\n"
+"  operator+(a * b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[9], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("class Foo {\n"
+"  void foo() {\n"
+"operator+(a * b);\n"
+"  }\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[10], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[12], tok::star, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, OverloadedOperatorInTemplate) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -310,8 +310,13 @@
   // If faced with "a.operator*(argument)" or "a->operator*(argument)",
   // i.e. the operator is called as a member function,
   // then the argument must be an expression.
+  // If faced with "operator+(argument)", i.e. the operator is called as
+  // a free function, then the argument is an expression only if the 
current
+  // line can't be a declaration.
   bool OperatorCalledAsMemberFunction =
-  Prev->Previous && Prev->Previous->isOneOf(tok::period, tok::arrow);
+  (Prev->Previous &&
+   Prev->Previous->isOneOf(tok::period, tok::arrow)) ||
+  (!Line.MustBeDeclaration && !Line.InMacroBody);
   Contexts.back().IsExpression = OperatorCalledAsMemberFunction;
 } else if (OpeningParen.is(TT_VerilogInstancePortLParen)) {
   Contexts.back().IsExpression = true;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -657,6 +657,33 @@
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+
+  Tokens = annotate("class Foo {\n"
+"  int operator+(a* b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[8], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() {\n"
+"  operator+(a * b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[9], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("class Foo {\n"
+"  void foo() {\n"
+"oper

[PATCH] D153798: [clang-format] Correctly annotate operator free function call

[Emilia Kond via Phabricator via cfe-commits]

rymiel added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:319
+   Prev->Previous->isOneOf(tok::period, tok::arrow)) ||
+  (!Line.MustBeDeclaration && !Line.InMacroBody);
   Contexts.back().IsExpression = OperatorCalledAsMemberFunction;

owenpan wrote:
> Why not `Line.InMacroBody`? Wouldn't it misformat the following snippet?
> ```
> #define FOO   \
>   void foo() {\
> operator+(a * b); \
>   }
> ```
Yes, but it would break this test case: 
https://github.com/llvm/llvm-project/blob/e469d0d636f36140b08d0b5f603c043008307bcf/clang/unittests/Format/FormatTest.cpp#L11573

I understand it's a nasty workaround, though, but it's quick. The other option 
seems to be rewriting how overloaded operators are annotated to instead be more 
like regular function declarations, but I haven't gauged how hard that would be


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153798

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

[PATCH] D153798: [clang-format] Correctly annotate operator free function call

[Emilia Kond via Phabricator via cfe-commits]

rymiel updated this revision to Diff 535859.
rymiel added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153798

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
@@ -657,6 +657,33 @@
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+
+  Tokens = annotate("class Foo {\n"
+"  int operator+(a* b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[8], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() {\n"
+"  operator+(a * b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[9], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("class Foo {\n"
+"  void foo() {\n"
+"operator+(a * b);\n"
+"  }\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[10], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[12], tok::star, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, OverloadedOperatorInTemplate) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11580,6 +11580,14 @@
"  }",
getLLVMStyleWithColumns(50)));
 
+// FIXME: We should be able to figure out this is an operator call
+#if 0
+  verifyFormat("#define FOO \\\n"
+   "  void foo() {  \\\n"
+   "operator+(a * b);   \\\n"
+   "  }", getLLVMStyleWithColumns(25));
+#endif
+
   // FIXME: We cannot handle this case yet; we might be able to figure out that
   // foo d > v; doesn't make sense.
   verifyFormat("foo d> v;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -310,9 +310,14 @@
   // If faced with "a.operator*(argument)" or "a->operator*(argument)",
   // i.e. the operator is called as a member function,
   // then the argument must be an expression.
-  bool OperatorCalledAsMemberFunction =
-  Prev->Previous && Prev->Previous->isOneOf(tok::period, tok::arrow);
-  Contexts.back().IsExpression = OperatorCalledAsMemberFunction;
+  // If faced with "operator+(argument)", i.e. the operator is called as
+  // a free function, then the argument is an expression only if the 
current
+  // line can't be a declaration.
+  bool IsOperatorCallSite =
+  (Prev->Previous &&
+   Prev->Previous->isOneOf(tok::period, tok::arrow)) ||
+  (!Line.MustBeDeclaration && !Line.InMacroBody);
+  Contexts.back().IsExpression = IsOperatorCallSite;
 } else if (OpeningParen.is(TT_VerilogInstancePortLParen)) {
   Contexts.back().IsExpression = true;
   Contexts.back().ContextType = Context::VerilogInstancePortList;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -657,6 +657,33 @@
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+
+  Tokens = annotate("class Foo {\n"
+"  int operator+(a* b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[8], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() {\n"
+"  operator+(a * b);\n"
+"}");
+ 

[PATCH] D153798: [clang-format] Correctly annotate operator free function call

[Emilia Kond 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 rG4986f3f2f220: [clang-format] Correctly annotate operator 
free function call (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153798

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
@@ -668,6 +668,33 @@
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+
+  Tokens = annotate("class Foo {\n"
+"  int operator+(a* b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[8], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("void foo() {\n"
+"  operator+(a * b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[9], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("class Foo {\n"
+"  void foo() {\n"
+"operator+(a * b);\n"
+"  }\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[10], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[12], tok::star, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, OverloadedOperatorInTemplate) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11580,6 +11580,14 @@
"  }",
getLLVMStyleWithColumns(50)));
 
+// FIXME: We should be able to figure out this is an operator call
+#if 0
+  verifyFormat("#define FOO \\\n"
+   "  void foo() {  \\\n"
+   "operator+(a * b);   \\\n"
+   "  }", getLLVMStyleWithColumns(25));
+#endif
+
   // FIXME: We cannot handle this case yet; we might be able to figure out that
   // foo d > v; doesn't make sense.
   verifyFormat("foo d> v;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -310,9 +310,14 @@
   // If faced with "a.operator*(argument)" or "a->operator*(argument)",
   // i.e. the operator is called as a member function,
   // then the argument must be an expression.
-  bool OperatorCalledAsMemberFunction =
-  Prev->Previous && Prev->Previous->isOneOf(tok::period, tok::arrow);
-  Contexts.back().IsExpression = OperatorCalledAsMemberFunction;
+  // If faced with "operator+(argument)", i.e. the operator is called as
+  // a free function, then the argument is an expression only if the 
current
+  // line can't be a declaration.
+  bool IsOperatorCallSite =
+  (Prev->Previous &&
+   Prev->Previous->isOneOf(tok::period, tok::arrow)) ||
+  (!Line.MustBeDeclaration && !Line.InMacroBody);
+  Contexts.back().IsExpression = IsOperatorCallSite;
 } else if (OpeningParen.is(TT_VerilogInstancePortLParen)) {
   Contexts.back().IsExpression = true;
   Contexts.back().ContextType = Context::VerilogInstancePortList;


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -668,6 +668,33 @@
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+
+  Tokens = annotate("class Foo {\n"
+"  int operator+(a* b);\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(

[PATCH] D154184: [clang-format] Correctly annotate */&/&& in operator function calls

[Emilia Kond via Phabricator via cfe-commits]

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

Thanks, this probably makes more sense than what I did before




Comment at: clang/lib/Format/TokenAnnotator.cpp:3320
 
+  // Annotate */&/&& in `operator` function calls as binary operators.
+  for (const auto *Tok = Line.First; Tok; Tok = Tok->Next) {

I assume this extra fixup step means that the context of the call's parens 
won't be set to `IsExpression = true`? That won't cause any problems?


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

https://reviews.llvm.org/D154184

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

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

[Emilia Kond via Phabricator via cfe-commits]

rymiel added a comment.

This patch also fixes https://github.com/llvm/llvm-project/issues/52911 (which 
is probably a duplicate anyway)




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:583-584
+  ProbablyBracedList ||
+  NextTok->is(tok::l_brace) && LBraceStack.back().PrevTok &&
+  LBraceStack.back().PrevTok->is(tok::identifier);
 

llvm-project/clang/lib/Format/UnwrappedLineParser.cpp:583:71: warning: '&&' 
within '||' [-Wlogical-op-parentheses]
  NextTok->is(tok::l_brace) && LBraceStack.back().PrevTok &&
  ^~
llvm-project/clang/lib/Format/UnwrappedLineParser.cpp:583:71: note: place 
parentheses around the '&&' expression to silence this warning
  NextTok->is(tok::l_brace) && LBraceStack.back().PrevTok &&
  ^
  (
1 warning generated.



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

https://reviews.llvm.org/D150403

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

[PATCH] D150057: [clang-format] Fix consecutive alignments in #else blocks

[Emilia Kond via Phabricator via cfe-commits]

rymiel added inline comments.



Comment at: clang/unittests/Format/FormatTestComments.cpp:2762
   // These comments should *not* be aligned
-  EXPECT_NE( // change for EQ when fixed
+  EXPECT_EQ( // change for EQ when fixed
   "#if FOO\n"

Should probably get rid of this comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150057

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

[PATCH] D150614: [clang-format] Ignore first token when finding MustBreak

[Emilia Kond 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.

When in ColumnLimit 0, the formatter looks for MustBreakBefore in the
line in order to check if a line is allowed to be merged onto one line.

However, since MustBreakBefore is really a property of the gap between
the token and the one previously, I belive the check is erroneous in
checking all the tokens in a line, since whether the previous line ended
with a forced line break should have no effect on whether the current
line is allowed to merge with the next one.

This patch changes the check to skip the first token in
`LineJoiner.containsMustBreak`.

This patch also changes a test, which is not ideal, but I believe the
test also suffered from this bug. The test case in question sets
AllowShortFunctionsOnASingleLine to "Empty", but the empty function in
said test case isn't merged to a single line, because of the very same
bug this patch fixes.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150614

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
@@ -13831,6 +13831,20 @@
 "}",
 format("A()\n:b(0)\n{\n}", NoColumnLimit));
 
+  FormatStyle NoColumnLimitWrapAfterFunction = NoColumnLimit;
+  NoColumnLimitWrapAfterFunction.BreakBeforeBraces = FormatStyle::BS_Custom;
+  NoColumnLimitWrapAfterFunction.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  int foo { return 0; }\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  void foo {}\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+
   FormatStyle DoNotMergeNoColumnLimit = NoColumnLimit;
   DoNotMergeNoColumnLimit.AllowShortFunctionsOnASingleLine =
   FormatStyle::SFS_None;
@@ -20119,9 +20133,7 @@
"  int i = 5;\n"
"  }\n"
"#ifdef _DEBUG\n"
-   "void bar()\n"
-   "  {\n"
-   "  }\n"
+   "void bar() {}\n"
"#else\n"
"void bar()\n"
"  {\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -888,9 +888,14 @@
   }
 
   bool containsMustBreak(const AnnotatedLine *Line) {
-for (const FormatToken *Tok = Line->First; Tok; Tok = Tok->Next)
+for (const FormatToken *Tok = Line->First; Tok; Tok = Tok->Next) {
+  // Ignore the first token, because in this situation, it applies more
+  // to the last token of the previous line.
+  if (Tok == Line->First)
+continue;
   if (Tok->MustBreakBefore)
 return true;
+}
 return false;
   }
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13831,6 +13831,20 @@
 "}",
 format("A()\n:b(0)\n{\n}", NoColumnLimit));
 
+  FormatStyle NoColumnLimitWrapAfterFunction = NoColumnLimit;
+  NoColumnLimitWrapAfterFunction.BreakBeforeBraces = FormatStyle::BS_Custom;
+  NoColumnLimitWrapAfterFunction.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  int foo { return 0; }\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  void foo {}\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+
   FormatStyle DoNotMergeNoColumnLimit = NoColumnLimit;
   DoNotMergeNoColumnLimit.AllowShortFunctionsOnASingleLine =
   FormatStyle::SFS_None;
@@ -20119,9 +20133,7 @@
"  int i = 5;\n"
"  }\n"
"#ifdef _DEBUG\n"
-   "void bar()\n"
-   "  {\n"
-   "  }\n"
+   "void bar() {}\n"
"#else\n"
"void bar()\n"
"  {\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -888,9 +888,14 @@
   }
 
   bool containsMustBreak(const AnnotatedLine *Line) {
-for (const FormatToken

[PATCH] D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine

[Emilia Kond via Phabricator via cfe-commits]

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

I see nothing wrong with this patch alone as it currently stands, since it's a 
quite simple change to the LineJoiner, and I see it as one of the final 
stopgaps in clang-format's support for requires clauses, and I think it 
definitely beats the current buggy behavior where behaviour is conflated with 
BraceWrapping.AfterFunction.
But I'd like other reviewers to maybe have another look?


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] D150629: [clang-format] Don't allow template to be preceded by closing brace

[Emilia Kond 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.

This check is similar to the right paren check right below it, but it
doesn't need the overloaded operator check.

This patch prevents brace-initialized objects that are being compared
from being mis-annotated as template parameters.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150629

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
@@ -10679,6 +10679,7 @@
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("a < 0 ? b : a > 0 ? c : d;");
+  verifyFormat("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;");
   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
@@ -144,6 +144,8 @@
 if (Previous.Previous) {
   if (Previous.Previous->Tok.isLiteral())
 return false;
+  if (Previous.Previous->is(tok::r_brace))
+return false;
   if (Previous.Previous->is(tok::r_paren) && Contexts.size() > 1 &&
   (!Previous.Previous->MatchingParen ||
!Previous.Previous->MatchingParen->is(


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10679,6 +10679,7 @@
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("a < 0 ? b : a > 0 ? c : d;");
+  verifyFormat("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;");
   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
@@ -144,6 +144,8 @@
 if (Previous.Previous) {
   if (Previous.Previous->Tok.isLiteral())
 return false;
+  if (Previous.Previous->is(tok::r_brace))
+return false;
   if (Previous.Previous->is(tok::r_paren) && Contexts.size() > 1 &&
   (!Previous.Previous->MatchingParen ||
!Previous.Previous->MatchingParen->is(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150614: [clang-format] Ignore first token when finding MustBreak

[Emilia Kond via Phabricator via cfe-commits]

rymiel updated this revision to Diff 522453.
rymiel marked an inline comment as done.
rymiel added a comment.

Start iteration from second token


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150614

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
@@ -13831,6 +13831,20 @@
 "}",
 format("A()\n:b(0)\n{\n}", NoColumnLimit));
 
+  FormatStyle NoColumnLimitWrapAfterFunction = NoColumnLimit;
+  NoColumnLimitWrapAfterFunction.BreakBeforeBraces = FormatStyle::BS_Custom;
+  NoColumnLimitWrapAfterFunction.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  int foo { return 0; }\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  void foo {}\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+
   FormatStyle DoNotMergeNoColumnLimit = NoColumnLimit;
   DoNotMergeNoColumnLimit.AllowShortFunctionsOnASingleLine =
   FormatStyle::SFS_None;
@@ -20119,9 +20133,7 @@
"  int i = 5;\n"
"  }\n"
"#ifdef _DEBUG\n"
-   "void bar()\n"
-   "  {\n"
-   "  }\n"
+   "void bar() {}\n"
"#else\n"
"void bar()\n"
"  {\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -888,7 +888,10 @@
   }
 
   bool containsMustBreak(const AnnotatedLine *Line) {
-for (const FormatToken *Tok = Line->First; Tok; Tok = Tok->Next)
+assert(Line->First);
+// Ignore the first token, because in this situation, it applies more to 
the
+// last token of the previous line.
+for (const FormatToken *Tok = Line->First->Next; Tok; Tok = Tok->Next)
   if (Tok->MustBreakBefore)
 return true;
 return false;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13831,6 +13831,20 @@
 "}",
 format("A()\n:b(0)\n{\n}", NoColumnLimit));
 
+  FormatStyle NoColumnLimitWrapAfterFunction = NoColumnLimit;
+  NoColumnLimitWrapAfterFunction.BreakBeforeBraces = FormatStyle::BS_Custom;
+  NoColumnLimitWrapAfterFunction.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  int foo { return 0; }\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  void foo {}\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+
   FormatStyle DoNotMergeNoColumnLimit = NoColumnLimit;
   DoNotMergeNoColumnLimit.AllowShortFunctionsOnASingleLine =
   FormatStyle::SFS_None;
@@ -20119,9 +20133,7 @@
"  int i = 5;\n"
"  }\n"
"#ifdef _DEBUG\n"
-   "void bar()\n"
-   "  {\n"
-   "  }\n"
+   "void bar() {}\n"
"#else\n"
"void bar()\n"
"  {\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -888,7 +888,10 @@
   }
 
   bool containsMustBreak(const AnnotatedLine *Line) {
-for (const FormatToken *Tok = Line->First; Tok; Tok = Tok->Next)
+assert(Line->First);
+// Ignore the first token, because in this situation, it applies more to the
+// last token of the previous line.
+for (const FormatToken *Tok = Line->First->Next; Tok; Tok = Tok->Next)
   if (Tok->MustBreakBefore)
 return true;
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150614: [clang-format] Ignore first token when finding MustBreak

[Emilia Kond via Phabricator via cfe-commits]

rymiel added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:891-895
+for (const FormatToken *Tok = Line->First; Tok; Tok = Tok->Next) {
+  // Ignore the first token, because in this situation, it applies more
+  // to the last token of the previous line.
+  if (Tok == Line->First)
+continue;

owenpan wrote:
> We can start from the second token.
Thanks! I'm really glad to have reviewers to catch times where I wasn't 
thinking straight


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150614

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

[PATCH] D150614: [clang-format] Ignore first token when finding MustBreak

[Emilia Kond via Phabricator via cfe-commits]

rymiel added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:20136
"#ifdef _DEBUG\n"
-   "void bar()\n"
-   "  {\n"
-   "  }\n"
+   "void bar() {}\n"
"#else\n"

HazardyKnusperkeks wrote:
> This doesn't seem right.
WhitesmithsBraceStyle.AllowShortFunctionsOnASingleLine = 
FormatStyle::SFS_Empty; is set above


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150614

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

[PATCH] D150614: [clang-format] Ignore first token when finding MustBreak

[Emilia Kond via Phabricator via cfe-commits]

rymiel added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:20136
"#ifdef _DEBUG\n"
-   "void bar()\n"
-   "  {\n"
-   "  }\n"
+   "void bar() {}\n"
"#else\n"

rymiel wrote:
> HazardyKnusperkeks wrote:
> > This doesn't seem right.
> WhitesmithsBraceStyle.AllowShortFunctionsOnASingleLine = 
> FormatStyle::SFS_Empty; is set above
(this is mentioned in the summary)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150614

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

[PATCH] D150629: [clang-format] Don't allow template to be preceded by closing brace

[Emilia Kond via Phabricator via cfe-commits]

rymiel updated this revision to Diff 522776.
rymiel added a comment.

Add annotator tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150629

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
@@ -480,6 +480,23 @@
   EXPECT_TOKEN(Tokens[8], tok::greater, TT_BinaryOperator);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsNonTemplateAngleBrackets) {
+  auto Tokens = annotate("return a < b && c > d;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[6], tok::greater, TT_BinaryOperator);
+
+  Tokens = annotate("a < 0 ? b : a > 0 ? c : d;");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[7], tok::greater, TT_BinaryOperator);
+
+  Tokens = annotate("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;");
+  ASSERT_EQ(Tokens.size(), 27u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[20], tok::greater, TT_BinaryOperator);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.WhitespaceSensitiveMacros.push_back("FOO");
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10679,6 +10679,7 @@
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("a < 0 ? b : a > 0 ? c : d;");
+  verifyFormat("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;");
   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
@@ -144,6 +144,8 @@
 if (Previous.Previous) {
   if (Previous.Previous->Tok.isLiteral())
 return false;
+  if (Previous.Previous->is(tok::r_brace))
+return false;
   if (Previous.Previous->is(tok::r_paren) && Contexts.size() > 1 &&
   (!Previous.Previous->MatchingParen ||
!Previous.Previous->MatchingParen->is(


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -480,6 +480,23 @@
   EXPECT_TOKEN(Tokens[8], tok::greater, TT_BinaryOperator);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsNonTemplateAngleBrackets) {
+  auto Tokens = annotate("return a < b && c > d;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[6], tok::greater, TT_BinaryOperator);
+
+  Tokens = annotate("a < 0 ? b : a > 0 ? c : d;");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[7], tok::greater, TT_BinaryOperator);
+
+  Tokens = annotate("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;");
+  ASSERT_EQ(Tokens.size(), 27u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[20], tok::greater, TT_BinaryOperator);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.WhitespaceSensitiveMacros.push_back("FOO");
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10679,6 +10679,7 @@
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("a < 0 ? b : a > 0 ? c : d;");
+  verifyFormat("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;");
   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
@@ -144,6 +144,8 @@
 if (Previous.Previous) {
   if (Previous.Previous->Tok.isLiteral())
 return false;
+  if (Previous.Previous->is(tok::r_brace))
+return false;
   if (Previous.Previous->is(tok::r_paren) && Contexts.size() > 1 &&
   (!Previous.Previous->MatchingParen ||
!Previous.Previous->MatchingParen->is(
___
cfe-commits mailing list

[PATCH] D150629: [clang-format] Don't allow template to be preceded by closing brace

[Emilia Kond 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 rGe4d3e8880239: [clang-format] Don't allow template to be 
preceded by closing brace (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150629

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
@@ -480,6 +480,23 @@
   EXPECT_TOKEN(Tokens[8], tok::greater, TT_BinaryOperator);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsNonTemplateAngleBrackets) {
+  auto Tokens = annotate("return a < b && c > d;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[6], tok::greater, TT_BinaryOperator);
+
+  Tokens = annotate("a < 0 ? b : a > 0 ? c : d;");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[7], tok::greater, TT_BinaryOperator);
+
+  Tokens = annotate("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;");
+  ASSERT_EQ(Tokens.size(), 27u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[20], tok::greater, TT_BinaryOperator);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.WhitespaceSensitiveMacros.push_back("FOO");
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10679,6 +10679,7 @@
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("a < 0 ? b : a > 0 ? c : d;");
+  verifyFormat("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;");
   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
@@ -144,6 +144,8 @@
 if (Previous.Previous) {
   if (Previous.Previous->Tok.isLiteral())
 return false;
+  if (Previous.Previous->is(tok::r_brace))
+return false;
   if (Previous.Previous->is(tok::r_paren) && Contexts.size() > 1 &&
   (!Previous.Previous->MatchingParen ||
!Previous.Previous->MatchingParen->is(


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -480,6 +480,23 @@
   EXPECT_TOKEN(Tokens[8], tok::greater, TT_BinaryOperator);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsNonTemplateAngleBrackets) {
+  auto Tokens = annotate("return a < b && c > d;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[6], tok::greater, TT_BinaryOperator);
+
+  Tokens = annotate("a < 0 ? b : a > 0 ? c : d;");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[7], tok::greater, TT_BinaryOperator);
+
+  Tokens = annotate("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;");
+  ASSERT_EQ(Tokens.size(), 27u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[20], tok::greater, TT_BinaryOperator);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.WhitespaceSensitiveMacros.push_back("FOO");
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10679,6 +10679,7 @@
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("a < 0 ? b : a > 0 ? c : d;");
+  verifyFormat("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;");
   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
@@ -144,6 +144,8 @@
 if (Previous.Previous) {
   if (Previous.Previous->Tok.isLiteral())
 return false;
+  if (Previous.Previous->is(tok::r_brace))
+return false;
   if (Previous.Previous->is(tok::r_paren) && Contexts.size() > 1 &&
 

[PATCH] D150614: [clang-format] Ignore first token when finding MustBreak

[Emilia Kond via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG06763ea5d8f9: [clang-format] Ignore first token when finding 
MustBreak (authored by rymiel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150614

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
@@ -13832,6 +13832,20 @@
 "}",
 format("A()\n:b(0)\n{\n}", NoColumnLimit));
 
+  FormatStyle NoColumnLimitWrapAfterFunction = NoColumnLimit;
+  NoColumnLimitWrapAfterFunction.BreakBeforeBraces = FormatStyle::BS_Custom;
+  NoColumnLimitWrapAfterFunction.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  int foo { return 0; }\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  void foo {}\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+
   FormatStyle DoNotMergeNoColumnLimit = NoColumnLimit;
   DoNotMergeNoColumnLimit.AllowShortFunctionsOnASingleLine =
   FormatStyle::SFS_None;
@@ -20120,9 +20134,7 @@
"  int i = 5;\n"
"  }\n"
"#ifdef _DEBUG\n"
-   "void bar()\n"
-   "  {\n"
-   "  }\n"
+   "void bar() {}\n"
"#else\n"
"void bar()\n"
"  {\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -888,7 +888,10 @@
   }
 
   bool containsMustBreak(const AnnotatedLine *Line) {
-for (const FormatToken *Tok = Line->First; Tok; Tok = Tok->Next)
+assert(Line->First);
+// Ignore the first token, because in this situation, it applies more to 
the
+// last token of the previous line.
+for (const FormatToken *Tok = Line->First->Next; Tok; Tok = Tok->Next)
   if (Tok->MustBreakBefore)
 return true;
 return false;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13832,6 +13832,20 @@
 "}",
 format("A()\n:b(0)\n{\n}", NoColumnLimit));
 
+  FormatStyle NoColumnLimitWrapAfterFunction = NoColumnLimit;
+  NoColumnLimitWrapAfterFunction.BreakBeforeBraces = FormatStyle::BS_Custom;
+  NoColumnLimitWrapAfterFunction.BraceWrapping.AfterFunction = true;
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  int foo { return 0; }\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+  verifyFormat("class C {\n"
+   "#pragma foo\n"
+   "  void foo {}\n"
+   "};",
+   NoColumnLimitWrapAfterFunction);
+
   FormatStyle DoNotMergeNoColumnLimit = NoColumnLimit;
   DoNotMergeNoColumnLimit.AllowShortFunctionsOnASingleLine =
   FormatStyle::SFS_None;
@@ -20120,9 +20134,7 @@
"  int i = 5;\n"
"  }\n"
"#ifdef _DEBUG\n"
-   "void bar()\n"
-   "  {\n"
-   "  }\n"
+   "void bar() {}\n"
"#else\n"
"void bar()\n"
"  {\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -888,7 +888,10 @@
   }
 
   bool containsMustBreak(const AnnotatedLine *Line) {
-for (const FormatToken *Tok = Line->First; Tok; Tok = Tok->Next)
+assert(Line->First);
+// Ignore the first token, because in this situation, it applies more to the
+// last token of the previous line.
+for (const FormatToken *Tok = Line->First->Next; Tok; Tok = Tok->Next)
   if (Tok->MustBreakBefore)
 return true;
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150848: [clang-format] Respect ColumnLimit 0 lines breaks in inline asm

[Emilia Kond 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.

Previously, using ColumnLimit: 0 with extended inline asm with the
BreakBeforeInlineASMColon: OnlyMultiline option (the default style),
the formatter would act as if in Always mode, meaning a line break was
added before every colon in an extended inline assembly block.

This patch respects the already existing line breaks, and doesn't add
any new ones, if in ColumnLimit 0 mode.

Behaviour with Always stays as expected, with a break before every colon
regardless of any existing line breaks.

Behaviour with Never was broken before, and remains broken with this patch,
it is just never respected in ColumnLimit 0 mode.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150848

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4617,6 +4617,42 @@
 format("__asm   {\n"
"}\n"
"int   i;"));
+
+  auto Style = getLLVMStyleWithColumns(0);
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline;
+  verifyFormat("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));", Style);
+  EXPECT_EQ("asm(\"xyz\"\n"
+": \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));",
+format("asm(\"xyz\"\n"
+   ": \"=a\"(a), \"=d\"(b)\n"
+   ": \"a\"(data));",
+   Style));
+  EXPECT_EQ("asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));",
+format("asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n"
+   ": \"a\"(data));",
+   Style));
+
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Always;
+  EXPECT_EQ(
+  "asm(\"xyz\"\n"
+  ": \"=a\"(a), \"=d\"(b)\n"
+  ": \"a\"(data));",
+  format("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));", Style));
+  EXPECT_EQ("asm(\"xyz\"\n"
+": \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));",
+format("asm(\"xyz\"\n"
+   ": \"=a\"(a), \"=d\"(b)\n"
+   ": \"a\"(data));",
+   Style));
+  EXPECT_EQ("asm(\"xyz\"\n"
+": \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));",
+format("asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n"
+   ": \"a\"(data));",
+   Style));
 }
 
 TEST_F(FormatTest, FormatTryCatch) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -357,7 +357,8 @@
   if (Current.MustBreakBefore ||
   (Current.is(TT_InlineASMColon) &&
(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always ||
-Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline))) 
{
+(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline &&
+ Style.ColumnLimit != 0 {
 return true;
   }
   if (CurrentState.BreakBeforeClosingBrace &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4617,6 +4617,42 @@
 format("__asm   {\n"
"}\n"
"int   i;"));
+
+  auto Style = getLLVMStyleWithColumns(0);
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline;
+  verifyFormat("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));", Style);
+  EXPECT_EQ("asm(\"xyz\"\n"
+": \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));",
+format("asm(\"xyz\"\n"
+   ": \"=a\"(a), \"=d\"(b)\n"
+   ": \"a\"(data));",
+   Style));
+  EXPECT_EQ("asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));",
+format("asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n"
+   ": \"a\"(data));",
+   Style));
+
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Always;
+  EXPECT_EQ(
+  "asm(\"xyz\"\n"
+  ": \"=a\"(a), \"=d\"(b)\n"
+  ": \"a\"(data));",
+  format("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));", Style));
+  EXPECT_EQ("asm(\"xyz\"\n"
+": \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));",
+format("asm(\"xyz\"\n"
+   ": \"=a\"(a), \"=d\"(b)\n"
+ 

[PATCH] D150848: [clang-format] Respect ColumnLimit 0 lines breaks in inline asm

[Emilia Kond via Phabricator via cfe-commits]

rymiel updated this revision to Diff 530486.
rymiel added a comment.

Apply suggestions from review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150848

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4617,6 +4617,24 @@
 format("__asm   {\n"
"}\n"
"int   i;"));
+
+  auto Style = getLLVMStyleWithColumns(0);
+  const StringRef Code1{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));"};
+  const StringRef Code2{"asm(\"xyz\"\n"
+": \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));"};
+  const StringRef Code3{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));"};
+
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline;
+  verifyFormat(Code1, Style);
+  EXPECT_EQ(Code2, format(Code2, Style));
+  EXPECT_EQ(Code3, format(Code3, Style));
+
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Always;
+  EXPECT_EQ(Code2, format(Code1, Style));
+  EXPECT_EQ(Code2, format(Code2, Style));
+  EXPECT_EQ(Code2, format(Code3, Style));
 }
 
 TEST_F(FormatTest, FormatTryCatch) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -357,7 +357,8 @@
   if (Current.MustBreakBefore ||
   (Current.is(TT_InlineASMColon) &&
(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always ||
-Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline))) 
{
+(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline &&
+ Style.ColumnLimit > 0 {
 return true;
   }
   if (CurrentState.BreakBeforeClosingBrace &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4617,6 +4617,24 @@
 format("__asm   {\n"
"}\n"
"int   i;"));
+
+  auto Style = getLLVMStyleWithColumns(0);
+  const StringRef Code1{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));"};
+  const StringRef Code2{"asm(\"xyz\"\n"
+": \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));"};
+  const StringRef Code3{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n"
+": \"a\"(data));"};
+
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline;
+  verifyFormat(Code1, Style);
+  EXPECT_EQ(Code2, format(Code2, Style));
+  EXPECT_EQ(Code3, format(Code3, Style));
+
+  Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Always;
+  EXPECT_EQ(Code2, format(Code1, Style));
+  EXPECT_EQ(Code2, format(Code2, Style));
+  EXPECT_EQ(Code2, format(Code3, Style));
 }
 
 TEST_F(FormatTest, FormatTryCatch) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -357,7 +357,8 @@
   if (Current.MustBreakBefore ||
   (Current.is(TT_InlineASMColon) &&
(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always ||
-Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline))) {
+(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline &&
+ Style.ColumnLimit > 0 {
 return true;
   }
   if (CurrentState.BreakBeforeClosingBrace &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits