[clangd] Add support for inline parameter hints

[nridge]

Some notes on this:

This feature is based on https://github.com/microsoft/language-server-protocol/issues/956, which combines inline parameter hints with other types of annotations that can be shown inline under a single feature called inlay hints. The protocol is designed to accommodate different types of hints, though the only kind currently provided are parameter hints.

The protocol bits are more specifically based on https://github.com/microsoft/vscode-languageserver-node/pull/609/, with some changes to reflect the discussion in the LSP issue (notably, the addition of a position field as discussed in this comment).

I have a corresponding client-side patch here: https://github.com/clangd/vscode-clangd/pull/168

I do have some questions about the details of the protocol, which I've asked inline.

clang-tools-extra/clangd/ClangdLSPServer.cpp

1478

I expect the protocol in this patch to be largely in line with the version standardized in LSP, but there may be some minor differences as that's not yet finalized.

Does that mean we should rather name this clangd/inlayHints for now?

clang-tools-extra/clangd/Protocol.h

1489

I'm not sure if there's much point in keeping these constants in the protocol. With the addition of InlayHintPosition, the InlayHintKind no longer carries any semantic meaning, beyond allowing the client to enable/disable hints on a per-kind basis.

I do think that enable/disable granularity is useful, but it could also be accomplished with a freeform string as the kind. Should I change it to a string? Or keep the enum for clangd's use but serialize it as a string?

1524

In the protocol documentation, I've repeatedly used the term "token" to describe the fragment of code to which a hint applies, but technically it's not necessary a single AST token. For example, for parameter hints, they apply to the entire argument expression. Should I say something like "AST node" instead?

[nridge]

Some improvements

[nridge]

Adding a few potential reviewers.

[sammccall]

Thanks for working on this! It looks like it's getting traction in VSCode/LSP now, which makes it much more compelling.

Protocol stuff: obviously in the long run we want to match where LSP goes, and the fewer changes between now and then the better.
But there's enough uncertainty that I don't think we have much chance of guessing right enough to be compatible at this point.
So rather than introducing another variant that's likely to be "wrong", let's use rust-analyzer's protocol as-is?
I don't think leaving out "position" is a big deal as long as we're only providing one type of hint.
(I'd even be tempted to call it "rust-analyzer/inlayHints" but maybe that's too silly)

---

I dug a bit into the differences based on:

• the pull request you mentioned: https://github.com/microsoft/vscode-languageserver-node/pull/609/files#
• the proposed API inside VSCode: https://github.com/microsoft/vscode/blob/main/src/vs/vscode.proposed.d.ts
• discussion on that PR: https://github.com/microsoft/vscode/pull/113285

Looks like VSCode calls this "inline hints" but has a TODO to change back to "inlay hint". So naming seems likely correct.

Ranges/positions are very different! In particular, I think range has the same name+type but different semantics.
For the parameter hint:

foo(bar: 42);
The rust-analyzer protocol wants range = `foo([[42]]);`, label="bar: " position = before
The VSCode API wants range = `foo([[]]42);`, label="bar:", whitespaceAfter = true

This is a tough call because I think the rust-analyzer proposal is better (retains more info + flexibility like you say) but the VSCode one seems fairly likely to win :-/ We should provide feedback here.

"text" and "label" seem an arbitrary different choice of name.

[sammccall]

clang-tools-extra/clangd/ClangdLSPServer.cpp

1478

I think differences are quite likely, as VSCode APIs diverge from what's in the PR.

So yeah, I think this should be clangd/inlayHints or rust-analyzer/inlayHints.

clang-tools-extra/clangd/Protocol.h

1489

We should at avoid having string literals scattered all over the impl + tests.

I like enum-serialized-as-string a little better than string-with-constants-exposed...

(Also it seems reasonably likely they'll pull the same dictionary trick as with semantic-tokens, so we can send enum numbers over the wire)

1494

any point including the unused kinds here, rather than leaving FIXMEs for ideas we have?

(I ask because it's not obvious to me that the caret is in the right place in that example, but it seems silly to worry about it now)

1494

implicit conversions/constructor calls is a potentially interesting set of hints too

1520

as mentioned I don't think we need to include position for now. If we do, 0/1 seems like an unlikely representation...

1524

I'd just say something like "the range of source code"... Both "token" and "ast node" seem unnessarily specific.

clang-tools-extra/clangd/XRefs.cpp

1977

It's not obvious to me exactly what cases "implicit constructor call" covers, can you be more precise?
I suspect this comes down to "does the constructor look like a function call with an argument list", in which case getParenOrBraceRange() is obviously the right check.

1979

I think you also want to exclude E->isStdListInitialization - in this case you have braces but it's not the arg list

1999

nit: can we take an ArrayRef instead of pointer+length?

2004

do we not want to assume that an incomplete call is a prefix?
Seems like the inlays might be helpful in diagnosing that situation :-)

Actually I guess we'd have to handle RecoveryExpr to get that to work. I did have an idea once that RecoveryExpr would contain a Stmt::Kind indicating what we *tried* to build, which we could use here.

Anyway, something for the future.

2004

should we also bail out if numParams > argcount and the function isn't variadic?
Not sure we'll actually get here, but that goes for the < condition too...

2012

Ah, redecls are interesting. Many possible variations on this strategy.

For example:

• we could iterate over all redecls instead of just canonical + definition
• we could fall back arg-by-arg rather than for the whole arg list

I think it might be worth moving this into getParameterNames (or introducing an intermediate function) to allow some more implementation flexibility (now or in the future).

2024

Very nice.
I think we also want to omit it if there's a /* foo= */ comment (whitespace and = optional).

It's extremely annoying that TokenBuffer doesn't capture comments, but I think it would work well enough to simply match the source code up to the insertion point textually.

2030

It's really tempting to include by-ref-ness as foo&: or something, but the distinction between const/non-const is critical and I can't think of a similarly compact way to denote that.

2046

How do you feel about stripping leading underscores from names, in order to prettify standard library calls?

e.g. libc++'s std::copy has parameters __first, __last and __result to guard against macros, libstdc++ and MSVCSTL seem to do the same. It's going to look really noisy!

(I don't think we currently do this anywhere, it's obviously relevant to signature help & code completion too... but have to start somewhere!)

2059

I don't think param decls can be special names, so instead of printing can you just grab the ASTContext-owned StringRef from the DeclarationName? Then we can return vector<StringRef> with no copies.

And while here, it seems like SmallVector<StringRef, 8> or so would be even better.

(Can merge this with the anonymous case: either getAsIdentifierInfo gives you an identifier or you insert empty)

2066

Ooh, macros :-)

