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 insert a space between :: and * #105043

Merged
merged 6 commits into from
Aug 23, 2024

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Aug 20, 2024

Also, don't insert a space after ::* for method pointers.

See #86253 (comment).

Fixes #100841.

Also, don't insert a space after ::* for method pointers.

See llvm#86253 (comment).

Fixes llvm#100841.
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 20, 2024

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Also, don't insert a space after ::* for method pointers.

See #86253 (comment).

Fixes #100841.


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

3 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+5-5)
  • (modified) clang/unittests/Format/FormatTest.cpp (+18-18)
  • (modified) clang/unittests/Format/QualifierFixerTest.cpp (+17-19)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 9d4204655b8ed6..f1f60bc0e8a72b 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4478,10 +4478,8 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
   }
   if (Left.is(tok::colon))
     return Left.isNot(TT_ObjCMethodExpr);
-  if (Left.is(tok::coloncolon)) {
-    return Right.is(tok::star) && Right.is(TT_PointerOrReference) &&
-           Style.PointerAlignment != FormatStyle::PAS_Left;
-  }
+  if (Left.is(tok::coloncolon))
+    return false;
   if (Left.is(tok::less) || Right.isOneOf(tok::greater, tok::less)) {
     if (Style.Language == FormatStyle::LK_TextProto ||
         (Style.Language == FormatStyle::LK_Proto &&
@@ -4591,7 +4589,9 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
     if (!BeforeLeft)
       return false;
     if (BeforeLeft->is(tok::coloncolon)) {
-      return Left.is(tok::star) &&
+      const auto *Prev = BeforeLeft->Previous;
+      return Left.is(tok::star) && Prev &&
+             !Prev->endsSequence(tok::identifier, TT_FunctionTypeLParen) &&
              Style.PointerAlignment != FormatStyle::PAS_Right;
     }
     return !BeforeLeft->isOneOf(tok::l_paren, tok::l_square);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 794ccab3704534..e895f16465491a 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -3646,8 +3646,8 @@ TEST_F(FormatTest, FormatsClasses) {
                "    : public aaaaaaaaaaaaaaaaaaa<aaaaaaaaaaaaaaaaaaaaa,\n"
                "                                 aaaaaaaaaaaaaaaaaaaaaa> {};");
   verifyFormat("template <class R, class C>\n"
-               "struct Aaaaaaaaaaaaaaaaa<R (C:: *)(int) const>\n"
-               "    : Aaaaaaaaaaaaaaaaa<R (C:: *)(int)> {};");
+               "struct Aaaaaaaaaaaaaaaaa<R (C::*)(int) const>\n"
+               "    : Aaaaaaaaaaaaaaaaa<R (C::*)(int)> {};");
   verifyFormat("class ::A::B {};");
 }
 
@@ -11166,10 +11166,10 @@ TEST_F(FormatTest, UnderstandsBinaryOperators) {
 }
 
 TEST_F(FormatTest, UnderstandsPointersToMembers) {
-  verifyFormat("int A:: *x;");
-  verifyFormat("int (S:: *func)(void *);");
-  verifyFormat("void f() { int (S:: *func)(void *); }");
-  verifyFormat("typedef bool *(Class:: *Member)() const;");
+  verifyFormat("int A::*x;");
+  verifyFormat("int (S::*func)(void *);");
+  verifyFormat("void f() { int (S::*func)(void *); }");
+  verifyFormat("typedef bool *(Class::*Member)() const;");
   verifyFormat("void f() {\n"
                "  (a->*f)();\n"
                "  a->*x;\n"
@@ -11187,16 +11187,16 @@ TEST_F(FormatTest, UnderstandsPointersToMembers) {
 
   FormatStyle Style = getLLVMStyle();
   EXPECT_EQ(Style.PointerAlignment, FormatStyle::PAS_Right);
-  verifyFormat("typedef bool *(Class:: *Member)() const;", Style);
-  verifyFormat("void f(int A:: *p) { int A:: *v = &A::B; }", Style);
+  verifyFormat("typedef bool *(Class::*Member)() const;", Style);
+  verifyFormat("void f(int A::*p) { int A::*v = &A::B; }", Style);
 
   Style.PointerAlignment = FormatStyle::PAS_Left;
-  verifyFormat("typedef bool* (Class::* Member)() const;", Style);
+  verifyFormat("typedef bool* (Class::*Member)() const;", Style);
   verifyFormat("void f(int A::* p) { int A::* v = &A::B; }", Style);
 
   Style.PointerAlignment = FormatStyle::PAS_Middle;
-  verifyFormat("typedef bool * (Class:: * Member)() const;", Style);
-  verifyFormat("void f(int A:: * p) { int A:: * v = &A::B; }", Style);
+  verifyFormat("typedef bool * (Class::*Member)() const;", Style);
+  verifyFormat("void f(int A::* p) { int A::* v = &A::B; }", Style);
 }
 
 TEST_F(FormatTest, UnderstandsUnaryOperators) {
@@ -12539,7 +12539,7 @@ TEST_F(FormatTest, FormatsFunctionTypes) {
   verifyFormat("int (*func)(void *);");
   verifyFormat("void f() { int (*func)(void *); }");
   verifyFormat("template <class CallbackClass>\n"
-               "using Callback = void (CallbackClass:: *)(SomeObject *Data);");
+               "using MyCallback = void (CallbackClass::*)(SomeObject *Data);");
 
   verifyGoogleFormat("A<void*(int*, SomeType*)>;");
   verifyGoogleFormat("void* (*a)(int);");
@@ -19462,13 +19462,13 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
                "int   bbbbbbb = 0;",
                Alignment);
   // http://llvm.org/PR68079
-  verifyFormat("using Fn   = int (A:: *)();\n"
-               "using RFn  = int (A:: *)() &;\n"
-               "using RRFn = int (A:: *)() &&;",
+  verifyFormat("using Fn   = int (A::*)();\n"
+               "using RFn  = int (A::*)() &;\n"
+               "using RRFn = int (A::*)() &&;",
                Alignment);
-  verifyFormat("using Fn   = int (A:: *)();\n"
-               "using RFn  = int *(A:: *)() &;\n"
-               "using RRFn = double (A:: *)() &&;",
+  verifyFormat("using Fn   = int (A::*)();\n"
+               "using RFn  = int *(A::*)() &;\n"
+               "using RRFn = double (A::*)() &&;",
                Alignment);
 
   // PAS_Right
diff --git a/clang/unittests/Format/QualifierFixerTest.cpp b/clang/unittests/Format/QualifierFixerTest.cpp
index 3a5f63e5de65b4..f9255c6e4c7088 100644
--- a/clang/unittests/Format/QualifierFixerTest.cpp
+++ b/clang/unittests/Format/QualifierFixerTest.cpp
@@ -305,7 +305,7 @@ TEST_F(QualifierFixerTest, RightQualifier) {
   verifyFormat("Foo inline static const;", "Foo inline const static;", Style);
   verifyFormat("Foo inline static const;", Style);
 
-  verifyFormat("Foo<T volatile>::Bar<Type const, 5> const volatile A:: *;",
+  verifyFormat("Foo<T volatile>::Bar<Type const, 5> const volatile A::*;",
                "volatile const Foo<volatile T>::Bar<const Type, 5> A::*;",
                Style);
 
@@ -523,15 +523,14 @@ TEST_F(QualifierFixerTest, RightQualifier) {
   verifyFormat("const INTPTR a;", Style);
 
   // Pointers to members
-  verifyFormat("int S:: *a;", Style);
-  verifyFormat("int const S:: *a;", "const int S:: *a;", Style);
-  verifyFormat("int const S:: *const a;", "const int S::* const a;", Style);
-  verifyFormat("int A:: *const A:: *p1;", Style);
-  verifyFormat("float (C:: *p)(int);", Style);
-  verifyFormat("float (C:: *const p)(int);", Style);
-  verifyFormat("float (C:: *p)(int) const;", Style);
-  verifyFormat("float const (C:: *p)(int);", "const float (C::*p)(int);",
-               Style);
+  verifyFormat("int S::*a;", Style);
+  verifyFormat("int const S::*a;", "const int S::*a;", Style);
+  verifyFormat("int const S::*const a;", "const int S::* const a;", Style);
+  verifyFormat("int A::*const A::*p1;", Style);
+  verifyFormat("float (C::*p)(int);", Style);
+  verifyFormat("float (C::*const p)(int);", Style);
+  verifyFormat("float (C::*p)(int) const;", Style);
+  verifyFormat("float const (C::*p)(int);", "const float (C::*p)(int);", Style);
 }
 
 TEST_F(QualifierFixerTest, LeftQualifier) {
@@ -831,15 +830,14 @@ TEST_F(QualifierFixerTest, LeftQualifier) {
   verifyFormat("INTPTR const a;", Style);
 
   // Pointers to members
-  verifyFormat("int S:: *a;", Style);
-  verifyFormat("const int S:: *a;", "int const S:: *a;", Style);
-  verifyFormat("const int S:: *const a;", "int const S::* const a;", Style);
-  verifyFormat("int A:: *const A:: *p1;", Style);
-  verifyFormat("float (C:: *p)(int);", Style);
-  verifyFormat("float (C:: *const p)(int);", Style);
-  verifyFormat("float (C:: *p)(int) const;", Style);
-  verifyFormat("const float (C:: *p)(int);", "float const (C::*p)(int);",
-               Style);
+  verifyFormat("int S::*a;", Style);
+  verifyFormat("const int S::*a;", "int const S::*a;", Style);
+  verifyFormat("const int S::*const a;", "int const S::*const a;", Style);
+  verifyFormat("int A::*const A::*p1;", Style);
+  verifyFormat("float (C::*p)(int);", Style);
+  verifyFormat("float (C::*const p)(int);", Style);
+  verifyFormat("float (C::*p)(int) const;", Style);
+  verifyFormat("const float (C::*p)(int);", "float const (C::*p)(int);", Style);
 }
 
 TEST_F(QualifierFixerTest, ConstVolatileQualifiersOrder) {

@owenca owenca requested a review from kadircet August 20, 2024 15:59
Copy link
Member

@kadircet kadircet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot for the quick fix!

clang/lib/Format/TokenAnnotator.cpp Outdated Show resolved Hide resolved
clang/lib/Format/TokenAnnotator.cpp Outdated Show resolved Hide resolved
@owenca owenca added this to the LLVM 19.X Release milestone Aug 22, 2024
Copy link
Member

@kadircet kadircet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot!

clang/lib/Format/TokenAnnotator.cpp Outdated Show resolved Hide resolved
@owenca owenca merged commit 714033a into llvm:main Aug 23, 2024
8 checks passed
@owenca owenca deleted the pointer-to-member branch August 23, 2024 03:02
@owenca
Copy link
Contributor Author

owenca commented Aug 23, 2024

/cherry-pick 714033a

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Aug 23, 2024
Also, don't insert a space after ::* for method pointers.

See
llvm#86253 (comment).

Fixes llvm#100841.

(cherry picked from commit 714033a)
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 23, 2024

/pull-request #105773

cjdb pushed a commit to cjdb/llvm-project that referenced this pull request Aug 23, 2024
Also, don't insert a space after ::* for method pointers.

See
llvm#86253 (comment).

Fixes llvm#100841.
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Aug 26, 2024
Also, don't insert a space after ::* for method pointers.

See
llvm#86253 (comment).

Fixes llvm#100841.

(cherry picked from commit 714033a)
dmpolukhin pushed a commit to dmpolukhin/llvm-project that referenced this pull request Sep 2, 2024
Also, don't insert a space after ::* for method pointers.

See
llvm#86253 (comment).

Fixes llvm#100841.
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 19-rc1 regression: member function pointers as template arguments
4 participants