From 070ab5cd47cde6bb757e1897a93e8c08edbdeaae Mon Sep 17 00:00:00 2001
From: Paul Hoad <mydeveloperday@gmail.com>
Date: Mon, 15 Apr 2019 07:47:15 +0000
Subject: [PATCH] [clang-format] [PR41170] Break after return type ignored with
 certain comments positions

Summary:
Addresses https://bugs.llvm.org/show_bug.cgi?id=41170

The AlwaysBreakAfterReturn type setting can go wrong if the line ends with a comment
```
void foo() /* comment */
```
or

```
void foo() // comment
```

It will incorrectly see such functions as Declarations and not Definitions

The following code addresses this by looking for function which end with `; <comment>` rather than just `;` or `<comment>`

Reviewers: klimek, djasper, reuk, russellmcc, owenpan, sammccall
Reviewed By: owenpan
Subscribers: lebedev.ri, cfe-commits, sammccall
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60363

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@358375 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Format/TokenAnnotator.h     | 14 +++++++++++---
 unittests/Format/FormatTest.cpp | 20 ++++++++++++++++++++
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/lib/Format/TokenAnnotator.h b/lib/Format/TokenAnnotator.h
index 0422c611b4..d21df4938b 100644
--- a/lib/Format/TokenAnnotator.h
+++ b/lib/Format/TokenAnnotator.h
@@ -99,9 +99,17 @@ public:
   /// function declaration. Asserts MightBeFunctionDecl.
   bool mightBeFunctionDefinition() const {
     assert(MightBeFunctionDecl);
-    // FIXME: Line.Last points to other characters than tok::semi
-    // and tok::lbrace.
-    return !Last->isOneOf(tok::semi, tok::comment);
+    // Try to determine if the end of a stream of tokens is either the
+    // Definition or the Declaration for a function. It does this by looking for
+    // the ';' in foo(); and using that it ends with a ; to know this is the
+    // Definition, however the line could end with
+    //    foo(); /* comment */
+    // or
+    //    foo(); // comment
+    // or
+    //    foo() // comment
+    // endsWith() ignores the comment.
+    return !endsWith(tok::semi);
   }
 
   /// \c true if this line starts a namespace definition.
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index b4605053ea..d95f5d64ef 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -5768,6 +5768,26 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) {
                "  return a;\n"
                "}\n",
                Style);
+
+  Style = getGNUStyle();
+
+  // Test for comments at the end of function declarations.
+  verifyFormat("void\n"
+               "foo (int a, /*abc*/ int b) // def\n"
+               "{\n"
+               "}\n",
+               Style);
+
+  verifyFormat("void\n"
+               "foo (int a, /* abc */ int b) /* def */\n"
+               "{\n"
+               "}\n",
+               Style);
+
+  // Definitions that should not break after return type
+  verifyFormat("void foo (int a, int b); // def\n", Style);
+  verifyFormat("void foo (int a, int b); /* def */\n", Style);
+  verifyFormat("void foo (int a, int b);\n", Style);
 }
 
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
-- 
2.40.0