[clangd] Add option to disable inlay hints for init lists when building array.

[njames93]

Summary

Inlay hints can be pretty annoying when using an initializer list to build an array.

string HelloW[] = {<0=>"Hello", <1=>"World"};

To silence these I've extended the Designators config option to work as an enum that will let users choose to enable or disable arrays while still letting them use the normal designators.

[njames93]

I'm not 100% sold on this way of setting this up.
There is definitely a use case for splitting the current Designators option into 2, one of them just controlling arrays.

[sammccall]

We have been thinking about problems with inlay hints being verbose, and array initializers that are visible by default are clearly seen as spammy.
Thanks for sending a patch. I filed https://github.com/clangd/clangd/issues/1277 about this just now and added some thoughts.
FWIW when *not* shown by default (VSCode's offUnlessPressed) these don't seem to be spammy.

Adding a new option is always easy, but actually helps relatively few people unless we change the default.
So I think we should change the default behavior, and consider making it configurable if we get requests to add it back.

(I have reservations about the details of the config schema in this patch, but we can improve it if needed).

clang-tools-extra/clangd/InlayHints.cpp

77

This means that both array-nested-in-struct and struct-nested-in-array will produce no hints at all.
This is probably a rare case, but IMO one that will likely benefit from designators.

The smallest behavior change to address the common case would be to drop designators that only contain a single array index.

I suppose the neatest way to achieve this is to bail out at (old) line 153:

if (Fields.isArray() && !BraceElidedSubobject && Prefix.empty())
  // Here we'd emit only a trivial array designator.
  // This isn't useful: https://github.com/clangd/clangd/issues/1277
  continue;

clang-tools-extra/clangd/Config.h

struct Config {
   } Completion;
 
   /// Configures hover feature.
   struct {
     /// Whether hover show a.k.a type.
     bool ShowAKA = true;
   } Hover;
 
-  struct {
+  struct InlayHints {
+    enum DesignatorsStlye { None = 0, All, IgnoreArrays };
+
     /// If false, inlay hints are completely disabled.
     bool Enabled = true;
 
     // Whether specific categories of hints are enabled.
     bool Parameters = true;
     bool DeducedTypes = true;
-    bool Designators = true;
+    DesignatorsStlye Designators = All;
   } InlayHints;
 };
 
 } // namespace clangd
 } // namespace clang
 
 namespace llvm {
 template <> struct DenseMapInfo<clang::clangd::Config::ExternalIndexSpec> {

clang-tools-extra/clangd/ConfigCompile.cpp

 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/YAMLParser.h"
 #include <algorithm>
 #include <memory>
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 namespace config {
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;
       });
-    if (F.Designators)
-      Out.Apply.push_back([Value(**F.Designators)](const Params &, Config &C) {
-        C.InlayHints.Designators = Value;
+    if (F.Designators) {
+      Optional<Config::InlayHints::DesignatorsStlye> Style;
+      if (auto Bool = llvm::yaml::parseBool(**F.Designators)) {
+        Style = *Bool ? Config::InlayHints::All : Config::InlayHints::None;
+      } else
+        Style = compileEnum<Config::InlayHints::DesignatorsStlye>(
+                    "Designators", *F.Designators)
+                    .map("All", Config::InlayHints::All)
+                    .map("IgnoreArrays", Config::InlayHints::IgnoreArrays)
+                    .map("None", Config::InlayHints::None)
+                    .value();
+      if (Style)
+        Out.Apply.push_back([Style(*Style)](const Params &, Config &C) {
+          C.InlayHints.Designators = Style;
         });
     }
+  }
 
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
   constexpr static llvm::SourceMgr::DiagKind Warning =
       llvm::SourceMgr::DK_Warning;
   void diag(llvm::SourceMgr::DiagKind Kind, llvm::StringRef Message,
             llvm::SMRange Range) {
     if (Range.isValid() && SourceMgr != nullptr)
       Diagnostic(SourceMgr->GetMessage(Range.Start, Kind, Message, Range));

clang-tools-extra/clangd/ConfigFragment.h

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;
     /// Show designators in aggregate initialization.
-    llvm::Optional<Located<bool>> Designators;
+    llvm::Optional<Located<std::string>> Designators;
   };
   InlayHintsBlock InlayHints;
 };
 
 } // namespace config
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/ConfigYAML.cpp

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.handle("Designators", [&](Node &N) {
-      if (auto Value = boolValue(N, "Designators"))
+      if (auto Value = scalarValue(N, "Designators"))
         F.Designators = *Value;
     });
     Dict.parse(N);
   }
 
   // Helper for parsing mapping nodes (dictionaries).
   // We don't use YamlIO as we want to control over unknown keys.
   class DictParser {

clang-tools-extra/clangd/InlayHints.cpp

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) {
+  bool append(std::string &Out, bool ForSubobject, bool CollectArrays) {
     if (IsArray) {
+      if (CollectArrays) {
         Out.push_back('[');
         Out.append(std::to_string(Index));
         Out.push_back(']');
         return true;
       }
+      return false;
+    }
     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.

 // 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) {
+                        std::string &Prefix, bool CollectArrays) {
   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))
