[clang-tools-extra] [clangd] Add inlay hints for default function arguments and implicit lambda captures (PR #95712)

Tor Shepherd via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 19 06:56:02 PDT 2024


https://github.com/torshepherd updated https://github.com/llvm/llvm-project/pull/95712

>From 3830a49faf5e20781f864d459f5478221740ac3d Mon Sep 17 00:00:00 2001
From: Tor Shepherd <tor.aksel.shepherd at gmail.com>
Date: Wed, 19 Jun 2024 09:55:34 -0400
Subject: [PATCH 1/2] after comments

---
 clang-tools-extra/clangd/Config.h             |  2 +
 clang-tools-extra/clangd/ConfigCompile.cpp    | 11 ++-
 clang-tools-extra/clangd/ConfigFragment.h     |  5 ++
 clang-tools-extra/clangd/ConfigYAML.cpp       |  9 +-
 clang-tools-extra/clangd/InlayHints.cpp       | 87 ++++++++++++++++++-
 clang-tools-extra/clangd/Protocol.cpp         |  6 ++
 clang-tools-extra/clangd/Protocol.h           | 15 ++++
 .../clangd/unittests/InlayHintTests.cpp       | 57 ++++++++++++
 8 files changed, 187 insertions(+), 5 deletions(-)

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..89486ce9fa034 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>
@@ -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..4e535db382443 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -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..bf5bbaf635917 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -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..52ad8e763fd80 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;
 }
 
@@ -1465,6 +1470,58 @@ TEST(TypeHints, DefaultTemplateArgs) {
                   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>

>From 51e6295ecf8b71e8016fc7d93926b110113074d7 Mon Sep 17 00:00:00 2001
From: Tor Shepherd <tor.aksel.shepherd at gmail.com>
Date: Wed, 19 Jun 2024 09:55:54 -0400
Subject: [PATCH 2/2] more comments

---
 clang-tools-extra/clangd/InlayHints.cpp | 26 ++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 2631c17409eab..420f392cd5910 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -36,6 +36,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -383,30 +384,38 @@ maybeDropCxxExplicitObjectParameters(ArrayRef<const ParmVarDecl *> Params) {
 }
 
 llvm::StringRef getLambdaCaptureName(const LambdaCapture &Capture) {
-  if (Capture.capturesVariable())
-    return Capture.getCapturedVar()->getName();
-  if (Capture.capturesThis())
+  switch (Capture.getCaptureKind()) {
+  case LCK_This:
+  case LCK_StarThis:
     return llvm::StringRef{"this"};
-  return llvm::StringRef{"unknown"};
+  case LCK_ByCopy:
+  case LCK_ByRef:
+  case LCK_VLAType:
+    return Capture.getCapturedVar()->getName();
+  }
+  llvm_unreachable("unhandled capture kind");
 }
 
 template <typename R, typename P>
 std::string joinAndTruncate(R &&Range, size_t MaxLength,
                             P &&GetAsStringFunction) {
   std::string Out;
+  llvm::raw_string_ostream OS(Out);
   bool IsFirst = true;
+  llvm::ListSeparator Sep(", ");
   for (auto &&Element : Range) {
     if (!IsFirst)
-      Out.append(", ");
+      OS << Sep;
     else
       IsFirst = false;
     auto AsString = GetAsStringFunction(Element);
     if (Out.size() + AsString.size() >= MaxLength) {
-      Out.append("...");
+      OS << "...";
       break;
     }
-    Out.append(AsString);
+    OS << AsString;
   }
+  OS.flush();
   return Out;
 }
 
@@ -812,8 +821,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
       if (Cfg.InlayHints.DefaultArguments && IsDefault) {
         auto SourceText = Lexer::getSourceText(
             CharSourceRange::getTokenRange(Params[I]->getDefaultArgRange()),
-            Callee.Decl->getASTContext().getSourceManager(),
-            Callee.Decl->getASTContext().getLangOpts());
+            AST.getSourceManager(), AST.getLangOpts());
         FormattedDefaultArgs.emplace_back(llvm::formatv(
             "{0} = {1}", Name,
             SourceText.size() > Cfg.InlayHints.TypeNameLimit ? "..."



More information about the cfe-commits mailing list