Author: paulhoad Date: Mon Apr 15 00:47:15 2019 New Revision: 358375 URL: http://llvm.org/viewvc/llvm-project?rev=358375&view=rev Log: [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 Modified: cfe/trunk/lib/Format/TokenAnnotator.h cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/lib/Format/TokenAnnotator.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=358375&r1=358374&r2=358375&view=diff ============================================================================== --- cfe/trunk/lib/Format/TokenAnnotator.h (original) +++ cfe/trunk/lib/Format/TokenAnnotator.h Mon Apr 15 00:47:15 2019 @@ -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. Modified: cfe/trunk/unittests/Format/FormatTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=358375&r1=358374&r2=358375&view=diff ============================================================================== --- cfe/trunk/unittests/Format/FormatTest.cpp (original) +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Apr 15 00:47:15 2019 @@ -5768,6 +5768,26 @@ TEST_F(FormatTest, ReturnTypeBreakingSty " 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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
