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 @@ -11,15 +11,29 @@ #include "HeuristicResolver.h" #include "ParsedAST.h" #include "clang/AST/DeclarationName.h" +#include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Builtins.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/ScopeExit.h" namespace clang { namespace clangd { + namespace { +AST_MATCHER(CallExpr, isStdForward) { + return Node.getBuiltinCallee() == Builtin::BIforward; + return Node.getCalleeDecl()->isInStdNamespace() && Node.getCalleeDecl() + ->getAsFunction() + ->getDeclName() + .getAsIdentifierInfo() + ->isStr("forward"); +} + // For now, inlay hints are always anchored at the left or right of their range. enum class HintSide { Left, Right }; @@ -369,6 +383,66 @@ private: using NameVec = SmallVector; + llvm::SmallVector matchForwardedParams( + const FunctionDecl *Callee, + const llvm::ArrayRef &VariadicParams) { + // Store forwarded ParmVarDecl if found, nullptr otherwise + llvm::SmallVector Result(VariadicParams.size()); + if (!Callee->hasBody()) { + return Result; + } + using namespace clang::ast_matchers; + // Match either std::forward(args)... or args... inside invocation + // arguments + auto ForwardedParmMatcher = compoundStmt(forEachDescendant( + invocation( + forEach(implicitCastExpr( + anyOf(has(declRefExpr( + hasDeclaration(parmVarDecl().bind("arg")))), + has(callExpr(isStdForward(), + has(declRefExpr(hasDeclaration( + parmVarDecl().bind("arg")))))))) + .bind("cast"))) + .bind("call"))); + auto Matches = match(ForwardedParmMatcher, *Callee->getBody(), AST); + // Store lookup table to find location of ParmVarDecls in VariadicParams + llvm::SmallDenseMap ParamLocations; + size_t ParamIdx = 0; + for (const auto *Param : VariadicParams) { + ParamLocations.insert({Param, ParamIdx}); + ParamIdx++; + } + for (auto Entry : Matches) { + const auto *Param = Entry.getNodeAs("arg"); + const auto LocIt = ParamLocations.find(Param); + // If the argument in the forwarded call matches one of our VariadicParams + if (LocIt != ParamLocations.end()) { + const auto *Cast = Entry.getNodeAs("cast"); + const auto *Call = Entry.getNodeAs("call"); + const auto *Constructor = Entry.getNodeAs("call"); + // Grab the corresponding forwarded parameter + if (Call) { + auto CallLoc = std::distance( + Call->arg_begin(), + std::find(Call->arg_begin(), Call->arg_end(), Cast)); + assert(CallLoc < Call->getNumArgs()); + Result[LocIt->second] = + Call->getCalleeDecl()->getAsFunction()->getParamDecl(CallLoc); + } else { + assert(Constructor); + auto CallLoc = std::distance(Constructor->arg_begin(), + std::find(Constructor->arg_begin(), + Constructor->arg_end(), Cast)); + assert(CallLoc < Constructor->getNumArgs()); + Result[LocIt->second] = + Constructor->getConstructor()->getParamDecl(CallLoc); + } + } + } + + return Result; + } + // The purpose of Anchor is to deal with macros. It should be the call's // opening or closing parenthesis or brace. (Always using the opening would // make more sense but CallExpr only exposes the closing.) We heuristically @@ -389,9 +463,10 @@ if (Ctor->isCopyOrMoveConstructor()) return; - // Don't show hints for variadic parameters. + // Skip variadic parameters for now. size_t FixedParamCount = getFixedParamCount(Callee); size_t ArgCount = std::min(FixedParamCount, Args.size()); + const auto Params = Callee->parameters(); NameVec ParameterNames = chooseParameterNames(Callee, ArgCount); @@ -400,7 +475,18 @@ if (isSetter(Callee, ParameterNames)) return; - for (size_t I = 0; I < ArgCount; ++I) { + // If the function call contains a variadic parameter, attempt to forward + // the parameter names from the wrapped function + if (Args.size() > FixedParamCount && Args.size() == Params.size()) { + auto ForwardedParams = matchForwardedParams( + Callee, Params.take_back(Args.size() - ArgCount)); + for (size_t I = ArgCount; I < Args.size(); ++I) { + const auto *Parm = ForwardedParams[I - ArgCount]; + ParameterNames.emplace_back(Parm ? getSimpleName(*Parm) : ""); + } + } + + for (size_t I = 0; I < ParameterNames.size(); ++I) { StringRef Name = ParameterNames[I]; if (!shouldHint(Args[I], Name)) continue; @@ -497,7 +583,7 @@ 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 + // use all the parameter names from the definition (if present in the // translation unit). // We could try a bit harder, e.g.: // - try all re-declarations, not just canonical + definition @@ -563,7 +649,7 @@ return Result; } - // We pass HintSide rather than SourceLocation because we want to ensure + // 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, llvm::StringRef Prefix, llvm::StringRef Label, @@ -617,7 +703,8 @@ std::string TypeName = T.getAsString(Policy); if (TypeName.length() < TypeNameLimit) addInlayHint(R, HintSide::Right, InlayHintKind::TypeHint, Prefix, - TypeName, /*Suffix=*/""); + TypeName, + /*Suffix=*/""); } void addDesignatorHint(SourceRange R, llvm::StringRef Text) { 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 @@ -162,6 +162,159 @@ ExpectedHint{"good: ", "good"}); } +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 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, VariadicForwardedConstructor) { + // No hint for anonymous variadic parameter + // The 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{$wrapped[[std::forward(args)...]]}; } + void baz() { + bar($param[[42]]); + } + )cpp", + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, VariadicPlainConstructor) { + // No hint for anonymous variadic parameter + assertParameterHints(R"cpp( + struct S { S(int a); }; + template + T bar(Args&&... args) { return T{$wrapped[[args...]]}; } + void baz() { + bar($param[[42]]); + } + )cpp", + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, VariadicForwardedNewConstructor) { + // No hint for anonymous variadic parameter + // The 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{$wrapped[[std::forward(args)...]]}; } + void baz() { + bar($param[[42]]); + } + )cpp", + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, VariadicPlainNewConstructor) { + // No hint for anonymous variadic parameter + assertParameterHints(R"cpp( + struct S { S(int a); }; + template + T* bar(Args&&... args) { return new T{$wrapped[[args...]]}; } + void baz() { + bar($param[[42]]); + } + )cpp", + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, VariadicForwarded) { + // No hint for anonymous variadic parameter + // The 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($wrapped[[std::forward(args)...]]); } + void baz() { + bar($param[[42]]); + } + )cpp", + ExpectedHint{"a: ", "wrapped"}, + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, VariadicPlain) { + // No hint for anonymous variadic parameter + assertParameterHints(R"cpp( + void foo(int a); + template + void bar(Args&&... args) { return foo($wrapped[[args...]]); } + void baz() { + bar($param[[42]]); + } + )cpp", + ExpectedHint{"a: ", "wrapped"}, + ExpectedHint{"a: ", "param"}); +} + +TEST(ParameterHints, MatchingNameVariadicForwarded) { + // No hint for anonymous variadic parameter + // The 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($wrapped[[std::forward(args)...]]); } + void baz() { + int a; + bar(a); + } + )cpp", + ExpectedHint{"a: ", "wrapped"}); +} + +TEST(ParameterHints, MatchingNameVariadicPlain) { + // No hint for anonymous variadic parameter + assertParameterHints(R"cpp( + void foo(int a); + template + void bar(Args&&... args) { return foo($wrapped[[args...]]); } + void baz() { + int a; + bar(a); + } + )cpp", + ExpectedHint{"a: ", "wrapped"}); +} + TEST(ParameterHints, Operator) { // No hint for operator call with operator syntax. assertParameterHints(R"cpp(