Page MenuHomePhabricator

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

Authored by njames93 on Aug 3 2022, 4:16 AM.

Details

Reviewers
sammccall
nridge
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.

Diff Detail

Event Timeline

njames93 created this revision.Aug 3 2022, 4:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2022, 4:16 AM
njames93 requested review of this revision.Aug 3 2022, 4:16 AM

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.

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;