clang-format: fix spacing in `operator const char*()`

[krasimir]

Summary

Revision a75f8d98d7ac9e557b238a229a9a2647c71feed1 fixed spacing for operators,
but caused the const and non-const versions to diverge:

// With Style.PointerAlignment = FormatStyle::PAS_Left:

struct A {
  operator char*() { return ""; }
  operator const char *() const { return ""; }
};

The code was checking if the type specifier was directly preceded by operator.
However there could be comments and const/volatile in between.

[krasimir]

• Add more tests

[mprobst]

clang/lib/Format/TokenAnnotator.cpp

2729

typo dependent

2734

how many const and volatile may be there, and is their order fixed?

We also have the endsSequence helper on FormatToken that might express the concept better (Left.endsSequence(tok::kw_const, tok::kw_volatile) || ...). YMMV, with the tokens being optional that might lead to too many alternates to check for.

[krasimir]

• Address review comments

[krasimir]

• Fix typo

[krasimir]

clang/lib/Format/TokenAnnotator.cpp

2734

Thank you! I think const and volatile can only appear once, and not both at the same time.
There are 3 cases to check for with endsSequence which is ok, and it's less code.

[krasimir]

• Fix typo

[MyDeveloperDay]

@krasimir Thanks for fixing this, as it looks like I broke it.. its much appreciated

[sylvestre.ledru]

@krasimir @MyDeveloperDay @hans Looks like it is a regression from https://reviews.llvm.org/D72911
and the fix isn't in 10.0rc2.
Should we take it?

[hans]

In D72911#1900091, @sylvestre.ledru wrote:

@krasimir @MyDeveloperDay @hans Looks like it is a regression from https://reviews.llvm.org/D72911
and the fix isn't in 10.0rc2.
Should we take it?

Seems pretty safe. I've pushed it as 99e5b2ff9df5ca4c7fe13b63f60d953058cd9ca3. Thanks!

clang/lib/Format/TokenAnnotator.cpp

return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
              !Line.IsMultiVariableDeclStmt) &&
             Left.Previous &&
             !Left.Previous->isOneOf(tok::l_paren, tok::coloncolon,
                                     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()) &&
-      Left.Previous && Left.Previous->is(tok::kw_operator))
-    // Space between the type and the *
-    // operator void*(), operator char*(), operator Foo*() dependant
-    // on PointerAlignment style.
+      // 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);
   const auto SpaceRequiredForArrayInitializerLSquare =
       [](const FormatToken &LSquareTok, const FormatStyle &Style) {
         return Style.SpacesInContainerLiterals ||
                ((Style.Language == FormatStyle::LK_Proto ||
                  Style.Language == FormatStyle::LK_TextProto) &&
                 !Style.Cpp11BracedListStyle &&
                 LSquareTok.endsSequence(tok::l_square, tok::colon,
                                         TT_SelectorName));
       };
   if (Left.is(tok::l_square))
     return (Left.is(TT_ArrayInitializerLSquare) && Right.isNot(tok::r_square) &&
             SpaceRequiredForArrayInitializerLSquare(Left, Style)) ||
            (Left.isOneOf(TT_ArraySubscriptLSquare, TT_StructuredBindingLSquare,
                          TT_LambdaLSquare) &&
             Style.SpacesInSquareBrackets && Right.isNot(tok::r_square));
   if (Right.is(tok::r_square))
     return Right.MatchingParen &&
            ((Right.MatchingParen->is(TT_ArrayInitializerLSquare) &&
              SpaceRequiredForArrayInitializerLSquare(*Right.MatchingParen,
                                                      Style)) ||
             (Style.SpacesInSquareBrackets &&

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, OperatorSpacing) {
   verifyFormat("Foo::operator()(void &&);", Style);
   verifyFormat("Foo::operator&&(void &&);", Style);
   verifyFormat("Foo::operator&&();", Style);
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 
   Style.PointerAlignment = FormatStyle::PAS_Left;
   verifyFormat("Foo::operator*();", Style);
   verifyFormat("Foo::operator void*();", Style);
+  verifyFormat("Foo::operator/*comment*/ void*();", Style);
+  verifyFormat("Foo::operator/*a*/ const /*b*/ void*();", Style);
+  verifyFormat("Foo::operator/*a*/ volatile /*b*/ void*();", Style);
   verifyFormat("Foo::operator()(void*);", Style);
   verifyFormat("Foo::operator*(void*);", Style);
   verifyFormat("Foo::operator*();", Style);
   verifyFormat("operator*(int (*)(), class Foo);", Style);
 
   verifyFormat("Foo::operator&();", Style);
   verifyFormat("Foo::operator void&();", Style);
+  verifyFormat("Foo::operator/*comment*/ void&();", Style);
+  verifyFormat("Foo::operator/*a*/ const /*b*/ void&();", Style);
+  verifyFormat("Foo::operator/*a*/ volatile /*b*/ void&();", Style);
   verifyFormat("Foo::operator()(void&);", Style);
   verifyFormat("Foo::operator&(void&);", Style);
   verifyFormat("Foo::operator&();", Style);
   verifyFormat("operator&(int (&)(), class Foo);", Style);
 
   verifyFormat("Foo::operator&&();", Style);
   verifyFormat("Foo::operator void&&();", Style);
+  verifyFormat("Foo::operator/*comment*/ void&&();", Style);
+  verifyFormat("Foo::operator/*a*/ const /*b*/ void&&();", Style);
+  verifyFormat("Foo::operator/*a*/ volatile /*b*/ void&&();", Style);
   verifyFormat("Foo::operator()(void&&);", Style);
   verifyFormat("Foo::operator&&(void&&);", Style);
   verifyFormat("Foo::operator&&();", Style);
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   verifyFormat("Foo::operator*();", Style);
   verifyFormat("Foo::operator void *();", Style);