[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
This commit is contained in:
Sam McCall 2022-01-07 04:15:07 +01:00
parent 8de76bd569
commit ce94432702
8 changed files with 312 additions and 14 deletions

View File

@ -130,6 +130,7 @@ struct Config {
// Whether specific categories of hints are enabled.
bool Parameters = true;
bool DeducedTypes = true;
bool Designators = false;
} InlayHints;
};

View File

@ -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;

View File

@ -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;
};

View File

@ -229,6 +229,10 @@ private:
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);
}

View File

@ -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 @@ public:
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 @@ private:
// 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 @@ private:
break
CHECK_KIND(ParameterHint, Parameters);
CHECK_KIND(TypeHint, DeducedTypes);
CHECK_KIND(DesignatorHint, Designators);
#undef CHECK_KIND
}
@ -378,6 +569,11 @@ private:
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;

View File

@ -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");
}

View File

@ -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

View File

@ -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;