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 @@ -10,16 +10,26 @@ #include "Config.h" #include "HeuristicResolver.h" #include "ParsedAST.h" +#include "clang/AST/Decl.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; +} + // For now, inlay hints are always anchored at the left or right of their range. enum class HintSide { Left, Right }; @@ -369,6 +379,63 @@ private: using NameVec = SmallVector; + llvm::SmallVector matchForwardedParams( + const FunctionDecl *Callee, + const llvm::ArrayRef &VariadicParams) { + 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++; + } + // Store forwarded ParmVarDecl if found, nullptr otherwise + llvm::SmallVector Result(VariadicParams.size()); + 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 +456,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()); + auto Params = Callee->parameters(); NameVec ParameterNames = chooseParameterNames(Callee, ArgCount); @@ -400,14 +468,31 @@ 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) { + 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 < Args.size(); ++I) { StringRef Name = ParameterNames[I]; - if (!shouldHint(Args[I], Name)) - continue; + bool NameHint = shouldNameHint(Args[I], Name); + std::string Suffix = ": "; + if (!NameHint) { + Name = ""; + Suffix = ""; + } + Suffix += getRefSuffix(Params[I]); - addInlayHint(Args[I]->getSourceRange(), HintSide::Left, - InlayHintKind::ParameterHint, /*Prefix=*/"", Name, - /*Suffix=*/": "); + if (!Name.empty() || !Suffix.empty()) { + addInlayHint(Args[I]->getSourceRange(), HintSide::Left, + InlayHintKind::ParameterHint, /*Prefix=*/"", Name, Suffix); + } } } @@ -434,12 +519,21 @@ return WhatItIsSetting.equals_insensitive(ParamNames[0]); } - bool shouldHint(const Expr *Arg, StringRef ParamName) { + StringRef getRefSuffix(const ParmVarDecl *Param) { + // If the parameter is a non-const reference type, print an inlay hint + auto Type = Param->getType(); + return Type->isReferenceType() && + !Type.getNonReferenceType().isConstQualified() + ? (Type->isLValueReferenceType() ? "&" : "&&") + : ""; + } + + bool shouldNameHint(const Expr *Arg, StringRef ParamName) { if (ParamName.empty()) return false; // If the argument expression is a single name and it matches the - // parameter name exactly, omit the hint. + // parameter name exactly, omit the name hint. if (ParamName == getSpelledIdentifier(Arg)) return false; @@ -497,7 +591,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 +657,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 +711,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 @@ -138,6 +138,39 @@ )cpp"); } +TEST(ParameterHints, NoNameConstReference) { + // No hint for anonymous const l-value ref parameter. + assertParameterHints(R"cpp( + void foo(const int&); + void bar() { + foo(42); + } + )cpp"); +} + +TEST(ParameterHints, NoNameReference) { + // Reference hint for anonymous l-value ref parameter. + assertParameterHints(R"cpp( + void foo(int&); + void bar() { + int i; + foo($param[[i]]); + } + )cpp", + ExpectedHint{"&", "param"}); +} + +TEST(ParameterHints, NoNameRValueReference) { + // Reference hint for anonymous r-value ref parameter. + assertParameterHints(R"cpp( + void foo(int&&); + void bar() { + foo($param[[42]]); + } + )cpp", + ExpectedHint{"&&", "param"}); +} + TEST(ParameterHints, NameInDefinition) { // Parameter name picked up from definition if necessary. assertParameterHints(R"cpp( @@ -162,6 +195,40 @@ ExpectedHint{"good: ", "good"}); } +TEST(ParameterHints, NameConstReference) { + // Only name hint for const l-value ref parameter. + assertParameterHints(R"cpp( + void foo(const int& param); + void bar() { + foo($param[[42]]); + } + )cpp", + ExpectedHint{"param: ", "param"}); +} + +TEST(ParameterHints, NameReference) { + // Reference and name hint for l-value ref parameter. + assertParameterHints(R"cpp( + void foo(int& param); + void bar() { + int i; + foo($param[[i]]); + } + )cpp", + ExpectedHint{"param: &", "param"}); +} + +TEST(ParameterHints, NameRValueReference) { + // Reference and name hint for r-value ref parameter. + assertParameterHints(R"cpp( + void foo(int&& param); + void bar() { + foo($param[[42]]); + } + )cpp", + ExpectedHint{"param: &&", "param"}); +} + TEST(ParameterHints, Operator) { // No hint for operator call with operator syntax. assertParameterHints(R"cpp( @@ -301,6 +368,21 @@ ExpectedHint{"param: ", "param"}); } +TEST(ParameterHints, ArgMatchesParamReference) { + assertParameterHints(R"cpp( + void foo(int& param); + void foo2(const int& param); + void bar() { + int param; + // show reference hint on mutable reference + foo($param[[param]]); + // but not on const reference + foo2(param); + } + )cpp", + ExpectedHint{"&", "param"}); +} + TEST(ParameterHints, LeadingUnderscore) { assertParameterHints(R"cpp( void foo(int p1, int _p2, int __p3); diff --git a/query.txt b/query.txt new file mode 100644 --- /dev/null +++ b/query.txt @@ -0,0 +1,30 @@ +functionTemplateDecl(has(templateTypeParmDecl(isParameterPack()).bind("parameter_pack"))) +functionTemplateDecl(has(functionDecl(hasBody(compoundStmt(hasDescendant(allOf(packExpansionExpr().bind("pack_expansion"),hasParent(invocation().bind("call"))))).bind("body"))))) +functionTemplateDecl(has(functionDecl(hasAnyParameter(allOf(isParameterPack(),parmVarDecl().bind("param")))))) + +functionTemplateDecl( + allOf( + has( + templateTypeParmDecl(isParameterPack()).bind("parameter_pack")), + has( + functionDecl( + allOf( + hasBody + compoundStmt( + has( + invocation( + hasDescendant( + packExpansionExpr().bind("pack_expansion") + ) + ).bind("call") + ) + ) + ), + hasAnyParameter( + parmVarDecl(isParameterPack()).bind("param") + ) + ) + ) + ) + ) +) \ No newline at end of file