diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -130,6 +130,7 @@ // 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 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -541,6 +541,10 @@ 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 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -293,6 +293,8 @@ llvm::Optional> ParameterNames; /// Show deduced types for `auto`. llvm::Optional> DeducedTypes; + /// Show designators in aggregate initialization. + llvm::Optional> Designators; }; InlayHintsBlock InlayHints; }; diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -229,6 +229,10 @@ 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 --- 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 @@ // 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(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 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 &Out, + const llvm::DenseSet &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(Init)) + continue; // a "hole" for a subobject that was not explicitly initialized + + const auto *BraceElidedSubobject = llvm::dyn_cast(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 +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 NestedBraces; + for (const Expr *Init : Syn->inits()) + if (auto *Nested = llvm::dyn_cast(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 Designators; + std::string EmptyPrefix; + collectDesignators(Syn->isSemanticForm() ? Syn : Syn->getSemanticForm(), + Designators, NestedBraces, EmptyPrefix); + return Designators; +} + class InlayHintVisitor : public RecursiveASTVisitor { public: InlayHintVisitor(std::vector &Results, ParsedAST &AST, @@ -127,6 +289,30 @@ 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 Designators = + getDesignators(Syn); + for (const Expr *Init : Syn->inits()) { + if (llvm::isa(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 @@ // 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 @@ break CHECK_KIND(ParameterHint, Parameters); CHECK_KIND(TypeHint, DeducedTypes); + CHECK_KIND(DesignatorHint, Designators); #undef CHECK_KIND } @@ -378,6 +569,11 @@ TypeName, /*Suffix=*/""); } + void addDesignatorHint(SourceRange R, llvm::StringRef Text) { + addInlayHint(R, HintSide::Left, InlayHintKind::DesignatorHint, + /*Prefix=*/"", Text, /*Suffix=*/"="); + } + std::vector &Results; ASTContext &AST; const Config &Cfg; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1538,6 +1538,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 diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/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"); } diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp --- 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 hintsOfKind(ParsedAST &AST, InlayHintKind Kind) { std::vector Result; @@ -45,17 +46,24 @@ 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 C; C.InlayHints.Parameters = false; C.InlayHints.DeducedTypes = false; + C.InlayHints.Designators = false; return C; } @@ -100,6 +109,15 @@ assertHints(InlayHintKind::TypeHint, AnnotatedSource, Expected...); } +template +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 @@ 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 struct Array { T __elements[N]; }; + Array 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;