sammccall created this revision.
sammccall added a reviewer: nridge.
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.
These make the init lists appear as if designated initialization was used.
Example:
ExpectedHint{"param: ", "arg"}
becomes
ExpectedHint{.Label="param: ", .RangeName="arg"}
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D116786
Files:
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
Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -11,18 +11,21 @@
#include "TestTU.h"
#include "TestWorkspace.h"
#include "XRefs.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::UnorderedElementsAre;
std::vector<InlayHint> hintsOfKind(ParsedAST &AST, InlayHintKind Kind) {
@@ -38,15 +41,24 @@
std::string Label;
std::string RangeName;
- 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);
+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;
}
template <typename... ExpectedHints>
@@ -58,7 +70,7 @@
auto AST = TU.build();
EXPECT_THAT(hintsOfKind(AST, Kind),
- UnorderedElementsAre(HintMatcher(Expected, Source)...));
+ ElementsAre(HintMatcher(Expected, Source)...));
}
template <typename... ExpectedHints>
@@ -73,6 +85,12 @@
assertHints(InlayHintKind::TypeHint, AnnotatedSource, Expected...);
}
+template <typename... ExpectedHints>
+void assertDesignatorHints(llvm::StringRef AnnotatedSource,
+ ExpectedHints... Expected) {
+ assertHints(InlayHintKind::DesignatorHint, AnnotatedSource, Expected...);
+}
+
TEST(ParameterHints, Smoke) {
assertParameterHints(R"cpp(
void foo(int param);
@@ -631,6 +649,71 @@
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 hints expected*/);
+}
+
// FIXME: Low-hanging fruit where we could omit a type hint:
// - auto x = TypeName(...);
// - auto x = (TypeName) (...);
Index: clang-tools-extra/clangd/Protocol.h
===================================================================
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1533,6 +1533,12 @@
/// 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
Index: clang-tools-extra/clangd/Protocol.cpp
===================================================================
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1326,6 +1326,8 @@
return "parameter";
case InlayHintKind::TypeHint:
return "type";
+ case InlayHintKind::DesignatorHint:
+ return "designator";
}
llvm_unreachable("Unknown clang.clangd.InlayHintKind");
}
Index: clang-tools-extra/clangd/InlayHints.cpp
===================================================================
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -8,15 +8,176 @@
#include "InlayHints.h"
#include "HeuristicResolver.h"
#include "ParsedAST.h"
-#include "support/Logger.h"
+#include "SourceCode.h"
#include "clang/AST/DeclarationName.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/SourceManager.h"
-#include "llvm/Support/raw_ostream.h"
+#include "llvm/ADT/ScopeExit.h"
namespace clang {
namespace clangd {
+namespace {
+
+// 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;
+ FieldsI = RD->field_begin();
+ FieldsE = RD->field_end();
+ if (const auto *CRD = llvm::dyn_cast<CXXRecordDecl>(RD)) {
+ BasesI = CRD->bases_begin();
+ BasesE = CRD->bases_end();
+ Valid = CRD->isAggregate();
+ }
+ OneField = Valid && BasesI == BasesE && FieldsI != FieldsE &&
+ std::next(FieldsI) == FieldsE;
+ }
+ }
+ }
+ // 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 (BasesI != BasesE)
+ ++BasesI;
+ else if (FieldsI != FieldsE)
+ ++FieldsI;
+ }
+ // 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 (BasesI != BasesE)
+ return false; // Bases can't be designated. Should we make one up?
+ if (FieldsI != FieldsE) {
+ llvm::StringRef FieldName;
+ if (const IdentifierInfo *II = FieldsI->getIdentifier())
+ FieldName = II->getName();
+
+ // For certain objects, their subobjects may be named directly.
+ if (ForSubobject &&
+ (FieldsI->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 BasesI;
+ CXXRecordDecl::base_class_const_iterator BasesE;
+ RecordDecl::field_iterator FieldsI;
+ RecordDecl::field_iterator FieldsE;
+};
+
+// 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.
+// 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 type 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.
+ collectDesignators(BraceElidedSubobject, Out, NestedBraces, Prefix);
+ continue;
+ }
+ Prefix.push_back('=');
+ 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());
+
+ // gatherDesignators 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 Scratch;
+ collectDesignators(Syn->isSemanticForm() ? Syn : Syn->getSemanticForm(),
+ Designators, NestedBraces, Scratch);
+ return Designators;
+}
class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
public:
@@ -119,6 +280,22 @@
return true;
}
+ bool VisitInitListExpr(InitListExpr *Syn) {
+ 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:
@@ -220,12 +397,16 @@
// 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.
@@ -344,6 +525,10 @@
addInlayHint(R, InlayHintKind::TypeHint, std::string(Prefix) + TypeName);
}
+ void addDesignatorHint(SourceRange R, llvm::StringRef Text) {
+ addInlayHint(R, InlayHintKind::DesignatorHint, Text);
+ }
+
std::vector<InlayHint> &Results;
ASTContext &AST;
FileID MainFileID;
@@ -361,6 +546,8 @@
static const size_t TypeNameLimit = 32;
};
+} // namespace
+
std::vector<InlayHint> inlayHints(ParsedAST &AST) {
std::vector<InlayHint> Results;
InlayHintVisitor Visitor(Results, AST);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
