[clangd] Improve hover on arguments to function call

[adamcz]

Summary

In cases like:

foo(a, ^b);

We now additionally show the name and type of the parameter to foo that
corresponds that "b" is passed as.

The name should help with understanding what it's used for and type can
be useful to find out if call to foo() can mutate variable "b" or not
(i.e. if it is pass by value, reference, const reference, etc).

[kadircet]

thanks! that looks really useful, especially knowing when a parameter is being passed as a ref/val.

clang-tools-extra/clangd/Hover.cpp

292

why not return a HoverInfo::Param instead ?

520

umm, is this a fixme or a leftover? I suppose it is from a previous version of the patch.

775

nit: move into anonymous namespace

777

nit: extract N->outerImplicit() to a variable, it seems to be repeated along.

790

i don't think this is necessary, we are baling out if FD->getNumParams() <= I anyways.

793

nit: put this before arg index finding logic and do an early exit:

if(!CE) return;
auto *FD = CE->getDirectCallee();
if(!FD || FD->isOverloaded || variadic..) return;

after doing this you can also change the arg index finding logic to:

auto *SelectedArg = N->outerImplicit().ASTNode.get<Expr>();
if(!SelectedParam) return; // you can also move this above other exit conditions.
for (unsigned I = 0; I < min(CE->getNumArgs, FD->getNumParams())...) { // I suppose these two can be different in presence of default params ?
  if (CE->getArg(I) == SelectedArg) {
      // do magic;
      break;
  }
}
``

794

i can see the reason for variadics, but why bail out on overloaded operators? also can we have some tests for these two?

833

this place is getting crowded. maybe we should start passing N into getHoverContents and encapsulate the logic in there or something. no action needed, just thinking out loud.

957

let's put this after Size and before Documentation we've been keeping the documentation and definition as kind of a footer for a while now.
that would also drop the need for a ruler.

961

i bet there will always be some people getting confused no matter what we say in here (especially since we are not just printing the type, and I totally find printing param name useful especially in case of literals). But to align with other types of information we provide maybe say:

Substitutes: ?

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

730

can you also add a test for a literal ?

2200

i find showing the default value a little bit confusing, as user is providing a different value. i can also see some value in it though, so up to you.

[adamcz]

addressed review comments, some UI questions remain open for now.

[adamcz]

clang-tools-extra/clangd/Hover.cpp

520

Yep, previous version, sorry about that.

790

It's possible for FD->getNumParams() to be higher than CE->getNumArgs(), for example due to default arguments. If we couldn't find selected node inside CallExpr, we shouldn't be trying to match it to some "random" argument in FunctionDecl.

It's a bit irrelevant now, after addressing other review comments ;-)

793

Yea, that's nicer, thanks.

(I opted for "continue" rather than "break" to reduce indentation).

794

I could totally see us supporting variadic, I'm just not sure how useful that will be, so I didn't do it right away.

As for the operators, I don't think we should trigger on those. There are two cases:

• things that do not look like a function call: operator+, operator<<, etc. When you hover over a variable it's not immediately obvious what the "passed to" would be referring to. Something like:

foo(a+b);
if I see "a passed as const int&" I might think this is about passing it to foo() (at least of first glance).
At the same time, the value of this is very low for operators. You know what their signature is already.

-operator() - this is much more like a function call. Maybe it would make sense to support it. It gets tricky with matching CallExpr args to FD params though. I'll leave a todo for now and worry about this in a separate change, if don't mind.jj

961

I am very much open to suggestions for phrasing, I'm not too happy about "Passed as".

However, I am not a fan of Substitutes. If I saw that, I would have no idea what it's referring to. Since there is no visible connection between this hover card and the function call, without knowing what I'm looking at I would not make this connection.

Might be just me. I am really as far from being a UI expert as possible. I'll ask around to see if people would get the Substitute: and get back to you on this one.

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

730

Currently there is no hover at all for literals. The reason was that it wouldn't be useful. This information, however, might be, especially in cases like:
printThis(that, true, false, true);
knowing what these refer to could be useful.

Do you think this is good enough reason to enable hover on literals, with just this information? I'm thinking yes, but I'm not sure.

2200

I did that for consistency with how we format function arguments. IMHO that's a good choice, but, as I already mentioned, I have no UI skills at all, so if you or someone else feels strongly about this, I won't argue.

[kadircet]

clang-tools-extra/clangd/Hover.cpp

689

nit: this is already checked before entering the function.

794

