Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-format] Don't remove parentheses in macro definitions #81444

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Feb 12, 2024

Closes #81399.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 12, 2024

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Closes #81399.


Full diff: https://github.com/llvm/llvm-project/pull/81444.diff

2 Files Affected:

  • (modified) clang/lib/Format/UnwrappedLineParser.cpp (+1-1)
  • (modified) clang/unittests/Format/FormatTest.cpp (+2)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index d4f9b3f9df524e..e660a1dfff968e 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2515,7 +2515,7 @@ bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
         parseChildBlock();
       break;
     case tok::r_paren:
-      if (!MightBeStmtExpr &&
+      if (!MightBeStmtExpr && !Line->InMacroBody &&
           Style.RemoveParentheses > FormatStyle::RPS_Leave) {
         const auto *Prev = LeftParen->Previous;
         const auto *Next = Tokens->peekNextToken();
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 7b65c8d6e21b00..13937a15fdaee2 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -26972,6 +26972,7 @@ TEST_F(FormatTest, RemoveParentheses) {
   EXPECT_EQ(Style.RemoveParentheses, FormatStyle::RPS_Leave);
 
   Style.RemoveParentheses = FormatStyle::RPS_MultipleParentheses;
+  verifyFormat("#define Foo(...) foo((__VA_ARGS__))", Style);
   verifyFormat("int x __attribute__((aligned(16))) = 0;", Style);
   verifyFormat("decltype((foo->bar)) baz;", Style);
   verifyFormat("class __declspec(dllimport) X {};",
@@ -27006,6 +27007,7 @@ TEST_F(FormatTest, RemoveParentheses) {
   verifyFormat("return (({ 0; }));", "return ((({ 0; })));", Style);
 
   Style.RemoveParentheses = FormatStyle::RPS_ReturnStatement;
+  verifyFormat("#define Return0 return (0);", Style);
   verifyFormat("return 0;", "return (0);", Style);
   verifyFormat("co_return 0;", "co_return ((0));", Style);
   verifyFormat("return 0;", "return (((0)));", Style);

@prj-
Copy link

prj- commented Feb 12, 2024

Any hope that this lands in 18.1? Thanks anyway!

@owenca owenca merged commit 4af24d4 into llvm:main Feb 13, 2024
5 of 6 checks passed
@owenca owenca deleted the RemoveParens branch February 13, 2024 03:20
@owenca owenca added this to the LLVM 18.X Release milestone Feb 13, 2024
@owenca
Copy link
Contributor Author

owenca commented Feb 13, 2024

/cherry-pick 4af24d4

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Feb 13, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 13, 2024

/pull-request #81566

@owenca
Copy link
Contributor Author

owenca commented Feb 13, 2024

Any hope that this lands in 18.1? Thanks anyway!

Most likely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

[clang-format] Faulty RemoveParentheses: MultipleParentheses/ReturnStatement
4 participants