[clang-tools-extra] a638648 - [clangd] add inlay hints for std::forward-ed parameter packs
Tobias Ribizel via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 6 13:32:38 PDT 2022
Author: Tobias Ribizel
Date: 2022-07-06T22:23:18+02:00
New Revision: a638648fef76146634c2199ce7b90c4bc6bfed01
URL: https://github.com/llvm/llvm-project/commit/a638648fef76146634c2199ce7b90c4bc6bfed01
DIFF: https://github.com/llvm/llvm-project/commit/a638648fef76146634c2199ce7b90c4bc6bfed01.diff
LOG: [clangd] add inlay hints for std::forward-ed parameter packs
This adds special-case treatment for parameter packs in
make_unique-like functions to forward parameter names to inlay hints.
The parameter packs are being resolved recursively by traversing the
function body of forwarding functions looking for expressions matching
the (std::forwarded) parameters expanded from a pack.
The implementation checks whether parameters are being passed by
(rvalue) reference or value and adds reference inlay hints accordingly.
The traversal has a limited recursion stack depth, and recursive calls
like std::make_tuple are cut off to avoid hinting duplicate parameter
names.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D124690
Added:
Modified:
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/unittests/InlayHintTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index ca838618badd9..1bf876102f991 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -16,19 +16,23 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/DeclarationName.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/NestedNameSpecifier.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/TemplateBase.h"
#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/Builtins.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/Specifiers.h"
#include "clang/Index/USRGeneration.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/None.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/raw_ostream.h"
@@ -667,5 +671,300 @@ bool isDeeplyNested(const Decl *D, unsigned MaxDepth) {
}
return false;
}
+
+namespace {
+
+// returns true for `X` in `template <typename... X> void foo()`
+bool isTemplateTypeParameterPack(NamedDecl *D) {
+ if (const auto *TTPD = dyn_cast<TemplateTypeParmDecl>(D)) {
+ return TTPD->isParameterPack();
+ }
+ return false;
+}
+
+// Returns the template parameter pack type from an instantiated function
+// template, if it exists, nullptr otherwise.
+const TemplateTypeParmType *getFunctionPackType(const FunctionDecl *Callee) {
+ if (const auto *TemplateDecl = Callee->getPrimaryTemplate()) {
+ auto TemplateParams = TemplateDecl->getTemplateParameters()->asArray();
+ // find the template parameter pack from the back
+ const auto It = std::find_if(TemplateParams.rbegin(), TemplateParams.rend(),
+ isTemplateTypeParameterPack);
+ if (It != TemplateParams.rend()) {
+ const auto *TTPD = dyn_cast<TemplateTypeParmDecl>(*It);
+ return TTPD->getTypeForDecl()->castAs<TemplateTypeParmType>();
+ }
+ }
+ return nullptr;
+}
+
+// Returns the template parameter pack type that this parameter was expanded
+// from (if in the Args... or Args&... or Args&&... form), if this is the case,
+// nullptr otherwise.
+const TemplateTypeParmType *getUnderylingPackType(const ParmVarDecl *Param) {
+ const auto *PlainType = Param->getType().getTypePtr();
+ if (auto *RT = dyn_cast<ReferenceType>(PlainType))
+ PlainType = RT->getPointeeTypeAsWritten().getTypePtr();
+ if (const auto *SubstType = dyn_cast<SubstTemplateTypeParmType>(PlainType)) {
+ const auto *ReplacedParameter = SubstType->getReplacedParameter();
+ if (ReplacedParameter->isParameterPack()) {
+ return dyn_cast<TemplateTypeParmType>(
+ ReplacedParameter->getCanonicalTypeUnqualified()->getTypePtr());
+ }
+ }
+ return nullptr;
+}
+
+// This visitor walks over the body of an instantiated function template.
+// The template accepts a parameter pack and the visitor records whether
+// the pack parameters were forwarded to another call. For example, given:
+//
+// template <typename T, typename... Args>
+// auto make_unique(Args... args) {
+// return unique_ptr<T>(new T(args...));
+// }
+//
+// When called as `make_unique<std::string>(2, 'x')` this yields a function
+// `make_unique<std::string, int, char>` with two parameters.
+// The visitor records that those two parameters are forwarded to the
+// `constructor std::string(int, char);`.
+//
+// This information is recorded in the `ForwardingInfo` split into fully
+// resolved parameters (passed as argument to a parameter that is not an
+// expanded template type parameter pack) and forwarding parameters (passed to a
+// parameter that is an expanded template type parameter pack).
+class ForwardingCallVisitor
+ : public RecursiveASTVisitor<ForwardingCallVisitor> {
+public:
+ ForwardingCallVisitor(ArrayRef<const ParmVarDecl *> Parameters)
+ : Parameters{Parameters}, PackType{getUnderylingPackType(
+ Parameters.front())} {}
+
+ bool VisitCallExpr(CallExpr *E) {
+ auto *Callee = getCalleeDeclOrUniqueOverload(E);
+ if (Callee) {
+ handleCall(Callee, E->arguments());
+ }
+ return !Info.hasValue();
+ }
+
+ bool VisitCXXConstructExpr(CXXConstructExpr *E) {
+ auto *Callee = E->getConstructor();
+ if (Callee) {
+ handleCall(Callee, E->arguments());
+ }
+ return !Info.hasValue();
+ }
+
+ // The expanded parameter pack to be resolved
+ ArrayRef<const ParmVarDecl *> Parameters;
+ // The type of the parameter pack
+ const TemplateTypeParmType *PackType;
+
+ struct ForwardingInfo {
+ // If the parameters were resolved to another FunctionDecl, these are its
+ // first non-variadic parameters (i.e. the first entries of the parameter
+ // pack that are passed as arguments bound to a non-pack parameter.)
+ ArrayRef<const ParmVarDecl *> Head;
+ // If the parameters were resolved to another FunctionDecl, these are its
+ // variadic parameters (i.e. the entries of the parameter pack that are
+ // passed as arguments bound to a pack parameter.)
+ ArrayRef<const ParmVarDecl *> Pack;
+ // If the parameters were resolved to another FunctionDecl, these are its
+ // last non-variadic parameters (i.e. the last entries of the parameter pack
+ // that are passed as arguments bound to a non-pack parameter.)
+ ArrayRef<const ParmVarDecl *> Tail;
+ // If the parameters were resolved to another forwarding FunctionDecl, this
+ // is it.
+ Optional<FunctionDecl *> PackTarget;
+ };
+
+ // The output of this visitor
+ Optional<ForwardingInfo> Info;
+
+private:
+ // inspects the given callee with the given args to check whether it
+ // contains Parameters, and sets Info accordingly.
+ void handleCall(FunctionDecl *Callee, typename CallExpr::arg_range Args) {
+ if (std::any_of(Args.begin(), Args.end(), [](const Expr *E) {
+ return dyn_cast<PackExpansionExpr>(E) != nullptr;
+ })) {
+ return;
+ }
+ auto OptPackLocation = findPack(Args);
+ if (OptPackLocation) {
+ size_t PackLocation = OptPackLocation.getValue();
+ ArrayRef<ParmVarDecl *> MatchingParams =
+ Callee->parameters().slice(PackLocation, Parameters.size());
+ // Check whether the function has a parameter pack as the last template
+ // parameter
+ if (const auto *TTPT = getFunctionPackType(Callee)) {
+ // In this case: Separate the parameters into head, pack and tail
+ auto IsExpandedPack = [&](const ParmVarDecl *P) {
+ return getUnderylingPackType(P) == TTPT;
+ };
+ ForwardingInfo FI;
+ FI.Head = MatchingParams.take_until(IsExpandedPack);
+ FI.Pack = MatchingParams.drop_front(FI.Head.size())
+ .take_while(IsExpandedPack);
+ FI.Tail = MatchingParams.drop_front(FI.Head.size() + FI.Pack.size());
+ FI.PackTarget = Callee;
+ Info = FI;
+ return;
+ }
+ // Default case: assume all parameters were fully resolved
+ ForwardingInfo FI;
+ FI.Head = MatchingParams;
+ Info = FI;
+ }
+ }
+
+ // Returns the beginning of the expanded pack represented by Parameters
+ // in the given arguments, if it is there.
+ llvm::Optional<size_t> findPack(typename CallExpr::arg_range Args) {
+ // find the argument directly referring to the first parameter
+ auto FirstMatch = std::find_if(Args.begin(), Args.end(), [&](Expr *Arg) {
+ const auto *RefArg = unwrapArgument(Arg);
+ if (RefArg) {
+ if (Parameters.front() == dyn_cast<ParmVarDecl>(RefArg->getDecl())) {
+ return true;
+ }
+ }
+ return false;
+ });
+ if (FirstMatch == Args.end()) {
+ return llvm::None;
+ }
+ return std::distance(Args.begin(), FirstMatch);
+ }
+
+ static FunctionDecl *getCalleeDeclOrUniqueOverload(CallExpr *E) {
+ Decl *CalleeDecl = E->getCalleeDecl();
+ auto *Callee = dyn_cast_or_null<FunctionDecl>(CalleeDecl);
+ if (!Callee) {
+ if (auto *Lookup = dyn_cast<UnresolvedLookupExpr>(E->getCallee())) {
+ Callee = resolveOverload(Lookup, E);
+ }
+ }
+ // Ignore the callee if the number of arguments is wrong (deal with va_args)
+ if (Callee->getNumParams() == E->getNumArgs())
+ return Callee;
+ return nullptr;
+ }
+
+ static FunctionDecl *resolveOverload(UnresolvedLookupExpr *Lookup,
+ CallExpr *E) {
+ FunctionDecl *MatchingDecl = nullptr;
+ if (!Lookup->requiresADL()) {
+ // Check whether there is a single overload with this number of
+ // parameters
+ for (auto *Candidate : Lookup->decls()) {
+ if (auto *FuncCandidate = dyn_cast_or_null<FunctionDecl>(Candidate)) {
+ if (FuncCandidate->getNumParams() == E->getNumArgs()) {
+ if (MatchingDecl) {
+ // there are multiple candidates - abort
+ return nullptr;
+ }
+ MatchingDecl = FuncCandidate;
+ }
+ }
+ }
+ }
+ return MatchingDecl;
+ }
+
+ // Removes any implicit cast expressions around the given expression.
+ static const Expr *unwrapImplicitCast(const Expr *E) {
+ while (const auto *Cast = dyn_cast<ImplicitCastExpr>(E)) {
+ E = Cast->getSubExpr();
+ }
+ return E;
+ }
+
+ // Maps std::forward(E) to E, nullptr otherwise
+ static const Expr *unwrapForward(const Expr *E) {
+ if (const auto *Call = dyn_cast<CallExpr>(E)) {
+ const auto Callee = Call->getBuiltinCallee();
+ if (Callee == Builtin::BIforward) {
+ return Call->getArg(0);
+ }
+ }
+ return E;
+ }
+
+ // Maps std::forward(DeclRefExpr) to DeclRefExpr, removing any intermediate
+ // implicit casts, nullptr otherwise
+ static const DeclRefExpr *unwrapArgument(const Expr *E) {
+ E = unwrapImplicitCast(E);
+ E = unwrapForward(E);
+ E = unwrapImplicitCast(E);
+ return dyn_cast<DeclRefExpr>(E);
+ }
+};
+
+} // namespace
+
+SmallVector<const ParmVarDecl *>
+resolveForwardingParameters(const FunctionDecl *D, unsigned MaxDepth) {
+ auto Parameters = D->parameters();
+ // If the function has a template parameter pack
+ if (const auto *TTPT = getFunctionPackType(D)) {
+ // Split the parameters into head, pack and tail
+ auto IsExpandedPack = [TTPT](const ParmVarDecl *P) {
+ return getUnderylingPackType(P) == TTPT;
+ };
+ ArrayRef<const ParmVarDecl *> Head = Parameters.take_until(IsExpandedPack);
+ ArrayRef<const ParmVarDecl *> Pack =
+ Parameters.drop_front(Head.size()).take_while(IsExpandedPack);
+ ArrayRef<const ParmVarDecl *> Tail =
+ Parameters.drop_front(Head.size() + Pack.size());
+ SmallVector<const ParmVarDecl *> Result(Parameters.size());
+ // Fill in non-pack parameters
+ auto HeadIt = std::copy(Head.begin(), Head.end(), Result.begin());
+ auto TailIt = std::copy(Tail.rbegin(), Tail.rend(), Result.rbegin());
+ // Recurse on pack parameters
+ size_t Depth = 0;
+ const FunctionDecl *CurrentFunction = D;
+ llvm::SmallSet<const FunctionTemplateDecl *, 4> SeenTemplates;
+ if (const auto *Template = D->getPrimaryTemplate()) {
+ SeenTemplates.insert(Template);
+ }
+ while (!Pack.empty() && CurrentFunction && Depth < MaxDepth) {
+ // Find call expressions involving the pack
+ ForwardingCallVisitor V{Pack};
+ V.TraverseStmt(CurrentFunction->getBody());
+ if (!V.Info) {
+ break;
+ }
+ // If we found something: Fill in non-pack parameters
+ auto Info = V.Info.getValue();
+ HeadIt = std::copy(Info.Head.begin(), Info.Head.end(), HeadIt);
+ TailIt = std::copy(Info.Tail.rbegin(), Info.Tail.rend(), TailIt);
+ // Prepare next recursion level
+ Pack = Info.Pack;
+ CurrentFunction = Info.PackTarget.getValueOr(nullptr);
+ Depth++;
+ // If we are recursing into a previously encountered function: Abort
+ if (CurrentFunction) {
+ if (const auto *Template = CurrentFunction->getPrimaryTemplate()) {
+ bool NewFunction = SeenTemplates.insert(Template).second;
+ if (!NewFunction) {
+ return {Parameters.begin(), Parameters.end()};
+ }
+ }
+ }
+ }
+ // Fill in the remaining unresolved pack parameters
+ HeadIt = std::copy(Pack.begin(), Pack.end(), HeadIt);
+ assert(TailIt.base() == HeadIt);
+ return Result;
+ }
+ return {Parameters.begin(), Parameters.end()};
+}
+
+bool isExpandedFromParameterPack(const ParmVarDecl *D) {
+ return getUnderylingPackType(D) != nullptr;
+}
+
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index c903a34e728fa..e0024c27aca3a 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -199,6 +199,18 @@ bool hasUnstableLinkage(const Decl *D);
/// reasonable.
bool isDeeplyNested(const Decl *D, unsigned MaxDepth = 10);
+/// Recursively resolves the parameters of a FunctionDecl that forwards its
+/// parameters to another function via variadic template parameters. This can
+/// for example be used to retrieve the constructor parameter ParmVarDecl for a
+/// make_unique or emplace_back call.
+llvm::SmallVector<const ParmVarDecl *>
+resolveForwardingParameters(const FunctionDecl *D, unsigned MaxDepth = 10);
+
+/// Checks whether D is instantiated from a function parameter pack
+/// whose type is a bare type parameter pack (e.g. `Args...`), or a
+/// reference to one (e.g. `Args&...` or `Args&&...`).
+bool isExpandedFromParameterPack(const ParmVarDecl *D);
+
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 3afadb517cfc2..9b3b2b1c53224 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -379,7 +379,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
// assume that if this location does not come from a macro definition, then
// the entire argument list likely appears in the main file and can be hinted.
void processCall(SourceLocation Anchor, const FunctionDecl *Callee,
- llvm::ArrayRef<const Expr *const> Args) {
+ llvm::ArrayRef<const Expr *> Args) {
if (!Cfg.InlayHints.Parameters || Args.size() == 0 || !Callee)
return;
@@ -393,12 +393,10 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
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();
+ // Resolve parameter packs to their forwarded parameter
+ auto ForwardedParams = resolveForwardingParameters(Callee);
- NameVec ParameterNames = chooseParameterNames(Callee, ArgCount);
+ NameVec ParameterNames = chooseParameterNames(ForwardedParams);
// Exclude setters (i.e. functions with one argument whose name begins with
// "set"), and builtins like std::move/forward/... as their parameter name
@@ -406,10 +404,18 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
if (isSetter(Callee, ParameterNames) || isSimpleBuiltin(Callee))
return;
- for (size_t I = 0; I < ArgCount; ++I) {
+ for (size_t I = 0; I < ParameterNames.size() && I < Args.size(); ++I) {
+ // Pack expansion expressions cause the 1:1 mapping between arguments and
+ // parameters to break down, so we don't add further inlay hints if we
+ // encounter one.
+ if (isa<PackExpansionExpr>(Args[I])) {
+ break;
+ }
+
StringRef Name = ParameterNames[I];
bool NameHint = shouldHintName(Args[I], Name);
- bool ReferenceHint = shouldHintReference(Params[I]);
+ bool ReferenceHint =
+ shouldHintReference(Callee->getParamDecl(I), ForwardedParams[I]);
if (NameHint || ReferenceHint) {
addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
@@ -473,11 +479,36 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
return true;
}
- bool shouldHintReference(const ParmVarDecl *Param) {
- // If the parameter is a non-const reference type, print an inlay hint
+ bool shouldHintReference(const ParmVarDecl *Param,
+ const ParmVarDecl *ForwardedParam) {
+ // We add a & hint only when the argument is passed as mutable reference.
+ // For parameters that are not part of an expanded pack, this is
+ // straightforward. For expanded pack parameters, it's likely that they will
+ // be forwarded to another function. In this situation, we only want to add
+ // the reference hint if the argument is actually being used via mutable
+ // reference. This means we need to check
+ // 1. whether the value category of the argument is preserved, i.e. each
+ // pack expansion uses std::forward correctly.
+ // 2. whether the argument is ever copied/cast instead of passed
+ // by-reference
+ // Instead of checking this explicitly, we use the following proxy:
+ // 1. the value category can only change from rvalue to lvalue during
+ // forwarding, so checking whether both the parameter of the forwarding
+ // function and the forwarded function are lvalue references detects such
+ // a conversion.
+ // 2. if the argument is copied/cast somewhere in the chain of forwarding
+ // calls, it can only be passed on to an rvalue reference or const lvalue
+ // reference parameter. Thus if the forwarded parameter is a mutable
+ // lvalue reference, it cannot have been copied/cast to on the way.
+ // Additionally, we should not add a reference hint if the forwarded
+ // parameter was only partially resolved, i.e. points to an expanded pack
+ // parameter, since we do not know how it will be used eventually.
auto Type = Param->getType();
+ auto ForwardedType = ForwardedParam->getType();
return Type->isLValueReferenceType() &&
- !Type.getNonReferenceType().isConstQualified();
+ ForwardedType->isLValueReferenceType() &&
+ !ForwardedType.getNonReferenceType().isConstQualified() &&
+ !isExpandedFromParameterPack(ForwardedParam);
}
// Checks if "E" is spelled in the main file and preceded by a C-style comment
@@ -524,23 +555,26 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
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(SmallVector<const ParmVarDecl *> Parameters) {
+ NameVec ParameterNames;
+ for (const auto *P : Parameters) {
+ if (isExpandedFromParameterPack(P)) {
+ // If we haven't resolved a pack paramater (e.g. foo(Args... args)) to a
+ // non-pack parameter, then hinting as foo(args: 1, args: 2, args: 3) is
+ // unlikely to be useful.
+ ParameterNames.emplace_back();
+ } else {
+ auto SimpleName = getSimpleName(*P);
+ // If the parameter is unnamed in the declaration:
+ // attempt to get its name from the definition
+ if (SimpleName.empty()) {
+ if (const auto *PD = getParamDefinition(P)) {
+ SimpleName = getSimpleName(*PD);
+ }
+ }
+ ParameterNames.emplace_back(SimpleName);
}
}
- assert(ParameterNames.size() == ArgCount);
// Standard library functions often have parameter names that start
// with underscores, which makes the hints noisy, so strip them out.
@@ -550,28 +584,24 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
return ParameterNames;
}
- static void stripLeadingUnderscores(StringRef &Name) {
- 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;
+ // for a ParmVarDecl from a function declaration, returns the corresponding
+ // ParmVarDecl from the definition if possible, nullptr otherwise.
+ static const ParmVarDecl *getParamDefinition(const ParmVarDecl *P) {
+ if (auto *Callee = dyn_cast<FunctionDecl>(P->getDeclContext())) {
+ if (auto *Def = Callee->getDefinition()) {
+ auto I = std::distance(
+ Callee->param_begin(),
+ std::find(Callee->param_begin(), Callee->param_end(), P));
+ if (I < Callee->getNumParams()) {
+ return Def->getParamDecl(I);
}
- ++Result;
}
- return Result;
}
- // C-style varargs don't need special handling, they're already
- // not included in getNumParams().
- return Function->getNumParams();
+ return nullptr;
+ }
+
+ static void stripLeadingUnderscores(StringRef &Name) {
+ Name = Name.ltrim('_');
}
static StringRef getSimpleName(const NamedDecl &D) {
@@ -582,17 +612,6 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
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,
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 5644ae0d7cc1f..f5741184d0351 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -174,6 +174,43 @@ TEST(ParameterHints, NoNameRValueReference) {
)cpp");
}
+TEST(ParameterHints, NoNameVariadicDeclaration) {
+ // No hint for anonymous variadic parameter
+ assertParameterHints(R"cpp(
+ template <typename... Args>
+ void foo(Args&& ...);
+ void bar() {
+ foo(42);
+ }
+ )cpp");
+}
+
+TEST(ParameterHints, NoNameVariadicForwarded) {
+ // No hint for anonymous variadic parameter
+ // This prototype of std::forward is sufficient for clang to recognize it
+ assertParameterHints(R"cpp(
+ namespace std { template <typename T> T&& forward(T&); }
+ void foo(int);
+ template <typename... Args>
+ void bar(Args&&... args) { return foo(std::forward<Args>(args)...); }
+ void baz() {
+ bar(42);
+ }
+ )cpp");
+}
+
+TEST(ParameterHints, NoNameVariadicPlain) {
+ // No hint for anonymous variadic parameter
+ assertParameterHints(R"cpp(
+ void foo(int);
+ template <typename... Args>
+ 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(
@@ -186,6 +223,36 @@ TEST(ParameterHints, NameInDefinition) {
ExpectedHint{"param: ", "param"});
}
+TEST(ParameterHints, NamePartiallyInDefinition) {
+ // Parameter name picked up from definition if necessary.
+ assertParameterHints(R"cpp(
+ void foo(int, int b);
+ void bar() {
+ foo($param1[[42]], $param2[[42]]);
+ }
+ void foo(int a, int) {};
+ )cpp",
+ ExpectedHint{"a: ", "param1"},
+ ExpectedHint{"b: ", "param2"});
+}
+
+TEST(ParameterHints, NameInDefinitionVariadic) {
+ // Parameter name picked up from definition in a resolved forwarded parameter.
+ assertParameterHints(R"cpp(
+ void foo(int, int);
+ template <typename... Args>
+ void bar(Args... args) {
+ foo(args...);
+ }
+ void baz() {
+ bar($param1[[42]], $param2[[42]]);
+ }
+ void foo(int a, int b) {};
+ )cpp",
+ ExpectedHint{"a: ", "param1"},
+ ExpectedHint{"b: ", "param2"});
+}
+
TEST(ParameterHints, NameMismatch) {
// Prefer name from declaration.
assertParameterHints(R"cpp(
@@ -258,6 +325,455 @@ TEST(ParameterHints, NameRValueReference) {
ExpectedHint{"param: ", "param"});
}
+TEST(ParameterHints, VariadicForwardedConstructor) {
+ // Name hint for variadic parameter using std::forward in a constructor call
+ // This prototype of std::forward is sufficient for clang to recognize it
+ assertParameterHints(R"cpp(
+ namespace std { template <typename T> T&& forward(T&); }
+ struct S { S(int a); };
+ template <typename T, typename... Args>
+ T bar(Args&&... args) { return T{std::forward<Args>(args)...}; }
+ void baz() {
+ int b;
+ bar<S>($param[[b]]);
+ }
+ )cpp",
+ ExpectedHint{"a: ", "param"});
+}
+
+TEST(ParameterHints, VariadicPlainConstructor) {
+ // Name hint for variadic parameter in a constructor call
+ assertParameterHints(R"cpp(
+ struct S { S(int a); };
+ template <typename T, typename... Args>
+ T bar(Args&&... args) { return T{args...}; }
+ void baz() {
+ int b;
+ bar<S>($param[[b]]);
+ }
+ )cpp",
+ ExpectedHint{"a: ", "param"});
+}
+
+TEST(ParameterHints, VariadicForwardedNewConstructor) {
+ // Name hint for variadic parameter using std::forward in a new expression
+ // This prototype of std::forward is sufficient for clang to recognize it
+ assertParameterHints(R"cpp(
+ namespace std { template <typename T> T&& forward(T&); }
+ struct S { S(int a); };
+ template <typename T, typename... Args>
+ T* bar(Args&&... args) { return new T{std::forward<Args>(args)...}; }
+ void baz() {
+ int b;
+ bar<S>($param[[b]]);
+ }
+ )cpp",
+ ExpectedHint{"a: ", "param"});
+}
+
+TEST(ParameterHints, VariadicPlainNewConstructor) {
+ // Name hint for variadic parameter in a new expression
+ assertParameterHints(R"cpp(
+ struct S { S(int a); };
+ template <typename T, typename... Args>
+ T* bar(Args&&... args) { return new T{args...}; }
+ void baz() {
+ int b;
+ bar<S>($param[[b]]);
+ }
+ )cpp",
+ ExpectedHint{"a: ", "param"});
+}
+
+TEST(ParameterHints, VariadicForwarded) {
+ // Name for variadic parameter using std::forward
+ // This prototype of std::forward is sufficient for clang to recognize it
+ assertParameterHints(R"cpp(
+ namespace std { template <typename T> T&& forward(T&); }
+ void foo(int a);
+ template <typename... Args>
+ void bar(Args&&... args) { return foo(std::forward<Args>(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 <typename... Args>
+ void bar(Args&&... args) { return foo(args...); }
+ void baz() {
+ bar($param[[42]]);
+ }
+ )cpp",
+ ExpectedHint{"a: ", "param"});
+}
+
+TEST(ParameterHints, VariadicPlainWithPackFirst) {
+ // Name hint for variadic parameter when the parameter pack is not the last
+ // template parameter
+ assertParameterHints(R"cpp(
+ void foo(int a);
+ template <typename... Args, typename Arg>
+ void bar(Arg, Args&&... args) { return foo(args...); }
+ void baz() {
+ bar(1, $param[[42]]);
+ }
+ )cpp",
+ ExpectedHint{"a: ", "param"});
+}
+
+TEST(ParameterHints, VariadicSplitTwolevel) {
+ // Name for variadic parameter that involves both head and tail parameters to
+ // deal with.
+ // This prototype of std::forward is sufficient for clang to recognize it
+ assertParameterHints(R"cpp(
+ namespace std { template <typename T> T&& forward(T&); }
+ void baz(int, int b, double);
+ template <typename... Args>
+ void foo(int a, Args&&... args) {
+ return baz(1, std::forward<Args>(args)..., 1.0);
+ }
+ template <typename... Args>
+ void bar(Args&&... args) { return foo(std::forward<Args>(args)...); }
+ void bazz() {
+ bar($param1[[32]], $param2[[42]]);
+ }
+ )cpp",
+ ExpectedHint{"a: ", "param1"},
+ ExpectedHint{"b: ", "param2"});
+}
+
+TEST(ParameterHints, VariadicNameFromSpecialization) {
+ // We don't try to resolve forwarding parameters if the function call uses a
+ // specialization.
+ assertParameterHints(R"cpp(
+ void foo(int a);
+ template <typename... Args>
+ void bar(Args... args) {
+ foo(args...);
+ }
+ template <>
+ void bar<int>(int b);
+ void baz() {
+ bar($param[[42]]);
+ }
+ )cpp",
+ ExpectedHint{"b: ", "param"});
+}
+
+TEST(ParameterHints, VariadicNameFromSpecializationRecursive) {
+ // We don't try to resolve forwarding parameters inside a forwarding function
+ // call if that function call uses a specialization.
+ assertParameterHints(R"cpp(
+ void foo2(int a);
+ template <typename... Args>
+ void foo(Args... args) {
+ foo2(args...);
+ }
+ template <typename... Args>
+ void bar(Args... args) {
+ foo(args...);
+ }
+ template <>
+ void foo<int>(int b);
+ void baz() {
+ bar($param[[42]]);
+ }
+ )cpp",
+ ExpectedHint{"b: ", "param"});
+}
+
+TEST(ParameterHints, VariadicOverloaded) {
+ // Name for variadic parameter for an overloaded function with unique number
+ // of parameters.
+ // This prototype of std::forward is sufficient for clang to recognize it
+ assertParameterHints(
+ R"cpp(
+ namespace std { template <typename T> T&& forward(T&); }
+ void baz(int b, int c);
+ void baz(int bb, int cc, int dd);
+ template <typename... Args>
+ void foo(int a, Args&&... args) {
+ return baz(std::forward<Args>(args)...);
+ }
+ template <typename... Args>
+ void bar(Args&&... args) { return foo(std::forward<Args>(args)...); }
+ void bazz() {
+ bar($param1[[32]], $param2[[42]], $param3[[52]]);
+ bar($param4[[1]], $param5[[2]], $param6[[3]], $param7[[4]]);
+ }
+ )cpp",
+ ExpectedHint{"a: ", "param1"}, ExpectedHint{"b: ", "param2"},
+ ExpectedHint{"c: ", "param3"}, ExpectedHint{"a: ", "param4"},
+ ExpectedHint{"bb: ", "param5"}, ExpectedHint{"cc: ", "param6"},
+ ExpectedHint{"dd: ", "param7"});
+}
+
+TEST(ParameterHints, VariadicRecursive) {
+ // make_tuple-like recursive variadic call
+ assertParameterHints(
+ R"cpp(
+ void foo();
+
+ template <typename Head, typename... Tail>
+ void foo(Head head, Tail... tail) {
+ foo(tail...);
+ }
+
+ template <typename... Args>
+ void bar(Args... args) {
+ foo(args...);
+ }
+
+ int main() {
+ bar(1, 2, 3);
+ }
+ )cpp");
+}
+
+TEST(ParameterHints, VariadicVarargs) {
+ // variadic call involving varargs (to make sure we don't crash)
+ assertParameterHints(R"cpp(
+ void foo(int fixed, ...);
+ template <typename... Args>
+ void bar(Args&&... args) {
+ foo(args...);
+ }
+
+ void baz() {
+ bar($fixed[[41]], 42, 43);
+ }
+ )cpp");
+}
+
+TEST(ParameterHints, VariadicTwolevelUnresolved) {
+ // the same setting as VariadicVarargs, only with parameter pack
+ assertParameterHints(R"cpp(
+ template <typename... Args>
+ void foo(int fixed, Args&& ... args);
+ template <typename... Args>
+ void bar(Args&&... args) {
+ foo(args...);
+ }
+
+ void baz() {
+ bar($fixed[[41]], 42, 43);
+ }
+ )cpp",
+ ExpectedHint{"fixed: ", "fixed"});
+}
+
+TEST(ParameterHints, VariadicTwoCalls) {
+ // only the first call using the parameter pack should be picked up
+ assertParameterHints(
+ R"cpp(
+ void f1(int a, int b);
+ void f2(int c, int d);
+
+ bool cond;
+
+ template <typename... Args>
+ void foo(Args... args) {
+ if (cond) {
+ f1(args...);
+ } else {
+ f2(args...);
+ }
+ }
+
+ int main() {
+ foo($param1[[1]], $param2[[2]]);
+ }
+ )cpp",
+ ExpectedHint{"a: ", "param1"}, ExpectedHint{"b: ", "param2"});
+}
+
+TEST(ParameterHints, VariadicInfinite) {
+ // infinite recursion should not break clangd
+ assertParameterHints(
+ R"cpp(
+ template <typename... Args>
+ void foo(Args...);
+
+ template <typename... Args>
+ void bar(Args... args) {
+ foo(args...);
+ }
+
+ template <typename... Args>
+ void foo(Args... args) {
+ bar(args...);
+ }
+
+ int main() {
+ foo(1, 2);
+ }
+ )cpp");
+}
+
+TEST(ParameterHints, VariadicDuplicatePack) {
+ // edge cases with multiple adjacent packs should work
+ assertParameterHints(
+ R"cpp(
+ void foo(int a, int b, int c, int);
+
+ template <typename... Args>
+ void bar(int, Args... args, int d) {
+ foo(args..., d);
+ }
+
+ template <typename... Args>
+ void baz(Args... args, Args... args2) {
+ bar<Args..., int>(1, args..., args2...);
+ }
+
+ int main() {
+ baz<int, int>($p1[[1]], $p2[[2]], $p3[[3]], $p4[[4]]);
+ }
+ )cpp",
+ ExpectedHint{"a: ", "p1"}, ExpectedHint{"b: ", "p2"},
+ ExpectedHint{"c: ", "p3"}, ExpectedHint{"d: ", "p4"});
+}
+
+TEST(ParameterHints, VariadicEmplace) {
+ // emplace-like calls should forward constructor parameters
+ // This prototype of std::forward is sufficient for clang to recognize it
+ assertParameterHints(
+ R"cpp(
+ namespace std { template <typename T> T&& forward(T&); }
+ using size_t = decltype(sizeof(0));
+ void *operator new(size_t, void *);
+ struct S {
+ S(int A);
+ S(int B, int C);
+ };
+ struct alloc {
+ template <typename T>
+ T* allocate();
+ template <typename T, typename... Args>
+ void construct(T* ptr, Args&&... args) {
+ ::new ((void*)ptr) T{std::forward<Args>(args)...};
+ }
+ };
+ template <typename T>
+ struct container {
+ template <typename... Args>
+ void emplace(Args&&... args) {
+ alloc a;
+ auto ptr = a.template allocate<T>();
+ a.construct(ptr, std::forward<Args>(args)...);
+ }
+ };
+ void foo() {
+ container<S> c;
+ c.emplace($param1[[1]]);
+ c.emplace($param2[[2]], $param3[[3]]);
+ }
+ )cpp",
+ ExpectedHint{"A: ", "param1"}, ExpectedHint{"B: ", "param2"},
+ ExpectedHint{"C: ", "param3"});
+}
+
+TEST(ParameterHints, VariadicReferenceHint) {
+ assertParameterHints(R"cpp(
+ void foo(int&);
+ template <typename... Args>
+ void bar(Args... args) { return foo(args...); }
+ void baz() {
+ int a;
+ bar(a);
+ bar(1);
+ }
+ )cpp");
+}
+
+TEST(ParameterHints, VariadicReferenceHintForwardingRef) {
+ assertParameterHints(R"cpp(
+ void foo(int&);
+ template <typename... Args>
+ void bar(Args&&... args) { return foo(args...); }
+ void baz() {
+ int a;
+ bar($param[[a]]);
+ bar(1);
+ }
+ )cpp",
+ ExpectedHint{"&: ", "param"});
+}
+
+TEST(ParameterHints, VariadicReferenceHintForwardingRefStdForward) {
+ assertParameterHints(R"cpp(
+ namespace std { template <typename T> T&& forward(T&); }
+ void foo(int&);
+ template <typename... Args>
+ void bar(Args&&... args) { return foo(std::forward<Args>(args)...); }
+ void baz() {
+ int a;
+ bar($param[[a]]);
+ }
+ )cpp",
+ ExpectedHint{"&: ", "param"});
+}
+
+TEST(ParameterHints, VariadicNoReferenceHintForwardingRefStdForward) {
+ assertParameterHints(R"cpp(
+ namespace std { template <typename T> T&& forward(T&); }
+ void foo(int);
+ template <typename... Args>
+ void bar(Args&&... args) { return foo(std::forward<Args>(args)...); }
+ void baz() {
+ int a;
+ bar(a);
+ bar(1);
+ }
+ )cpp");
+}
+
+TEST(ParameterHints, VariadicNoReferenceHintUnresolvedForward) {
+ assertParameterHints(R"cpp(
+ template <typename... Args>
+ void foo(Args&&... args);
+ void bar() {
+ int a;
+ foo(a);
+ }
+ )cpp");
+}
+
+TEST(ParameterHints, MatchingNameVariadicForwarded) {
+ // No name hint for variadic parameter with matching name
+ // This prototype of std::forward is sufficient for clang to recognize it
+ assertParameterHints(R"cpp(
+ namespace std { template <typename T> T&& forward(T&); }
+ void foo(int a);
+ template <typename... Args>
+ void bar(Args&&... args) { return foo(std::forward<Args>(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 <typename... Args>
+ 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(
@@ -470,7 +986,7 @@ TEST(ParameterHints, VarargsFunction) {
assertParameterHints(R"cpp(
void foo(int fixed, ...);
- void bar() {
+ void bar() {
foo($fixed[[41]], 42, 43);
}
)cpp",
More information about the cfe-commits
mailing list