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][NFC] Refactor getting first/last non-comment of line #74570

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Dec 6, 2023

No description provided.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 6, 2023

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

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

2 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.h (+5)
  • (modified) clang/lib/Format/UnwrappedLineFormatter.cpp (+6-13)
diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h
index 58e2cf79f488f..05a6daa87d803 100644
--- a/clang/lib/Format/TokenAnnotator.h
+++ b/clang/lib/Format/TokenAnnotator.h
@@ -156,6 +156,11 @@ class AnnotatedLine {
     return First->is(tok::comment) ? First->getNextNonComment() : First;
   }
 
+  FormatToken *getLastNonComment() const {
+    assert(Last);
+    return Last->is(tok::comment) ? Last->getPreviousNonComment() : Last;
+  }
+
   FormatToken *First;
   FormatToken *Last;
 
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 40730cd53529e..b4930c2e4621d 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -346,14 +346,10 @@ class LineJoiner {
             return false;
 
           // Check if the found line starts a record.
-          const FormatToken *LastNonComment = Line->Last;
+          const auto *LastNonComment = Line->getLastNonComment();
+          // There must be another token (usually `{`), because we chose a
+          // non-PPDirective and non-comment line that has a smaller level.
           assert(LastNonComment);
-          if (LastNonComment->is(tok::comment)) {
-            LastNonComment = LastNonComment->getPreviousNonComment();
-            // There must be another token (usually `{`), because we chose a
-            // non-PPDirective and non-comment line that has a smaller level.
-            assert(LastNonComment);
-          }
           return isRecordLBrace(*LastNonComment);
         }
       }
@@ -363,12 +359,9 @@ class LineJoiner {
 
     bool MergeShortFunctions = ShouldMergeShortFunctions();
 
-    const FormatToken *FirstNonComment = TheLine->First;
-    if (FirstNonComment->is(tok::comment)) {
-      FirstNonComment = FirstNonComment->getNextNonComment();
-      if (!FirstNonComment)
-        return 0;
-    }
+    const auto *FirstNonComment = TheLine->getFirstNonComment();
+    if (!FirstNonComment)
+      return 0;
     // FIXME: There are probably cases where we should use FirstNonComment
     // instead of TheLine->First.
 

@owenca owenca merged commit 1241b5b into llvm:main Dec 6, 2023
4 of 5 checks passed
@owenca owenca deleted the refactor branch December 6, 2023 19:45
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.

None yet

3 participants