Author: Sam McCall
Date: 2022-01-10T10:49:35+01:00
New Revision: 16fd5c278488b3d3275afc381a00ba0b51b070ee

URL: 
https://github.com/llvm/llvm-project/commit/16fd5c278488b3d3275afc381a00ba0b51b070ee
DIFF: 
https://github.com/llvm/llvm-project/commit/16fd5c278488b3d3275afc381a00ba0b51b070ee.diff

LOG: [clangd] Support configuration of inlay hints.

The idea is that the feature will always be advertised at the LSP level, but
depending on config we'll return partial or no responses.

We try to avoid doing the analysis for hints we're not going to return.

Examples of syntax:
```
InlayHints:
  Enabled: No
---
InlayHints:
  ParameterNames: No
---
InlayHints:
  ParameterNames: Yes
  DeducedTypes: Yes
```

Differential Revision: https://reviews.llvm.org/D116713

Added: 
    

Modified: 
    clang-tools-extra/clangd/Config.h
    clang-tools-extra/clangd/ConfigCompile.cpp
    clang-tools-extra/clangd/ConfigFragment.h
    clang-tools-extra/clangd/ConfigYAML.cpp
    clang-tools-extra/clangd/InlayHints.cpp
    clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
    clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index 38fc93fa361c..952626db31e4 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -122,6 +122,15 @@ struct Config {
     /// Whether hover show a.k.a type.
     bool ShowAKA = false;
   } Hover;
+
+  struct {
+    /// If false, inlay hints are completely disabled.
+    bool Enabled = true;
+
+    // Whether specific categories of hints are enabled.
+    bool Parameters = true;
+    bool DeducedTypes = true;
+  } InlayHints;
 };
 
 } // namespace clangd

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 18afdeb3cb5c..a606b98a2dba 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -197,6 +197,7 @@ struct FragmentCompiler {
     compile(std::move(F.Diagnostics));
     compile(std::move(F.Completion));
     compile(std::move(F.Hover));
+    compile(std::move(F.InlayHints));
   }
 
   void compile(Fragment::IfBlock &&F) {
@@ -526,6 +527,22 @@ struct FragmentCompiler {
     }
   }
 
+  void compile(Fragment::InlayHintsBlock &&F) {
+    if (F.Enabled)
+      Out.Apply.push_back([Value(**F.Enabled)](const Params &, Config &C) {
+        C.InlayHints.Enabled = Value;
+      });
+    if (F.ParameterNames)
+      Out.Apply.push_back(
+          [Value(**F.ParameterNames)](const Params &, Config &C) {
+            C.InlayHints.Parameters = Value;
+          });
+    if (F.DeducedTypes)
+      Out.Apply.push_back([Value(**F.DeducedTypes)](const Params &, Config &C) 
{
+        C.InlayHints.DeducedTypes = Value;
+      });
+  }
+
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
   constexpr static llvm::SourceMgr::DiagKind Warning =
       llvm::SourceMgr::DK_Warning;

diff  --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index 31c4636efa0b..d165b6305aa5 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -283,6 +283,18 @@ struct Fragment {
     llvm::Optional<Located<bool>> ShowAKA;
   };
   HoverBlock Hover;
+
+  /// Configures labels shown inline with the code.
+  struct InlayHintsBlock {
+    /// Enables/disables the inlay-hints feature.
+    llvm::Optional<Located<bool>> Enabled;
+
+    /// Show parameter names before function arguments.
+    llvm::Optional<Located<bool>> ParameterNames;
+    /// Show deduced types for `auto`.
+    llvm::Optional<Located<bool>> DeducedTypes;
+  };
+  InlayHintsBlock InlayHints;
 };
 
 } // namespace config

diff  --git a/clang-tools-extra/clangd/ConfigYAML.cpp 
b/clang-tools-extra/clangd/ConfigYAML.cpp
index 0487c3281576..04c0c633a3bb 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -66,6 +66,7 @@ class Parser {
     Dict.handle("Diagnostics", [&](Node &N) { parse(F.Diagnostics, N); });
     Dict.handle("Completion", [&](Node &N) { parse(F.Completion, N); });
     Dict.handle("Hover", [&](Node &N) { parse(F.Hover, N); });
+    Dict.handle("InlayHints", [&](Node &N) { parse(F.InlayHints, N); });
     Dict.parse(N);
     return !(N.failed() || HadError);
   }
