diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -164,6 +164,9 @@ /// If true, use the dirty buffer contents when building Preambles. bool UseDirtyHeaders = false; + // If true, parse make_unique-like functions in the preamble. + bool PreambleParseForwardingFunctions = false; + explicit operator TUScheduler::Options() const; }; // Sensible default options for use in tests. @@ -416,6 +419,8 @@ bool UseDirtyHeaders = false; + bool PreambleParseForwardingFunctions = false; + // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex) llvm::StringMap> CachedCompletionFuzzyFindRequestByFile; diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -137,6 +137,7 @@ Opts.AsyncThreadsCount = AsyncThreadsCount; Opts.RetentionPolicy = RetentionPolicy; Opts.StorePreamblesInMemory = StorePreamblesInMemory; + Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions; Opts.UpdateDebounce = UpdateDebounce; Opts.ContextProvider = ContextProvider; return Opts; @@ -148,7 +149,9 @@ : FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS), DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr), ClangTidyProvider(Opts.ClangTidyProvider), - UseDirtyHeaders(Opts.UseDirtyHeaders), WorkspaceRoot(Opts.WorkspaceRoot), + UseDirtyHeaders(Opts.UseDirtyHeaders), + PreambleParseForwardingFunctions(Opts.PreambleParseForwardingFunctions), + WorkspaceRoot(Opts.WorkspaceRoot), Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate : TUScheduler::NoInvalidation), DirtyFS(std::make_unique(TFS, DraftMgr)) { @@ -910,7 +913,7 @@ // It's safe to pass in the TU, as dumpAST() does not // deserialize the preamble. auto Node = DynTypedNode::create( - *Inputs->AST.getASTContext().getTranslationUnitDecl()); + *Inputs->AST.getASTContext().getTranslationUnitDecl()); return CB(dumpAST(Node, Inputs->AST.getTokens(), Inputs->AST.getASTContext())); } diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -14,6 +14,7 @@ #include "clang/AST/DeclarationName.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Basic/Builtins.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/ScopeExit.h" @@ -185,6 +186,168 @@ return Designators; } +bool isExpandedParameter(const ParmVarDecl *Param) { + auto PlainType = Param->getType().getNonReferenceType(); + if (const auto *SubstType = dyn_cast(PlainType)) { + return SubstType->getReplacedParameter()->isParameterPack(); + } + return false; +} + +class ForwardingParameterVisitor + : public RecursiveASTVisitor { +public: + void + resolveForwardedParameters(const FunctionDecl *Callee, + llvm::SmallVector &Params) { + if (!TraversedFunctions.contains(Callee)) { + UnresolvedParams.clear(); + size_t ParamIdx = 0; + for (const auto *Param : Params) { + // If the parameter is part of an expanded pack and not yet resolved + if (isExpandedParameter(Param) && + ForwardedParams.find(Param) == ForwardedParams.end()) { + UnresolvedParams.insert(Param); + } + handleParmVarDeclName(Callee, ParamIdx); + ParamIdx++; + } + // Recursively resolve forwarded parameters + if (!UnresolvedParams.empty()) { + TraversalQueue.push_back(const_cast(Callee)); + while (!TraversalQueue.empty()) { + auto *CurrentFunction = TraversalQueue.front(); + TraversedFunctions.insert(CurrentFunction); + TraverseFunctionDecl(CurrentFunction); + TraversalQueue.pop_front(); + } + } + } + // Replace parameters by their forwarded counterpart + for (auto &Param : Params) { + Param = getForwardedParameter(Param); + Param = getNamedParameter(Param); + } + } + + bool VisitCXXConstructExpr(CXXConstructExpr *E) { + // if there are no va_args, we can forward parameters + if (E->getConstructor()->getNumParams() == E->getNumArgs()) { + handleCallOrConstructExpr(E->getConstructor(), E->arguments()); + } + return false; + } + + bool VisitCallExpr(CallExpr *D) { + if (auto *Callee = dyn_cast(D->getCalleeDecl())) { + // if there are no va_args, we can forward parameters + if (Callee->getNumParams() == D->getNumArgs()) { + handleCallOrConstructExpr(Callee, D->arguments()); + } + } + return false; + } + +private: + void handleParmVarDeclName(const FunctionDecl *Callee, size_t I) { + const auto *Param = Callee->getParamDecl(I); + // If the declaration doesn't provide a name + if (!Param->getDeclName().getAsIdentifierInfo()) { + // Try the definition + if (const auto *Def = Callee->getDefinition()) { + const auto *DefParam = Def->getParamDecl(I); + if (DefParam != Param && + DefParam->getDeclName().getAsIdentifierInfo()) { + // Store the mapping declaration parameter -> definition parameter + ParamDeclToDef.insert({Param, DefParam}); + } + } + } + } + + void handleCallOrConstructExpr(FunctionDecl *Callee, + CallExpr::arg_range Args) { + size_t ArgIdx = 0; + bool Recurse = false; + for (const auto *Arg : Args) { + if (const auto *ArgRef = unpackArgument(Arg)) { + if (const auto *ArgSource = dyn_cast(ArgRef->getDecl())) { + const auto It = UnresolvedParams.find(ArgSource); + if (It == UnresolvedParams.end()) { + continue; + } + const auto *Param = Callee->getParamDecl(ArgIdx); + // If this parameter is part of an expanded pack, we need to recurse + if (isExpandedParameter(Param)) { + UnresolvedParams.insert(Param); + Recurse = true; + } else { + // If it is a concrete parameter, we need to check whether it + // provides a name, and if not check its definition + handleParmVarDeclName(Callee, ArgIdx); + } + // This parameter has now been resolved + UnresolvedParams.erase(It); + ForwardedParams.insert({ArgSource, Param}); + } + } + ArgIdx++; + } + if (Recurse && !TraversedFunctions.contains(Callee)) { + TraversalQueue.push_back(Callee); + } + } + + const Expr *unpackImplicitCast(const Expr *E) { + if (const auto *Cast = dyn_cast(E)) { + return Cast->getSubExpr(); + } + return E; + } + + const Expr *unpackForward(const Expr *E) { + if (const auto *Call = dyn_cast(E)) { + const auto Callee = Call->getBuiltinCallee(); + if (Callee == Builtin::BIforward) { + return Call->getArg(0); + } + } + return E; + } + + const DeclRefExpr *unpackArgument(const Expr *E) { + E = unpackImplicitCast(E); + E = unpackForward(E); + return dyn_cast(E); + } + + const ParmVarDecl *getForwardedParameter(const ParmVarDecl *Param) { + auto It = ForwardedParams.find(Param); + if (It == ForwardedParams.end()) { + return Param; + } + // path compression + It->second = getForwardedParameter(It->second); + return It->second; + } + + const ParmVarDecl *getNamedParameter(const ParmVarDecl *Param) { + auto It = ParamDeclToDef.find(Param); + if (It == ParamDeclToDef.end()) { + return Param; + } + return It->second; + } + + llvm::DenseSet UnresolvedParams; + std::deque TraversalQueue; + llvm::DenseSet TraversedFunctions; + llvm::DenseMap ForwardedParams; + // For parameters that don't have a name in the function declaration, but do + // have one in the definition, store this mapping here. + llvm::DenseMap ParamDeclToDef; +}; + class InlayHintVisitor : public RecursiveASTVisitor { public: InlayHintVisitor(std::vector &Results, ParsedAST &AST, @@ -392,21 +555,29 @@ if (Ctor->isCopyOrMoveConstructor()) return; - // Don't show hints for variadic parameters. - size_t FixedParamCount = getFixedParamCount(Callee); - size_t ArgCount = std::min(FixedParamCount, Args.size()); - auto Params = Callee->parameters(); + SmallVector Params{Callee->param_begin(), + Callee->param_end()}; + + FwdVisitor.resolveForwardedParameters(Callee, Params); - NameVec ParameterNames = chooseParameterNames(Callee, ArgCount); + NameVec ParameterNames = chooseParameterNames(Params); // Exclude setters (i.e. functions with one argument whose name begins with // "set"), as their parameter name is also not likely to be interesting. if (isSetter(Callee, ParameterNames)) return; - for (size_t I = 0; I < ArgCount; ++I) { + // Exclude builtins like std::move and std::forward + if (Callee->getBuiltinID() == Builtin::BImove || + Callee->getBuiltinID() == Builtin::BIforward) + return; + + for (size_t I = 0; I < Params.size(); ++I) { StringRef Name = ParameterNames[I]; - bool NameHint = shouldHintName(Args[I], Name); + // Skip unexpanded pack expansion expressions + if (I >= Args.size() || isa(Args[I])) + break; + bool NameHint = shouldHintName(Args[I], Params[I], Name); bool ReferenceHint = shouldHintReference(Params[I]); if (NameHint || ReferenceHint) { @@ -440,7 +611,8 @@ return WhatItIsSetting.equals_insensitive(ParamNames[0]); } - bool shouldHintName(const Expr *Arg, StringRef ParamName) { + bool shouldHintName(const Expr *Arg, const ParmVarDecl *Param, + StringRef ParamName) { if (ParamName.empty()) return false; @@ -453,6 +625,11 @@ if (isPrecededByParamNameComment(Arg, ParamName)) return false; + // Exclude parameters that are unresolved parameter packs + if (isExpandedParameter(Param)) { + return false; + } + return true; } @@ -507,23 +684,12 @@ return {}; } - NameVec chooseParameterNames(const FunctionDecl *Callee, size_t ArgCount) { - // The current strategy here is to use all the parameter names from the - // canonical declaration, unless they're all empty, in which case we - // use all the parameter names from the definition (in present in the - // translation unit). - // We could try a bit harder, e.g.: - // - try all re-declarations, not just canonical + definition - // - fall back arg-by-arg rather than wholesale - - NameVec ParameterNames = getParameterNamesForDecl(Callee, ArgCount); - - if (llvm::all_of(ParameterNames, std::mem_fn(&StringRef::empty))) { - if (const FunctionDecl *Def = Callee->getDefinition()) { - ParameterNames = getParameterNamesForDecl(Def, ArgCount); - } + NameVec + chooseParameterNames(const SmallVector Parameters) { + NameVec ParameterNames(Parameters.size()); + for (size_t I = 0; I < Parameters.size(); I++) { + ParameterNames[I] = getSimpleName(*Parameters[I]); } - assert(ParameterNames.size() == ArgCount); // Standard library functions often have parameter names that start // with underscores, which makes the hints noisy, so strip them out. @@ -537,26 +703,6 @@ Name = Name.ltrim('_'); } - // Return the number of fixed parameters Function has, that is, not counting - // parameters that are variadic (instantiated from a parameter pack) or - // C-style varargs. - static size_t getFixedParamCount(const FunctionDecl *Function) { - if (FunctionTemplateDecl *Template = Function->getPrimaryTemplate()) { - FunctionDecl *F = Template->getTemplatedDecl(); - size_t Result = 0; - for (ParmVarDecl *Parm : F->parameters()) { - if (Parm->isParameterPack()) { - break; - } - ++Result; - } - return Result; - } - // C-style varargs don't need special handling, they're already - // not included in getNumParams(). - return Function->getNumParams(); - } - static StringRef getSimpleName(const NamedDecl &D) { if (IdentifierInfo *Ident = D.getDeclName().getAsIdentifierInfo()) { return Ident->getName(); @@ -565,17 +711,6 @@ return StringRef(); } - NameVec getParameterNamesForDecl(const FunctionDecl *Function, - size_t ArgCount) { - NameVec Result; - for (size_t I = 0; I < ArgCount; ++I) { - const ParmVarDecl *Parm = Function->getParamDecl(I); - assert(Parm); - Result.emplace_back(getSimpleName(*Parm)); - } - return Result; - } - // We pass HintSide rather than SourceLocation because we want to ensure // it is in the same file as the common file range. void addInlayHint(SourceRange R, HintSide Side, InlayHintKind Kind, @@ -645,6 +780,7 @@ FileID MainFileID; StringRef MainFileBuf; const HeuristicResolver *Resolver; + ForwardingParameterVisitor FwdVisitor; // We want to suppress default template arguments, but otherwise print // canonical types. Unfortunately, they're conflicting policies so we can't // have both. For regular types, suppressing template arguments is more diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -100,6 +100,7 @@ std::shared_ptr buildPreamble(PathRef FileName, CompilerInvocation CI, const ParseInputs &Inputs, bool StoreInMemory, + bool ParseForwardingFunctions, PreambleParsedCallback PreambleCallback, PreambleBuildStats *Stats = nullptr); diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -66,9 +66,10 @@ public: CppFilePreambleCallbacks( PathRef File, PreambleParsedCallback ParsedCallback, - PreambleBuildStats *Stats, + PreambleBuildStats *Stats, bool ParseForwardingFunctions, std::function BeforeExecuteCallback) : File(File), ParsedCallback(ParsedCallback), Stats(Stats), + ParseForwardingFunctions(ParseForwardingFunctions), BeforeExecuteCallback(std::move(BeforeExecuteCallback)) {} IncludeStructure takeIncludes() { return std::move(Includes); } @@ -139,12 +140,38 @@ bool shouldSkipFunctionBody(Decl *D) override { // Generally we skip function bodies in preambles for speed. // We can make exceptions for functions that are cheap to parse and - // instantiate, widely used, and valuable (e.g. commonly produce errors). - if (const auto *FT = llvm::dyn_cast(D)) { - if (const auto *II = FT->getDeclName().getAsIdentifierInfo()) - // std::make_unique is trivial, and we diagnose bad constructor calls. - if (II->isStr("make_unique") && FT->isInStdNamespace()) - return false; + // instantiate, widely used, and valuable (e.g. commonly produce errors, + // necessary for code completion/inlay hints). + if (auto *FT = llvm::dyn_cast(D)) { + // Fast path: only take care of make_unique + if (!ParseForwardingFunctions) { + if (const auto *II = FT->getDeclName().getAsIdentifierInfo()) { + // std::make_unique is trivial, and we diagnose bad constructor calls. + if (FT->isInStdNamespace() && II->isStr("make_unique")) + return false; + } + return true; + } + // Slow path: Check whether its last parameter is a parameter pack... + if (const auto *FD = FT->getAsFunction()) { + const auto NumParams = FD->getNumParams(); + if (NumParams > 0) { + const auto *LastParam = FD->getParamDecl(NumParams - 1); + if (isa(LastParam->getType())) { + const auto *PackTypePtr = + dyn_cast(LastParam->getType() + .getNonPackExpansionType() + .getNonReferenceType() + .getTypePtr()); + // ... whose template parameter comes from the function directly + if (FT->getTemplateParameters()->getDepth() == + PackTypePtr->getDepth()) { + return false; + } + } + } + return true; + } } return true; } @@ -161,6 +188,7 @@ const clang::LangOptions *LangOpts = nullptr; const SourceManager *SourceMgr = nullptr; PreambleBuildStats *Stats; + bool ParseForwardingFunctions; std::function BeforeExecuteCallback; }; @@ -426,11 +454,10 @@ } // namespace -std::shared_ptr -buildPreamble(PathRef FileName, CompilerInvocation CI, - const ParseInputs &Inputs, bool StoreInMemory, - PreambleParsedCallback PreambleCallback, - PreambleBuildStats *Stats) { +std::shared_ptr buildPreamble( + PathRef FileName, CompilerInvocation CI, const ParseInputs &Inputs, + bool StoreInMemory, bool ParseForwardingFunctions, + PreambleParsedCallback PreambleCallback, PreambleBuildStats *Stats) { // Note that we don't need to copy the input contents, preamble can live // without those. auto ContentsBuffer = @@ -484,6 +511,7 @@ CI.getPreprocessorOpts().WriteCommentListToPCH = false; CppFilePreambleCallbacks CapturedInfo(FileName, PreambleCallback, Stats, + ParseForwardingFunctions, [&ASTListeners](CompilerInstance &CI) { for (const auto &L : ASTListeners) L->beforeExecute(CI); diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -204,6 +204,9 @@ /// Typically to inject per-file configuration. /// If the path is empty, context sholud be "generic". std::function ContextProvider; + + // If true, parse make_unique-like functions in the preamble. + bool PreambleParseForwardingFunctions = false; }; TUScheduler(const GlobalCompilationDatabase &CDB, const Options &Opts, diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -389,11 +389,12 @@ public: PreambleThread(llvm::StringRef FileName, ParsingCallbacks &Callbacks, bool StorePreambleInMemory, bool RunSync, - SynchronizedTUStatus &Status, + bool ParseForwardingFunctions, SynchronizedTUStatus &Status, TUScheduler::HeaderIncluderCache &HeaderIncluders, ASTWorker &AW) : FileName(FileName), Callbacks(Callbacks), - StoreInMemory(StorePreambleInMemory), RunSync(RunSync), Status(Status), + StoreInMemory(StorePreambleInMemory), RunSync(RunSync), + ParseForwardingFunctions(ParseForwardingFunctions), Status(Status), ASTPeer(AW), HeaderIncluders(HeaderIncluders) {} /// It isn't guaranteed that each requested version will be built. If there @@ -518,6 +519,7 @@ ParsingCallbacks &Callbacks; const bool StoreInMemory; const bool RunSync; + bool ParseForwardingFunctions; SynchronizedTUStatus &Status; ASTWorker &ASTPeer; @@ -778,7 +780,8 @@ ContextProvider(Opts.ContextProvider), CDB(CDB), Callbacks(Callbacks), Barrier(Barrier), Done(false), Status(FileName, Callbacks), PreamblePeer(FileName, Callbacks, Opts.StorePreamblesInMemory, RunSync, - Status, HeaderIncluders, *this) { + Opts.PreambleParseForwardingFunctions, Status, + HeaderIncluders, *this) { // Set a fallback command because compile command can be accessed before // `Inputs` is initialized. Other fields are only used after initialization // from client inputs. @@ -1012,7 +1015,7 @@ PreambleBuildStats Stats; bool IsFirstPreamble = !LatestBuild; LatestBuild = clang::clangd::buildPreamble( - FileName, *Req.CI, Inputs, StoreInMemory, + FileName, *Req.CI, Inputs, StoreInMemory, ParseForwardingFunctions, [this, Version(Inputs.Version)](ASTContext &Ctx, Preprocessor &PP, const CanonicalIncludes &CanonIncludes) { Callbacks.onPreambleAST(FileName, Version, Ctx, PP, CanonIncludes); diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -161,6 +161,7 @@ bool buildAST() { log("Building preamble..."); Preamble = buildPreamble(File, *Invocation, Inputs, /*StoreInMemory=*/true, + Opts.PreambleParseForwardingFunctions, [&](ASTContext &Ctx, Preprocessor &PP, const CanonicalIncludes &Includes) { if (!Opts.BuildDynamicSymbolIndex) diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -497,6 +497,11 @@ Hidden, init(ClangdServer::Options().UseDirtyHeaders)}; +opt PreambleParseForwardingFunctions{ + "preamble-parse-forwarding", cat(Misc), + desc("Parse all make_unique-like functions in the preamble"), Hidden, + init(ClangdServer::Options().PreambleParseForwardingFunctions)}; + #if defined(__GLIBC__) && CLANGD_MALLOC_TRIM opt EnableMallocTrim{ "malloc-trim", @@ -934,6 +939,7 @@ Opts.ClangTidyProvider = ClangTidyOptProvider; } Opts.UseDirtyHeaders = UseDirtyHeaders; + Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions; Opts.QueryDriverGlobs = std::move(QueryDriverGlobs); Opts.TweakFilter = [&](const Tweak &T) { if (T.hidden() && !HiddenFeatures) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -121,8 +121,10 @@ ADD_FAILURE() << "Couldn't build CompilerInvocation"; return {}; } - auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, - /*InMemory=*/true, /*Callback=*/nullptr); + auto Preamble = + buildPreamble(testPath(TU.Filename), *CI, Inputs, + /*StoreInMemory=*/true, + /*PreambleCallback=*/false, /*Callback=*/nullptr); return codeComplete(testPath(TU.Filename), Point, Preamble.get(), Inputs, Opts); } @@ -1191,8 +1193,10 @@ ADD_FAILURE() << "Couldn't build CompilerInvocation"; return {}; } - auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, - /*InMemory=*/true, /*Callback=*/nullptr); + auto Preamble = + buildPreamble(testPath(TU.Filename), *CI, Inputs, + /*StoreInMemory=*/true, + /*PreambleCallback=*/false, /*Callback=*/nullptr); if (!Preamble) { ADD_FAILURE() << "Couldn't build Preamble"; return {}; @@ -1439,8 +1443,10 @@ auto Inputs = TU.inputs(FS); auto CI = buildCompilerInvocation(Inputs, Diags); ASSERT_TRUE(CI); - auto EmptyPreamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, - /*InMemory=*/true, /*Callback=*/nullptr); + auto EmptyPreamble = + buildPreamble(testPath(TU.Filename), *CI, Inputs, + /*StoreInMemory=*/true, + /*PreambleCallback=*/false, /*Callback=*/nullptr); ASSERT_TRUE(EmptyPreamble); TU.AdditionalFiles["a.h"] = "int foo(int x);"; diff --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp --- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp @@ -306,7 +306,7 @@ FileIndex Index; bool IndexUpdated = false; buildPreamble(FooCpp, *CI, PI, - /*StoreInMemory=*/true, + /*StoreInMemory=*/true, /*ParseForwardingFunctions=*/false, [&](ASTContext &Ctx, Preprocessor &PP, const CanonicalIncludes &CanonIncludes) { EXPECT_FALSE(IndexUpdated) diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -170,6 +170,43 @@ )cpp"); } +TEST(ParameterHints, NoNameVariadicDeclaration) { + // No hint for anonymous variadic parameter + assertParameterHints(R"cpp( + template + void foo(Args&& ...); + void bar() { + foo(42); + } + )cpp"); +} + +TEST(ParameterHints, NoNameVariadicForwarded) { + // No hint for anonymous variadic parameter + // The forward prototype is not correct, but is converted into builtin anyways + assertParameterHints(R"cpp( + namespace std { template T&& forward(T&); } + void foo(int); + template + void bar(Args&&... args) { return foo(std::forward(args)...); } + void baz() { + bar(42); + } + )cpp"); +} + +TEST(ParameterHints, NoNameVariadicPlain) { + // No hint for anonymous variadic parameter + assertParameterHints(R"cpp( + void foo(int); + template + void bar(Args&&... args) { return foo(args...); } + void baz() { + bar(42); + } + )cpp"); +} + TEST(ParameterHints, NameInDefinition) { // Parameter name picked up from definition if necessary. assertParameterHints(R"cpp( @@ -254,6 +291,123 @@ ExpectedHint{"param: ", "param"}); } +TEST(ParameterHints, VariadicForwardedConstructor) { + // Name hint for variadic parameter + // The forward prototype is not correct, but is converted into builtin anyways + assertParameterHints(R"cpp( + namespace std { template T&& forward(T&); } + struct S { S(int a); }; + template + T bar(Args&&... args) { return T{std::forward(args)...}; } + void baz() { + int b; + bar($param[[b]]); + } + )cpp", + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, VariadicPlainConstructor) { + // Name hint for variadic parameter + assertParameterHints(R"cpp( + struct S { S(int a); }; + template + T bar(Args&&... args) { return T{args...}; } + void baz() { + int b; + bar($param[[b]]); + } + )cpp", + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, VariadicForwardedNewConstructor) { + // Name hint for variadic parameter + // The forward prototype is not correct, but is converted into builtin anyways + assertParameterHints(R"cpp( + namespace std { template T&& forward(T&); } + struct S { S(int a); }; + template + T* bar(Args&&... args) { return new T{std::forward(args)...}; } + void baz() { + int b; + bar($param[[b]]); + } + )cpp", + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, VariadicPlainNewConstructor) { + // Name hint for variadic parameter + assertParameterHints(R"cpp( + struct S { S(int a); }; + template + T* bar(Args&&... args) { return new T{args...}; } + void baz() { + int b; + bar($param[[b]]); + } + )cpp", + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, VariadicForwarded) { + // Name for variadic parameter + // The forward prototype is not correct, but is converted into builtin anyways + assertParameterHints(R"cpp( + namespace std { template T&& forward(T&); } + void foo(int a); + template + void bar(Args&&... args) { return foo(std::forward(args)...); } + void baz() { + int b; + bar($param[[b]]); + } + )cpp", + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, VariadicPlain) { + // Name hint for variadic parameter + assertParameterHints(R"cpp( + void foo(int a); + template + void bar(Args&&... args) { return foo(args...); } + void baz() { + bar($param[[42]]); + } + )cpp", + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, MatchingNameVariadicForwarded) { + // No name hint for variadic parameter with matching name + // The forward prototype is not correct, but is converted into builtin anyways + assertParameterHints(R"cpp( + namespace std { template T&& forward(T&); } + void foo(int a); + template + void bar(Args&&... args) { return foo(std::forward(args)...); } + void baz() { + int a; + bar(a); + } + )cpp"); +} + +TEST(ParameterHints, MatchingNameVariadicPlain) { + // No name hint for variadic parameter with matching name + assertParameterHints(R"cpp( + void foo(int a); + template + void bar(Args&&... args) { return foo(args...); } + void baz() { + int a; + bar(a); + } + )cpp"); +} + TEST(ParameterHints, Operator) { // No hint for operator call with operator syntax. assertParameterHints(R"cpp( diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -497,7 +497,8 @@ auto Inputs = TU.inputs(FS); auto CI = buildCompilerInvocation(Inputs, Diags); auto EmptyPreamble = - buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr); + buildPreamble(testPath("foo.cpp"), *CI, Inputs, /*StoreInMemory=*/true, + /*PreambleCallback=*/false, nullptr); ASSERT_TRUE(EmptyPreamble); EXPECT_THAT(EmptyPreamble->Includes.MainFileIncludes, IsEmpty()); @@ -540,7 +541,8 @@ auto Inputs = TU.inputs(FS); auto CI = buildCompilerInvocation(Inputs, Diags); auto BaselinePreamble = - buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr); + buildPreamble(testPath("foo.cpp"), *CI, Inputs, /*StoreInMemory=*/true, + /*PreambleCallback=*/false, nullptr); ASSERT_TRUE(BaselinePreamble); EXPECT_THAT(BaselinePreamble->Includes.MainFileIncludes, ElementsAre(testing::Field(&Inclusion::Written, ""))); diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -173,7 +173,8 @@ TU.AdditionalFiles["c.h"] = ""; auto PI = TU.inputs(FS); auto BaselinePreamble = buildPreamble( - TU.Filename, *buildCompilerInvocation(PI, Diags), PI, true, nullptr); + TU.Filename, *buildCompilerInvocation(PI, Diags), PI, + /*StoreInMemory=*/true, /*ParseForwardingFunctions=*/false, nullptr); // We drop c.h from modified and add a new header. Since the latter is patched // we should only get a.h in preamble includes. TU.Code = R"cpp( diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -101,7 +101,9 @@ auto ModuleCacheDeleter = llvm::make_scope_exit( std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath)); return clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, - /*StoreInMemory=*/true, PreambleCallback); + /*StoreInMemory=*/true, + /*ParseForwardingFunctions=*/false, + PreambleCallback); } ParsedAST TestTU::build() const { @@ -115,9 +117,10 @@ auto ModuleCacheDeleter = llvm::make_scope_exit( std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath)); - auto Preamble = clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, - /*StoreInMemory=*/true, - /*PreambleCallback=*/nullptr); + auto Preamble = + clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, + /*StoreInMemory=*/true, + /*PreambleCallback=*/false, nullptr); auto AST = ParsedAST::build(testPath(Filename), Inputs, std::move(CI), Diags.take(), Preamble); if (!AST.hasValue()) {