sammccall created this revision.
sammccall added reviewers: nridge, hokein.
Herald added subscribers: usaxena95, kadircet, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
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:
# No hints
InlayHints:
Enabled: No
---
# Turn off a default category
InlayHints:
ParameterNames: No
---
# Turn on some categories
InlayHints:
ParameterNames: Yes
DeducedTypes: Yes
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D116713
Files:
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
Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ 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"
@@ -23,6 +25,7 @@
namespace {
+using ::testing::IsEmpty;
using ::testing::UnorderedElementsAre;
std::vector<InlayHint> hintsOfKind(ParsedAST &AST, InlayHintKind Kind) {
@@ -49,6 +52,13 @@
arg.range == Code.range(Expected.RangeName);
}
+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) {
@@ -59,6 +69,10 @@
EXPECT_THAT(hintsOfKind(AST, Kind),
UnorderedElementsAre(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), IsEmpty());
}
template <typename... ExpectedHints>
Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -228,6 +228,23 @@
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
Index: clang-tools-extra/clangd/InlayHints.cpp
===================================================================
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
#include "InlayHints.h"
+#include "Config.h"
#include "HeuristicResolver.h"
#include "ParsedAST.h"
#include "support/Logger.h"
@@ -20,8 +21,9 @@
class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
public:
- InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST)
- : Results(Results), AST(AST.getASTContext()),
+ InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST,
+ const Config &Cfg)
+ : Results(Results), AST(AST.getASTContext()), Cfg(Cfg),
MainFileID(AST.getSourceManager().getMainFileID()),
Resolver(AST.getHeuristicResolver()),
TypeHintPolicy(this->AST.getPrintingPolicy()),
@@ -46,6 +48,9 @@
}
bool VisitCXXConstructExpr(CXXConstructExpr *E) {
+ if (!Cfg.InlayHints.DeducedTypes)
+ return true;
+
// Weed out constructor calls that don't look like a function call with
// an argument list, by checking the validity of getParenOrBraceRange().
// Also weed out std::initializer_list constructors as there are no names
@@ -61,6 +66,9 @@
}
bool VisitCallExpr(CallExpr *E) {
+ if (!Cfg.InlayHints.DeducedTypes)
+ 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
@@ -84,6 +92,9 @@
}
bool VisitFunctionDecl(FunctionDecl *D) {
+ if (!Cfg.InlayHints.DeducedTypes)
+ return true;
+
if (auto *AT = D->getReturnType()->getContainedAutoType()) {
QualType Deduced = AT->getDeducedType();
if (!Deduced.isNull()) {
@@ -96,6 +107,9 @@
}
bool VisitVarDecl(VarDecl *D) {
+ if (!Cfg.InlayHints.DeducedTypes)
+ return true;
+
// Do not show hints for the aggregate in a structured binding,
// but show hints for the individual bindings.
if (auto *DD = dyn_cast<DecompositionDecl>(D)) {
@@ -314,6 +328,23 @@
}
void addInlayHint(SourceRange R, InlayHintKind Kind, llvm::StringRef Label) {
+ // 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)
@@ -346,6 +377,7 @@
std::vector<InlayHint> &Results;
ASTContext &AST;
+ const Config &Cfg;
FileID MainFileID;
StringRef MainFileBuf;
const HeuristicResolver *Resolver;
@@ -363,7 +395,10 @@
std::vector<InlayHint> inlayHints(ParsedAST &AST) {
std::vector<InlayHint> Results;
- InlayHintVisitor Visitor(Results, AST);
+ const auto &Cfg = Config::current();
+ if (!Cfg.InlayHints.Enabled)
+ return Results;
+ InlayHintVisitor Visitor(Results, AST, Cfg);
Visitor.TraverseAST(AST.getASTContext());
// De-duplicate hints. Duplicates can sometimes occur due to e.g. explicit
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -66,6 +66,7 @@
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 @@
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 @@
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 @@
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;
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -283,6 +283,18 @@
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
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -197,6 +197,7 @@
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 @@
}
}
+ 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;
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -122,6 +122,15 @@
/// 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