ok makes sense, you are definitely right about nested case, I wasn't thinking about it.

915

I know you are planning to make more changes on how we render this, but let's not forget to drop the ruler in here.

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

730

you are right, i remember carving the codepath for handling literals but forgot that we've disabled them at the time because there wasn't much info we could surface. I believe this one is a pretty good candidate (assuming LSP doesn't get "inline parameter annotations").

But definitely doesn't need to be handled in this patch, feel free to update the fixme on getHoverContents(Expr ...) though, saying we might want to surface callee information.

[adamcz]

Formatting changed to spell out [const] reference and hide type unless conversion happens.
Defining what the "correct" behavior is on various cases is very difficult.
This is my best shot at incorporating previous requests and comments.

Also addressed more review comments.

[kadircet]

thanks a lot! haven't checked the tests yet.

clang-tools-extra/clangd/Hover.cpp

707

nit: Arg seems to be used only once, consider inlining.

712

nit: braces are usually omitted when "then" part has a single expression. (applies to multiple other places in the patch)

717

nit: you can drop hasValue, we mostly make use of the implicit bool conversion for checking optionals.

726

isn't this same as PVD->getType().isConstQualified()? maybe set it in there?

I think it is more explicit that way, because in theory the Expr above is always the DeclRefExpr referencing the current parameter, which is quite subtle to figure out while looking at this. Or am I missing something?

751

*sigh* wish we didn't have more than 60 types of implicit casts in c++.

I can see how it is hard to list everything in here and even if you did it would still be really hard to reason about them.
I can't help myself but wonder though, was there a reasoning when choosing what to leave out? Because that would help me a lot, and possibly future readers. (I guess I am also fine with "the list looked comprehensive enough", as we can always iterate over)

I am a little bit hesitant about the fail policy though(both in here and in other branches), as I can't prove to myself that being passed as a reference would be false in all the other cases for example. And showing incorrect information in here could really end up confusing the user. Unless there's a clear reasoning behind this failure mode, I would suggest dropping CalleeArgInfo completely to be on the safe side. That way we can improve the handling for important cases later on, without surfacing possibly wrong results. WDYT?

clang-tools-extra/clangd/Hover.h

86

why is this true by default ?

87

s/false/const/ ?

what would you say to having an enum instead ?

enum PassBy {
  Value,
   Ref,
   ConstRef,
};

[adamcz]

Addressed review comments, also added some "const" here and there.

[adamcz]

clang-tools-extra/clangd/Hover.cpp

712

Ah, yes, that. I keep doing that because I am personally strongly opposed to this convention. I respect the rules, of course, and avoid {} in those cases, but I think my personal feelings about it make it hard for me to avoid mistakes like this ;-). Sorry.

Fixed.

726

It's different. Consider:
class C {
public:

C(int &x) {}

};
void foo(const C &c) {}

int y;
foo(y);

PVD is const-qualified (and it's type is const C&), but y is int and passed by non-const reference to the C c-tor, which might mutate y, even though the resulting instance of C will be const.

751

I basically went through the list of all CK_ values and looked for ones that preserve the ref bit. There aren't many. Basically any type conversion other than casting to base must drop the ref bit (because it's a different type then) and then there's few that are explicitly about copying data. So in the end the default is to drop the reference bit and there's only a handful that preserve it (DerivedToBase, Decay and NoOp, basically).

I do agree with you that this is fragile, likely to be slightly wrong and too complicated. I can't think of a better solution though.

I prefer to not drop the whole CalleeArgInfo / hover thing in those cases because, even if you ignore the whole mutable/non-mutable bit of information, I still find the name of the argument it's passed as to be very valuable).