+    if (!Fields.append(Prefix, BraceElidedSubobject != nullptr, CollectArrays))
       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);
+      collectDesignators(BraceElidedSubobject, Out, NestedBraces, Prefix,
+                         CollectArrays);
       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) {
+getDesignators(const InitListExpr *Syn, bool CollectArrays) {
   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);
+                     Designators, NestedBraces, EmptyPrefix, CollectArrays);
   return Designators;
 }
 
 class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
 public:
   InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST,
                    const Config &Cfg, llvm::Optional<Range> RestrictRange)
       : Results(Results), AST(AST.getASTContext()), Cfg(Cfg),
public:
 
   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)
+    if (Cfg.InlayHints.Designators == Config::InlayHints::None)
       return true;
     if (Syn->isIdiomaticZeroInitializer(AST.getLangOpts()))
       return true;
-    llvm::DenseMap<SourceLocation, std::string> Designators =
-        getDesignators(Syn);
+    llvm::DenseMap<SourceLocation, std::string> Designators = getDesignators(
+        Syn, Cfg.InlayHints.Designators == Config::InlayHints::All);
     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);
     }

clang-tools-extra/clangd/unittests/InlayHintTests.cpp

 }
 
 MATCHER_P(labelIs, Label, "") { return arg.label == Label; }
 
 Config noHintsConfig() {
   Config C;
   C.InlayHints.Parameters = false;
   C.InlayHints.DeducedTypes = false;
-  C.InlayHints.Designators = false;
+  C.InlayHints.Designators = Config::InlayHints::None;
   return C;
 }
 
 template <typename... ExpectedHints>
 void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
                  ExpectedHints... Expected) {
   Annotations Source(AnnotatedSource);
   TestTU TU = TestTU::withCode(Source.code());

 template <typename... ExpectedHints>
 void assertTypeHints(llvm::StringRef AnnotatedSource,
                      ExpectedHints... Expected) {
   ignore(Expected.Side = Right...);
   assertHints(InlayHintKind::Type, AnnotatedSource, Expected...);
 }
 
 template <typename... ExpectedHints>
-void assertDesignatorHints(llvm::StringRef AnnotatedSource,
+void assertDesignatorHints(llvm::StringRef AnnotatedSource, bool IncludeArray,
                            ExpectedHints... Expected) {
   Config Cfg;
-  Cfg.InlayHints.Designators = true;
+  Cfg.InlayHints.Designators =
+      IncludeArray ? Config::InlayHints::All : Config::InlayHints::IgnoreArrays;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
   assertHints(InlayHintKind::Designator, AnnotatedSource, Expected...);
 }
 
 TEST(ParameterHints, Smoke) {
   assertParameterHints(R"cpp(
     void foo(int param);
     void bar() {
TEST(TypeHints, Aliased) {
   TestTU TU = TestTU::withCode("void foo(void){} extern typeof(foo) foo;");
   TU.ExtraArgs.push_back("-xc");
   auto AST = TU.build();
 
   EXPECT_THAT(hintsOfKind(AST, InlayHintKind::Type), IsEmpty());
 }
 
 TEST(DesignatorHints, Basic) {
-  assertDesignatorHints(R"cpp(
+  StringRef Source = 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"});
+  )cpp";
+  assertDesignatorHints(Source, true, ExpectedHint{".x=", "x"},
+                        ExpectedHint{".y=", "y"}, ExpectedHint{"[0]=", "0"},
+                        ExpectedHint{"[1]=", "1"});
+  assertDesignatorHints(Source, false, ExpectedHint{".x=", "x"},
+                        ExpectedHint{".y=", "y"});
 }
 
 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"});
+                        true, 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"});
+                        true, 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"});
+                        true, 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"});
+                        true, ExpectedHint{"[0]=", "0"},
+                        ExpectedHint{"[1]=", "1"});
 }
 
 TEST(DesignatorHints, OnlyAggregateInit) {
-  assertDesignatorHints(R"cpp(
+  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!)*/);
+  )cpp",
+      true /*no designator hints expected (but param hints!)*/);
 }
 
 TEST(InlayHints, RestrictRange) {
   Annotations Code(R"cpp(
     auto a = false;
     [[auto b = 1;
     auto c = '2';]]
     auto d = 3.f;