[clang-tools-extra] ce94432 - [clangd] Add designator inlay hints for initializer lists.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 25 15:44:42 PST 2022
Author: Sam McCall
Date: 2022-01-26T00:35:29+01:00
New Revision: ce94432702bf42a0b95a2693aa47177f37dd0bb3
URL: https://github.com/llvm/llvm-project/commit/ce94432702bf42a0b95a2693aa47177f37dd0bb3
DIFF: https://github.com/llvm/llvm-project/commit/ce94432702bf42a0b95a2693aa47177f37dd0bb3.diff
LOG: [clangd] Add designator inlay hints for initializer lists.
These make the init lists appear as if designated initialization was used.
Example:
ExpectedHint{"param: ", "arg"}
becomes
ExpectedHint{.Label="param: ", .RangeName="arg"}
Differential Revision: https://reviews.llvm.org/D116786
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/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
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 952626db31e4..f84b5ef1ffb5 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -130,6 +130,7 @@ struct Config {
// Whether specific categories of hints are enabled.
bool Parameters = true;
bool DeducedTypes = true;
+ bool Designators = false;
} InlayHints;
};
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index a606b98a2dba..268f214639b9 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -541,6 +541,10 @@ struct FragmentCompiler {
Out.Apply.push_back([Value(**F.DeducedTypes)](const Params &, Config &C) {
C.InlayHints.DeducedTypes = Value;
});
+ if (F.Designators)
+ Out.Apply.push_back([Value(**F.Designators)](const Params &, Config &C) {
+ C.InlayHints.Designators = Value;
+ });
}
constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index d165b6305aa5..0be906036c87 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -293,6 +293,8 @@ struct Fragment {
llvm::Optional<Located<bool>> ParameterNames;
/// Show deduced types for `auto`.
llvm::Optional<Located<bool>> DeducedTypes;
+ /// Show designators in aggregate initialization.
+ llvm::Optional<Located<bool>> Designators;
};
InlayHintsBlock InlayHints;
};
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index 04c0c633a3bb..0c758b6d296d 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -229,6 +229,10 @@ class Parser {
if (auto Value = boolValue(N, "DeducedTypes"))
F.DeducedTypes = *Value;
});
+ Dict.handle("Designators", [&](Node &N) {
+ if (auto Value = boolValue(N, "Designators"))
+ F.Designators = *Value;
+ });
Dict.parse(N);
}
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index e534faa80c4b..671f9a151d40 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/ScopeExit.h"
namespace clang {
namespace clangd {
@@ -21,6 +22,167 @@ namespace {
// For now, inlay hints are always anchored at the left or right of their range.
enum class HintSide { Left, Right };
+// Helper class to iterate over the designator names of an aggregate type.
+//
+// For an array type, yields [0], [1], [2]...
+// For aggregate classes, yields null for each base, then .field1, .field2, ...
+class AggregateDesignatorNames {
+public:
+ AggregateDesignatorNames(QualType T) {
+ if (!T.isNull()) {
+ T = T.getCanonicalType();
+ if (T->isArrayType()) {
+ IsArray = true;
+ Valid = true;
+ return;
+ }
+ if (const RecordDecl *RD = T->getAsRecordDecl()) {
+ Valid = true;
+ FieldsIt = RD->field_begin();
+ FieldsEnd = RD->field_end();
+ if (const auto *CRD = llvm::dyn_cast<CXXRecordDecl>(RD)) {
+ BasesIt = CRD->bases_begin();
+ BasesEnd = CRD->bases_end();
+ Valid = CRD->isAggregate();
+ }
+ OneField = Valid && BasesIt == BasesEnd && FieldsIt != FieldsEnd &&
+ std::next(FieldsIt) == FieldsEnd;
+ }
+ }
+ }
+ // Returns false if the type was not an aggregate.
+ operator bool() { return Valid; }
+ // Advance to the next element in the aggregate.
+ void next() {
+ if (IsArray)
+ ++Index;
+ else if (BasesIt != BasesEnd)
+ ++BasesIt;
+ else if (FieldsIt != FieldsEnd)
+ ++FieldsIt;
+ }
+ // Print the designator to Out.
+ // Returns false if we could not produce a designator for this element.
+ bool append(std::string &Out, bool ForSubobject) {
+ if (IsArray) {
+ Out.push_back('[');
+ Out.append(std::to_string(Index));
+ Out.push_back(']');
+ return true;
+ }
+ if (BasesIt != BasesEnd)
+ return false; // Bases can't be designated. Should we make one up?
+ if (FieldsIt != FieldsEnd) {
+ llvm::StringRef FieldName;
+ if (const IdentifierInfo *II = FieldsIt->getIdentifier())
+ FieldName = II->getName();
+
+ // For certain objects, their subobjects may be named directly.
+ if (ForSubobject &&
+ (FieldsIt->isAnonymousStructOrUnion() ||
+ // std::array<int,3> x = {1,2,3}. Designators not strictly valid!
+ (OneField && isReservedName(FieldName))))
+ return true;
+
+ if (!FieldName.empty() && !isReservedName(FieldName)) {
+ Out.push_back('.');
+ Out.append(FieldName.begin(), FieldName.end());
+ return true;
+ }
+ return false;
+ }
+ return false;
+ }
+
+private:
+ bool Valid = false;
+ bool IsArray = false;
+ bool OneField = false; // e.g. std::array { T __elements[N]; }
+ unsigned Index = 0;
+ CXXRecordDecl::base_class_const_iterator BasesIt;
+ CXXRecordDecl::base_class_const_iterator BasesEnd;
+ RecordDecl::field_iterator FieldsIt;
+ RecordDecl::field_iterator FieldsEnd;
+};
+
+// Collect designator labels describing the elements of an init list.
+//
+// This function contributes the designators of some (sub)object, which is
+// represented by the semantic InitListExpr Sem.
+// This includes any nested subobjects, but *only* if they are part of the same
+// original syntactic init list (due to brace elision).
+// In other words, it may descend into subobjects but not written init-lists.
+//
+// For example: struct Outer { Inner a,b; }; struct Inner { int x, y; }
+// Outer o{{1, 2}, 3};
+// This function will be called with Sem = { {1, 2}, {3, ImplicitValue} }
+// It should generate designators '.a:' and '.b.x:'.
+// '.a:' is produced directly without recursing into the written sublist.
+// (The written sublist will have a separate collectDesignators() call later).
+// Recursion with Prefix='.b' and Sem = {3, ImplicitValue} produces '.b.x:'.
+void collectDesignators(const InitListExpr *Sem,
+ llvm::DenseMap<SourceLocation, std::string> &Out,
+ const llvm::DenseSet<SourceLocation> &NestedBraces,
+ std::string &Prefix) {
+ if (!Sem || Sem->isTransparent())
+ return;
+ assert(Sem->isSemanticForm());
+
+ // The elements of the semantic form all correspond to direct subobjects of
+ // the aggregate type. `Fields` iterates over these subobject names.
+ AggregateDesignatorNames Fields(Sem->getType());
+ if (!Fields)
+ return;
+ for (const Expr *Init : Sem->inits()) {
+ auto Next = llvm::make_scope_exit([&, Size(Prefix.size())] {
+ Fields.next(); // Always advance to the next subobject name.
+ Prefix.resize(Size); // Erase any designator we appended.
+ });
+ if (llvm::isa<ImplicitValueInitExpr>(Init))
+ continue; // a "hole" for a subobject that was not explicitly initialized
+
+ const auto *BraceElidedSubobject = llvm::dyn_cast<InitListExpr>(Init);
+ if (BraceElidedSubobject &&
+ NestedBraces.contains(BraceElidedSubobject->getLBraceLoc()))
+ BraceElidedSubobject = nullptr; // there were braces!
+
+ if (!Fields.append(Prefix, BraceElidedSubobject != nullptr))
+ continue; // no designator available for this subobject
+ if (BraceElidedSubobject) {
+ // If the braces were elided, this aggregate subobject is initialized
+ // inline in the same syntactic list.
+ // Descend into the semantic list describing the subobject.
+ // (NestedBraces are still correct, they're from the same syntactic list).
+ collectDesignators(BraceElidedSubobject, Out, NestedBraces, Prefix);
+ continue;
+ }
+ Out.try_emplace(Init->getBeginLoc(), Prefix);
+ }
+}
+
+// Get designators describing the elements of a (syntactic) init list.
+// This does not produce designators for any explicitly-written nested lists.
+llvm::DenseMap<SourceLocation, std::string>
+getDesignators(const InitListExpr *Syn) {
+ assert(Syn->isSyntacticForm());
+
+ // collectDesignators needs to know which InitListExprs in the semantic tree
+ // were actually written, but InitListExpr::isExplicit() lies.
+ // Instead, record where braces of sub-init-lists occur in the syntactic form.
+ llvm::DenseSet<SourceLocation> NestedBraces;
+ for (const Expr *Init : Syn->inits())
+ if (auto *Nested = llvm::dyn_cast<InitListExpr>(Init))
+ NestedBraces.insert(Nested->getLBraceLoc());
+
+ // Traverse the semantic form to find the designators.
+ // We use their SourceLocation to correlate with the syntactic form later.
+ llvm::DenseMap<SourceLocation, std::string> Designators;
+ std::string EmptyPrefix;
+ collectDesignators(Syn->isSemanticForm() ? Syn : Syn->getSemanticForm(),
+ Designators, NestedBraces, EmptyPrefix);
+ return Designators;
+}
+
class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
public:
InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST,
@@ -127,6 +289,30 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
return true;
}
+ bool VisitInitListExpr(InitListExpr *Syn) {
+ // We receive the syntactic form here (shouldVisitImplicitCode() is false).
+ // This is the one we will ultimately attach designators to.
+ // It may have subobject initializers inlined without braces. The *semantic*
+ // form of the init-list has nested init-lists for these.
+ // getDesignators will look at the semantic form to determine the labels.
+ assert(Syn->isSyntacticForm() && "RAV should not visit implicit code!");
+ if (!Cfg.InlayHints.Designators)
+ return true;
+ if (Syn->isIdiomaticZeroInitializer(AST.getLangOpts()))
+ return true;
+ llvm::DenseMap<SourceLocation, std::string> Designators =
+ getDesignators(Syn);
+ for (const Expr *Init : Syn->inits()) {
+ if (llvm::isa<DesignatedInitExpr>(Init))
+ continue;
+ auto It = Designators.find(Init->getBeginLoc());
+ if (It != Designators.end() &&
+ !isPrecededByParamNameComment(Init, It->second))
+ addDesignatorHint(Init->getSourceRange(), It->second);
+ }
+ return true;
+ }
+
// FIXME: Handle RecoveryExpr to try to hint some invalid calls.
private:
@@ -229,12 +415,16 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
// Check for comment ending.
if (!SourcePrefix.consume_back("*/"))
return false;
- // Allow whitespace and "=" at end of comment.
- SourcePrefix = SourcePrefix.rtrim().rtrim('=').rtrim();
+ // Ignore some punctuation and whitespace around comment.
+ // In particular this allows designators to match nicely.
+ llvm::StringLiteral IgnoreChars = " =.";
+ SourcePrefix = SourcePrefix.rtrim(IgnoreChars);
+ ParamName = ParamName.trim(IgnoreChars);
// Other than that, the comment must contain exactly ParamName.
if (!SourcePrefix.consume_back(ParamName))
return false;
- return SourcePrefix.rtrim().endswith("/*");
+ SourcePrefix = SourcePrefix.rtrim(IgnoreChars);
+ return SourcePrefix.endswith("/*");
}
// If "E" spells a single unqualified identifier, return that name.
@@ -341,6 +531,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
break
CHECK_KIND(ParameterHint, Parameters);
CHECK_KIND(TypeHint, DeducedTypes);
+ CHECK_KIND(DesignatorHint, Designators);
#undef CHECK_KIND
}
@@ -378,6 +569,11 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
TypeName, /*Suffix=*/"");
}
+ void addDesignatorHint(SourceRange R, llvm::StringRef Text) {
+ addInlayHint(R, HintSide::Left, InlayHintKind::DesignatorHint,
+ /*Prefix=*/"", Text, /*Suffix=*/"=");
+ }
+
std::vector<InlayHint> &Results;
ASTContext &AST;
const Config &Cfg;
diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index 42f452f74f97..4dee1373a149 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1326,6 +1326,8 @@ llvm::json::Value toJSON(InlayHintKind K) {
return "parameter";
case InlayHintKind::TypeHint:
return "type";
+ case InlayHintKind::DesignatorHint:
+ return "designator";
}
llvm_unreachable("Unknown clang.clangd.InlayHintKind");
}
diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index d7ca580dceff..1945c766608b 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -1538,6 +1538,12 @@ enum class InlayHintKind {
/// which shows the deduced type of the variable.
TypeHint,
+ /// A hint before an element of an aggregate braced initializer list,
+ /// indicating what it is initializing.
+ /// Pair{^1, ^2};
+ /// Uses designator syntax, e.g. `.first:`.
+ DesignatorHint,
+
/// 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 992ec0e012ae..6c3ac0d62e0e 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -13,21 +13,22 @@
#include "TestWorkspace.h"
#include "XRefs.h"
#include "support/Context.h"
+#include "llvm/Support/ScopedPrinter.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
namespace clang {
namespace clangd {
-std::ostream &operator<<(std::ostream &Stream, const InlayHint &Hint) {
- return Stream << Hint.label;
+llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
+ const InlayHint &Hint) {
+ return Stream << Hint.label << "@" << Hint.range;
}
namespace {
using ::testing::ElementsAre;
using ::testing::IsEmpty;
-using ::testing::UnorderedElementsAre;
std::vector<InlayHint> hintsOfKind(ParsedAST &AST, InlayHintKind Kind) {
std::vector<InlayHint> Result;
@@ -45,17 +46,24 @@ struct ExpectedHint {
std::string RangeName;
HintSide Side = Left;
- friend std::ostream &operator<<(std::ostream &Stream,
- const ExpectedHint &Hint) {
- return Stream << Hint.RangeName << ": " << Hint.Label;
+ friend llvm::raw_ostream &operator<<(llvm::raw_ostream &Stream,
+ const ExpectedHint &Hint) {
+ return Stream << Hint.Label << "@$" << Hint.RangeName;
}
};
-MATCHER_P2(HintMatcher, Expected, Code, "") {
- return arg.label == Expected.Label &&
- arg.range == Code.range(Expected.RangeName) &&
- arg.position ==
- ((Expected.Side == Left) ? arg.range.start : arg.range.end);
+MATCHER_P2(HintMatcher, Expected, Code, llvm::to_string(Expected)) {
+ if (arg.label != Expected.Label) {
+ *result_listener << "label is " << arg.label;
+ return false;
+ }
+ if (arg.range != Code.range(Expected.RangeName)) {
+ *result_listener << "range is " << arg.label << " but $"
+ << Expected.RangeName << " is "
+ << llvm::to_string(Code.range(Expected.RangeName));
+ return false;
+ }
+ return true;
}
MATCHER_P(labelIs, Label, "") { return arg.label == Label; }
@@ -64,6 +72,7 @@ Config noHintsConfig() {
Config C;
C.InlayHints.Parameters = false;
C.InlayHints.DeducedTypes = false;
+ C.InlayHints.Designators = false;
return C;
}
@@ -100,6 +109,15 @@ void assertTypeHints(llvm::StringRef AnnotatedSource,
assertHints(InlayHintKind::TypeHint, AnnotatedSource, Expected...);
}
+template <typename... ExpectedHints>
+void assertDesignatorHints(llvm::StringRef AnnotatedSource,
+ ExpectedHints... Expected) {
+ Config Cfg;
+ Cfg.InlayHints.Designators = true;
+ WithContextValue WithCfg(Config::Key, std::move(Cfg));
+ assertHints(InlayHintKind::DesignatorHint, AnnotatedSource, Expected...);
+}
+
TEST(ParameterHints, Smoke) {
assertParameterHints(R"cpp(
void foo(int param);
@@ -658,6 +676,71 @@ TEST(TypeHints, Deduplication) {
ExpectedHint{": int", "var"});
}
+TEST(DesignatorHints, Basic) {
+ assertDesignatorHints(R"cpp(
+ struct S { int x, y, z; };
+ S s {$x[[1]], $y[[2+2]]};
+
+ int x[] = {$0[[0]], $1[[1]]};
+ )cpp",
+ ExpectedHint{".x=", "x"}, ExpectedHint{".y=", "y"},
+ ExpectedHint{"[0]=", "0"}, ExpectedHint{"[1]=", "1"});
+}
+
+TEST(DesignatorHints, Nested) {
+ assertDesignatorHints(R"cpp(
+ struct Inner { int x, y; };
+ struct Outer { Inner a, b; };
+ Outer o{ $a[[{ $x[[1]], $y[[2]] }]], $bx[[3]] };
+ )cpp",
+ ExpectedHint{".a=", "a"}, ExpectedHint{".x=", "x"},
+ ExpectedHint{".y=", "y"}, ExpectedHint{".b.x=", "bx"});
+}
+
+TEST(DesignatorHints, AnonymousRecord) {
+ assertDesignatorHints(R"cpp(
+ struct S {
+ union {
+ struct {
+ struct {
+ int y;
+ };
+ } x;
+ };
+ };
+ S s{$xy[[42]]};
+ )cpp",
+ ExpectedHint{".x.y=", "xy"});
+}
+
+TEST(DesignatorHints, Suppression) {
+ assertDesignatorHints(R"cpp(
+ struct Point { int a, b, c, d, e, f, g, h; };
+ Point p{/*a=*/1, .c=2, /* .d = */3, $e[[4]]};
+ )cpp",
+ ExpectedHint{".e=", "e"});
+}
+
+TEST(DesignatorHints, StdArray) {
+ // Designators for std::array should be [0] rather than .__elements[0].
+ // While technically correct, the designator is useless and horrible to read.
+ assertDesignatorHints(R"cpp(
+ template <typename T, int N> struct Array { T __elements[N]; };
+ Array<int, 2> x = {$0[[0]], $1[[1]]};
+ )cpp",
+ ExpectedHint{"[0]=", "0"}, ExpectedHint{"[1]=", "1"});
+}
+
+TEST(DesignatorHints, OnlyAggregateInit) {
+ assertDesignatorHints(R"cpp(
+ struct Copyable { int x; } c;
+ Copyable d{c};
+
+ struct Constructible { Constructible(int x); };
+ Constructible x{42};
+ )cpp" /*no designator hints expected (but param hints!)*/);
+}
+
TEST(InlayHints, RestrictRange) {
Annotations Code(R"cpp(
auto a = false;
More information about the cfe-commits
mailing list