[clang-tidy] add support for lambdas in use-trailing-return-type

Author: vbvictor

clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp

@@ -17,6 +17,30 @@
 #include <cctype>
 #include <optional>
 
+namespace clang::tidy {
+
+template <>
+struct OptionEnumMapping<
+    modernize::UseTrailingReturnTypeCheck::TransformLambda> {
+  static llvm::ArrayRef<std::pair<
+      modernize::UseTrailingReturnTypeCheck::TransformLambda, StringRef>>
+  getEnumMapping() {
+    static constexpr std::pair<
+        modernize::UseTrailingReturnTypeCheck::TransformLambda, StringRef>
+        Mapping[] = {
+            {modernize::UseTrailingReturnTypeCheck::TransformLambda::All,
+             "All"},
+            {modernize::UseTrailingReturnTypeCheck::TransformLambda::
+                 AllExceptAuto,
+             "AllExceptAuto"},
+            {modernize::UseTrailingReturnTypeCheck::TransformLambda::None,
+             "None"}};
+    return Mapping;
+  }
+};
+
+} // namespace clang::tidy
+
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::modernize {
@@ -111,10 +135,17 @@ struct UnqualNameVisitor : public RecursiveASTVisitor<UnqualNameVisitor> {
 private:
   const FunctionDecl &F;
 };
+
+AST_MATCHER(LambdaExpr, hasExplicitResultType) {
+  return Node.hasExplicitResultType();
+}
+
 } // namespace
 
-constexpr llvm::StringLiteral Message =
+constexpr llvm::StringLiteral MessageFunction =
     "use a trailing return type for this function";
