llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Tor Shepherd (torshepherd) <details> <summary>Changes</summary> Title. This PR adds support for showing implicit lambda captures:  and defaulted function arguments:  as inlay hints. If the list of captures (or default args) is too long (based on TypeNameLimit), shows "..." instead:   Note: I wrote this before https://github.com/llvm/llvm-project/pull/85497 landed. With this we should probably also follow up with go-to-definition on the lambda captures at least; I could see that being helpful --- Full diff: https://github.com/llvm/llvm-project/pull/95712.diff 8 Files Affected: - (modified) clang-tools-extra/clangd/Config.h (+2) - (modified) clang-tools-extra/clangd/ConfigCompile.cpp (+14-5) - (modified) clang-tools-extra/clangd/ConfigFragment.h (+5) - (modified) clang-tools-extra/clangd/ConfigYAML.cpp (+8-1) - (modified) clang-tools-extra/clangd/InlayHints.cpp (+84-3) - (modified) clang-tools-extra/clangd/Protocol.cpp (+7-1) - (modified) clang-tools-extra/clangd/Protocol.h (+24-9) - (modified) clang-tools-extra/clangd/unittests/InlayHintTests.cpp (+62-4) ``````````diff diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index 41143b9ebc8d2..5100214244454 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -148,6 +148,8 @@ struct Config { bool DeducedTypes = true; bool Designators = true; bool BlockEnd = false; + bool LambdaCaptures = false; + bool DefaultArguments = false; // Limit the length of type names in inlay hints. (0 means no limit) uint32_t TypeNameLimit = 32; } InlayHints; diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index f32f674443ffe..dcdb71ea01bb2 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -43,7 +43,6 @@ #include "llvm/Support/Regex.h" #include "llvm/Support/SMLoc.h" #include "llvm/Support/SourceMgr.h" -#include <algorithm> #include <memory> #include <optional> #include <string> @@ -504,10 +503,10 @@ struct FragmentCompiler { auto Fast = isFastTidyCheck(Str); if (!Fast.has_value()) { diag(Warning, - llvm::formatv( - "Latency of clang-tidy check '{0}' is not known. " - "It will only run if ClangTidy.FastCheckFilter is Loose or None", - Str) + llvm::formatv("Latency of clang-tidy check '{0}' is not known. " + "It will only run if ClangTidy.FastCheckFilter is " + "Loose or None", + Str) .str(), Arg.Range); } else if (!*Fast) { @@ -654,6 +653,16 @@ struct FragmentCompiler { Out.Apply.push_back([Value(**F.BlockEnd)](const Params &, Config &C) { C.InlayHints.BlockEnd = Value; }); + if (F.LambdaCaptures) + Out.Apply.push_back( + [Value(**F.LambdaCaptures)](const Params &, Config &C) { + C.InlayHints.LambdaCaptures = Value; + }); + if (F.DefaultArguments) + Out.Apply.push_back( + [Value(**F.DefaultArguments)](const Params &, Config &C) { + C.InlayHints.DefaultArguments = Value; + }); if (F.TypeNameLimit) Out.Apply.push_back( [Value(**F.TypeNameLimit)](const Params &, Config &C) { diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index f3e51a9b6dbc4..ad30b43f34874 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -331,6 +331,11 @@ struct Fragment { std::optional<Located<bool>> Designators; /// Show defined symbol names at the end of a definition block. std::optional<Located<bool>> BlockEnd; + /// Show names of captured variables by default capture groups in lambdas. + std::optional<Located<bool>> LambdaCaptures; + /// Show parameter names and default values of default arguments after all + /// of the explicit arguments. + std::optional<Located<bool>> DefaultArguments; /// Limit the length of type name hints. (0 means no limit) std::optional<Located<uint32_t>> TypeNameLimit; }; diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp index 3e9b6a07d3b32..58e5f8bead101 100644 --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -14,7 +14,6 @@ #include "llvm/Support/YAMLParser.h" #include <optional> #include <string> -#include <system_error> namespace clang { namespace clangd { @@ -264,6 +263,14 @@ class Parser { if (auto Value = boolValue(N, "BlockEnd")) F.BlockEnd = *Value; }); + Dict.handle("LambdaCaptures", [&](Node &N) { + if (auto Value = boolValue(N, "LambdaCaptures")) + F.LambdaCaptures = *Value; + }); + Dict.handle("DefaultArguments", [&](Node &N) { + if (auto Value = boolValue(N, "DefaultArguments")) + F.DefaultArguments = *Value; + }); Dict.handle("TypeNameLimit", [&](Node &N) { if (auto Value = uint32Value(N, "TypeNameLimit")) F.TypeNameLimit = *Value; diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index cd4f1931b3ce1..2631c17409eab 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -11,27 +11,37 @@ #include "Config.h" #include "HeuristicResolver.h" #include "ParsedAST.h" +#include "Protocol.h" #include "SourceCode.h" #include "clang/AST/ASTDiagnostic.h" #include "clang/AST/Decl.h" +#include "clang/AST/DeclBase.h" #include "clang/AST/DeclarationName.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" +#include "clang/AST/LambdaCapture.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Stmt.h" #include "clang/AST/StmtVisitor.h" #include "clang/AST/Type.h" #include "clang/Basic/Builtins.h" +#include "clang/Basic/Lambda.h" #include "clang/Basic/OperatorKinds.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/Twine.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/FormatVariadic.h" #include "llvm/Support/SaveAndRestore.h" #include "llvm/Support/ScopedPrinter.h" #include "llvm/Support/raw_ostream.h" +#include <algorithm> +#include <iterator> #include <optional> #include <string> @@ -372,6 +382,34 @@ maybeDropCxxExplicitObjectParameters(ArrayRef<const ParmVarDecl *> Params) { return Params; } +llvm::StringRef getLambdaCaptureName(const LambdaCapture &Capture) { + if (Capture.capturesVariable()) + return Capture.getCapturedVar()->getName(); + if (Capture.capturesThis()) + return llvm::StringRef{"this"}; + return llvm::StringRef{"unknown"}; +} + +template <typename R, typename P> +std::string joinAndTruncate(R &&Range, size_t MaxLength, + P &&GetAsStringFunction) { + std::string Out; + bool IsFirst = true; + for (auto &&Element : Range) { + if (!IsFirst) + Out.append(", "); + else + IsFirst = false; + auto AsString = GetAsStringFunction(Element); + if (Out.size() + AsString.size() >= MaxLength) { + Out.append("..."); + break; + } + Out.append(AsString); + } + return Out; +} + struct Callee { // Only one of Decl or Loc is set. // Loc is for calls through function pointers. @@ -422,7 +460,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { Callee.Decl = E->getConstructor(); if (!Callee.Decl) return true; - processCall(Callee, {E->getArgs(), E->getNumArgs()}); + processCall(Callee, E->getParenOrBraceRange().getEnd(), + {E->getArgs(), E->getNumArgs()}); return true; } @@ -495,7 +534,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { dyn_cast_or_null<CXXMethodDecl>(Callee.Decl)) if (IsFunctor || Method->hasCXXExplicitFunctionObjectParameter()) Args = Args.drop_front(1); - processCall(Callee, Args); + processCall(Callee, E->getRParenLoc(), Args); return true; } @@ -598,6 +637,15 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { } bool VisitLambdaExpr(LambdaExpr *E) { + if (Cfg.InlayHints.LambdaCaptures && E->getCaptureDefault() != LCD_None && + !E->implicit_captures().empty()) { + std::string FormattedCaptureList = + joinAndTruncate(E->implicit_captures(), Cfg.InlayHints.TypeNameLimit, + [](const LambdaCapture &ImplicitCapture) { + return getLambdaCaptureName(ImplicitCapture); + }); + addImplicitCaptureHint(E->getCaptureDefaultLoc(), FormattedCaptureList); + } FunctionDecl *D = E->getCallOperator(); if (!E->hasExplicitResultType()) addReturnTypeHint(D, E->hasExplicitParameters() @@ -709,7 +757,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { private: using NameVec = SmallVector<StringRef, 8>; - void processCall(Callee Callee, llvm::ArrayRef<const Expr *> Args) { + void processCall(Callee Callee, SourceRange LParenOrBraceRange, + llvm::ArrayRef<const Expr *> Args) { assert(Callee.Decl || Callee.Loc); if (!Cfg.InlayHints.Parameters || Args.size() == 0) @@ -721,6 +770,9 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { if (Ctor->isCopyOrMoveConstructor()) return; + SmallVector<std::string> FormattedDefaultArgs; + bool HasNonDefaultArgs = false; + ArrayRef<const ParmVarDecl *> Params, ForwardedParams; // Resolve parameter packs to their forwarded parameter SmallVector<const ParmVarDecl *> ForwardedParamsStorage; @@ -755,12 +807,34 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { bool NameHint = shouldHintName(Args[I], Name); bool ReferenceHint = shouldHintReference(Params[I], ForwardedParams[I]); + bool IsDefault = isa<CXXDefaultArgExpr>(Args[I]); + HasNonDefaultArgs |= !IsDefault; + if (Cfg.InlayHints.DefaultArguments && IsDefault) { + auto SourceText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Params[I]->getDefaultArgRange()), + Callee.Decl->getASTContext().getSourceManager(), + Callee.Decl->getASTContext().getLangOpts()); + FormattedDefaultArgs.emplace_back(llvm::formatv( + "{0} = {1}", Name, + SourceText.size() > Cfg.InlayHints.TypeNameLimit ? "..." + : SourceText)); + } + if (NameHint || ReferenceHint) { addInlayHint(Args[I]->getSourceRange(), HintSide::Left, InlayHintKind::Parameter, ReferenceHint ? "&" : "", NameHint ? Name : "", ": "); } } + + if (!FormattedDefaultArgs.empty()) { + std::string Hint = + joinAndTruncate(FormattedDefaultArgs, Cfg.InlayHints.TypeNameLimit, + [](const auto &E) { return E; }); + addInlayHint(LParenOrBraceRange, HintSide::Left, + InlayHintKind::DefaultArgument, + HasNonDefaultArgs ? ", " : "", Hint, ""); + } } static bool isSetter(const FunctionDecl *Callee, const NameVec &ParamNames) { @@ -968,6 +1042,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { CHECK_KIND(Type, DeducedTypes); CHECK_KIND(Designator, Designators); CHECK_KIND(BlockEnd, BlockEnd); + CHECK_KIND(LambdaCapture, LambdaCaptures); + CHECK_KIND(DefaultArgument, DefaultArguments); #undef CHECK_KIND } @@ -1021,6 +1097,11 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { /*Prefix=*/"", Text, /*Suffix=*/"="); } + void addImplicitCaptureHint(SourceRange R, llvm::StringRef Text) { + addInlayHint(R, HintSide::Right, InlayHintKind::LambdaCapture, + /*Prefix=*/": ", Text, /*Suffix=*/""); + } + bool shouldPrintTypeHint(llvm::StringRef TypeName) const noexcept { return Cfg.InlayHints.TypeNameLimit == 0 || TypeName.size() < Cfg.InlayHints.TypeNameLimit; diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index c08f80442eaa0..dafc3f4eab567 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -209,7 +209,7 @@ bool fromJSON(const llvm::json::Value &Params, ChangeAnnotation &R, O.map("needsConfirmation", R.needsConfirmation) && O.mapOptional("description", R.description); } -llvm::json::Value toJSON(const ChangeAnnotation & CA) { +llvm::json::Value toJSON(const ChangeAnnotation &CA) { llvm::json::Object Result{{"label", CA.label}}; if (CA.needsConfirmation) Result["needsConfirmation"] = *CA.needsConfirmation; @@ -1477,6 +1477,8 @@ llvm::json::Value toJSON(const InlayHintKind &Kind) { return 2; case InlayHintKind::Designator: case InlayHintKind::BlockEnd: + case InlayHintKind::LambdaCapture: + case InlayHintKind::DefaultArgument: // This is an extension, don't serialize. return nullptr; } @@ -1517,6 +1519,10 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, InlayHintKind Kind) { return "designator"; case InlayHintKind::BlockEnd: return "block-end"; + case InlayHintKind::LambdaCapture: + return "lambda-capture"; + case InlayHintKind::DefaultArgument: + return "default-argument"; } llvm_unreachable("Unknown clang.clangd.InlayHintKind"); }; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index a0f8b04bc4ffd..793eebf4d51df 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -281,7 +281,7 @@ struct TextDocumentEdit { /// The text document to change. VersionedTextDocumentIdentifier textDocument; - /// The edits to be applied. + /// The edits to be applied. /// FIXME: support the AnnotatedTextEdit variant. std::vector<TextEdit> edits; }; @@ -557,7 +557,7 @@ struct ClientCapabilities { /// The client supports versioned document changes for WorkspaceEdit. bool DocumentChanges = false; - + /// The client supports change annotations on text edits, bool ChangeAnnotation = false; @@ -1013,12 +1013,12 @@ struct WorkspaceEdit { /// Versioned document edits. /// /// If a client neither supports `documentChanges` nor - /// `workspace.workspaceEdit.resourceOperations` then only plain `TextEdit`s - /// using the `changes` property are supported. + /// `workspace.workspaceEdit.resourceOperations` then only plain `TextEdit`s + /// using the `changes` property are supported. std::optional<std::vector<TextDocumentEdit>> documentChanges; - + /// A map of change annotations that can be referenced in - /// AnnotatedTextEdit. + /// AnnotatedTextEdit. std::map<std::string, ChangeAnnotation> changeAnnotations; }; bool fromJSON(const llvm::json::Value &, WorkspaceEdit &, llvm::json::Path); @@ -1274,13 +1274,13 @@ enum class InsertTextFormat { /// Additional details for a completion item label. struct CompletionItemLabelDetails { /// An optional string which is rendered less prominently directly after label - /// without any spacing. Should be used for function signatures or type + /// without any spacing. Should be used for function signatures or type /// annotations. std::string detail; /// An optional string which is rendered less prominently after - /// CompletionItemLabelDetails.detail. Should be used for fully qualified - /// names or file path. + /// CompletionItemLabelDetails.detail. Should be used for fully qualified + /// names or file path. std::string description; }; llvm::json::Value toJSON(const CompletionItemLabelDetails &); @@ -1681,6 +1681,21 @@ enum class InlayHintKind { /// This is a clangd extension. BlockEnd = 4, + /// An inlay hint that is for a variable captured implicitly in a lambda. + /// + /// An example of parameter hint for implicit lambda captures: + /// [&^] { return A; }; + /// Adds an inlay hint ": A". + LambdaCapture = 5, + + /// An inlay hint that is for a default argument. + /// + /// An example of a parameter hint for a default argument: + /// void foo(bool A = true); + /// foo(^); + /// Adds an inlay hint "A = true". + DefaultArgument = 6, + /// Other ideas for hints that are not currently implemented: /// /// * Chaining hints, showing the types of intermediate expressions diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp index 5b1531eb2fa60..65ab743fb3e38 100644 --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -15,9 +15,12 @@ #include "support/Context.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/ScopedPrinter.h" +#include "llvm/Support/raw_ostream.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include <optional> #include <string> +#include <utility> #include <vector> namespace clang { @@ -81,6 +84,8 @@ Config noHintsConfig() { C.InlayHints.DeducedTypes = false; C.InlayHints.Designators = false; C.InlayHints.BlockEnd = false; + C.InlayHints.LambdaCaptures = false; + C.InlayHints.DefaultArguments = false; return C; } @@ -1458,13 +1463,66 @@ TEST(TypeHints, DefaultTemplateArgs) { struct A {}; A<float> foo(); auto $var[[var]] = foo(); - A<float> bar[1]; + A<float> baz; + A<float> bar[1]{}; auto [$binding[[value]]] = bar; )cpp", ExpectedHint{": A<float>", "var"}, ExpectedHint{": A<float>", "binding"}); } +TEST(LambdaCaptures, Smoke) { + Config Cfg; + Cfg.InlayHints.Parameters = false; + Cfg.InlayHints.DeducedTypes = false; + Cfg.InlayHints.Designators = false; + Cfg.InlayHints.BlockEnd = false; + Cfg.InlayHints.DefaultArguments = false; + + Cfg.InlayHints.LambdaCaptures = true; + Cfg.InlayHints.TypeNameLimit = 10; + WithContextValue WithCfg(Config::Key, std::move(Cfg)); + + assertHints(InlayHintKind::LambdaCapture, R"cpp( + void foo() { + int A = 1; + int ReallyLongName = 1; + auto B = [$byvalue[[=]]] { return A; }; + auto C = [$byref[[&]]] { return A; }; + auto D = [$trunc[[=]], &A] { B(); (void)ReallyLongName; return A; }; + } + )cpp", + ExpectedHint{": A", "byvalue", Right}, + ExpectedHint{": A", "byref", Right}, + ExpectedHint{": B, ...", "trunc", Right}); +} + +TEST(DefaultArguments, Smoke) { + Config Cfg; + Cfg.InlayHints.Parameters = + true; // To test interplay of parameters and default parameters + Cfg.InlayHints.DeducedTypes = false; + Cfg.InlayHints.Designators = false; + Cfg.InlayHints.BlockEnd = false; + Cfg.InlayHints.LambdaCaptures = false; + + Cfg.InlayHints.DefaultArguments = true; + WithContextValue WithCfg(Config::Key, std::move(Cfg)); + + const auto *Code = R"cpp( + int foo(int A = 4) { return A; } + int bar(int A, int B = 1, bool C = foo($default1[[)]]) { return A; } + int A = bar($explicit[[2]]$default2[[)]]; + )cpp"; + + assertHints(InlayHintKind::DefaultArgument, Code, + ExpectedHint{"A = 4", "default1", Left}, + ExpectedHint{", B = 1, C = foo()", "default2", Left}); + + assertHints(InlayHintKind::Parameter, Code, + ExpectedHint{"A: ", "explicit", Left}); +} + TEST(TypeHints, Deduplication) { assertTypeHints(R"cpp( template <typename T> @@ -1568,7 +1626,7 @@ TEST(TypeHints, SubstTemplateParameterAliases) { )cpp"; llvm::StringRef VectorIntPtr = R"cpp( - vector<int *> array; + vector<int *> $init[[array]]; auto $no_modifier[[x]] = array[3]; auto* $ptr_modifier[[ptr]] = &array[3]; auto& $ref_modifier[[ref]] = array[3]; @@ -1592,7 +1650,7 @@ TEST(TypeHints, SubstTemplateParameterAliases) { ExpectedHint{": non_template_iterator", "end"}); llvm::StringRef VectorInt = R"cpp( - vector<int> array; + vector<int> $init[[array]]; auto $no_modifier[[by_value]] = array[3]; auto* $ptr_modifier[[ptr]] = &array[3]; auto& $ref_modifier[[ref]] = array[3]; @@ -1741,7 +1799,7 @@ TEST(ParameterHints, PseudoObjectExpr) { int printf(const char *Format, ...); int main() { - S s; + S $init[[s]]; __builtin_dump_struct(&s, printf); // Not `Format: __builtin_dump_struct()` printf($Param[["Hello, %d"]], 42); // Normal calls are not affected. // This builds a PseudoObjectExpr, but here it's useful for showing the `````````` </details> https://github.com/llvm/llvm-project/pull/95712 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