@@ -199,12 +200,8 @@ class Parser {
   void parse(Fragment::CompletionBlock &F, Node &N) {
     DictParser Dict("Completion", this);
     Dict.handle("AllScopes", [&](Node &N) {
-      if (auto Value = scalarValue(N, "AllScopes")) {
-        if (auto AllScopes = llvm::yaml::parseBool(**Value))
-          F.AllScopes = *AllScopes;
-        else
-          warning("AllScopes should be a boolean", N);
-      }
+      if (auto AllScopes = boolValue(N, "AllScopes"))
+        F.AllScopes = *AllScopes;
     });
     Dict.parse(N);
   }
@@ -212,12 +209,25 @@ class Parser {
   void parse(Fragment::HoverBlock &F, Node &N) {
     DictParser Dict("Hover", this);
     Dict.handle("ShowAKA", [&](Node &N) {
-      if (auto Value = scalarValue(N, "ShowAKA")) {
-        if (auto ShowAKA = llvm::yaml::parseBool(**Value))
-          F.ShowAKA = *ShowAKA;
-        else
-          warning("ShowAKA should be a boolean", N);
-      }
+      if (auto ShowAKA = boolValue(N, "ShowAKA"))
+        F.ShowAKA = *ShowAKA;
+    });
+    Dict.parse(N);
+  }
+
+  void parse(Fragment::InlayHintsBlock &F, Node &N) {
+    DictParser Dict("InlayHints", this);
+    Dict.handle("Enabled", [&](Node &N) {
+      if (auto Value = boolValue(N, "Enabled"))
+        F.Enabled = *Value;
+    });
+    Dict.handle("ParameterNames", [&](Node &N) {
+      if (auto Value = boolValue(N, "ParameterNames"))
+        F.ParameterNames = *Value;
+    });
+    Dict.handle("DeducedTypes", [&](Node &N) {
+      if (auto Value = boolValue(N, "DeducedTypes"))
+        F.DeducedTypes = *Value;
     });
     Dict.parse(N);
   }
@@ -331,6 +341,16 @@ class Parser {
     return llvm::None;
   }
 
+  llvm::Optional<Located<bool>> boolValue(Node &N, llvm::StringRef Desc) {
+    if (auto Scalar = scalarValue(N, Desc)) {
+      if (auto Bool = llvm::yaml::parseBool(**Scalar))
+        return Located<bool>(*Bool, Scalar->Range);
+      else
+        warning(Desc + " should be a boolean", N);
+    }
+    return llvm::None;
+  }
+
   // Try to parse a list of single scalar values, or just a single value.
   llvm::Optional<std::vector<Located<std::string>>> scalarValues(Node &N) {
     std::vector<Located<std::string>> Result;

diff  --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 60d20a38ecd2..e534faa80c4b 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -6,6 +6,7 @@
 //
 
//===----------------------------------------------------------------------===//
 #include "InlayHints.h"
+#include "Config.h"
 #include "HeuristicResolver.h"
 #include "ParsedAST.h"
 #include "clang/AST/DeclarationName.h"
@@ -23,8 +24,8 @@ enum class HintSide { Left, Right };
 class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
 public:
   InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST,
-                   llvm::Optional<Range> RestrictRange)
-      : Results(Results), AST(AST.getASTContext()),
+                   const Config &Cfg, llvm::Optional<Range> RestrictRange)
+      : Results(Results), AST(AST.getASTContext()), Cfg(Cfg),
         RestrictRange(std::move(RestrictRange)),
         MainFileID(AST.getSourceManager().getMainFileID()),
         Resolver(AST.getHeuristicResolver()),
@@ -65,6 +66,9 @@ class InlayHintVisitor : public 
RecursiveASTVisitor<InlayHintVisitor> {
   }
 
   bool VisitCallExpr(CallExpr *E) {
+    if (!Cfg.InlayHints.Parameters)
+      return true;
+
     // Do not show parameter hints for operator calls written using operator
     // syntax or user-defined literals. (Among other reasons, the resulting
     // hints can look awkard, e.g. the expression can itself be a function
@@ -135,7 +139,7 @@ class InlayHintVisitor : public 
RecursiveASTVisitor<InlayHintVisitor> {
   // 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) {
-    if (Args.size() == 0 || !Callee)
+    if (!Cfg.InlayHints.Parameters || Args.size() == 0 || !Callee)
       return;
 
     // If the anchor location comes from a macro defintion, there's nowhere to
@@ -323,6 +327,23 @@ class InlayHintVisitor : public 
RecursiveASTVisitor<InlayHintVisitor> {
   void addInlayHint(SourceRange R, HintSide Side, InlayHintKind Kind,
                     llvm::StringRef Prefix, llvm::StringRef Label,
                     llvm::StringRef Suffix) {
+    // We shouldn't get as far as adding a hint if the category is disabled.
+    // We'd like to disable as much of the analysis as possible above instead.
+    // Assert in debug mode but add a dynamic check in production.
+    assert(Cfg.InlayHints.Enabled && "Shouldn't get here if disabled!");
+    switch (Kind) {
+#define CHECK_KIND(Enumerator, ConfigProperty)                                 
\
+  case InlayHintKind::Enumerator:                                              
\
+    assert(Cfg.InlayHints.ConfigProperty &&                                    
\
+           "Shouldn't get here if kind is disabled!");                         
\
+    if (!Cfg.InlayHints.ConfigProperty)                                        
\
+      return;                                                                  
\
+    break
+      CHECK_KIND(ParameterHint, Parameters);
+      CHECK_KIND(TypeHint, DeducedTypes);
+#undef CHECK_KIND
+    }
+
     auto FileRange =
         toHalfOpenFileRange(AST.getSourceManager(), AST.getLangOpts(), R);
     if (!FileRange)
@@ -348,8 +369,7 @@ class InlayHintVisitor : public 
RecursiveASTVisitor<InlayHintVisitor> {
 
   void addTypeHint(SourceRange R, QualType T, llvm::StringRef Prefix,
                    const PrintingPolicy &Policy) {
-    // Do not print useless "NULL TYPE" hint.
-    if (!T.getTypePtrOrNull())
+    if (!Cfg.InlayHints.DeducedTypes || T.isNull())
       return;
 
     std::string TypeName = T.getAsString(Policy);
@@ -360,6 +380,7 @@ class InlayHintVisitor : public 
RecursiveASTVisitor<InlayHintVisitor> {
 
   std::vector<InlayHint> &Results;
   ASTContext &AST;
+  const Config &Cfg;
   llvm::Optional<Range> RestrictRange;
   FileID MainFileID;
   StringRef MainFileBuf;
@@ -381,7 +402,10 @@ class InlayHintVisitor : public 
RecursiveASTVisitor<InlayHintVisitor> {
 std::vector<InlayHint> inlayHints(ParsedAST &AST,
                                   llvm::Optional<Range> RestrictRange) {
   std::vector<InlayHint> Results;
-  InlayHintVisitor Visitor(Results, AST, std::move(RestrictRange));
+  const auto &Cfg = Config::current();
+  if (!Cfg.InlayHints.Enabled)
+    return Results;
+  InlayHintVisitor Visitor(Results, AST, Cfg, std::move(RestrictRange));
   Visitor.TraverseAST(AST.getASTContext());
 
   // De-duplicate hints. Duplicates can sometimes occur due to e.g. explicit

diff  --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp 
b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
index aeab1beb2ec2..3722ee297665 100644
--- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -228,6 +228,23 @@ TEST(ParseYAML, ShowAKA) {
   ASSERT_EQ(Results.size(), 1u);
   EXPECT_THAT(Results[0].Hover.ShowAKA, llvm::ValueIs(Val(true)));
 }
+
+TEST(ParseYAML, InlayHints) {
+  CapturedDiags Diags;
+  Annotations YAML(R"yaml(
+InlayHints:
+  Enabled: No
+  ParameterNames: Yes
+  )yaml");
+  auto Results =
+      Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_EQ(Results.size(), 1u);
+  EXPECT_THAT(Results[0].InlayHints.Enabled, llvm::ValueIs(Val(false)));
+  EXPECT_THAT(Results[0].InlayHints.ParameterNames, llvm::ValueIs(Val(true)));
+  EXPECT_EQ(Results[0].InlayHints.DeducedTypes, llvm::None);
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd

diff  --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index e8880981c1a4..992ec0e012ae 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -6,11 +6,13 @@
 //
 
//===----------------------------------------------------------------------===//
 #include "Annotations.h"
+#include "Config.h"
 #include "InlayHints.h"
 #include "Protocol.h"
 #include "TestTU.h"
 #include "TestWorkspace.h"
 #include "XRefs.h"
+#include "support/Context.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -24,6 +26,8 @@ std::ostream &operator<<(std::ostream &Stream, const 
InlayHint &Hint) {
 namespace {
 
 using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::UnorderedElementsAre;
 
 std::vector<InlayHint> hintsOfKind(ParsedAST &AST, InlayHintKind Kind) {
   std::vector<InlayHint> Result;
@@ -56,6 +60,13 @@ MATCHER_P2(HintMatcher, Expected, Code, "") {
 
 MATCHER_P(labelIs, Label, "") { return arg.label == Label; }
 
+Config noHintsConfig() {
+  Config C;
+  C.InlayHints.Parameters = false;
+  C.InlayHints.DeducedTypes = false;
+  return C;
+}
+
 template <typename... ExpectedHints>
 void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
                  ExpectedHints... Expected) {
@@ -66,6 +77,10 @@ void assertHints(InlayHintKind Kind, llvm::StringRef 
AnnotatedSource,
 
   EXPECT_THAT(hintsOfKind(AST, Kind),
               ElementsAre(HintMatcher(Expected, Source)...));
+  // Sneak in a cross-cutting check that hints are disabled by config.
+  // We'll hit an assertion failure if addInlayHint still gets called.
+  WithContextValue WithCfg(Config::Key, noHintsConfig());
+  EXPECT_THAT(inlayHints(AST, llvm::None), IsEmpty());
 }
 
 // Hack to allow expression-statements operating on parameter packs in C++14.


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to