Merge branch 'main' into quatradic-func-ordering

Author: mscuttari

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,
+             "all_except_auto"},
+            {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 ErrorMessageOnFunction =
     "use a trailing return type for this function";
+constexpr llvm::StringLiteral ErrorMessageOnLambda =
+    "use a trailing return type for this lambda";
 
 static SourceLocation expandIfMacroId(SourceLocation Loc,
                                       const SourceManager &SM) {
@@ -329,6 +360,48 @@ findReturnTypeAndCVSourceRange(const FunctionDecl &F, const TypeLoc &ReturnLoc,
   return ReturnTypeRange;
 }
 
+static SourceLocation 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 {};
+}
+
 static void keepSpecifiers(std::string &ReturnType, std::string &Auto,
                            SourceRange ReturnTypeCVRange, const FunctionDecl &F,
                            const FriendDecl *Fr, const ASTContext &Ctx,
@@ -382,14 +455,43 @@ static void keepSpecifiers(std::string &ReturnType, std::string &Auto,
   }
 }
 
+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(
@@ -401,8 +503,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
@@ -495,4 +602,41 @@ void UseTrailingReturnTypeCheck::check(const MatchFinder::MatchResult &Result) {
       << 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(), ErrorMessageOnLambda);
+    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(), ErrorMessageOnLambda);
+}
+
 } // namespace clang::tidy::modernize

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

@@ -26,18 +26,25 @@ 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;
+
+  void diagOnLambda(const LambdaExpr *Lambda,
+                    const ast_matchers::MatchFinder::MatchResult &Result);
 };
 
 } // namespace clang::tidy::modernize

clang-tools-extra/clangd/ModulesBuilder.cpp

