[PATCH] D80933: [clang-format] [PR46157] Wrong spacing of negative literals with use of operator

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: krasimir, JakeMerdichAMD, curdeius, mitchell-stellar.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay added a reviewer: thopre.
MyDeveloperDay edited the summary of this revision.

https://bugs.llvm.org/show_bug.cgi?id=46157

  class A;
  
  void foo(A (*)(const A&, const A&), int);
  A operator+(const A&, const A&);
  
  void bar() {
    return foo(operator+, -42);
  }

is incorrect formatted putting a space between the `-` and the 42

  return foo(operator+, - 42);

This revision tries to refine the annotating of the OverloadOperator to only mark the comma as an operator if its immediately after the operator keyword and to not mark the closing '(' as an TT_OverloadedOperatorLParen unless its actually a `(`

This code was assuming the usage of operator too much I suspect and not considering its use as a function ptr.

Probably a fairly rare corner case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80933

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
@@ -16417,6 +16417,17 @@
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 }
 
+TEST_F(FormatTest, OperatorPassedAsAFunctionPtr) {
+  FormatStyle Style = getLLVMStyle();
+  // PR46157
+  verifyFormat("foo(operator+, -42);", Style);
+  verifyFormat("foo(operator++, -42);", Style);
+  verifyFormat("foo(operator--, -42);", Style);
+  verifyFormat("foo(-42, operator--);", Style);
+  verifyFormat("foo(-42, operator, );", Style);
+  verifyFormat("foo(operator, , -42);", Style);
+}
+
 TEST_F(FormatTest, VeryLongNamespaceCommentSplit) {
   // These tests are not in NamespaceFixer because that doesn't
   // test its interaction with line wrapping
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -978,16 +978,18 @@
         if (CurrentToken->isOneOf(tok::star, tok::amp))
           CurrentToken->setType(TT_PointerOrReference);
         consumeToken();
+        if (CurrentToken->is(tok::comma) &&
+            CurrentToken->Previous->isNot(tok::kw_operator))
+          break;
         if (CurrentToken && CurrentToken->Previous->isOneOf(
                                 TT_BinaryOperator, TT_UnaryOperator, tok::comma,
                                 tok::star, tok::arrow, tok::amp, tok::ampamp))
           CurrentToken->Previous->setType(TT_OverloadedOperator);
       }
-      if (CurrentToken) {
+      if (CurrentToken && CurrentToken->is(tok::l_paren))
         CurrentToken->setType(TT_OverloadedOperatorLParen);
-        if (CurrentToken->Previous->is(TT_BinaryOperator))
-          CurrentToken->Previous->setType(TT_OverloadedOperator);
-      }
+      if (CurrentToken && CurrentToken->Previous->is(TT_BinaryOperator))
+        CurrentToken->Previous->setType(TT_OverloadedOperator);
       break;
     case tok::question:
       if (Tok->is(TT_CSharpNullConditionalLSquare)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80933.267635.patch
Type: text/x-patch
Size: 2148 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200601/10e72684/attachment.bin>