[PATCH] D76850: clang-format: Fix pointer alignment for overloaded operators (PR45107)

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.
hans added reviewers: sammccall, MyDeveloperDay, thakis.
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

LGTM


This fixes a regression from D69573 <https://reviews.llvm.org/D69573> which broke the following example:

  $ echo 'operator C<T>*();' | bin/clang-format --style=Chromium
  operator C<T> *();

(There should be no space between before the asterisk.)

It seems the problem is in TokenAnnotator::spaceRequiredBetween(), which only looked at the token to the left of the * to see if it was a type or not. That code only handled simple types or identifiers, not templates or qualified types. This patch addresses that.

Please take a look. I'm not familiar with the code here, so comments are welcome.


https://reviews.llvm.org/D76850

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15386,6 +15386,11 @@
   verifyFormat("Foo::operator&&();", Style);
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 
+  // PR45107
+  verifyFormat("operator Vector<String>&();", Style);
+  verifyFormat("operator foo::Bar*();", Style);
+  verifyFormat("operator const Foo<X>::Bar<Y>*();", Style);
+
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   verifyFormat("Foo::operator*();", Style);
   verifyFormat("Foo::operator void *();", Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2799,20 +2799,38 @@
                                     tok::l_square));
   if (Right.is(tok::star) && Left.is(tok::l_paren))
     return false;
-  if (Right.isOneOf(tok::star, tok::amp, tok::ampamp) &&
-      (Left.is(tok::identifier) || Left.isSimpleTypeSpecifier()) &&
-      // Space between the type and the * in:
-      //   operator void*()
-      //   operator char*()
-      //   operator /*comment*/ const char*()
-      //   operator volatile /*comment*/ char*()
-      //   operator Foo*()
-      // dependent on PointerAlignment style.
-      Left.Previous &&
-      (Left.Previous->endsSequence(tok::kw_operator) ||
-       Left.Previous->endsSequence(tok::kw_const, tok::kw_operator) ||
-       Left.Previous->endsSequence(tok::kw_volatile, tok::kw_operator)))
-    return (Style.PointerAlignment != FormatStyle::PAS_Left);
+  if (Right.isOneOf(tok::star, tok::amp, tok::ampamp)) {
+    const FormatToken *Previous = &Left;
+    while (Previous && !Previous->is(tok::kw_operator)) {
+      if (Previous->is(tok::identifier) || Previous->isSimpleTypeSpecifier()) {
+        Previous = Previous->Previous;
+        continue;
+      }
+      if (Previous->is(TT_TemplateCloser) && Previous->MatchingParen) {
+        Previous = Previous->MatchingParen->Previous;
+        continue;
+      }
+      if (Previous->is(tok::coloncolon)) {
+        Previous = Previous->Previous;
+        continue;
+      }
+      break;
+    }
+    // Space between the type and the * in:
+    //   operator void*()
+    //   operator char*()
+    //   operator /*comment*/ const char*()
+    //   operator volatile /*comment*/ char*()
+    //   operator Foo*()
+    //   operator C<T>*()
+    //   operator std::Foo*()
+    //   operator C<T>::D<U>*()
+    // dependent on PointerAlignment style.
+    if (Previous && (Previous->endsSequence(tok::kw_operator) ||
+       Previous->endsSequence(tok::kw_const, tok::kw_operator) ||
+       Previous->endsSequence(tok::kw_volatile, tok::kw_operator)))
+      return (Style.PointerAlignment != FormatStyle::PAS_Left);
+  }
   const auto SpaceRequiredForArrayInitializerLSquare =
       [](const FormatToken &LSquareTok, const FormatStyle &Style) {
         return Style.SpacesInContainerLiterals ||


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76850.252842.patch
Type: text/x-patch
Size: 3095 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200326/a5399e74/attachment.bin>