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] Fix parsing of operator<() {} #75144

Merged
merged 3 commits into from Dec 13, 2023

Conversation

XDeme1
Copy link
Contributor

@XDeme1 XDeme1 commented Dec 12, 2023

Fixes #74876.

During the parsing of operator<(Foo&) {}, there was no handling for the operator<, so it called consumeToken() again, causing the AnnotationParser::Scopes to have one additional left brace each time it tried to parse it, leaving it unbalanced.
Because of this, in the following code:

class Foo {
  void operator<(Foo&) {}
  Foo& f;
};

The & in the reference member, was being interpreted as TT_BinaryOperator instead of TT_PointerOrReference.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 12, 2023

@llvm/pr-subscribers-clang-format

Author: None (XDeme)

Changes

Fixes llvm/llvm-project#74876.

During the parsing of operator&lt;(Foo&amp;) {}, there was no handling for the operator<, so it called consumeToken() again starting from (Foo&amp;), causing the AnnotationParser::Scopes to have one additional left brace each time it tried to parse it, leaving it unbalanced.
Because of this, in the following code:

class Foo {
  void operator&lt;(Foo&amp;) {}
  Foo&amp; f;
};

The &amp; in the reference member, was being interpreted as TT_BinaryOperator instead of TT_PointerOrReference.


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

3 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+9)
  • (modified) clang/unittests/Format/FormatTest.cpp (+7)
  • (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+11)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index eaccb5881ca30f..957612088c7bb0 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -23,6 +23,9 @@
 
 namespace clang {
 namespace format {
+static bool isFunctionDeclarationName(bool IsCpp, const FormatToken &Current,
+                                      const AnnotatedLine &Line,
+                                      FormatToken *&ClosingParen);
 
 static bool mustBreakAfterAttributes(const FormatToken &Tok,
                                      const FormatStyle &Style) {
@@ -164,6 +167,12 @@ class AnnotatingParser {
                TT_OverloadedOperatorLParen))) {
         return false;
       }
+      FormatToken *ClosingParen = nullptr;
+      if (Previous.Previous->is(tok::kw_operator) &&
+          isFunctionDeclarationName(Style.isCpp(), *Previous.Previous, Line,
+                                    ClosingParen)) {
+        return false;
+      }
     }
 
     FormatToken *Left = CurrentToken->Previous;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 24b2fd599dc397..79013a473a7c2e 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -11727,6 +11727,13 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
                "  void func(type &a) { a & member; }\n"
                "  anotherType &member;\n"
                "}");
+
+  Style.ReferenceAlignment = FormatStyle::RAS_Left;
+  verifyFormat("class Foo {\n"
+               "  void operator<(Foo&) {}\n"
+               "  Foo& f;\n"
+               "};",
+               Style);
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 65b1f0f4b57659..58a782f909d6aa 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -298,6 +298,17 @@ TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::identifier, TT_TypeName);
   EXPECT_TOKEN(Tokens[3], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("class Foo {\n"
+                    "void operator<(Foo&) {}\n"
+                    "Foo& f;\n"
+                    "};");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::kw_operator, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::less, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_OverloadedOperatorLParen);
+  EXPECT_TOKEN(Tokens[10], tok::l_brace, TT_FunctionLBrace);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {

clang/unittests/Format/FormatTest.cpp Outdated Show resolved Hide resolved
clang/unittests/Format/TokenAnnotatorTest.cpp Outdated Show resolved Hide resolved
@XDeme1
Copy link
Contributor Author

XDeme1 commented Dec 13, 2023

Thanks for reviewing. If the patch is ok, could you land this patch for me? I don't have commit access.

@owenca owenca merged commit 9512d6d into llvm:main Dec 13, 2023
3 of 4 checks passed
@XDeme1 XDeme1 deleted the ReferenceAlignment branch December 13, 2023 21:58
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.

Extra space is sometimes placed before the C++ '&' operator, regression in clang-format 17
3 participants