I like the idea of now showing the ref/const-ref/value bit in cases where we fear it might not be correct, but I do not know how to display this to the user without confusing them (i.e. if there's no mention of ref/const-ref, does that mean it's by-value or that we couldn't tell?). Note that I intentionally avoided showing the words "by value" to the user, since it will confuse everyone when used on pointers.

This is essentially why I was hesitant to extract the reference bit and why my initial change had only the type, to let user figure it out themselves. It's going to be quite hard to get this right.

clang-tools-extra/clangd/Hover.h

86

Passing by reference is the default. It takes extra nodes in the AST to turn it to pass by value. Basically the defaults are what we see on the very inner node and then we adjust this as we go up the tree. Does that make sense?

87

I initially had an enum for the whole thing, but the Ref/Const/Ref/Value is very independent from Converted, so it resulted in too many values and awkward code. A PassBy enum and a boolean for Converted bit is better.

Makes the code slightly and tests a lot more readable. Thanks!

[kadircet]

Thanks, LGTM!

clang-tools-extra/clangd/Hover.cpp

751

I prefer to not drop the whole CalleeArgInfo / hover thing in those cases because, even if you ignore the whole mutable/non-mutable bit of information, I still find the name of the argument it's passed as to be very valuable).

I would expect people to file bugs in such cases for missing calleearginfo on hover card, hence we catching up quickly to a big enough coverage (which I think is already provided by this baseline). But the same argument goes both ways, users will also likely file bugs when the passby info is wrong, only problem is they'll likely be confused for a while :D

I like the idea of now showing the ref/const-ref/value bit in cases where we fear it might not be correct, but I do not know how to display this to the user without confusing them (i.e. if there's no mention of ref/const-ref, does that mean it's by-value or that we couldn't tell?). Note that I intentionally avoided showing the words "by value" to the user, since it will confuse everyone when used on pointers.

I agree, hence just dropping the passby field isn't enough, if we chose to go down this way I think we should drop full CalleArgInfo.

I don't think it is a decision we should block this patch on, we can decide what to do by looking at user reactions.

920

nit:

OS << "by ";
if(CallPassType->PassBy == ConstRef)
  OS << "const ";
OS << "reference ";

925

nit: again some redundant braces :D

clang-tools-extra/clangd/Hover.h

86

yes totally! (actually I had figured this one out while reading the rest, but forgot to delete the comment, sorry)

[adamcz]

Final review comment + rebase on top of HEAD

[adamcz]

removed some more {}

clang-tools-extra/clangd/Hover.h

struct HoverInfo {
   /// Set for all templates(function, class, variable).
   llvm::Optional<std::vector<Param>> TemplateParameters;
   /// Contains the evaluated value of the symbol if available.
   llvm::Optional<std::string> Value;
   /// Contains the byte-size of fields and types where it's interesting.
   llvm::Optional<uint64_t> Size;
   /// Contains the offset of fields within the enclosing class.
   llvm::Optional<uint64_t> Offset;
+  // Set when symbol is inside function call. Contains information extracted
+  // from the callee definition about the argument this is passed as.
+  llvm::Optional<Param> CalleeArgInfo;
+  struct PassType {
+    // How the variable is passed to callee.
+    enum PassMode { Ref, ConstRef, Value };
+    PassMode PassBy = Ref;
+    // True if type conversion happened. This includes calls to implicit
+    // constructor, as well as built-in type conversions. Casting to base class
+    // is not considered conversion.
+    bool Converted = false;
+  };
+  // Set only if CalleeArgInfo is set.
+  llvm::Optional<PassType> CallPassType;
 
   /// Produce a user-readable information.
   markup::Document present() const;
 };
 
+inline bool operator==(const HoverInfo::PassType &LHS,
+                       const HoverInfo::PassType &RHS) {
+  return std::tie(LHS.PassBy, LHS.Converted) ==
+         std::tie(RHS.PassBy, RHS.Converted);
+}
+
 // Try to infer structure of a documentation comment (e.g. line breaks).
 // FIXME: move to another file so CodeComplete doesn't depend on Hover.
 void parseDocumentation(llvm::StringRef Input, markup::Document &Output);
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo::Param &);
 inline bool operator==(const HoverInfo::Param &LHS,
                        const HoverInfo::Param &RHS) {
   return std::tie(LHS.Type, LHS.Name, LHS.Default) ==

clang-tools-extra/clangd/Hover.cpp

const Expr *getDefaultArg(const ParmVarDecl *PVD) {
   // attached to the AST node. For the latter though, it is safe to proceed as
   // the expression is still valid.
   if (!PVD->hasDefaultArg() || PVD->hasUnparsedDefaultArg())
     return nullptr;
   return PVD->hasUninstantiatedDefaultArg() ? PVD->getUninstantiatedDefaultArg()
                                             : PVD->getDefaultArg();
 }
 
-// Populates Type, ReturnType, and Parameters for function-like decls.
-void fillFunctionTypeAndParams(HoverInfo &HI, const Decl *D,
-                               const FunctionDecl *FD,
+HoverInfo::Param toHoverInfoParam(const ParmVarDecl *PVD,
                                   const PrintingPolicy &Policy) {
-  HI.Parameters.emplace();
-  for (const ParmVarDecl *PVD : FD->parameters()) {
-    HI.Parameters->emplace_back();
-    auto &P = HI.Parameters->back();
-    P.Type = printType(PVD->getType(), Policy);
-    if (!PVD->getName().empty())
-      P.Name = PVD->getNameAsString();
-    if (const Expr *DefArg = getDefaultArg(PVD)) {
-      P.Default.emplace();
-      llvm::raw_string_ostream Out(*P.Default);
-      DefArg->printPretty(Out, nullptr, Policy);
-    }
-  }
+  HoverInfo::Param Out;
+  Out.Type = printType(PVD->getType(), Policy);
+    Out.Name = PVD->getNameAsString();
+    Out.Default.emplace();
+    llvm::raw_string_ostream OS(*Out.Default);
+    DefArg->printPretty(OS, nullptr, Policy);
+  return Out;
 
+// Populates Type, ReturnType, and Parameters for function-like decls.
+void fillFunctionTypeAndParams(HoverInfo &HI, const Decl *D,
+                               const FunctionDecl *FD,
+                               const PrintingPolicy &Policy) {
+  HI.Parameters.emplace();
+  for (const ParmVarDecl *PVD : FD->parameters())
+    HI.Parameters->emplace_back(toHoverInfoParam(PVD, Policy));
+
   // We don't want any type info, if name already contains it. This is true for
   // constructors/destructors and conversion operators.
   const auto NK = FD->getDeclName().getNameKind();
   if (NK == DeclarationName::CXXConstructorName ||
       NK == DeclarationName::CXXDestructorName ||
       NK == DeclarationName::CXXConversionFunctionName)
     return;
 
if (const auto *Var = dyn_cast<VarDecl>(D)) {
     if (const Expr *Init = Var->getInit())
       HI.Value = printExprValue(Init, Ctx);
   } else if (const auto *ECD = dyn_cast<EnumConstantDecl>(D)) {
     // Dependent enums (e.g. nested in template classes) don't have values yet.
     if (!ECD->getType()->isDependentType())
       HI.Value = ECD->getInitVal().toString(10);
   }
 
   HI.Definition = printDefinition(D);
   return HI;
 }
 
 /// Generate a \p Hover object given the type \p T.
 HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx,
                            const SymbolIndex *Index) {
   HoverInfo HI;
 
if (Record && !Record->isDependentType()) {
         HI.Size = Size->getQuantity();
         HI.Offset = OffsetBits / 8;
       }
     }
     return;
   }
 }
 
+// If N is passed as argument to a function, fill HI.CalleeArgInfo with
+// information about that argument.
+void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI,
+                           const PrintingPolicy &Policy) {
+  const auto &OuterNode = N->outerImplicit();
+  if (!OuterNode.Parent)
+    return;
+  const auto *CE = OuterNode.Parent->ASTNode.get<CallExpr>();
+  if (!CE)
+    return;
+  const FunctionDecl *FD = CE->getDirectCallee();
+  // For non-function-call-like operatators (e.g. operator+, operator<<) it's
+  // not immediattely obvious what the "passed as" would refer to and, given
+  // fixed function signature, the value would be very low anyway, so we choose
+  // to not support that.
+  // Both variadic functions and operator() (especially relevant for lambdas)
+  // should be supported in the future.
+  if (!FD || FD->isOverloadedOperator() || FD->isVariadic())
+    return;
+
+  // Find argument index for N.
+  for (unsigned I = 0; I < CE->getNumArgs() && I < FD->getNumParams(); ++I) {
+    if (CE->getArg(I) != OuterNode.ASTNode.get<Expr>())
+      continue;
+
+    // Extract matching argument from function declaration.
+    if (const ParmVarDecl *PVD = FD->getParamDecl(I))
+      HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, Policy));
+    break;
+  }
+  if (!HI.CalleeArgInfo)
+    return;
+
+  // If we found a matching argument, also figure out if it's a
+  // [const-]reference. For this we need to walk up the AST from the arg itself
+  // to CallExpr and check all implicit casts, constructor calls, etc.
+  HoverInfo::PassType PassType;
+  if (const auto *E = N->ASTNode.get<Expr>()) {
+    if (E->getType().isConstQualified())
+      PassType.PassBy = HoverInfo::PassType::ConstRef;
+  }
+
+  for (auto *CastNode = N->Parent;
+       CastNode != OuterNode.Parent && !PassType.Converted;
+       CastNode = CastNode->Parent) {
+    if (const auto *ImplicitCast = CastNode->ASTNode.get<ImplicitCastExpr>()) {
+      switch (ImplicitCast->getCastKind()) {
+      case CK_NoOp:
+      case CK_DerivedToBase:
+      case CK_UncheckedDerivedToBase:
+        // If it was a reference before, it's still a reference.
+        if (PassType.PassBy != HoverInfo::PassType::Value)
+          PassType.PassBy = ImplicitCast->getType().isConstQualified()
+                                ? HoverInfo::PassType::ConstRef
+                                : HoverInfo::PassType::Ref;
+        break;
+      case CK_LValueToRValue:
+      case CK_ArrayToPointerDecay:
+      case CK_FunctionToPointerDecay:
+      case CK_NullToPointer:
+      case CK_NullToMemberPointer:
+        // No longer a reference, but we do not show this as type conversion.
+        PassType.PassBy = HoverInfo::PassType::Value;
+        break;
+      default:
+        PassType.PassBy = HoverInfo::PassType::Value;
+        PassType.Converted = true;
+        break;
+      }
+    } else if (const auto *CtorCall =
+                   CastNode->ASTNode.get<CXXConstructExpr>()) {
+      // We want to be smart about copy constructors. They should not show up as
+      // type conversion, but instead as passing by value.
+      if (CtorCall->getConstructor()->isCopyConstructor())
+        PassType.PassBy = HoverInfo::PassType::Value;
+      else
+        PassType.Converted = true;
+    } else { // Unknown implicit node, assume type conversion.
+      PassType.PassBy = HoverInfo::PassType::Value;
+      PassType.Converted = true;
+    }
+  }
+
+  HI.CallPassType.emplace(PassType);
+}
+
 } // namespace
 
 llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
                                    format::FormatStyle Style,
                                    const SymbolIndex *Index) {
   const SourceManager &SM = AST.getSourceManager();
   auto CurLoc = sourceLocationInMainFile(SM, Pos);
   if (!CurLoc) {
     llvm::consumeError(CurLoc.takeError());
     return llvm::None;
   }
   const auto &TB = AST.getTokens();
   auto TokensTouchingCursor = syntax::spelledTokensTouching(*CurLoc, TB);
   // Early exit if there were no tokens around the cursor.
   if (TokensTouchingCursor.empty())
     return llvm::None;
 
   // To be used as a backup for highlighting the selected token, we use back as
   // it aligns better with biases elsewhere (editors tend to send the position
   // for the left of the hovered token).
   CharSourceRange HighlightRange =
       TokensTouchingCursor.back().range(SM).toCharRange(SM);
   llvm::Optional<HoverInfo> HI;
   // Macros and deducedtype only works on identifiers and auto/decltype keywords
   // respectively. Therefore they are only trggered on whichever works for them,
   // similar to SelectionTree::create().
   for (const auto &Tok : TokensTouchingCursor) {
     if (Tok.kind() == tok::identifier) {
       // Prefer the identifier token as a fallback highlighting range.
       HighlightRange = Tok.range(SM).toCharRange(SM);
       if (auto M = locateMacroAt(Tok, AST.getPreprocessor())) {
         HI = getHoverContents(*M, AST);
if (const SelectionTree::Node *N = ST.commonAncestor()) {
       if (!Decls.empty()) {
         HI = getHoverContents(Decls.front(), Index);
         // Layout info only shown when hovering on the field/class itself.
         if (Decls.front() == N->ASTNode.get<Decl>())
           addLayoutInfo(*Decls.front(), *HI);
         // Look for a close enclosing expression to show the value of.
         if (!HI->Value)
           HI->Value = printExprValue(N, AST.getASTContext());
+        maybeAddCalleeArgInfo(N, *HI, AST.getASTContext().getPrintingPolicy());
       } else if (const Expr *E = N->ASTNode.get<Expr>()) {
         HI = getHoverContents(E, AST);
       }
       // FIXME: support hovers for other nodes?
       //  - built-in types
     }
   }
 
markup::Document HoverInfo::present() const {
   if (Offset)
     Output.addParagraph().appendText(
         llvm::formatv("Offset: {0} byte{1}", *Offset, *Offset == 1 ? "" : "s")
             .str());
   if (Size)
     Output.addParagraph().appendText(
         llvm::formatv("Size: {0} byte{1}", *Size, *Size == 1 ? "" : "s").str());
 
+  if (CalleeArgInfo) {
+    assert(CallPassType);
+    std::string Buffer;
+    llvm::raw_string_ostream OS(Buffer);
+    OS << "Passed ";
+    if (CallPassType->PassBy != HoverInfo::PassType::Value) {
+      OS << "by ";
+      if (CallPassType->PassBy == HoverInfo::PassType::ConstRef)
+        OS << "const ";
+      OS << "reference ";
+    }
+    if (CalleeArgInfo->Name)
+      OS << "as " << CalleeArgInfo->Name;
+    if (CallPassType->Converted && CalleeArgInfo->Type)
+      OS << " (converted to " << CalleeArgInfo->Type << ")";
+    Output.addParagraph().appendText(OS.str());
+  }
+
   if (!Documentation.empty())
     parseDocumentation(Documentation, Output);
 
   if (!Definition.empty()) {
     Output.addRuler();
     std::string ScopeComment;
     // Drop trailing "::".
     if (!LocalScope.empty()) {
       // Container name, e.g. class, method, function.
       // We might want to propagate some info about container type to print
       // function foo, class X, method X::bar, etc.
       ScopeComment =
           "// In " + llvm::StringRef(LocalScope).rtrim(':').str() + '\n';
     } else if (NamespaceScope && !NamespaceScope->empty()) {
       ScopeComment = "// In namespace " +
                      llvm::StringRef(*NamespaceScope).rtrim(':').str() + '\n';
     }
     std::string DefinitionWithAccess = !AccessSpecifier.empty()
                                            ? AccessSpecifier + ": " + Definition
                                            : Definition;
     // Note that we don't print anything for global namespace, to not annoy
     // non-c++ projects or projects that are not making use of namespaces.
     Output.addCodeBlock(ScopeComment + DefinitionWithAccess);
   }
+
   return Output;
 }
 
 // If the backtick at `Offset` starts a probable quoted range, return the range
 // (including the quotes).
 llvm::Optional<llvm::StringRef> getBacktickQuoteRange(llvm::StringRef Line,
                                                       unsigned Offset) {
   assert(Line[Offset] == '`');
 
   // The open-quote is usually preceded by whitespace.
   llvm::StringRef Prefix = Line.substr(0, Offset);
   constexpr llvm::StringLiteral BeforeStartChars = " \t(=";
   if (!Prefix.empty() && !BeforeStartChars.contains(Prefix.back()))

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

 #include "gtest/gtest.h"
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 namespace {
 
+using PassMode = HoverInfo::PassType::PassMode;
+
 TEST(Hover, Structured) {
   struct {
     const char *const Code;
     const std::function<void(HoverInfo &)> ExpectedBuilder;
   } Cases[] = {
       // Global scope.
       {R"cpp(
           // Best foo ever.
class Foo {})cpp";
          HI.Name = "x";
          HI.Kind = index::SymbolKind::Variable;
          HI.NamespaceScope = "";
          HI.Definition = "";
          HI.Type = "NULL TYPE";
          // Bindings are in theory public members of an anonymous struct.
          HI.AccessSpecifier = "public";
        }},
+      {// Extra info for function call.
+       R"cpp(
+          void fun(int arg_a, int &arg_b) {};
+          void code() {
+            int a = 1, b = 2;
+            fun(a, [[^b]]);
+          }
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "b";
+         HI.Kind = index::SymbolKind::Variable;
+         HI.NamespaceScope = "";
+         HI.Definition = "int b = 2";
+         HI.LocalScope = "code::";
+         HI.Value = "2";
+         HI.Type = "int";
+         HI.CalleeArgInfo.emplace();
+         HI.CalleeArgInfo->Name = "arg_b";
+         HI.CalleeArgInfo->Type = "int &";
+         HI.CallPassType.emplace();
+         HI.CallPassType->PassBy = PassMode::Ref;
+         HI.CallPassType->Converted = false;
+       }},
+      {// Extra info for method call.
+       R"cpp(
+          class C {
+           public:
+            void fun(int arg_a = 3, int arg_b = 4) {}
+          };
+          void code() {
+            int a = 1, b = 2;
+            C c;
+            c.fun([[^a]], b);
+          }
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "a";
+         HI.Kind = index::SymbolKind::Variable;
+         HI.NamespaceScope = "";
+         HI.Definition = "int a = 1";
+         HI.LocalScope = "code::";
+         HI.Value = "1";
+         HI.Type = "int";
+         HI.CalleeArgInfo.emplace();
+         HI.CalleeArgInfo->Name = "arg_a";
+         HI.CalleeArgInfo->Type = "int";
+         HI.CalleeArgInfo->Default = "3";
+         HI.CallPassType.emplace();
+         HI.CallPassType->PassBy = PassMode::Value;
+         HI.CallPassType->Converted = false;
+       }},
   };
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Code);
 
     Annotations T(Case.Code);
     TestTU TU = TestTU::withCode(T.code());
     TU.ExtraArgs.push_back("-std=c++17");
     // Types might be different depending on the target triplet, we chose a
for (const auto &Case : Cases) {
     EXPECT_EQ(H->ReturnType, Expected.ReturnType);
     EXPECT_EQ(H->Parameters, Expected.Parameters);
     EXPECT_EQ(H->TemplateParameters, Expected.TemplateParameters);
     EXPECT_EQ(H->SymRange, Expected.SymRange);
     EXPECT_EQ(H->Value, Expected.Value);
     EXPECT_EQ(H->Size, Expected.Size);
     EXPECT_EQ(H->Offset, Expected.Offset);
     EXPECT_EQ(H->AccessSpecifier, Expected.AccessSpecifier);
+    EXPECT_EQ(H->CalleeArgInfo, Expected.CalleeArgInfo);
+    EXPECT_EQ(H->CallPassType, Expected.CallPassType);
+  }
+}
+
+TEST(Hover, CallPassType) {
+  const llvm::StringRef CodePrefix = R"cpp(
+class Base {};
+class Derived : public Base {};
+class CustomClass {
+ public:
+  CustomClass() {}
+  CustomClass(const Base &x) {}
+  CustomClass(int &x) {}
+  CustomClass(float x) {}
+};
+
+void int_by_ref(int &x) {}
+void int_by_const_ref(const int &x) {}
+void int_by_value(int x) {}
+void base_by_ref(Base &x) {}
+void base_by_const_ref(const Base &x) {}
+void base_by_value(Base x) {}
+void float_by_value(float x) {}
+void custom_by_value(CustomClass x) {}
+
+void fun() {
+  int int_x;
+  int &int_ref = int_x;
+  const int &int_const_ref = int_x;
+  Base base;
+  const Base &base_const_ref = base;
+  Derived derived;
+  float float_x;
+)cpp";
+  const llvm::StringRef CodeSuffix = "}";
+
+  struct {
+    const char *const Code;
+    HoverInfo::PassType::PassMode PassBy;
+    bool Converted;
+  } Tests[] = {
+      // Integer tests
+      {"int_by_value([[^int_x]]);", PassMode::Value, false},
+      {"int_by_ref([[^int_x]]);", PassMode::Ref, false},
+      {"int_by_const_ref([[^int_x]]);", PassMode::ConstRef, false},
+      {"int_by_value([[^int_ref]]);", PassMode::Value, false},
+      {"int_by_const_ref([[^int_ref]]);", PassMode::ConstRef, false},
+      {"int_by_const_ref([[^int_ref]]);", PassMode::ConstRef, false},
+      {"int_by_const_ref([[^int_const_ref]]);", PassMode::ConstRef, false},
+      // Custom class tests
+      {"base_by_ref([[^base]]);", PassMode::Ref, false},
+      {"base_by_const_ref([[^base]]);", PassMode::ConstRef, false},
+      {"base_by_const_ref([[^base_const_ref]]);", PassMode::ConstRef, false},
+      {"base_by_value([[^base]]);", PassMode::Value, false},
+      {"base_by_value([[^base_const_ref]]);", PassMode::Value, false},
+      {"base_by_ref([[^derived]]);", PassMode::Ref, false},
+      {"base_by_const_ref([[^derived]]);", PassMode::ConstRef, false},
+      {"base_by_value([[^derived]]);", PassMode::Value, false},
+      // Converted tests
+      {"float_by_value([[^int_x]]);", PassMode::Value, true},
+      {"float_by_value([[^int_ref]]);", PassMode::Value, true},
+      {"float_by_value([[^int_const_ref]]);", PassMode::Value, true},
+      {"custom_by_value([[^int_x]]);", PassMode::Ref, true},
+      {"custom_by_value([[^float_x]]);", PassMode::Value, true},
+      {"custom_by_value([[^base]]);", PassMode::ConstRef, true},
+  };
+  for (const auto &Test : Tests) {
+    SCOPED_TRACE(Test.Code);
+
+    const auto Code = (CodePrefix + Test.Code + CodeSuffix).str();
+    Annotations T(Code);
+    TestTU TU = TestTU::withCode(T.code());
+    TU.ExtraArgs.push_back("-std=c++17");
+    auto AST = TU.build();
+    auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+    ASSERT_TRUE(H);
+    EXPECT_EQ(H->CallPassType->PassBy, Test.PassBy);
+    EXPECT_EQ(H->CallPassType->Converted, Test.Converted);
   }
 }
 
 TEST(Hover, NoHover) {
   llvm::StringRef Tests[] = {
       "^int main() {}",
       "void foo() {^}",
       R"cpp(// structured binding. Not supported yet
protected: int method())",
             HI.Name = "foo";
             HI.NamespaceScope = "ns1::";
             HI.Definition = "union foo {}";
           },
           R"(union foo
 
 // In namespace ns1
 private: union foo {})",
+      },
+      {
+          [](HoverInfo &HI) {
+            HI.Kind = index::SymbolKind::Variable;
+            HI.Name = "foo";
+            HI.Definition = "int foo = 3";
+            HI.LocalScope = "test::Bar::";
+            HI.Value = "3";
+            HI.Type = "int";
+            HI.CalleeArgInfo.emplace();
+            HI.CalleeArgInfo->Name = "arg_a";
+            HI.CalleeArgInfo->Type = "int";
+            HI.CalleeArgInfo->Default = "7";
+            HI.CallPassType.emplace();
+            HI.CallPassType->PassBy = PassMode::Value;
+            HI.CallPassType->Converted = false;
+          },
+          R"(variable foo
+
+Type: int
+Value = 3
+Passed as arg_a
+
+// In test::Bar
+int foo = 3)",
+      },
+      {
+          [](HoverInfo &HI) {
+            HI.Kind = index::SymbolKind::Variable;
+            HI.Name = "foo";
+            HI.Definition = "int foo = 3";
+            HI.LocalScope = "test::Bar::";
+            HI.Value = "3";
+            HI.Type = "int";
+            HI.CalleeArgInfo.emplace();
+            HI.CalleeArgInfo->Name = "arg_a";
+            HI.CalleeArgInfo->Type = "int";
+            HI.CalleeArgInfo->Default = "7";
+            HI.CallPassType.emplace();
+            HI.CallPassType->PassBy = PassMode::Ref;
+            HI.CallPassType->Converted = false;
+          },
+          R"(variable foo
+
+Type: int
+Value = 3
+Passed by reference as arg_a
+
+// In test::Bar
+int foo = 3)",
+      },
+      {
+          [](HoverInfo &HI) {
+            HI.Kind = index::SymbolKind::Variable;
+            HI.Name = "foo";
+            HI.Definition = "int foo = 3";
+            HI.LocalScope = "test::Bar::";
+            HI.Value = "3";
+            HI.Type = "int";
+            HI.CalleeArgInfo.emplace();
+            HI.CalleeArgInfo->Name = "arg_a";
+            HI.CalleeArgInfo->Type = "int";
+            HI.CalleeArgInfo->Default = "7";
+            HI.CallPassType.emplace();
+            HI.CallPassType->PassBy = PassMode::Value;
+            HI.CallPassType->Converted = true;
+          },
+          R"(variable foo
+
+Type: int
+Value = 3
+Passed as arg_a (converted to int)
+
+// In test::Bar
+int foo = 3)",
+      },
+      {
+          [](HoverInfo &HI) {
+            HI.Kind = index::SymbolKind::Variable;
+            HI.Name = "foo";
+            HI.Definition = "int foo = 3";
+            HI.LocalScope = "test::Bar::";
+            HI.Value = "3";
+            HI.Type = "int";
+            HI.CalleeArgInfo.emplace();
+            HI.CalleeArgInfo->Name = "arg_a";
+            HI.CalleeArgInfo->Type = "int";
+            HI.CalleeArgInfo->Default = "7";
+            HI.CallPassType.emplace();
+            HI.CallPassType->PassBy = PassMode::ConstRef;
+            HI.CallPassType->Converted = true;
+          },
+          R"(variable foo
+
+Type: int
+Value = 3
+Passed by const reference as arg_a (converted to int)
+
+// In test::Bar
+int foo = 3)",
       }};
 
   for (const auto &C : Cases) {
     HoverInfo HI;
     C.Builder(HI);
     EXPECT_EQ(HI.present().asPlainText(), C.ExpectedRender);
   }
 }