Page MenuHomePhabricator

[clangd] Improve inlay hints of things expanded from macros
ClosedPublic

Authored by sammccall on Sep 15 2022, 3:48 PM.

Details

Summary

When we aim a hint at some expanded tokens, we're only willing to attach it
to spelled tokens that exactly corresponde.

e.g.
int zoom(int x, int y, int z);
int dummy = zoom(NUMBERS);

Here we want to place a hint "x:" on the expanded "1", but we shouldn't
be willing to place it on NUMBERS, because it doesn't *exactly*
correspond (it has more tokens).

Fortunately we don't even have to implement this algorithm from scratch,
TokenBuffer has it.

Fixes https://github.com/clangd/clangd/issues/1289
Fixes https://github.com/clangd/clangd/issues/1118
Fixes https://github.com/clangd/clangd/issues/1018

Diff Detail

Event Timeline

sammccall created this revision.Sep 15 2022, 3:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 15 2022, 3:48 PM
Herald added a subscriber: arphaman. · View Herald Transcript
sammccall requested review of this revision.Sep 15 2022, 3:48 PM
nridge accepted this revision.Sep 17 2022, 2:16 AM

Thanks for fixing!

clang-tools-extra/clangd/InlayHints.cpp
281

Is this change related to the fix?

This revision is now accepted and ready to land.Sep 17 2022, 2:16 AM
sammccall added inline comments.Sep 19 2022, 7:34 AM
clang-tools-extra/clangd/InlayHints.cpp
281

Only tangentially...

This change makes the whole range significant (previously it was serialized but I think not used by any clients these days). So hiding the token->range conversion inside this function might encourage bugs in the future, even if today all the callers do that.

(I did consider attaching the return type hints to e.g. the parens range rather than a single paren, but can't think of a motivating case in either direction really)