Index: clang-tidy/modernize/CMakeLists.txt =================================================================== --- clang-tidy/modernize/CMakeLists.txt +++ clang-tidy/modernize/CMakeLists.txt @@ -30,6 +30,7 @@ UseNoexceptCheck.cpp UseNullptrCheck.cpp UseOverrideCheck.cpp + UseTrailingReturnTypeCheck.cpp UseTransparentFunctorsCheck.cpp UseUncaughtExceptionsCheck.cpp UseUsingCheck.cpp Index: clang-tidy/modernize/ModernizeTidyModule.cpp =================================================================== --- clang-tidy/modernize/ModernizeTidyModule.cpp +++ clang-tidy/modernize/ModernizeTidyModule.cpp @@ -35,6 +35,7 @@ #include "UseNoexceptCheck.h" #include "UseNullptrCheck.h" #include "UseOverrideCheck.h" +#include "UseTrailingReturnTypeCheck.h" #include "UseTransparentFunctorsCheck.h" #include "UseUncaughtExceptionsCheck.h" #include "UseUsingCheck.h" @@ -87,6 +88,8 @@ CheckFactories.registerCheck("modernize-use-noexcept"); CheckFactories.registerCheck("modernize-use-nullptr"); CheckFactories.registerCheck("modernize-use-override"); + CheckFactories.registerCheck( + "modernize-use-trailing-return-type"); CheckFactories.registerCheck( "modernize-use-transparent-functors"); CheckFactories.registerCheck( Index: clang-tidy/modernize/UseTrailingReturnTypeCheck.h =================================================================== --- /dev/null +++ clang-tidy/modernize/UseTrailingReturnTypeCheck.h @@ -0,0 +1,48 @@ +//===--- UseTrailingReturnTypeCheck.h - clang-tidy---------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USETRAILINGRETURNTYPECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USETRAILINGRETURNTYPECHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace modernize { + +/// Rewrites function signatures to use a trailing return type. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-trailing-type-return.html +class UseTrailingReturnTypeCheck : public ClangTidyCheck { +public: + UseTrailingReturnTypeCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + SourceLocation findTrailingReturnTypeSourceLocation( + const FunctionDecl &F, const FunctionTypeLoc &FTL, const ASTContext &Ctx, + const SourceManager &SM, const LangOptions &LangOpts); + llvm::Optional> + nonMacroTokensBeforeFunctionName(const FunctionDecl &F, const ASTContext &Ctx, + const SourceManager &SM, + const LangOptions &LangOpts); + SourceRange findReturnTypeAndCVSourceRange(const FunctionDecl &F, + const ASTContext &Ctx, + const SourceManager &SM, + const LangOptions &LangOpts); +}; + +} // namespace modernize +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USETRAILINGRETURNTYPECHECK_H Index: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp @@ -0,0 +1,350 @@ +//===--- UseTrailingReturnTypeCheck.cpp - clang-tidy-----------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "UseTrailingReturnTypeCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Tooling/FixIt.h" + +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace modernize { +namespace { +struct UnqualNameVisitor : public RecursiveASTVisitor { +public: + UnqualNameVisitor(const FunctionDecl &F, const SourceManager &SM) + : F(F), SM(SM) {} + + bool Collision = false; + + bool shouldWalkTypesOfTypeLocs() const { return false; } + + bool VisitUnqualName(StringRef UnqualName) { + // Check for collisions with function arguments + for (ParmVarDecl *Param : F.parameters()) + if (const IdentifierInfo *Ident = Param->getIdentifier()) + if (Ident->getName() == UnqualName) { + Collision = true; + return true; + } + return false; + } + + bool TraverseTypeLoc(TypeLoc TL, bool Elaborated = false) { + if (TL.isNull()) + return true; + + if (!Elaborated) { + switch (TL.getTypeLocClass()) { + case TypeLoc::Record: + if (VisitUnqualName( + TL.getAs().getTypePtr()->getDecl()->getName())) + return false; + break; + case TypeLoc::Enum: + if (VisitUnqualName( + TL.getAs().getTypePtr()->getDecl()->getName())) + return false; + break; + case TypeLoc::TemplateSpecialization: + if (VisitUnqualName(TL.getAs() + .getTypePtr() + ->getTemplateName() + .getAsTemplateDecl() + ->getName())) + return false; + break; + default: + break; + } + } + + return RecursiveASTVisitor::TraverseTypeLoc(TL); + } + + // Replace the base method in order to call ower own + // TraverseTypeLoc(). + bool TraverseQualifiedTypeLoc(QualifiedTypeLoc TL) { + return TraverseTypeLoc(TL.getUnqualifiedLoc()); + } + + // Replace the base version to inform TraverseTypeLoc that the type is + // elaborated. + bool TraverseElaboratedTypeLoc(ElaboratedTypeLoc TL) { + if (TL.getQualifierLoc()) + if (!TraverseNestedNameSpecifierLoc(TL.getQualifierLoc())) + return false; + if (!TraverseTypeLoc(TL.getNamedTypeLoc(), true)) + return false; + return true; + } + + bool VisitDeclRefExpr(DeclRefExpr *S) { + const DeclarationName Name = S->getNameInfo().getName(); + if (!S->getQualifierLoc() && Name.isIdentifier() && + VisitUnqualName(Name.getAsIdentifierInfo()->getName())) + return false; + return true; + } + +private: + const FunctionDecl &F; + const SourceManager &SM; +}; +} // namespace + +constexpr llvm::StringLiteral Message = + "use a trailing return type for this function"; + +static SourceLocation expandIfMacroId(SourceLocation Loc, + const SourceManager &SM) { + if (Loc.isMacroID()) + Loc = SM.getImmediateExpansionRange(Loc).getBegin(); + return Loc; +} + +SourceLocation UseTrailingReturnTypeCheck::findTrailingReturnTypeSourceLocation( + const FunctionDecl &F, const FunctionTypeLoc &FTL, const ASTContext &Ctx, + const SourceManager &SM, const LangOptions &LangOpts) { + // We start with the location of the closing parenthesis. + SourceRange ExceptionSpecRange = F.getExceptionSpecSourceRange(); + if (ExceptionSpecRange.isValid()) + return Lexer::getLocForEndOfToken(ExceptionSpecRange.getEnd(), 0, SM, + LangOpts); + + // If the function argument list ends inside of a macro, it is dangerous to + // start lexing from here - bail out. + SourceLocation ClosingParen = FTL.getRParenLoc(); + if (ClosingParen.isMacroID()) + return {}; + + SourceLocation Result = + Lexer::getLocForEndOfToken(ClosingParen, 0, SM, LangOpts); + + // Skip subsequent CV and ref qualifiers. + std::pair Loc = SM.getDecomposedLoc(Result); + StringRef File = SM.getBufferData(Loc.first); + const char *TokenBegin = File.data() + Loc.second; + Lexer Lexer(SM.getLocForStartOfFile(Loc.first), LangOpts, File.begin(), + TokenBegin, File.end()); + Token T; + while (!Lexer.LexFromRawLexer(T)) { + if (T.is(tok::raw_identifier)) { + IdentifierInfo &Info = Ctx.Idents.get( + StringRef(SM.getCharacterData(T.getLocation()), T.getLength())); + T.setIdentifierInfo(&Info); + T.setKind(Info.getTokenID()); + } + + if (T.isOneOf(tok::amp, tok::ampamp, tok::kw_const, tok::kw_volatile)) { + Result = T.getEndLoc(); + continue; + } + + break; + } + + return Result; +} + +llvm::Optional> +UseTrailingReturnTypeCheck::nonMacroTokensBeforeFunctionName( + const FunctionDecl &F, const ASTContext &Ctx, const SourceManager &SM, + const LangOptions &LangOpts) { + SourceLocation BeginF = expandIfMacroId(F.getBeginLoc(), SM); + SourceLocation BeginNameF = expandIfMacroId(F.getLocation(), SM); + + // Create tokens for everything before the name of the function. + std::pair Loc = SM.getDecomposedLoc(BeginF); + StringRef File = SM.getBufferData(Loc.first); + const char *TokenBegin = File.data() + Loc.second; + Lexer Lexer(SM.getLocForStartOfFile(Loc.first), LangOpts, File.begin(), + TokenBegin, File.end()); + Token T; + SmallVector Tokens; + while (!Lexer.LexFromRawLexer(T) && + SM.isBeforeInTranslationUnit(T.getLocation(), BeginNameF)) { + if (T.is(tok::raw_identifier)) { + IdentifierInfo &Info = Ctx.Idents.get( + StringRef(SM.getCharacterData(T.getLocation()), T.getLength())); + if (Info.hasMacroDefinition()) { + // The CV qualifiers of the return type are inside macros + diag(F.getLocation(), Message); + return {}; + } + T.setIdentifierInfo(&Info); + T.setKind(Info.getTokenID()); + } + Tokens.push_back(T); + } + return Tokens; +} + +static bool hasAnyNestedLocalQualifiers(QualType Type) { + bool Result = Type.hasLocalQualifiers(); + if (Type->isPointerType()) + Result = Result || hasAnyNestedLocalQualifiers( + Type->castAs()->getPointeeType()); + if (Type->isReferenceType()) + Result = Result || hasAnyNestedLocalQualifiers( + Type->castAs()->getPointeeType()); + return Result; +} + +SourceRange UseTrailingReturnTypeCheck::findReturnTypeAndCVSourceRange( + const FunctionDecl &F, const ASTContext &Ctx, const SourceManager &SM, + const LangOptions &LangOpts) { + + // We start with the range of the return type and expand to neighboring + // 'const' and 'volatile'. + SourceRange ReturnTypeRange = F.getReturnTypeSourceRange(); + if (ReturnTypeRange.isInvalid()) { + // Happens if e.g. clang cannot resolve all includes and the return type is + // unknown. + diag(F.getLocation(), Message); + return {}; + } + + // If the return type has no local qualifiers, it's source range is accurate. + if (!hasAnyNestedLocalQualifiers(F.getReturnType())) + return ReturnTypeRange; + + // Include const and volatile to the left and right of the return type. + llvm::Optional> MaybeTokens = + nonMacroTokensBeforeFunctionName(F, Ctx, SM, LangOpts); + if (!MaybeTokens) + return {}; + const SmallVector &Tokens = *MaybeTokens; + + auto IsCV = [](Token T) { + return T.isOneOf(tok::kw_const, tok::kw_volatile); + }; + + bool ExtendedLeft = false; + for (size_t I = 0; I < Tokens.size(); I++) { + // If we found the beginning of the return type, include const and volatile + // to the left. + if (!SM.isBeforeInTranslationUnit(Tokens[I].getLocation(), + ReturnTypeRange.getBegin()) && + !ExtendedLeft) { + for (int J = static_cast(I) - 1; J >= 0 && IsCV(Tokens[J]); J--) + ReturnTypeRange.setBegin(Tokens[J].getLocation()); + ExtendedLeft = true; + } + // If we found the end of the return type, include const and volatile to the + // right. + if (SM.isBeforeInTranslationUnit(ReturnTypeRange.getEnd(), + Tokens[I].getLocation())) { + for (size_t J = I; J < Tokens.size() && IsCV(Tokens[J]); J++) + ReturnTypeRange.setEnd(Tokens[J].getLocation()); + break; + } + } + + return ReturnTypeRange; +} + +void UseTrailingReturnTypeCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus11) + return; + + Finder->addMatcher( + functionDecl(unless(anyOf(hasTrailingReturn(), returns(voidType()), + returns(autoType()), cxxConversionDecl(), + cxxMethodDecl(isImplicit())))) + .bind("f"), + this); +} + +void UseTrailingReturnTypeCheck::check(const MatchFinder::MatchResult &Result) { + const auto *F = Result.Nodes.getNodeAs("f"); + assert(F && "Matcher is expected to find only FunctionDecls"); + + if (F->getLocation().isInvalid()) + return; + + // TODO: implement those + if (F->getDeclaredReturnType()->isFunctionPointerType() || + F->getDeclaredReturnType()->isMemberFunctionPointerType() || + F->getDeclaredReturnType()->isMemberPointerType() || + F->getDeclaredReturnType()->getAs() != nullptr) { + diag(F->getLocation(), Message); + return; + } + + const ASTContext &Ctx = *Result.Context; + const SourceManager &SM = *Result.SourceManager; + const LangOptions &LangOpts = getLangOpts(); + + const TypeSourceInfo *TSI = F->getTypeSourceInfo(); + if (!TSI) { + diag(F->getLocation(), Message); + return; + } + FunctionTypeLoc FTL = + TSI->getTypeLoc().IgnoreParens().getAs(); + if (!FTL) { + diag(F->getLocation(), Message); + return; + } + + SourceLocation InsertionLoc = + findTrailingReturnTypeSourceLocation(*F, FTL, Ctx, SM, LangOpts); + if (InsertionLoc.isInvalid()) { + diag(F->getLocation(), Message); + return; + } + + // Using the declared return type via F->getDeclaredReturnType().getAsString() + // discards user formatting and order of const, volatile, type, whitespace, + // space before & ... . + SourceRange ReturnTypeCVRange = + findReturnTypeAndCVSourceRange(*F, Ctx, SM, LangOpts); + if (ReturnTypeCVRange.isInvalid()) + return; + + // Check if unqualified names in the return type conflict with other entities + // after the rewrite. + // FIXME: this could be done better, by performing a lookup of all + // unqualified names in the return type in the scope of the function. If the + // lookup finds a different entity than the original entity identified by the + // name, then we can either not perform a rewrite or explicitely qualify the + // entity. Such entities could be function parameter names, (inherited) class + // members, template parameters, etc. + UnqualNameVisitor UNV{*F, SM}; + UNV.TraverseTypeLoc(FTL.getReturnLoc()); + if (UNV.Collision) { + diag(F->getLocation(), Message); + return; + } + + SourceLocation ReturnTypeEnd = + Lexer::getLocForEndOfToken(ReturnTypeCVRange.getEnd(), 0, SM, LangOpts); + StringRef CharAfterReturnType = Lexer::getSourceText( + CharSourceRange::getCharRange(ReturnTypeEnd, + ReturnTypeEnd.getLocWithOffset(1)), + SM, LangOpts); + bool NeedSpaceAfterAuto = + CharAfterReturnType.empty() || !std::isspace(CharAfterReturnType[0]); + + StringRef ReturnType = tooling::fixit::getText(ReturnTypeCVRange, Ctx); + StringRef Auto = NeedSpaceAfterAuto ? "auto " : "auto"; + diag(F->getLocation(), Message) + << FixItHint::CreateReplacement(ReturnTypeCVRange, Auto) + << FixItHint::CreateInsertion(InsertionLoc, (" -> " + ReturnType).str()); +} + +} // namespace modernize +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -102,6 +102,11 @@ :doc:`objc-property-declaration ` check have been removed. +- New :doc:`modernize-use-trailing-type-return + ` check. + + Rewrites function signatures to use a trailing return type. + Improvements to include-fixer ----------------------------- Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -215,6 +215,7 @@ modernize-use-noexcept modernize-use-nullptr modernize-use-override + modernize-use-trailing-return-type modernize-use-transparent-functors modernize-use-uncaught-exceptions modernize-use-using Index: docs/clang-tidy/checks/modernize-use-trailing-return-type.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/modernize-use-trailing-return-type.rst @@ -0,0 +1,68 @@ +.. title:: clang-tidy - modernize-use-trailing-return-type + +modernize-use-trailing-return-type +================================== + +Rewrites function signatures to use a trailing return type +(introduced in C++11). This transformation is purely stylistic. +The return type before the function name is replaced by ``auto`` +and inserted after the function parameter list (and qualifiers). + +Example +------- + +.. code-block:: c++ + + int f1(); + inline int f2(int arg) noexcept; + virtual float f3() const && = delete; + +transforms to: + +.. code-block:: c++ + + auto f1() -> int; + inline auto f2(int arg) -> int noexcept; + virtual auto f3() const && -> float = delete; + +Known Limitations +----------------- + +The following categories of return types cannot be rewritten currently: +* function pointers +* member function pointers +* member pointers +* decltype, when it is the top level expression + +Unqualified names in the return type might erroneously refer to different entities after the rewrite. +Preventing such errors requires a full lookup of all unqualified names present in the return type in the scope of the trailing return type location. +This location includes e.g. function parameter names and members of the enclosing class (including all inherited classes). +Such a lookup is currently not implemented. + +Given the following piece of code + +.. code-block:: c++ + + struct Object { long long value; }; + Object f(unsigned Object) { return {Object * 2}; } + class CC { + int Object; + struct Object m(); + }; + Object CC::m() { return {0}; } + +a careless rewrite would produce the following output: + +.. code-block:: c++ + + struct Object { long long value; }; + auto f(unsigned Object) -> Object { return {Object * 2}; } // error + class CC { + int Object; + auto m() -> struct Object; + }; + auto CC::m() -> Object { return {0}; } // error + +This code fails to compile because the Object in the context of f refers to the equally named function parameter. +Similarly, the Object in the context of m refers to the equally named class member. +The check can currently only detect a clash with a function parameter name. Index: test/clang-tidy/modernize-use-trailing-return-type.cpp =================================================================== --- /dev/null +++ test/clang-tidy/modernize-use-trailing-return-type.cpp @@ -0,0 +1,437 @@ +// RUN: %check_clang_tidy %s modernize-use-trailing-return-type %t -- -- --std=c++14 -fdeclspec + +namespace std { + template + class vector; + + template + class array; + + class string; + + template + auto declval() -> T; +} + +// +// Functions +// + +int f(); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto f() -> int;{{$}} +int ((f))(); +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto ((f))() -> int;{{$}} +int f(int); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto f(int) -> int;{{$}} +int f(int arg); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto f(int arg) -> int;{{$}} +int f(int arg1, int arg2, int arg3); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3) -> int;{{$}} +int f(int arg1, int arg2, int arg3, ...); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3, ...) -> int;{{$}} +template int f(T t); +// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}template auto f(T t) -> int;{{$}} + +// +// Functions with formatting +// + +int a1() { return 42; } +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto a1() -> int { return 42; }{{$}} +int a2() { + return 42; +} +// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto a2() -> int {{{$}} +int a3() +{ + return 42; +} +// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto a3() -> int{{$}} +int a4(int arg ) ; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto a4(int arg ) -> int ;{{$}} +int a5 +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto a5{{$}} +(int arg); +// CHECK-FIXES: {{^}}(int arg) -> int;{{$}} +const +int +* +a7 +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +() +// CHECK-FIXES: {{^}}() -> const{{$}} +// CHECK-FIXES: {{^}}int{{$}} +// CHECK-FIXES: {{^}}*{{$}} +; + +int*a7(int arg); +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type] +// CHECK-FIXES: {{^}}auto a7(int arg) -> int*;{{$}} +template