+constexpr llvm::StringLiteral MessageLambda =
+    "use a trailing return type for this lambda";
 
 static SourceLocation expandIfMacroId(SourceLocation Loc,
                                       const SourceManager &SM) {
@@ -242,7 +273,7 @@ UseTrailingReturnTypeCheck::classifyTokensBeforeFunctionName(
         const MacroInfo *MI = PP->getMacroInfo(&Info);
         if (!MI || MI->isFunctionLike()) {
           // Cannot handle function style macros.
-          diag(F.getLocation(), Message);
+          diag(F.getLocation(), MessageFunction);
           return std::nullopt;
         }
       }
@@ -254,7 +285,7 @@ UseTrailingReturnTypeCheck::classifyTokensBeforeFunctionName(
     if (std::optional<ClassifiedToken> CT = classifyToken(F, *PP, T))
       ClassifiedTokens.push_back(*CT);
     else {
-      diag(F.getLocation(), Message);
+      diag(F.getLocation(), MessageFunction);
       return std::nullopt;
     }
   }
@@ -283,7 +314,7 @@ SourceRange UseTrailingReturnTypeCheck::findReturnTypeAndCVSourceRange(
   if (ReturnTypeRange.isInvalid()) {
     // Happens if e.g. clang cannot resolve all includes and the return type is
     // unknown.
-    diag(F.getLocation(), Message);
+    diag(F.getLocation(), MessageFunction);
     return {};
   }
 
@@ -383,14 +414,44 @@ void UseTrailingReturnTypeCheck::keepSpecifiers(
   }
 }
 
+UseTrailingReturnTypeCheck::UseTrailingReturnTypeCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      TransformFunctions(Options.get("TransformFunctions", true)),
+      TransformLambdas(Options.get("TransformLambdas", TransformLambda::All)) {
+
+  if (TransformFunctions == false && TransformLambdas == TransformLambda::None)
+    this->configurationDiag(
+        "The check 'modernize-use-trailing-return-type' will not perform any "
+        "analysis because 'TransformFunctions' and 'TransformLambdas' are "
+        "disabled.");
+}
+
+void UseTrailingReturnTypeCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "TransformFunctions", TransformFunctions);
+  Options.store(Opts, "TransformLambdas", TransformLambdas);
+}
+
 void UseTrailingReturnTypeCheck::registerMatchers(MatchFinder *Finder) {
-  auto F = functionDecl(
-               unless(anyOf(hasTrailingReturn(), returns(voidType()),
-                            cxxConversionDecl(), cxxMethodDecl(isImplicit()))))
-               .bind("Func");
+  auto F =
+      functionDecl(
+          unless(anyOf(
+              hasTrailingReturn(), returns(voidType()), cxxConversionDecl(),
+              cxxMethodDecl(
+                  anyOf(isImplicit(),
+                        hasParent(cxxRecordDecl(hasParent(lambdaExpr()))))))))
+          .bind("Func");
+
+  if (TransformFunctions) {
+    Finder->addMatcher(F, this);
+    Finder->addMatcher(friendDecl(hasDescendant(F)).bind("Friend"), this);
+  }
 
-  Finder->addMatcher(F, this);
-  Finder->addMatcher(friendDecl(hasDescendant(F)).bind("Friend"), this);
+  if (TransformLambdas != TransformLambda::None) {
+    Finder->addMatcher(
+        lambdaExpr(unless(hasExplicitResultType())).bind("Lambda"), this);
+  }
 }
 
 void UseTrailingReturnTypeCheck::registerPPCallbacks(
@@ -402,8 +463,13 @@ void UseTrailingReturnTypeCheck::check(const MatchFinder::MatchResult &Result) {
   assert(PP && "Expected registerPPCallbacks() to have been called before so "
                "preprocessor is available");
 
-  const auto *F = Result.Nodes.getNodeAs<FunctionDecl>("Func");
+  if (const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("Lambda")) {
+    diagOnLambda(Lambda, Result);
+    return;
+  }
+
   const auto *Fr = Result.Nodes.getNodeAs<FriendDecl>("Friend");
+  const auto *F = Result.Nodes.getNodeAs<FunctionDecl>("Func");
   assert(F && "Matcher is expected to find only FunctionDecls");
 
   // Three-way comparison operator<=> is syntactic sugar and generates implicit
@@ -423,7 +489,7 @@ void UseTrailingReturnTypeCheck::check(const MatchFinder::MatchResult &Result) {
   if (F->getDeclaredReturnType()->isFunctionPointerType() ||
       F->getDeclaredReturnType()->isMemberFunctionPointerType() ||
       F->getDeclaredReturnType()->isMemberPointerType()) {
-    diag(F->getLocation(), Message);
+    diag(F->getLocation(), MessageFunction);
     return;
   }
 
@@ -440,14 +506,14 @@ void UseTrailingReturnTypeCheck::check(const MatchFinder::MatchResult &Result) {
     // FIXME: This may happen if we have __attribute__((...)) on the function.
     // We abort for now. Remove this when the function type location gets
     // available in clang.
-    diag(F->getLocation(), Message);
+    diag(F->getLocation(), MessageFunction);
     return;
   }
 
   SourceLocation InsertionLoc =
       findTrailingReturnTypeSourceLocation(*F, FTL, Ctx, SM, LangOpts);
   if (InsertionLoc.isInvalid()) {
-    diag(F->getLocation(), Message);
+    diag(F->getLocation(), MessageFunction);
     return;
   }
 
@@ -470,7 +536,7 @@ void UseTrailingReturnTypeCheck::check(const MatchFinder::MatchResult &Result) {
   UnqualNameVisitor UNV{*F};
   UNV.TraverseTypeLoc(FTL.getReturnLoc());
   if (UNV.Collision) {
-    diag(F->getLocation(), Message);
+    diag(F->getLocation(), MessageFunction);
     return;
   }
 
@@ -489,9 +555,90 @@ void UseTrailingReturnTypeCheck::check(const MatchFinder::MatchResult &Result) {
   keepSpecifiers(ReturnType, Auto, ReturnTypeCVRange, *F, Fr, Ctx, SM,
                  LangOpts);
 
-  diag(F->getLocation(), Message)
+  diag(F->getLocation(), MessageFunction)
       << FixItHint::CreateReplacement(ReturnTypeCVRange, Auto)
       << FixItHint::CreateInsertion(InsertionLoc, " -> " + ReturnType);
 }
 
+void UseTrailingReturnTypeCheck::diagOnLambda(
+    const LambdaExpr *Lambda,
+    const ast_matchers::MatchFinder::MatchResult &Result) {
+
+  const CXXMethodDecl *Method = Lambda->getCallOperator();
+  if (!Method || Lambda->hasExplicitResultType())
+    return;
+
+  const ASTContext *Ctx = Result.Context;
+  const QualType ReturnType = Method->getReturnType();
+
+  // We can't write 'auto' in C++11 mode, try to write generic msg and bail out.
+  if (ReturnType->isDependentType() &&
+      Ctx->getLangOpts().LangStd == LangStandard::lang_cxx11) {
+    if (TransformLambdas == TransformLambda::All)
+      diag(Lambda->getBeginLoc(), MessageLambda);
+    return;
+  }
+
+  if (ReturnType->isUndeducedAutoType() &&
+      TransformLambdas == TransformLambda::AllExceptAuto)
+    return;
+
+  const SourceLocation TrailingReturnInsertLoc =
+      findLambdaTrailingReturnInsertLoc(Method, *Result.SourceManager,
+                                        getLangOpts(), *Result.Context);
+
+  if (TrailingReturnInsertLoc.isValid())
+    diag(Lambda->getBeginLoc(), "use a trailing return type for this lambda")
+        << FixItHint::CreateInsertion(
+               TrailingReturnInsertLoc,
+               " -> " +
+                   ReturnType.getAsString(Result.Context->getPrintingPolicy()));
+  else
+    diag(Lambda->getBeginLoc(), MessageLambda);
+}
+
+SourceLocation UseTrailingReturnTypeCheck::findLambdaTrailingReturnInsertLoc(
+    const CXXMethodDecl *Method, const SourceManager &SM,
+    const LangOptions &LangOpts, const ASTContext &Ctx) {
+  // 'requires' keyword is present in lambda declaration
+  if (Method->getTrailingRequiresClause()) {
+    SourceLocation ParamEndLoc;
+    if (Method->param_empty()) {
+      ParamEndLoc = Method->getBeginLoc();
+    } else {
+      ParamEndLoc = Method->getParametersSourceRange().getEnd();
+    }
+
+    std::pair<FileID, unsigned> ParamEndLocInfo =
+        SM.getDecomposedLoc(ParamEndLoc);
+    StringRef Buffer = SM.getBufferData(ParamEndLocInfo.first);
+
+    Lexer Lexer(SM.getLocForStartOfFile(ParamEndLocInfo.first), LangOpts,
+                Buffer.begin(), Buffer.data() + ParamEndLocInfo.second,
+                Buffer.end());
+
+    Token Token;
+    while (!Lexer.LexFromRawLexer(Token)) {
+      if (Token.is(tok::raw_identifier)) {
+        IdentifierInfo &Info = Ctx.Idents.get(StringRef(
+            SM.getCharacterData(Token.getLocation()), Token.getLength()));
+        Token.setIdentifierInfo(&Info);
+        Token.setKind(Info.getTokenID());
+      }
+
+      if (Token.is(tok::kw_requires)) {
+        return Token.getLocation().getLocWithOffset(-1);
+      }
+    }
+
+    return {};
+  }
+
+  // If no requires clause, insert before the body
+  if (const Stmt *Body = Method->getBody())
+    return Body->getBeginLoc().getLocWithOffset(-1);
+
+  return {};
+}
+
 } // namespace clang::tidy::modernize

clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h

@@ -27,18 +27,22 @@ struct ClassifiedToken {
 /// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-trailing-return-type.html
 class UseTrailingReturnTypeCheck : public ClangTidyCheck {
 public:
-  UseTrailingReturnTypeCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  UseTrailingReturnTypeCheck(StringRef Name, ClangTidyContext *Context);
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus11;
   }
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
                            Preprocessor *ModuleExpanderPP) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
+  enum TransformLambda { All, AllExceptAuto, None };
+
 private:
   Preprocessor *PP = nullptr;
+  const bool TransformFunctions;
+  const TransformLambda TransformLambdas;
 
   SourceLocation findTrailingReturnTypeSourceLocation(
       const FunctionDecl &F, const FunctionTypeLoc &FTL, const ASTContext &Ctx,
@@ -56,6 +60,13 @@ class UseTrailingReturnTypeCheck : public ClangTidyCheck {
                       SourceRange ReturnTypeCVRange, const FunctionDecl &F,
                       const FriendDecl *Fr, const ASTContext &Ctx,
                       const SourceManager &SM, const LangOptions &LangOpts);
+
+  void diagOnLambda(const LambdaExpr *Lambda,
+                    const ast_matchers::MatchFinder::MatchResult &Result);
+  SourceLocation findLambdaTrailingReturnInsertLoc(const CXXMethodDecl *Method,
+                                                   const SourceManager &SM,
+                                                   const LangOptions &LangOpts,
+                                                   const ASTContext &Ctx);
 };
 
 } // namespace clang::tidy::modernize

clang-tools-extra/docs/ReleaseNotes.rst

@@ -185,6 +185,13 @@ Changes in existing checks
   <clang-tidy/checks/modernize/use-std-numbers>` check to support math
   functions of different precisions.
 
+- Improved :doc:`modernize-use-trailing-return-type
+  <clang-tidy/checks/modernize/use-trailing-return-type>` check by adding
+  support to modernize lambda signatures to use trailing return type and adding
+  two new options: `TransformFunctions` and `TransformLambdas` to control
+  whether function declarations and lambdas should be transformed by the check.
+  Fixed false positives when lambda was matched as a function in C++11 mode.
+
 - Improved :doc:`performance-move-const-arg
   <clang-tidy/checks/performance/move-const-arg>` check by fixing false
   negatives on ternary operators calling ``std::move``.

clang-tools-extra/docs/clang-tidy/checks/modernize/use-trailing-return-type.rst

@@ -3,7 +3,7 @@
 modernize-use-trailing-return-type
 ==================================
 
-Rewrites function signatures to use a trailing return type
+Rewrites function and lambda 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).
@@ -16,6 +16,7 @@ Example
   int f1();
   inline int f2(int arg) noexcept;
   virtual float f3() const && = delete;
+  auto lambda = []() {};
 
 transforms to:
 
@@ -24,6 +25,7 @@ transforms to:
   auto f1() -> int;
   inline auto f2(int arg) -> int noexcept;
   virtual auto f3() const && -> float = delete;
+  auto lambda = []() -> void {};
 
 Known Limitations
 -----------------
@@ -66,3 +68,25 @@ a careless rewrite would produce the following output:
 This code fails to compile because the S in the context of f refers to the equally named function parameter.
 Similarly, the S in the context of m refers to the equally named class member.
 The check can currently only detect and avoid a clash with a function parameter name.
+
+Options
+-------
+
+.. option:: TransformFunctions
+
+  When set to `true`, function declarations will be transformed to use trailing
+  return. Default is `true`.
+
+.. option:: TransformLambdas
+
+  Controls how lambda expressions are transformed to use trailing
+  return type. Possible values are:
+
+  * `All` - Transform all lambda expressions without an explicit return type
+    to use trailing return type. If type can not be deduced, ``auto`` will be
+    used in since C++14 and generic message will be emitted otherwise.
+  * `AllExceptAuto` - Transform all lambda expressions except those whose return
+    type can not be deduced.
+  * `None` - Do not transform any lambda expressions.
+
+  Default is `All`.

clang-tools-extra/test/clang-tidy/checkers/modernize/use-trailing-return-type-cxx20.cpp

@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++20 %s modernize-use-trailing-return-type %t
+// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-trailing-return-type %t
 
 namespace std {
 template <typename T, typename U>