sammccall updated this revision to Diff 398228.
sammccall marked 2 inline comments as done.
sammccall added a comment.
Simplify & fix category checks
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116713/new/
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"
@@ -24,6 +26,8 @@
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_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 @@
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.
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 "clang/AST/DeclarationName.h"
@@ -23,8 +24,8 @@
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 @@
}
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 @@
// 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 @@
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 @@
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 @@
std::vector<InlayHint> &Results;
ASTContext &AST;
+ const Config &Cfg;
llvm::Optional<Range> RestrictRange;
FileID MainFileID;
StringRef MainFileBuf;
@@ -381,7 +402,10 @@
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
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
