[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: mitchell-stellar, klimek, owenpan, sammccall.
MyDeveloperDay added projects: clang-format, clang-tools-extra.
Herald added a project: clang.

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

Addressing bug related to returning after return type not being honoured for 
some operator types.

  $ bin/clang-format --style="{BasedOnStyle: llvm, AlwaysBreakAfterReturnType: 
TopLevelDefinitions}" /tmp/foo.cpp
  class Foo {
  public:
    bool operator!() const;
    bool operator<(Foo const &) const;
    bool operator*() const;
    bool operator->() const;
    bool operator+() const;
    bool operator-() const;
    bool f() const;
  };
  
  bool Foo::operator!() const { return true; }
  bool
  Foo::operator<(Foo const &) const {
    return true;
  }
  bool Foo::operator*() const { return true; }
  bool Foo::operator->() const { return true; }
  bool
  Foo::operator+() const {
    return true;
  }
  bool
  Foo::operator-() const {
    return true;
  }
  bool
  Foo::f() const {
    return true;
  }


Repository:
  rC Clang

https://reviews.llvm.org/D69573

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
@@ -6111,7 +6111,13 @@
                "void\n"
                "A::operator>>() {}\n"
                "void\n"
-               "A::operator+() {}\n",
+               "A::operator+() {}\n"
+               "void\n"
+               "A::operator*() {}\n"
+               "void\n"
+               "A::operator->() {}\n"
+               "void\n"
+               "A::operator!() {}\n",
                Style);
   verifyFormat("void *operator new(std::size_t s);", // No break here.
                Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1344,8 +1344,10 @@
         Contexts.back().IsExpression = false;
     } else if (Current.is(tok::kw_new)) {
       Contexts.back().CanBeExpression = false;
-    } else if (Current.isOneOf(tok::semi, tok::exclaim)) {
+    } else if (Current.isOneOf(tok::semi, tok::exclaim) &&
+               !(Current.Previous && Current.Previous->is(tok::kw_operator))) {
       // This should be the condition or increment in a for-loop.
+      // but not operator !()
       Contexts.back().IsExpression = true;
     }
   }
@@ -2085,6 +2087,8 @@
         return Next;
       if (Next->is(TT_OverloadedOperator))
         continue;
+      if (Next->isOneOf(tok::star, tok::arrow))
+        continue;
       if (Next->isOneOf(tok::kw_new, tok::kw_delete)) {
         // For 'new[]' and 'delete[]'.
         if (Next->Next && Next->Next->is(tok::l_square) && Next->Next->Next &&



Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6111,7 +6111,13 @@
                "void\n"
                "A::operator>>() {}\n"
                "void\n"
-               "A::operator+() {}\n",
+               "A::operator+() {}\n"
+               "void\n"
+               "A::operator*() {}\n"
+               "void\n"
+               "A::operator->() {}\n"
+               "void\n"
+               "A::operator!() {}\n",
                Style);
   verifyFormat("void *operator new(std::size_t s);", // No break here.
                Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1344,8 +1344,10 @@
         Contexts.back().IsExpression = false;
     } else if (Current.is(tok::kw_new)) {
       Contexts.back().CanBeExpression = false;
-    } else if (Current.isOneOf(tok::semi, tok::exclaim)) {
+    } else if (Current.isOneOf(tok::semi, tok::exclaim) &&
+               !(Current.Previous && Current.Previous->is(tok::kw_operator))) {
       // This should be the condition or increment in a for-loop.
+      // but not operator !()
       Contexts.back().IsExpression = true;
     }
   }
@@ -2085,6 +2087,8 @@
         return Next;
       if (Next->is(TT_OverloadedOperator))
         continue;
+      if (Next->isOneOf(tok::star, tok::arrow))
+        continue;
       if (Next->isOneOf(tok::kw_new, tok::kw_delete)) {
         // For 'new[]' and 'delete[]'.
         if (Next->Next && Next->Next->is(tok::l_square) && Next->Next->Next &&

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits