[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 ↗

(On Diff #333778)

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

76

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

189

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

189

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

41

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

44

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

68

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?

71

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

82

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

clang-tools-extra/clangd/InlayHints.h

10

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

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

84

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

104

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

can we also avoid printName here?

maybe change getDeclRefExpr to StringRef getSimpleExprName or so

151

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

Good call, thanks!

104

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

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

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/CMakeLists.txt

add_clang_library(clangDaemon
   FS.cpp
   FuzzyMatch.cpp
   GlobalCompilationDatabase.cpp
   Headers.cpp
   HeaderSourceSwitch.cpp
   HeuristicResolver.cpp
   Hover.cpp
   IncludeFixer.cpp
+  InlayHints.cpp
   JSONTransport.cpp
   PathMapping.cpp
   Protocol.cpp
   Quality.cpp
   ParsedAST.cpp
   Preamble.cpp
   RIFF.cpp
   Selection.cpp

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},
+      {"clangdInlayHintsProvider", 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/semanticTokens/full", this, &ClangdLSPServer::onSemanticTokens);
   Bind.method("textDocument/semanticTokens/full/delta", this, &ClangdLSPServer::onSemanticTokensDelta);
+  Bind.method("clangd/inlayHints", this, &ClangdLSPServer::onInlayHints);
   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);
 
   ApplyWorkspaceEdit = Bind.outgoingMethod("workspace/applyEdit");
   PublishDiagnostics = Bind.outgoingNotification("textDocument/publishDiagnostics");

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

 #include "CodeComplete.h"
 #include "Config.h"
 #include "Diagnostics.h"
 #include "DumpAST.h"
 #include "FindSymbols.h"
 #include "Format.h"
 #include "HeaderSourceSwitch.h"
 #include "Headers.h"
+#include "InlayHints.h"
 #include "ParsedAST.h"
 #include "Preamble.h"
 #include "Protocol.h"
 #include "SemanticHighlighting.h"
 #include "SemanticSelection.h"
 #include "SourceCode.h"
 #include "TUScheduler.h"
 #include "XRefs.h"
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/InlayHints.h

+//===--- InlayHints.h --------------------------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+//
+// Support for the proposed "inlay hints" LSP feature.
+// The version currently implemented is the one proposed here:
+// https://github.com/microsoft/vscode-languageserver-node/pull/609/.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INLAY_HINTS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INLAY_HINTS_H
+
+#include "Protocol.h"
+#include <vector>
+
+namespace clang {
+namespace clangd {
+class ParsedAST;
+
+// Compute and return all inlay hints for a file.
+std::vector<InlayHint> inlayHints(ParsedAST &AST);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INLAY_HINTS_H

clang-tools-extra/clangd/InlayHints.cpp

+//===--- InlayHints.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 "InlayHints.h"
+#include "ParsedAST.h"
+#include "support/Logger.h"
+#include "clang/AST/DeclarationName.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/SourceManager.h"
+
+namespace clang {
+namespace clangd {
+
+class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
+public:
+  InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST)
+      : Results(Results), AST(AST.getASTContext()) {}
+
+  bool VisitCXXConstructExpr(CXXConstructExpr *E) {
+    // Weed out constructor calls that don't look like a function call with
+    // an argument list, by checking the validity of getParenOrBraceRange().
+    // Also weed out std::initializer_list constructors as there are no names
+    // for the individual arguments.
+    if (!E->getParenOrBraceRange().isValid() ||
+        E->isStdInitListInitialization()) {
+      return true;
+    }
+
+    processCall(E->getParenOrBraceRange().getBegin(), E->getConstructor(),
+                {E->getArgs(), E->getNumArgs()});
+    return true;
+  }
+
+  bool VisitCallExpr(CallExpr *E) {
+    // Do not show parameter hints for operator calls written using operator
+    // syntax. (Among other reasons, 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(E->getRParenLoc(),
+                dyn_cast_or_null<FunctionDecl>(E->getCalleeDecl()),
+                {E->getArgs(), E->getNumArgs()});
+    return true;
+  }
+
+  // FIXME: Handle RecoveryExpr to try to hint some invalid calls.
+
+private:
+  // The purpose of Anchor is to deal with macros. It should be the call's
+  // opening or closing parenthesis or brace. (Always using the opening would
+  // make more sense but CallExpr only exposes the closing.) We heuristically
+  // assume that if this location does not come from a macro definition, then
+  // the entire argument list likely appears in the main file and can be hinted.
+  void processCall(SourceLocation Anchor, const FunctionDecl *Callee,
+                   llvm::ArrayRef<const Expr *const> Args) {
+    if (Args.size() == 0 || !Callee)
+      return;
+
+    // If the anchor location comes from a macro defintion, there's nowhere to
+    // put hints.
+    if (!AST.getSourceManager().getTopMacroCallerLoc(Anchor).isFileID())
+      return;
+
+    // The parameter name of a move or copy constructor is not very interesting.
+    if (auto *Ctor = dyn_cast<CXXConstructorDecl>(Callee))
+      if (Ctor->isCopyOrMoveConstructor())
+        return;
+
+    // FIXME: Exclude setters (i.e. functions with one argument whose name
+    // begins with "set"), as their parameter name is also not likely to be
+    // interesting.
+
+    // Don't show hints for variadic parameters.
+    size_t FixedParamCount = getFixedParamCount(Callee);
+    size_t ArgCount = std::min(FixedParamCount, Args.size());
+
+    std::vector<std::string> ParameterNames =
+        chooseParameterNames(Callee, ArgCount);
+
+    for (size_t I = 0; I < ArgCount; ++I) {
+      std::string Name = ParameterNames[I];
+      if (!shouldHint(Args[I], Name))
+        continue;
+
+      addInlayHint(Args[I]->getSourceRange(), InlayHintKind::ParameterHint,
+                   Name + ": ");
+    }
+  }
+
+  bool shouldHint(const Expr *Arg, StringRef ParamName) {
+    if (ParamName.empty())
+      return false;
+
+    // If the argument expression is a single name and it matches the
+    // parameter name exactly, omit the hint.
+    if (auto *DRE = getDeclRefExpr(Arg)) {
+      if (!DRE->getQualifier() && ParamName == printName(AST, *DRE->getDecl()))
+        return false;
+    }
+
+    // FIXME: Exclude argument expressions preceded by a /*paramName=*/ comment.
+
+    return true;
+  }
+
+  static const DeclRefExpr *getDeclRefExpr(const Expr *E) {
+    return dyn_cast<DeclRefExpr>(E->IgnoreUnlessSpelledInSource());
+  }
+
+  std::vector<std::string> chooseParameterNames(const FunctionDecl *Callee,
+                                                size_t ArgCount) {
+    // The current strategy here is to use all the parameter names from the
+    // canonical declaration, unless they're all empty, in which case we
+    // use all the parameter names from the definition (in present in the
+    // translation unit).
+    // We could try a bit harder, e.g.:
+    //   - try all re-declarations, not just canonical + definition
+    //   - fall back arg-by-arg rather than wholesale
+
+    std::vector<std::string> ParameterNames =
+        getParameterNamesForDecl(Callee, ArgCount);
+
+    if (llvm::all_of(ParameterNames, std::mem_fn(&std::string::empty))) {
+      if (const FunctionDecl *Def = Callee->getDefinition()) {
+        ParameterNames = getParameterNamesForDecl(Def, ArgCount);
+      }
+    }
+    assert(ParameterNames.size() == ArgCount);
+
+    // Standard library functions often have parameter names that start
+    // with underscores, which makes the hints noisy, so strip them out.
+    for (auto &Name : ParameterNames)
+      stripLeadingUnderscores(Name);
+
+    return ParameterNames;
+  }
+
+  static void stripLeadingUnderscores(std::string &Name) {
+    while (!Name.empty() && Name[0] == '_')
+      Name = Name.substr(1);
+  }
+
+  // Return the number of parameters Function has, not counting parameters
+  // instantiated from a parameter pack.
+  static size_t getFixedParamCount(const FunctionDecl *Function) {
+    if (FunctionTemplateDecl *Template = Function->getPrimaryTemplate()) {
+      FunctionDecl *F = Template->getTemplatedDecl();
+      size_t Result = 0;
+      for (ParmVarDecl *Parm : F->parameters()) {
+        if (Parm->isParameterPack()) {
+          break;
+        }
+        ++Result;
+      }
+      return Result;
+    }
+    return Function->getNumParams();
+  }
+
+  std::vector<std::string>
+  getParameterNamesForDecl(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);
+
+      if (IdentifierInfo *Ident = Parm->getDeclName().getAsIdentifierInfo()) {
+        Result.emplace_back(Ident->getName());
+      } else {
+        Result.emplace_back();
+      }
+    }
+    return Result;
+  }
+
+  void addInlayHint(SourceRange R, InlayHintKind Kind, llvm::StringRef Label) {
+    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())},
+        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;
+}
+
+} // namespace clangd
+} // namespace clang

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 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,
+
+  /// Other ideas for hints that are not currently implemented:
+  ///
+  /// * Type hints, showing deduced types.
+  /// * Chaining hints, showing the types of intermediate expressions
+  ///   in a chain of function calls.
+  /// * Hints indicating implicit conversions or implicit constructor calls.
+};
+llvm::json::Value toJSON(InlayHintKind);
+
+/// An annotation to be displayed inline next to a range of source code.
+struct InlayHint {
+  /// The range of source code 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 range, rather than displaying it all the time.
+  Range range;
+
+  /// 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;
 };
 bool fromJSON(const llvm::json::Value &, ReferenceParams &, llvm::json::Path);
 

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";
+  }
+  llvm_unreachable("Unknown clang.clangd.InlayHintKind");
+}
+
+llvm::json::Value toJSON(const InlayHint &H) {
+  return llvm::json::Object{
+      {"range", H.range}, {"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.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"
llvm::DenseSet<const Decl *> getNonLocalDeclRefs(ParsedAST &AST,
   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

 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
 # Test initialize request parameters with rootUri
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootUri":"test:///workspace","capabilities":{},"trace":"off"}}
 #      CHECK:  "id": 0,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:    "capabilities": {
 # CHECK-NEXT:      "astProvider": true,
 # CHECK-NEXT:      "callHierarchyProvider": true,
+# CHECK-NEXT:      "clangdInlayHintsProvider": true,
 # CHECK-NEXT:      "codeActionProvider": true,
 # CHECK-NEXT:      "compilationDatabase": {
 # CHECK-NEXT:        "automaticReload": true
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "completionProvider": {
 # CHECK-NEXT:        "allCommitCharacters": [
 # CHECK-NEXT:          " ",
 # CHECK-NEXT:          "\t",

 # 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:      "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:        },
 # CHECK-NEXT:        "legend": {

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 "InlayHints.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());
+  TU.ExtraArgs.push_back("-std=c++11");
+  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, Macros) {
+  // Handling of macros depends on where the call's argument list comes from.
+
+  // If it comes from a macro definition, there's nothing to hint
+  // at the invocation site.
+  assertParameterHints(R"cpp(
+    void foo(int param);
+    #define ExpandsToCall() foo(42)
+    void bar() {
+      ExpandsToCall();
+    }
+  )cpp");
+
+  // The argument expression being a macro invocation shouldn't interfere
+  // with hinting.
+  assertParameterHints(R"cpp(
+    #define PI 3.14
+    void foo(double param);
+    void bar() {
+      foo($param[[PI]]);
+    }
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+
+  // If the whole argument list comes from a macro parameter, hint it.
+  assertParameterHints(R"cpp(
+    void abort();
+    #define ASSERT(expr) if (!expr) abort()
+    int foo(int param);
+    void bar() {
+      ASSERT(foo($param[[42]]) == 0);
+    }
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+}
+
+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, ConstructorStdInitList) {
+  // Do not show hints for std::initializer_list constructors.
+  assertParameterHints(R"cpp(
+    namespace std {
+      template <typename> class initializer_list {};
+    }
+    struct S {
+      S(std::initializer_list<int> param);
+    };
+    void bar() {
+      S obj{42, 43};
+    }
+  )cpp");
+}
+
+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, LeadingUnderscore) {
+  assertParameterHints(R"cpp(
+    void foo(int p1, int _p2, int __p3);
+    void bar() {
+      foo($p1[[41]], $p2[[42]], $p3[[43]]);
+    }
+  )cpp",
+                       ExpectedHint{"p1: ", "p1"}, ExpectedHint{"p2: ", "p2"},
+                       ExpectedHint{"p3: ", "p3"});
+}
+
+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");
+}
+
+TEST(ParameterHints, VariadicFunction) {
+  assertParameterHints(R"cpp(
+    template <typename... T>
+    void foo(int fixed, T... variadic);
+
+    void bar() {
+      foo($fixed[[41]], 42, 43);
+    }
+  )cpp",
+                       ExpectedHint{"fixed: ", "fixed"});
+}
+
+TEST(ParameterHints, VarargsFunction) {
+  assertParameterHints(R"cpp(
+    void foo(int fixed, ...);
+
+    void bar() { 
+      foo($fixed[[41]], 42, 43);
+    }
+  )cpp",
+                       ExpectedHint{"fixed: ", "fixed"});
+}
+
+TEST(ParameterHints, CopyOrMoveConstructor) {
+  // Do not show hint for parameter of copy or move constructor.
+  assertParameterHints(R"cpp(
+    struct S {
+      S();
+      S(const S& other);
+      S(S&& other);
+    };
+    void bar() {
+      S a;
+      S b(a);    // copy
+      S c(S());  // move
+    }
+  )cpp");
+}
+
+TEST(ParameterHints, AggregateInit) {
+  // FIXME: This is not implemented yet, but it would be a natural
+  // extension to show member names as hints here.
+  assertParameterHints(R"cpp(
+    struct Point {
+      int x;
+      int y;
+    };
+    void bar() {
+      Point p{41, 42};
+    }
+  )cpp");
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang