From ae1b7859cbd61d2284d9690bc53482d0b6a46f63 Mon Sep 17 00:00:00 2001 From: Krasimir Georgiev Date: Wed, 9 Oct 2019 14:46:08 +0000 Subject: [PATCH] [clang-format] Update noexcept reference qualifiers detection Summary: r373165 fixed an issue where a templated noexcept member function with a reference qualifier would be indented more than expected: ``` // Formatting produced with LLVM style with AlwaysBreakTemplateDeclarations: Yes // before r373165: struct f { template void bar() && noexcept {} }; // after: struct f { template void bar() && noexcept {} }; ``` The way this is done is that in the AnnotatingParser in `lib/FormatTokenAnnotator.cpp` the determination of the usage of a `&` or `&&` (the line in determineTokenType ``` Current.Type = determineStarAmpUsage(... ``` is not performed in some cases anymore, combining with a few additional related checks afterwards. The net effect of these checks results in the `&` or `&&` token to start being classified as `TT_Unknown` in cases where before `r373165` it would be classified as `TT_UnaryOperator` or `TT_PointerOrReference` by `determineStarAmpUsage`. This inadvertently caused 2 classes of regressions I'm aware of: - The address-of `&` after a function assignment would be classified as `TT_Unknown`, causing spaces to surround it, disregarding style options: ``` // before r373165: void (*fun_ptr)(void) = &fun; // after: void (*fun_ptr)(void) = & fun; ``` - In cases where there is a function declaration list -- looking macro between a template line and the start of the function declaration, an `&` as part of the return type would be classified as `TT_Unknown`, causing spaces to surround it: ``` // before r373165: template DEPRECATED("lala") Type& foo(); // after: template DEPRECATED("lala") Type & foo(); ``` In these cases the problems are rooted in the skipping of the classification of a `&` (and similarly `&&`) by determineStarAmpUsage which effects the formatting decisions later in the pipeline. I've looked into the goal of r373165 and noticed that replacing `noexcept` with `const` in the given example produces no extra indentation with the old code: ``` // before r373165: struct f { template int foo() & const {} }; struct f { template int foo() & noexcept {} }; ``` I investigated how clang-format annotated these two examples differently to determine the places where the processing of both diverges in the pipeline. There were two places where the processing diverges, causing the extra indent in the `noexcept` case: 1. The `const` is annotated as a `TT_TrailingAnnotation`, whereas `noexcept` is annotated as `TT_Unknown`. I've updated the `determineTokenType` function to account for this by adding a missing `tok:kw_noexcept` to the clause that marks a token as `TT_TrailingAnnotation`. 2. The `&` in the second example is wrongly identified as `TT_BinaryOperator` in `determineStarAmpUsage`. This is the reason for the extra indentation -- clang-format gets confused and thinks this is an expression. I've updated `determineStarAmpUsage` to check for `tok:kw_noexcept`. With these two updates in place, the additional parsing introduced by r373165 becomes unnecessary and all added tests pass (with updates, as now clang-format respects the style configuration for spaces around the `&` in the test examples). I've removed these additions and added regression tests for the cases above. Reviewers: AndWass, MyDeveloperDay Reviewed By: MyDeveloperDay Subscribers: cfe-commits Tags: #clang, #clang-format Differential Revision: https://reviews.llvm.org/D68695 llvm-svn: 374172 --- clang/lib/Format/TokenAnnotator.cpp | 22 ++++++++-------------- clang/unittests/Format/FormatTest.cpp | 32 +++++++++++++++++++++++++------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index d4519fe..88f7468 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -15,6 +15,7 @@ #include "TokenAnnotator.h" #include "FormatToken.h" #include "clang/Basic/SourceManager.h" +#include "clang/Basic/TokenKinds.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/Support/Debug.h" @@ -65,7 +66,7 @@ public: AnnotatingParser(const FormatStyle &Style, AnnotatedLine &Line, const AdditionalKeywords &Keywords) : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false), - TrailingReturnFound(false), Keywords(Keywords) { + Keywords(Keywords) { Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false)); resetTokenMetadata(CurrentToken); } @@ -1397,11 +1398,7 @@ private: !Current.Previous->is(tok::kw_operator)) { // not auto operator->() -> xxx; Current.Type = TT_TrailingReturnArrow; - TrailingReturnFound = true; - } else if (Current.is(tok::star) || - (Current.isOneOf(tok::amp, tok::ampamp) && - (Current.NestingLevel != 0 || !Line.MightBeFunctionDecl || - TrailingReturnFound))) { + } else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) { Current.Type = determineStarAmpUsage(Current, Contexts.back().CanBeExpression && Contexts.back().IsExpression, @@ -1424,8 +1421,6 @@ private: Current.Type = TT_ConditionalExpr; } } else if (Current.isBinaryOperator() && - !(Line.MightBeFunctionDecl && Current.NestingLevel == 0 && - Current.isOneOf(tok::amp, tok::ampamp)) && (!Current.Previous || Current.Previous->isNot(tok::l_square)) && (!Current.is(tok::greater) && Style.Language != FormatStyle::LK_TextProto)) { @@ -1500,12 +1495,11 @@ private: // colon after this, this is the only place which annotates the identifier // as a selector.) Current.Type = TT_SelectorName; - } else if (Current.isOneOf(tok::identifier, tok::kw_const, tok::amp, - tok::ampamp) && + } else if (Current.isOneOf(tok::identifier, tok::kw_const, + tok::kw_noexcept) && Current.Previous && !Current.Previous->isOneOf(tok::equal, tok::at) && - Line.MightBeFunctionDecl && !TrailingReturnFound && - Contexts.size() == 1) { + Line.MightBeFunctionDecl && Contexts.size() == 1) { // Line.MightBeFunctionDecl can only be true after the parentheses of a // function declaration have been found. Current.Type = TT_TrailingAnnotation; @@ -1689,7 +1683,8 @@ private: const FormatToken *NextToken = Tok.getNextNonComment(); if (!NextToken || - NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const) || + NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_const, + tok::kw_noexcept) || (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment())) return TT_PointerOrReference; @@ -1790,7 +1785,6 @@ private: AnnotatedLine &Line; FormatToken *CurrentToken; bool AutoFound; - bool TrailingReturnFound; const AdditionalKeywords &Keywords; // Set of "<" tokens that do not open a template parameter list. If parseAngle diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 8c2bc1a..dde065a 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -7037,31 +7037,31 @@ TEST_F(FormatTest, UnderstandsFunctionRefQualification) { verifyFormat("struct f {\n" " template \n" - " int &foo(const std::string &str) & noexcept {}\n" + " int &foo(const std::string &str) &noexcept {}\n" "};", BreakTemplate); verifyFormat("struct f {\n" " template \n" - " int &foo(const std::string &str) && noexcept {}\n" + " int &foo(const std::string &str) &&noexcept {}\n" "};", BreakTemplate); verifyFormat("struct f {\n" " template \n" - " int &foo(const std::string &str) const & noexcept {}\n" + " int &foo(const std::string &str) const &noexcept {}\n" "};", BreakTemplate); verifyFormat("struct f {\n" " template \n" - " int &foo(const std::string &str) const & noexcept {}\n" + " int &foo(const std::string &str) const &noexcept {}\n" "};", BreakTemplate); verifyFormat("struct f {\n" " template \n" - " auto foo(const std::string &str) && noexcept -> int & {}\n" + " auto foo(const std::string &str) &&noexcept -> int & {}\n" "};", BreakTemplate); @@ -7084,13 +7084,13 @@ TEST_F(FormatTest, UnderstandsFunctionRefQualification) { verifyFormat("struct f {\n" " template \n" - " int& foo(const std::string& str) const & noexcept {}\n" + " int& foo(const std::string& str) const& noexcept {}\n" "};", AlignLeftBreakTemplate); verifyFormat("struct f {\n" " template \n" - " int& foo(const std::string& str) const & noexcept {}\n" + " int& foo(const std::string& str) const&& noexcept {}\n" "};", AlignLeftBreakTemplate); @@ -7099,6 +7099,24 @@ TEST_F(FormatTest, UnderstandsFunctionRefQualification) { " auto foo(const std::string& str) && noexcept -> int& {}\n" "};", AlignLeftBreakTemplate); + + // The `&` in `Type&` should not be confused with a trailing `&` of + // DEPRECATED(reason) member function. + verifyFormat("struct f {\n" + " template \n" + " DEPRECATED(reason)\n" + " Type &foo(arguments) {}\n" + "};", + BreakTemplate); + + verifyFormat("struct f {\n" + " template \n" + " DEPRECATED(reason)\n" + " Type& foo(arguments) {}\n" + "};", + AlignLeftBreakTemplate); + + verifyFormat("void (*foopt)(int) = &func;"); } TEST_F(FormatTest, UnderstandsNewAndDelete) { -- 2.7.4