@@ -430,10 +430,10 @@ class CachingProjectModules : public ProjectModules {
 /// Collect the directly and indirectly required module names for \param
 /// ModuleName in topological order. The \param ModuleName is guaranteed to
 /// be the last element in \param ModuleNames.
-llvm::SmallVector<StringRef> getAllRequiredModules(PathRef RequiredSource,
-                                                   CachingProjectModules &MDB,
-                                                   StringRef ModuleName) {
-  llvm::SmallVector<llvm::StringRef> ModuleNames;
+llvm::SmallVector<std::string> getAllRequiredModules(PathRef RequiredSource,
+                                                     CachingProjectModules &MDB,
+                                                     StringRef ModuleName) {
+  llvm::SmallVector<std::string> ModuleNames;
   llvm::StringSet<> ModuleNamesSet;
 
   auto VisitDeps = [&](StringRef ModuleName, auto Visitor) -> void {
@@ -444,7 +444,7 @@ llvm::SmallVector<StringRef> getAllRequiredModules(PathRef RequiredSource,
       if (ModuleNamesSet.insert(RequiredModuleName).second)
         Visitor(RequiredModuleName, Visitor);
 
-    ModuleNames.push_back(ModuleName);
+    ModuleNames.push_back(ModuleName.str());
   };
   VisitDeps(ModuleName, VisitDeps);
 
@@ -494,28 +494,30 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
   // Get Required modules in topological order.
   auto ReqModuleNames = getAllRequiredModules(RequiredSource, MDB, ModuleName);
   for (llvm::StringRef ReqModuleName : ReqModuleNames) {
-    if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
+    if (BuiltModuleFiles.isModuleUnitBuilt(ReqModuleName))
       continue;
 
     if (auto Cached = Cache.getModule(ReqModuleName)) {
       if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles,
                                TFS.view(std::nullopt))) {
-        log("Reusing module {0} from {1}", ModuleName,
+        log("Reusing module {0} from {1}", ReqModuleName,
             Cached->getModuleFilePath());
         BuiltModuleFiles.addModuleFile(std::move(Cached));
         continue;
       }
       Cache.remove(ReqModuleName);
     }
 
+    std::string ReqFileName =
+        MDB.getSourceForModuleName(ReqModuleName, RequiredSource);
     llvm::Expected<ModuleFile> MF = buildModuleFile(
-        ModuleName, ModuleUnitFileName, getCDB(), TFS, BuiltModuleFiles);
+        ReqModuleName, ReqFileName, getCDB(), TFS, BuiltModuleFiles);
     if (llvm::Error Err = MF.takeError())
       return Err;
 
-    log("Built module {0} to {1}", ModuleName, MF->getModuleFilePath());
+    log("Built module {0} to {1}", ReqModuleName, MF->getModuleFilePath());
     auto BuiltModuleFile = std::make_shared<const ModuleFile>(std::move(*MF));
-    Cache.add(ModuleName, BuiltModuleFile);
+    Cache.add(ReqModuleName, BuiltModuleFile);
     BuiltModuleFiles.addModuleFile(std::move(BuiltModuleFile));
   }
 

clang-tools-extra/clangd/test/module_dependencies.test

@@ -0,0 +1,96 @@
+# A smoke test to check that a simple dependency chain for modules can work.
+#
+# FIXME: This fails on the Windows ARM64 build server. Not entirely sure why as it has been tested on
+#        an ARM64 Windows VM and appears to work there.
+# UNSUPPORTED: host=aarch64-pc-windows-msvc
+#
+# RUN: rm -fr %t
+# RUN: mkdir -p %t
+# RUN: split-file %s %t
+#
+# RUN: sed -e "s|DIR|%/t|g" %t/compile_commands.json.tmpl > %t/compile_commands.json.tmp
+# RUN: sed -e "s|CLANG_CC|%clang|g" %t/compile_commands.json.tmp > %t/compile_commands.json
+# RUN: sed -e "s|DIR|%/t|g" %t/definition.jsonrpc.tmpl > %t/definition.jsonrpc.tmp
+#
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' %/t/definition.jsonrpc.tmp > %/t/definition.jsonrpc
+#
+# RUN: clangd -experimental-modules-support -lit-test < %t/definition.jsonrpc \
+# RUN:      | FileCheck -strict-whitespace %t/definition.jsonrpc
+
+#--- A-frag.cppm
+export module A:frag;
+export void printA() {}
+
+#--- A.cppm
+export module A;
+export import :frag;
+
+#--- Use.cpp
+import A;
+void foo() {
+    print
+}
+
+#--- compile_commands.json.tmpl
+[
+    {
+      "directory": "DIR",
+      "command": "CLANG_CC -fprebuilt-module-path=DIR -std=c++20 -o DIR/main.cpp.o -c DIR/Use.cpp",
+      "file": "DIR/Use.cpp"
+    },
+    {
+      "directory": "DIR",
+      "command": "CLANG_CC -std=c++20 DIR/A.cppm --precompile -o DIR/A.pcm",
+      "file": "DIR/A.cppm"
+    },
+    {
+      "directory": "DIR",
+      "command": "CLANG_CC -std=c++20 DIR/A-frag.cppm --precompile -o DIR/A-frag.pcm",
+      "file": "DIR/A-frag.cppm"
+    }
+]
+
+#--- definition.jsonrpc.tmpl
+{
+  "jsonrpc": "2.0",
+  "id": 0,
+  "method": "initialize",
+  "params": {
+    "processId": 123,
+    "rootPath": "clangd",
+    "capabilities": {
+      "textDocument": {
+        "completion": {
+          "completionItem": {
+            "snippetSupport": true
+          }
+        }
+      }
+    },
+    "trace": "off"
+  }
+}
+---
+{
+  "jsonrpc": "2.0",
+  "method": "textDocument/didOpen",
+  "params": {
+    "textDocument": {
+      "uri": "file://DIR/Use.cpp",
+      "languageId": "cpp",
+      "version": 1,
+      "text": "import A;\nvoid foo() {\n    print\n}\n"
+    }
+  }
+}
+
+# CHECK: "message"{{.*}}printA{{.*}}(fix available)
+
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file://DIR/Use.cpp"},"context":{"triggerKind":1},"position":{"line":2,"character":6}}}
+---
+{"jsonrpc":"2.0","id":2,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}