This is fine for now, but because this will be displayed always (for some users) failures are going to be fairly visible.

I don't think this check is quite sufficient to be safe:

#define PATHOLOGICAL(X) X , X
atan2(y: 1 + PATHOLOGICAL(2) + 4); // confusing! prefer no hint

and also excludes useful cases. Consider:

#define PI 3.14
atan2(PI, PI); // want hints
ASSERT(atan2(0, 1) == 0); // want hints

We don't really want to place a macro-handling tax on every type of hint we emit, so it would be nice to handle it generically here.
But it doesn't seem trivial and I'm wary of embarking on another toHalfOpenFileRange-like mission.

Maybe a compromise:

• we pass an "anchor" location, which for param-hints will be the ( - and we bail out if the ) is in a different file
• we bail out if the anchor is in a macro body (getTopMacroCallerLoc isn't the main file)
• we convert R to a range in the anchor file by following expansion edges. This range must be precise, i.e. not cover any other parsed tokens. This means the start must always be isAtStartOfImmediateMacroExpansion etc.
• now this range is guaranteed to be usable, just take the spelling locations of the endpoints

(Doing this with *just* the sourcerange R is probably possible but the heuristics to determine the anchor fileID seem complicated)

clang-tools-extra/clangd/XRefs.h

135

XRefs.cpp has too much stuff in it today.
This patch doesn't add too much but... any reason not to put it in a new file?

clang-tools-extra/clangd/test/initialize-params.test

75

I think it's important we namespace this one for now (probably more important than the method itself!)

Note that vscode-clangd and clangd aren't version-locked

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

188

I guess/hope you don't want to handle CXXUnresolvedConstructExpr too. If so, there are cases to deal with like when we can't tell whether or not we're calling an init-list-constructor...

[nridge]

Thanks for the review!

In D98748#2661629, @sammccall wrote:

So rather than introducing another variant that's likely to be "wrong", let's use rust-analyzer's protocol as-is?
I don't think leaving out "position" is a big deal as long as we're only providing one type of hint.

I do plan on working on other hint types (especially TypeHint) as a follow-up.

But we could still leave out "position", and let the client infer before vs. after based on the hint type (this what the Rust client does).

I dug a bit into the differences based on:

• the pull request you mentioned: https://github.com/microsoft/vscode-languageserver-node/pull/609/files#
• the proposed API inside VSCode: https://github.com/microsoft/vscode/blob/main/src/vs/vscode.proposed.d.ts
• discussion on that PR: https://github.com/microsoft/vscode/pull/113285

Ranges/positions are very different! In particular, I think range has the same name+type but different semantics.
For the parameter hint:

foo(bar: 42);
The rust-analyzer protocol wants range = `foo([[42]]);`, label="bar: " position = before
The VSCode API wants range = `foo([[]]42);`, label="bar:", whitespaceAfter = true

This is a tough call because I think the rust-analyzer proposal is better (retains more info + flexibility like you say) but the VSCode one seems fairly likely to win :-/ We should provide feedback here.

Good catch. Digging into that PR a bit more, it seems like the reason for the range working in this way is that the C# language server has an "overwrite" mode for inline hints that you can enter, where the provided range is overwritten (temporarily) with the hint contents.

For example, if the hint range were [[auto]] x = 42;, in overwrite mode that would be rendered as int x = 42;.

In this model, it makes sense to make the range nonempty for some hint types (like the [[auto]] case), but not others (like parameter hints, where you wouldn't want the parameter name to replace the argument value).

So... if we're going to provide feedback here, we should probably suggest something that accommodates this use case. Perhaps an additional flag in the hint, to indicate if the "overwrite" behaviour is sensible for it? (Or perhaps the hint type is enough to infer that?)

[nridge]

Address some review comments (others remain)

clang-tools-extra/clangd/Protocol.h

1494

The motivation for the proposed placement of type hints is that in several other languages (e.g. Rust, TypeScript), you can write both let var : type = init; (where the type is explicitly specified) and let var = init; (where the type is deduced). In those languages, the natural placement of the type hint is the place where the explicit annotation would otherwise go.

For consistency / familiarity, I figured it would make sense to use the same placement for C++.

Anyways, I've removed the enumerator for type hints now, we can discuss them further in the patch that adds them.

clang-tools-extra/clangd/XRefs.cpp

1979

Good catch, thanks!

I tried to write a test for this, but TestTU doesn't seem to like #include <initializer_list> (or any other standard library include). Do you know why that might be?

2004

You make some great points here:

1. We don't seem to get here for any sort of invalid call. I guess that means we could probably replace this check with an assert, but given the complexity of C++ I always worry about edge cases...
2. For variadic functions, we currently repeat the variadic parameter name for each matched argument. That doesn't seem great, it's probably better not to show hints at all for those args?

2004

Ha, I did find a legitimate case where the Callee->getNumParams() < ArgCount condition holds: C-style varargs functions. We currently don't show any hints for those, but presumably it would be better to show hints for the non-vararg parameters.

2024

I think we also want to omit it if there's a /* foo= */ comment (whitespace and = optional).

In my mind, one of the points of inline parameter hints is that they fulfil the functions of /* foo = */ comments without you actually having to write them.

Thus, I would argue for leaving the hints there, as a signal that the comment can now be removed.

2046

Sounds like a good idea to me!

2066

#define PATHOLOGICAL(X) X , X
atan2(y: 1 + PATHOLOGICAL(2) + 4); // confusing! prefer no hint

Surely, people who write this deserve a confusing hint?

(I kid, I kid. Your macro suggestions are reasonable and I will give them a try.)

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

188

It hadn't occurred to me, but yeah, that seems like a good reason to avoid handling CXXUnresolvedConstructExpr for now.

[nridge]

Marking this as "Changes Planned" because there are more review comments I need to address.

[njames93]

clang-tools-extra/clangd/XRefs.cpp

2024

In my mind, one of the points of inline parameter hints is that they fulfil the functions of /* foo = */ comments without you actually having to write them.

Thus, I would argue for leaving the hints there, as a signal that the comment can now be removed.

I disagree with that mentality, Not everyone on a project will be using a language server/ide that supports inline hints. A push to remove those comments will harm other contributors who don't see the inline hints. It would also harm readability where the code is hosted as they wont show inline hints.

Having said that, inline hints definitely makes code easier to work on so there is still a lot of value in this feature.

[sammccall]

This is looking pretty good! Can't wait to try it, and some of my suggestions are just scope creep, so feel free to draw the line :-)

Sorry, I think phabricator has lost track of line numbers that some of the comments were attached to :-(

In D98748#2667965, @nridge wrote:

Thanks for the review!

In D98748#2661629, @sammccall wrote:

So rather than introducing another variant that's likely to be "wrong", let's use rust-analyzer's protocol as-is?
I don't think leaving out "position" is a big deal as long as we're only providing one type of hint.

I do plan on working on other hint types (especially TypeHint) as a follow-up.

But we could still leave out "position", and let the client infer before vs. after based on the hint type (this what the Rust client does).

Yeah, this would be my preference (mostly because Rust does it and it's fewer fields, not because it's actually better).

I dug a bit into the differences based on:

• the pull request you mentioned: https://github.com/microsoft/vscode-languageserver-node/pull/609/files#
• the proposed API inside VSCode: https://github.com/microsoft/vscode/blob/main/src/vs/vscode.proposed.d.ts
• discussion on that PR: https://github.com/microsoft/vscode/pull/113285

Ranges/positions are very different! In particular, I think range has the same name+type but different semantics.
For the parameter hint:

foo(bar: 42);
The rust-analyzer protocol wants range = `foo([[42]]);`, label="bar: " position = before
The VSCode API wants range = `foo([[]]42);`, label="bar:", whitespaceAfter = true

This is a tough call because I think the rust-analyzer proposal is better (retains more info + flexibility like you say) but the VSCode one seems fairly likely to win :-/ We should provide feedback here.

Good catch. Digging into that PR a bit more, it seems like the reason for the range working in this way is that the C# language server has an "overwrite" mode for inline hints that you can enter, where the provided range is overwritten (temporarily) with the hint contents.

For example, if the hint range were [[auto]] x = 42;, in overwrite mode that would be rendered as int x = 42;.

In this model, it makes sense to make the range nonempty for some hint types (like the [[auto]] case), but not others (like parameter hints, where you wouldn't want the parameter name to replace the argument value).

So... if we're going to provide feedback here, we should probably suggest something that accommodates this use case. Perhaps an additional flag in the hint, to indicate if the "overwrite" behaviour is sensible for it? (Or perhaps the hint type is enough to infer that?)

Right. There are a few interlocking points here:

• when inserting text, it's nice to be able to say what range it applies to (so it can appear "on demand") - do we have examples of editors where this interaction works well?
• it's also nice to be able to either insert or overwrite (e.g. for expanding deduced types)
• the mode/presentation is related to the hint type, but not in a way that predictably spans languages

One neat way to resolve this is to make position explicit, and have the allowed values be before/after/replace.

My other pieces of feedback would be

• whitespaceBefore/whitespaceAfter doesn't belong in the protocol and can be inferred from the context/content.
• it will be nice to eventually have good support for code actions/commands targeting these hints, as they're often for deduced info that can be explicit

(Happy to leave some version of this myself, but my "experience" is indirect :-))

clang-tools-extra/clangd/InlayHints.cpp

40 ↗

(On Diff #335210)

nit: This is *a* reason, but I think the names used in practice wouldn't be much help, either.
(Typically "A" or "LHS").

43 ↗

(On Diff #335210)

nit: you've written the skipped cases (constructor vs call) in different styles, unify?

67 ↗

(On Diff #335210)

A few more "obvious" cases to exclude: if there's exactly one parameter and the callee is (a memberexpr/declrefexpr named) "set*"?

If the callee is a copy/move constructor?

70 ↗

(On Diff #335210)

we may want to rather factor this as shouldHint(Expr *E, StringRef Hint) - I suspect we'll hit other cases where the hints are not useful and may add heuristics

81 ↗

(On Diff #335210)

consider E->IgnoreUnlessSpelledInSource() or E->IgnoreParenCasts() or one of the other (countless!) variants...

clang-tools-extra/clangd/InlayHints.h

9 ↗

(On Diff #335210)

maybe include a reference to the version being implemented, as there's likely to be more proposals soon

clang-tools-extra/clangd/XRefs.cpp

1979

Well, we'd need to include a standard library implementation in the tests (libc++ isn't part of the compiler).
initializer_list is not a built-in header, I don't know particularly why.

This appears to be enough to get init list construction to work:

namespace std { template <class> class initializer_list {}; }

2004

Yeah for varargs we should hint the fixed params, and then could choose to apply the vararg name to none/first/all of the varargs.
I'd suggest none, to be conservative - can always tweak later.

We don't seem to get here for any sort of invalid call. I guess that means we could probably replace this check with an assert, but given the complexity of C++ I always worry about edge cases

Agree it's better to just be a bit defensive here.
A simple option is just iterating up to min(actual args, num required params) and ignoring the rest.

2024

+1 to njames93 - people are using other tools (we don't have these features in review workflows... yet!)

e.g. Google has a style guide that encourages these, and having this feature in clangd won't change that. Given that people *shouldn't* remove the comment, showing the hint as well is annoying.

Eventually maybe the "overwrite" mode would be a good fit here (you can make a reasonable case for either the comment or the hint being preferable). But either way I think we need to detect them.
If detecting /*foo=*/ seems too specific, we could also bail out if we find any inline param comment...

2066

atan2(y: 1 + PATHOLOGICAL(2) + 4); // confusing! prefer no hint

Surely, people who write this deserve a confusing hint?

Yeah, on reflection this case isn't important, feel free to ignore it entirely.

Supporting idiomatic macro usage *is* useful but if the filtering/location logic gets too complicated feel free to defer with a FIXME.

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

230

maybe add a FIXME test asserting no-hints-yet for aggregate initialization?
(not something to tackle in this patch, but seems like a fairly natural extension)

[kbobyrev]

Hey, I just wanted to say thank you! This is a rust-analyzer feature I really like when writing Rust code but somehow never thought about bringing it to Clangd. This is a nice idea, huge thanks for implementing it!

[nridge]

Address most remaining review comments

I've left a couple of things as FIXMEs, as indicated

clang-tools-extra/clangd/XRefs.cpp

2024

Good points, thanks. I'd like to get to a stage where even review tools have these kinds of smarts, but we're not there yet.

I've left this as a FIXME for now (along with the setter-function case), but I'm happy to implement it in a follow-up (or in another revision of this patch if you prefer).

2059

I revised the code to avoid printName(), but using StringRef clashes with stripping leading underscores, so I kept it as std::string.

2066

I ended up implementing a simplified version of what you suggest:

• I only check the ( (or rather, with CallExpr, only the ) since that's all it seems to expose), and assume the matching one is in the same place.
• I don't worry about pathological cases where a macro overlaps an argument boundary and such.

The result does support both of your atan2 examples (which I agree are idiomatic and useful to support).

[njames93]

clang-tools-extra/clangd/ClangdLSPServer.cpp

1478

If this is compatible with rust analysers feature. Maybe we should be using their methods and feature flags spec. It will mean make life easier for clients wishing to support this before it gets baked into the official protocol.

[sammccall]

clang-tools-extra/clangd/InlayHints.cpp

44 ↗

(On Diff #336774)

apparently user-defined literals are also CallExpr (what?) so I guess you want to exclude UserDefinedLiteral here

84 ↗

(On Diff #336774)

nit: SmallVector<8> seems like a clear (minor) win here

104 ↗

(On Diff #336774)

A MemberExpr with implicit-this should also not be hinted.

We may want to be looser than this, depending how much we value regularity vs brevity when it's obvious.
e.g. omit Foo: hint in

• Bar::Foo (qualifiers present)
• foo (case difference)
• getFoo() etc

But we can certainly revisit this later.

104 ↗

(On Diff #336774)

can we also avoid printName here?

maybe change getDeclRefExpr to StringRef getSimpleExprName or so

151 ↗

(On Diff #336774)

nit: or C varargs

clang-tools-extra/clangd/XRefs.cpp

2059

using StringRef clashes with stripping leading underscores

How so? Name.ltrim('_') points to the same data as Name

[nridge]

Address remaining review comments

clang-tools-extra/clangd/ClangdLSPServer.cpp

1478

It's not 100% compatible. For example, for parameter hints, Rust expects the server to provide only the parameter name itself in label, and post-processes the labels on the client side to tack on the ": ".

We could make it 100% compatible. Do you think that would be valuable? That is, do you expect there will be cross-language client implementations of the pre-standard protocol, in places that are not a plugin shipped with a language server (like vscode-clangd or the rust-analyzer vscode plugin)?

(Would accommodating such client impls limit us from adding things like the suggested "implicit conversion hints"?)

clang-tools-extra/clangd/InlayHints.cpp

44 ↗

(On Diff #336774)

Good call, thanks!

104 ↗

(On Diff #336774)

Fixed the "MemberExpr with implicit-this" case.

As for the other suggestions: I can see the argument, but I think there's also value in simplicity ("the argument expression matches the hint exactly"), including for the purpose of not leaving the user wondering why we're not showing a hint.

151 ↗

(On Diff #336774)

As it happens, for C varargs, the notional "varargs parameter" doesn't count towards getNumParams() in the first place, so this function has (and needs) no logic for them.

clang-tools-extra/clangd/XRefs.cpp

2059

Just didn't think of that, thanks :-)

[sammccall]

Still LG

clang-tools-extra/clangd/ClangdLSPServer.cpp

1478

FWIW *I* don't think many pre-standard clients are likely or that important in the long run. Rather we should focus on proving the concept, getting a head start on the standard feature, being well-positioned to give feedback on standard proposals. So if it's not totally compatible, I like the clangd prefixes.

clang-tools-extra/clangd/InlayHints.cpp

151 ↗

(On Diff #336774)

Right - but maybe the caller doesn't know that :-)

[nridge]

Address final nit

[nridge]

Btw., thanks for providing feedback on the LSP proposal!

clang-tools-extra/clangd/ClangdLSPServer.h

private:
   void onPrepareCallHierarchy(const CallHierarchyPrepareParams &,
                               Callback<std::vector<CallHierarchyItem>>);
   void onCallHierarchyIncomingCalls(
       const CallHierarchyIncomingCallsParams &,
       Callback<std::vector<CallHierarchyIncomingCall>>);
   void onCallHierarchyOutgoingCalls(
       const CallHierarchyOutgoingCallsParams &,
       Callback<std::vector<CallHierarchyOutgoingCall>>);
+  void onInlayHints(const InlayHintsParams &, Callback<std::vector<InlayHint>>);
   void onChangeConfiguration(const DidChangeConfigurationParams &);
   void onSymbolInfo(const TextDocumentPositionParams &,
                     Callback<std::vector<SymbolDetails>>);
   void onSelectionRange(const SelectionRangeParams &,
                         Callback<std::vector<SelectionRange>>);
   void onDocumentLink(const DocumentLinkParams &,
                       Callback<std::vector<DocumentLink>>);
   void onSemanticTokens(const SemanticTokensParams &, Callback<SemanticTokens>);

clang-tools-extra/clangd/ClangdLSPServer.cpp

llvm::json::Object ServerCaps{
        }},
       {"hoverProvider", true},
       {"selectionRangeProvider", true},
       {"documentSymbolProvider", true},
       {"workspaceSymbolProvider", true},
       {"referencesProvider", true},
       {"astProvider", true}, // clangd extension
       {"typeHierarchyProvider", true},
+      {"inlayHintsProvider", true},
       {"memoryUsageProvider", true}, // clangd extension
       {"compilationDatabase",        // clangd extension
        llvm::json::Object{{"automaticReload", true}}},
       {"callHierarchyProvider", true},
   };
 
   {
     LSPBinder Binder(Handlers, *this);

 
 void ClangdLSPServer::onCallHierarchyOutgoingCalls(
     const CallHierarchyOutgoingCallsParams &Params,
     Callback<std::vector<CallHierarchyOutgoingCall>> Reply) {
   // FIXME: To be implemented.
   Reply(std::vector<CallHierarchyOutgoingCall>{});
 }
 
+void ClangdLSPServer::onInlayHints(const InlayHintsParams &Params,
+                                   Callback<std::vector<InlayHint>> Reply) {
+  Server->inlayHints(Params.textDocument.uri.file(), std::move(Reply));
+}
+
 void ClangdLSPServer::applyConfiguration(
     const ConfigurationSettings &Settings) {
   // Per-file update to the compilation database.
   llvm::StringSet<> ModifiedFiles;
   for (auto &Entry : Settings.compilationDatabaseChanges) {
     PathRef File = Entry.first;
     auto Old = CDB->getCompileCommand(File);
     auto New =
void ClangdLSPServer::bindMethods(LSPBinder &Bind,
   Bind.method("textDocument/symbolInfo", this, &ClangdLSPServer::onSymbolInfo);
   Bind.method("textDocument/typeHierarchy", this, &ClangdLSPServer::onTypeHierarchy);
   Bind.method("typeHierarchy/resolve", this, &ClangdLSPServer::onResolveTypeHierarchy);
   Bind.method("textDocument/prepareCallHierarchy", this, &ClangdLSPServer::onPrepareCallHierarchy);
   Bind.method("callHierarchy/incomingCalls", this, &ClangdLSPServer::onCallHierarchyIncomingCalls);
   Bind.method("callHierarchy/outgoingCalls", this, &ClangdLSPServer::onCallHierarchyOutgoingCalls);
   Bind.method("textDocument/selectionRange", this, &ClangdLSPServer::onSelectionRange);
   Bind.method("textDocument/documentLink", this, &ClangdLSPServer::onDocumentLink);
+  Bind.method("textDocument/inlayHints", this, &ClangdLSPServer::onInlayHints);
   Bind.method("textDocument/semanticTokens/full", this, &ClangdLSPServer::onSemanticTokens);
   Bind.method("textDocument/semanticTokens/full/delta", this, &ClangdLSPServer::onSemanticTokensDelta);
   Bind.method("$/memoryUsage", this, &ClangdLSPServer::onMemoryUsage);
   if (Opts.FoldingRanges)
     Bind.method("textDocument/foldingRange", this, &ClangdLSPServer::onFoldingRange);
   Bind.command(APPLY_FIX_COMMAND, this, &ClangdLSPServer::onCommandApplyEdit);
   Bind.command(APPLY_TWEAK_COMMAND, this, &ClangdLSPServer::onCommandApplyTweak);
 

clang-tools-extra/clangd/ClangdServer.h

public:
   /// Get information about call hierarchy for a given position.
   void prepareCallHierarchy(PathRef File, Position Pos,
                             Callback<std::vector<CallHierarchyItem>> CB);
 
   /// Resolve incoming calls for a given call hierarchy item.
   void incomingCalls(const CallHierarchyItem &Item,
                      Callback<std::vector<CallHierarchyIncomingCall>>);
 
+  /// Resolve inlay hints for a given document.
+  void inlayHints(PathRef File, Callback<std::vector<InlayHint>>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
                         Callback<std::vector<SymbolInformation>> CB);
 
   /// Retrieve the symbols within the specified file.
   void documentSymbols(StringRef File,
                        Callback<std::vector<DocumentSymbol>> CB);
 

clang-tools-extra/clangd/ClangdServer.cpp

void ClangdServer::incomingCalls(
     const CallHierarchyItem &Item,
     Callback<std::vector<CallHierarchyIncomingCall>> CB) {
   WorkScheduler->run("Incoming Calls", "",
                      [CB = std::move(CB), Item, this]() mutable {
                        CB(clangd::incomingCalls(Item, Index));
                      });
 }
 
+void ClangdServer::inlayHints(PathRef File,
+                              Callback<std::vector<InlayHint>> CB) {
+  auto Action = [File = File.str(),
+                 CB = std::move(CB)](Expected<InputsAndAST> InpAST) mutable {
+    if (!InpAST)
+      return CB(InpAST.takeError());
+    CB(clangd::inlayHints(InpAST->AST));
+  };
+  WorkScheduler->runWithAST("InlayHints", File, std::move(Action));
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
 }
 
 void ClangdServer::workspaceSymbols(
     llvm::StringRef Query, int Limit,
     Callback<std::vector<SymbolInformation>> CB) {

clang-tools-extra/clangd/Protocol.h

struct CallHierarchyOutgoingCall {
   CallHierarchyItem to;
 
   /// The range at which this item is called.
   /// This is the range relative to the caller, and not `To`.
   std::vector<Range> fromRanges;
 };
 llvm::json::Value toJSON(const CallHierarchyOutgoingCall &);
 
+/// The parameter of a `textDocument/inlayHints` request.
+struct InlayHintsParams {
+  /// The text document for which inlay hints are requested.
+  TextDocumentIdentifier textDocument;
+};
+bool fromJSON(const llvm::json::Value &, InlayHintsParams &, llvm::json::Path);
+
+/// A set of predefined hint kinds.
+enum class InlayHintKind {
+  /// The hint is for type information.
+  /// An example of a type hint is a hint in this position:
+  ///    auto var^ = expr;
+  /// which shows the deduced type of `var`.
+  TypeHint,
+
+  /// The hint corresponds to parameter information.
+  /// An example of a parameter hint is a hint in this position:
+  ///    func(^arg);
+  /// which shows the name of the corresponding parameter.
+  ParameterHint,
+
+  /// The hint corresponds to method chaining.
+  /// An example of a chaining hint is a hint in this position:
+  ///    a.foo()^
+  ///     .bar();
+  /// which shows the type of the intermediate expression `a.foo()`.
+  ChainingHint
+};
+llvm::json::Value toJSON(InlayHintKind);
+
+/// Defines possible positions of an inlay hint relevant to the token
+/// to which it applies.
+enum class InlayHintPosition {
+  /// The inlay hint should precede the token.
+  Before,
+
+  /// The inlay hint should follow the token.
+  After
+};
+llvm::json::Value toJSON(InlayHintPosition);
+
+/// An annotation to be displayed inline next to a source code token.
+struct InlayHint {
+  /// The source range of the token to which the hint applies.
+  /// We provide the entire range, rather than just the endpoint
+  /// relevant to `position` (e.g. the start of the range for
+  /// InlayHintPosition::Before), to give clients the flexibility
+  /// to make choices like only displaying the hint while the cursor
+  /// is over the token, rather than displaying it all the time.
+  Range range;
+
+  /// The position of the hint relative to the token.
+  InlayHintPosition position;
+
+  /// The type of hint.
+  InlayHintKind kind;
+
+  /// The label that is displayed in the editor.
+  std::string label;
+};
+llvm::json::Value toJSON(const InlayHint &);
+
 struct ReferenceContext {
   /// Include the declaration of the current symbol.
   bool includeDeclaration = false;
 };
 
 struct ReferenceParams : public TextDocumentPositionParams {
   ReferenceContext context;
 };

clang-tools-extra/clangd/Protocol.cpp

bool fromJSON(const llvm::json::Value &Params,
   llvm::json::ObjectMapper O(Params, P);
   return O.map("item", C.item);
 }
 
 llvm::json::Value toJSON(const CallHierarchyOutgoingCall &C) {
   return llvm::json::Object{{"to", C.to}, {"fromRanges", C.fromRanges}};
 }
 
+bool fromJSON(const llvm::json::Value &Params, InlayHintsParams &R,
+              llvm::json::Path P) {
+  llvm::json::ObjectMapper O(Params, P);
+  return O && O.map("textDocument", R.textDocument);
+}
+
+llvm::json::Value toJSON(InlayHintKind K) {
+  switch (K) {
+  case InlayHintKind::ParameterHint:
+    return "parameter";
+  case InlayHintKind::TypeHint:
+    return "type";
+  case InlayHintKind::ChainingHint:
+    return "chaining";
+  }
+  llvm_unreachable("Unknown clang.clangd.InlayHintKind");
+}
+
+llvm::json::Value toJSON(InlayHintPosition P) {
+  return llvm::json::Value(static_cast<int>(P));
+}
+
+llvm::json::Value toJSON(const InlayHint &H) {
+  return llvm::json::Object{{"range", H.range},
+                            {"position", H.position},
+                            {"kind", H.kind},
+                            {"label", H.label}};
+}
+
 static const char *toString(OffsetEncoding OE) {
   switch (OE) {
   case OffsetEncoding::UTF8:
     return "utf-8";
   case OffsetEncoding::UTF16:
     return "utf-16";
   case OffsetEncoding::UTF32:
     return "utf-32";

clang-tools-extra/clangd/XRefs.h

 
 /// Get call hierarchy information at \p Pos.
 std::vector<CallHierarchyItem>
 prepareCallHierarchy(ParsedAST &AST, Position Pos, PathRef TUPath);
 
 std::vector<CallHierarchyIncomingCall>
 incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index);
 
+std::vector<InlayHint> inlayHints(ParsedAST &AST);
+
 /// Returns all decls that are referenced in the \p FD except local symbols.
 llvm::DenseSet<const Decl *> getNonLocalDeclRefs(ParsedAST &AST,
                                                  const FunctionDecl *FD);
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_XREFS_H

clang-tools-extra/clangd/XRefs.cpp

 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Attrs.inc"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
   // Sort results by name of container.
   llvm::sort(Results, [](const CallHierarchyIncomingCall &A,
                          const CallHierarchyIncomingCall &B) {
     return A.from.name < B.from.name;
   });
   return Results;
 }
 
+static bool isAnonymous(const DeclarationName &N) {
+  return N.isIdentifier() && !N.getAsIdentifierInfo();
+}
+
+class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
+public:
+  InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST)
+      : Results(Results), AST(AST.getASTContext()) {}
+
+  bool VisitCXXConstructExpr(CXXConstructExpr *E) {
+    // Weed out implicit constructor calls by checking the validity of
+    // getParenOrBraceRange(). (FIXME: Is there a better way to check this?)
+    if (E->getParenOrBraceRange().isValid()) {
+      processCall(E->getConstructor(), E->getArgs(), E->getNumArgs());
+    }
+    return true;
+  }
+
+  bool VisitCallExpr(CallExpr *E) {
+    // Do not show parameter hints for operator calls written using operator
+    // syntax. The resulting hints can look awkard (e.g. the operator expression
+    // can itself be a function argument and then we'd get two hints side by
+    // side).
+    if (isa<CXXOperatorCallExpr>(E))
+      return true;
+
+    processCall(dyn_cast_or_null<FunctionDecl>(E->getCalleeDecl()),
+                E->getArgs(), E->getNumArgs());
+    return true;
+  }
+
+private:
+  void processCall(const FunctionDecl *Callee, const Expr *const *Args,
+                   size_t ArgCount) {
+    if (ArgCount == 0 || !Callee)
+      return;
+
+    if (Callee->getNumParams() < ArgCount)
+      return;
+
+    std::vector<std::string> ParameterNames =
+        getParameterNames(Callee, ArgCount);
+
+    // If all parameter names are empty, see if the function's definition is
+    // present in the translation unit, maybe it has names.
+    if (llvm::all_of(ParameterNames, std::mem_fn(&std::string::empty))) {
+      if (const FunctionDecl *Def = Callee->getDefinition()) {
+        ParameterNames = getParameterNames(Def, ArgCount);
+      }
+    }
+    assert(ParameterNames.size() == ArgCount);
+    for (size_t I = 0; I < ArgCount; ++I) {
+      std::string Name = ParameterNames[I];
+      if (Name.empty())
+        continue;
+
+      // If the argument expression is a single name and it matches the
+      // parameter name exactly, omit the hint.
+      if (auto *DRE = getDeclRefExpr(Args[I])) {
+        if (!DRE->getQualifier() && Name == printName(AST, *DRE->getDecl()))
+          continue;
+      }
+
+      addInlayHint(Args[I]->getSourceRange(), InlayHintPosition::Before,
+                   InlayHintKind::ParameterHint, Name + ": ");
+    }
+  }
+
+  static const DeclRefExpr *getDeclRefExpr(const Expr *E) {
+    // FIXME: Are there other implicit wrappers we should handle?
+    if (auto *ICE = dyn_cast<ImplicitCastExpr>(E)) {
+      E = ICE->getSubExpr();
+    }
+    if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+      return DRE;
+    }
+    return nullptr;
+  }
+
+  std::vector<std::string> getParameterNames(const FunctionDecl *Function,
+                                             size_t ArgCount) {
+    std::vector<std::string> Result;
+    for (size_t I = 0; I < ArgCount; ++I) {
+      const ParmVarDecl *Parm = Function->getParamDecl(I);
+      assert(Parm);
+
+      // Rather than showing "(anonymous)", just don't show a hint.
+      if (isAnonymous(Parm->getDeclName())) {
+        Result.emplace_back();
+        continue;
+      }
+
+      Result.emplace_back(printName(AST, *Parm));
+    }
+    return Result;
+  }
+
+  void addInlayHint(SourceRange R, InlayHintPosition Pos, InlayHintKind Kind,
+                    llvm::StringRef Label) {
+    // Do not annotate calls in macro expansions.
+    if (!R.getBegin().isFileID())
+      return;
+    auto FileRange =
+        toHalfOpenFileRange(AST.getSourceManager(), AST.getLangOpts(), R);
+    if (!FileRange)
+      return;
+    Results.push_back(InlayHint{
+        Range{
+            sourceLocToPosition(AST.getSourceManager(), FileRange->getBegin()),
+            sourceLocToPosition(AST.getSourceManager(), FileRange->getEnd())},
+        Pos, Kind, Label.str()});
+  }
+
+  std::vector<InlayHint> &Results;
+  ASTContext &AST;
+};
+
+std::vector<InlayHint> inlayHints(ParsedAST &AST) {
+  std::vector<InlayHint> Results;
+  InlayHintVisitor Visitor(Results, AST);
+  Visitor.TraverseAST(AST.getASTContext());
+  return Results;
+}
+
 llvm::DenseSet<const Decl *> getNonLocalDeclRefs(ParsedAST &AST,
                                                  const FunctionDecl *FD) {
   if (!FD->hasBody())
     return {};
   llvm::DenseSet<const Decl *> DeclRefs;
   findExplicitReferences(
       FD,
       [&](ReferenceLoc Ref) {
         for (const Decl *D : Ref.Targets) {
           if (!index::isFunctionLocalSymbol(D) && !D->isTemplateParameter() &&
               !Ref.IsDecl)
             DeclRefs.insert(D);
         }
       },
       AST.getHeuristicResolver());
   return DeclRefs;
 }
 } // namespace clangd
-} // namespace clang
+} // namespace clang
 No newline at end of file

clang-tools-extra/clangd/test/initialize-params.test

 # CHECK-NEXT:      "executeCommandProvider": {
 # CHECK-NEXT:        "commands": [
 # CHECK-NEXT:          "clangd.applyFix",
 # CHECK-NEXT:          "clangd.applyTweak"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "hoverProvider": true,
 # CHECK-NEXT:      "implementationProvider": true,
+# CHECK-NEXT:      "inlayHintsProvider": true,
 # CHECK-NEXT:      "memoryUsageProvider": true,
 # CHECK-NEXT:      "referencesProvider": true,
 # CHECK-NEXT:      "renameProvider": true,
 # CHECK-NEXT:      "selectionRangeProvider": true,
 # CHECK-NEXT:      "semanticTokensProvider": {
 # CHECK-NEXT:        "full": {
 # CHECK-NEXT:          "delta": true
 # CHECK-NEXT:        },

clang-tools-extra/clangd/unittests/CMakeLists.txt

add_unittest(ClangdUnitTests ClangdTests
   FSTests.cpp
   FuzzyMatchTests.cpp
   GlobalCompilationDatabaseTests.cpp
   HeadersTests.cpp
   HeaderSourceSwitchTests.cpp
   HoverTests.cpp
   IndexActionTests.cpp
   IndexTests.cpp
+  InlayHintTests.cpp
   JSONTransportTests.cpp
   LoggerTests.cpp
   LSPBinderTests.cpp
   LSPClient.cpp
   ModulesTests.cpp
   ParsedASTTests.cpp
   PathMappingTests.cpp
   PreambleTests.cpp

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

+//===-- InlayHintTests.cpp  -------------------------------*- C++ -*-------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#include "Annotations.h"
+#include "Protocol.h"
+#include "TestTU.h"
+#include "XRefs.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using ::testing::UnorderedElementsAre;
+
+std::vector<InlayHint> parameterHints(ParsedAST &AST) {
+  std::vector<InlayHint> Result;
+  for (auto &Hint : inlayHints(AST)) {
+    if (Hint.kind == InlayHintKind::ParameterHint)
+      Result.push_back(Hint);
+  }
+  return Result;
+}
+
+struct ExpectedHint {
+  std::string Label;
+  std::string RangeName;
+};
+
+MATCHER_P2(HintMatcher, Expected, Code, "") {
+  return arg.label == Expected.Label &&
+         arg.range == Code.range(Expected.RangeName);
+}
+
+template <typename... ExpectedHints>
+void assertParameterHints(llvm::StringRef AnnotatedSource,
+                          ExpectedHints... Expected) {
+  Annotations Source(AnnotatedSource);
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+
+  EXPECT_THAT(parameterHints(AST),
+              UnorderedElementsAre(HintMatcher(Expected, Source)...));
+}
+
+TEST(ParameterHints, Smoke) {
+  assertParameterHints(R"cpp(
+    void foo(int param);
+    void bar() {
+      foo($param[[42]]);
+    }
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+}
+
+TEST(ParameterHints, NoName) {
+  // No hint for anonymous parameter.
+  assertParameterHints(R"cpp(
+    void foo(int);
+    void bar() {
+      foo(42);
+    }
+  )cpp");
+}
+
+TEST(ParameterHints, NameInDefinition) {
+  // Parameter name picked up from definition if necessary.
+  assertParameterHints(R"cpp(
+    void foo(int);
+    void bar() {
+      foo($param[[42]]);
+    }
+    void foo(int param) {};
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+}
+
+TEST(ParameterHints, NameMismatch) {
+  // Prefer name from declaration.
+  assertParameterHints(R"cpp(
+    void foo(int good);
+    void bar() {
+      foo($good[[42]]);
+    }
+    void foo(int bad) {};
+  )cpp",
+                       ExpectedHint{"good: ", "good"});
+}
+
+TEST(ParameterHints, Operator) {
+  // No hint for operator call with operator syntax.
+  assertParameterHints(R"cpp(
+    struct S {};
+    void operator+(S lhs, S rhs);
+    void bar() {
+      S a, b;
+      a + b;
+    }
+  )cpp");
+}
+
+TEST(ParameterHints, Macro) {
+  // No hint for call in macro expansion.
+  assertParameterHints(R"cpp(
+    void foo(int param);
+    #define ExpandsToCall() foo(42)
+    void bar() {
+      ExpandsToCall();
+    }
+  )cpp");
+}
+
+TEST(ParameterHints, ConstructorParens) {
+  assertParameterHints(R"cpp(
+    struct S {
+      S(int param);
+    };
+    void bar() {
+      S obj($param[[42]]);
+    }
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+}
+
+TEST(ParameterHints, ConstructorBraces) {
+  assertParameterHints(R"cpp(
+    struct S {
+      S(int param);
+    };
+    void bar() {
+      S obj{$param[[42]]};
+    }
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+}
+
+TEST(ParameterHints, MemberInit) {
+  assertParameterHints(R"cpp(
+    struct S {
+      S(int param);
+    };
+    struct T {
+      S member;
+      T() : member($param[[42]]) {}
+    };
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+}
+
+TEST(ParameterHints, ImplicitConstructor) {
+  assertParameterHints(R"cpp(
+    struct S {
+      S(int param);
+    };
+    void bar(S);
+    S foo() {
+      // Do not show hint for implicit constructor call in argument.
+      bar(42);
+      // Do not show hint for implicit constructor call in return.
+      return 42;
+    }
+  )cpp");
+}
+
+TEST(ParameterHints, ArgMatchesParam) {
+  assertParameterHints(R"cpp(
+    void foo(int param);
+    struct S {
+      static const int param = 42;
+    };
+    void bar() {
+      int param = 42;
+      // Do not show redundant "param: param".
+      foo(param);
+      // But show it if the argument is qualified.
+      foo($param[[S::param]]);
+    }
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+}
+
+TEST(ParameterHints, DependentCall) {
+  // FIXME: This doesn't currently produce a hint but should.
+  assertParameterHints(R"cpp(
+    template <typename T>
+    void foo(T param);
+
+    template <typename T>
+    struct S {
+      void bar(T par) {
+        foo($param[[par]]);
+      }
+    };
+  )cpp");
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang