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 over-indent comment below unbraced body #95354

Merged
merged 1 commit into from
Jun 15, 2024

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Jun 13, 2024

Fixes #45002.

@llvmbot
Copy link
Member

llvmbot commented Jun 13, 2024

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Fixes #45002.


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

3 Files Affected:

  • (modified) clang/lib/Format/UnwrappedLineParser.cpp (+5)
  • (modified) clang/lib/Format/UnwrappedLineParser.h (+3)
  • (modified) clang/unittests/Format/FormatTestComments.cpp (+30)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 08387d2e08ee0..899e68eadf70e 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -112,6 +112,7 @@ class ScopedLineState {
     Parser.Line->PPLevel = PreBlockLine->PPLevel;
     Parser.Line->InPPDirective = PreBlockLine->InPPDirective;
     Parser.Line->InMacroBody = PreBlockLine->InMacroBody;
+    Parser.Line->UnbracedBodyLevel = PreBlockLine->UnbracedBodyLevel;
   }
 
   ~ScopedLineState() {
@@ -2708,7 +2709,9 @@ void UnwrappedLineParser::parseUnbracedBody(bool CheckEOF) {
 
   addUnwrappedLine();
   ++Line->Level;
+  ++Line->UnbracedBodyLevel;
   parseStructuralElement();
+  --Line->UnbracedBodyLevel;
 
   if (Tok) {
     assert(!Line->InPPDirective);
@@ -4836,6 +4839,8 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
           PPBranchLevel > 0) {
         Line->Level += PPBranchLevel;
       }
+      assert(Line->Level >= Line->UnbracedBodyLevel);
+      Line->Level -= Line->UnbracedBodyLevel;
       flushComments(isOnNewLine(*FormatTok));
       parsePPDirective();
       PreviousWasComment = FormatTok->is(tok::comment);
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index d7963a4211bb9..d5eeb3d57149c 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -49,6 +49,9 @@ struct UnwrappedLine {
   /// Whether it is part of a macro body.
   bool InMacroBody = false;
 
+  /// Nesting level of unbraced body of a control statement.
+  unsigned UnbracedBodyLevel = 0;
+
   bool MustBeDeclaration = false;
 
   /// Whether the parser has seen \c decltype(auto) in this line.
diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp
index d2baace6a7d80..3e75707a9faec 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -1087,6 +1087,36 @@ TEST_F(FormatTestComments, KeepsLevelOfCommentBeforePPDirective) {
                Style);
 }
 
+TEST_F(FormatTestComments, CommentsBetweenUnbracedBodyAndPPDirective) {
+  verifyFormat("{\n"
+               "  if (a)\n"
+               "    f(); // comment\n"
+               "#define A\n"
+               "}");
+
+  verifyFormat("{\n"
+               "  while (a)\n"
+               "    f();\n"
+               "// comment\n"
+               "#define A\n"
+               "}");
+
+  verifyNoChange("{\n"
+                 "  if (a)\n"
+                 "    f();\n"
+                 "  // comment\n"
+                 "#define A\n"
+                 "}");
+
+  verifyNoChange("{\n"
+                 "  while (a)\n"
+                 "    if (b)\n"
+                 "      f();\n"
+                 "  // comment\n"
+                 "#define A\n"
+                 "}");
+}
+
 TEST_F(FormatTestComments, SplitsLongLinesInComments) {
   // FIXME: Do we need to fix up the "  */" at the end?
   // It doesn't look like any of our current logic triggers this.

@owenca owenca merged commit cddb9ce into llvm:main Jun 15, 2024
9 checks passed
@owenca owenca deleted the 45002 branch June 15, 2024 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad comment indentation before ifdef after braceless if
3 participants