[clangd] Hover: show CalleeArgInfo for literals and expressions

This is very useful when inlay hints are disabled.

Also, improve presentation of Hover when variable is passed by value to
a function with an unnamed parameter

Differential Revision: https://reviews.llvm.org/D140775

diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index d05ea6a..7f09e09 100644 (file)
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -795,7 +795,7 @@ bool isLiteral(const Expr *E) {
          llvm::isa<CXXNullPtrLiteralExpr>(E) ||
          llvm::isa<FixedPointLiteral>(E) || llvm::isa<FloatingLiteral>(E) ||
          llvm::isa<ImaginaryLiteral>(E) || llvm::isa<IntegerLiteral>(E) ||
-         llvm::isa<UserDefinedLiteral>(E);
+         llvm::isa<StringLiteral>(E) || llvm::isa<UserDefinedLiteral>(E);
 }
 
 llvm::StringLiteral getNameForExpr(const Expr *E) {
@@ -809,34 +809,53 @@ llvm::StringLiteral getNameForExpr(const Expr *E) {
   return llvm::StringLiteral("expression");
 }
 
+void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI,
+                           const PrintingPolicy &PP);
+
 // Generates hover info for `this` and evaluatable expressions.
 // FIXME: Support hover for literals (esp user-defined)
-std::optional<HoverInfo> getHoverContents(const Expr *E, ParsedAST &AST,
+std::optional<HoverInfo> getHoverContents(const SelectionTree::Node *N,
+                                          const Expr *E, ParsedAST &AST,
                                           const PrintingPolicy &PP,
                                           const SymbolIndex *Index) {
-  // There's not much value in hovering over "42" and getting a hover card
-  // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  std::optional<HoverInfo> HI;
+
+  if (const StringLiteral *SL = dyn_cast<StringLiteral>(E)) {
+    // Print the type and the size for string literals
+    HI = getStringLiteralContents(SL, PP);
+  } else if (isLiteral(E)) {
+    // There's not much value in hovering over "42" and getting a hover card
+    // saying "42 is an int", similar for most other literals.
+    // However, if we have CalleeArgInfo, it's still useful to show it.
+    maybeAddCalleeArgInfo(N, HI.emplace(), PP);
+    if (HI->CalleeArgInfo) {
+      // FIXME Might want to show the expression's value here instead?
+      // E.g. if the literal is in hex it might be useful to show the decimal
+      // value here.
+      HI->Name = "literal";
+      return HI;
+    }
     return std::nullopt;
+  }
 
-  HoverInfo HI;
-  // Print the type and the size for string literals
-  if (const StringLiteral *SL = dyn_cast<StringLiteral>(E))
-    return getStringLiteralContents(SL, PP);
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast<CXXThisExpr>(E))
-    return getThisExprHoverContents(CTE, AST.getASTContext(), PP);
+    HI = getThisExprHoverContents(CTE, AST.getASTContext(), PP);
   if (const PredefinedExpr *PE = dyn_cast<PredefinedExpr>(E))
-    return getPredefinedExprHoverContents(*PE, AST.getASTContext(), PP);
+    HI = getPredefinedExprHoverContents(*PE, AST.getASTContext(), PP);
   // For expressions we currently print the type and the value, iff it is
   // evaluatable.
   if (auto Val = printExprValue(E, AST.getASTContext())) {
-    HI.Type = printType(E->getType(), AST.getASTContext(), PP);
-    HI.Value = *Val;
-    HI.Name = std::string(getNameForExpr(E));
-    return HI;
+    HI.emplace();
+    HI->Type = printType(E->getType(), AST.getASTContext(), PP);
+    HI->Value = *Val;
+    HI->Name = std::string(getNameForExpr(E));
   }
-  return std::nullopt;
+
+  if (HI)
+    maybeAddCalleeArgInfo(N, *HI, PP);
+
+  return HI;
 }
 
 // Generates hover info for attributes.
@@ -973,6 +992,10 @@ void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI,
   if (const auto *E = N->ASTNode.get<Expr>()) {
     if (E->getType().isConstQualified())
       PassType.PassBy = HoverInfo::PassType::ConstRef;
+
+    // No implicit node, literal passed by value
+    if (isLiteral(E) && N->Parent == OuterNode.Parent)
+      PassType.PassBy = HoverInfo::PassType::Value;
   }
 
   for (auto *CastNode = N->Parent;
@@ -1010,6 +1033,10 @@ void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI,
         PassType.PassBy = HoverInfo::PassType::Value;
       else
         PassType.Converted = true;
+    } else if (const auto *MTE =
+                   CastNode->ASTNode.get<MaterializeTemporaryExpr>()) {
+      // Can't bind a non-const-ref to a temporary, so has to be const-ref
+      PassType.PassBy = HoverInfo::PassType::ConstRef;
     } else { // Unknown implicit node, assume type conversion.
       PassType.PassBy = HoverInfo::PassType::Value;
       PassType.Converted = true;
@@ -1135,7 +1162,7 @@ std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
           HI->Value = printExprValue(N, AST.getASTContext());
         maybeAddCalleeArgInfo(N, *HI, PP);
       } else if (const Expr *E = N->ASTNode.get<Expr>()) {
-        HI = getHoverContents(E, AST, PP, Index);
+        HI = getHoverContents(N, E, AST, PP, Index);
       } else if (const Attr *A = N->ASTNode.get<Attr>()) {
         HI = getHoverContents(A, AST);
       }
@@ -1240,6 +1267,8 @@ markup::Document HoverInfo::present() const {
     }
     if (CalleeArgInfo->Name)
       OS << "as " << CalleeArgInfo->Name;
+    else if (CallPassType->PassBy == HoverInfo::PassType::Value)
+      OS << "by value";
     if (CallPassType->Converted && CalleeArgInfo->Type)
       OS << " (converted to " << CalleeArgInfo->Type->Type << ")";
     Output.addParagraph().appendText(OS.str());

diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 3493813..7fbb885 100644 (file)
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -900,6 +900,40 @@ class Foo final {})cpp";
          HI.CalleeArgInfo->Type = "int &";
          HI.CallPassType = HoverInfo::PassType{PassMode::Ref, false};
        }},
+      {// Literal passed to function call
+       R"cpp(
+          void fun(int arg_a, const int &arg_b) {};
+          void code() {
+            int a = 1;
+            fun(a, [[^2]]);
+          }
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "literal";
+         HI.Kind = index::SymbolKind::Unknown;
+         HI.CalleeArgInfo.emplace();
+         HI.CalleeArgInfo->Name = "arg_b";
+         HI.CalleeArgInfo->Type = "const int &";
+         HI.CallPassType = HoverInfo::PassType{PassMode::ConstRef, false};
+       }},
+      {// Expression passed to function call
+       R"cpp(
+          void fun(int arg_a, const int &arg_b) {};
+          void code() {
+            int a = 1;
+            fun(a, 1 [[^+]] 2);
+          }
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "expression";
+         HI.Kind = index::SymbolKind::Unknown;
+         HI.Type = "int";
+         HI.Value = "3";
+         HI.CalleeArgInfo.emplace();
+         HI.CalleeArgInfo->Name = "arg_b";
+         HI.CalleeArgInfo->Type = "const int &";
+         HI.CallPassType = HoverInfo::PassType{PassMode::ConstRef, false};
+       }},
       {// Extra info for method call.
        R"cpp(
           class C {
@@ -1226,8 +1260,10 @@ void fun() {
   } Tests[] = {
       // Integer tests
       {"int_by_value([[^int_x]]);", PassMode::Value, false},
+      {"int_by_value([[^123]]);", PassMode::Value, false},
       {"int_by_ref([[^int_x]]);", PassMode::Ref, false},
       {"int_by_const_ref([[^int_x]]);", PassMode::ConstRef, false},
+      {"int_by_const_ref([[^123]]);", 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},
@@ -1245,6 +1281,8 @@ void fun() {
       {"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},
+      {"float_by_value([[^123.0f]]);", PassMode::Value, false},
+      {"float_by_value([[^123]]);", 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},
@@ -3048,6 +3086,18 @@ int foo = 3)",
           [](HoverInfo &HI) {
             HI.Kind = index::SymbolKind::Variable;
             HI.Name = "foo";
+            HI.CalleeArgInfo.emplace();
+            HI.CalleeArgInfo->Type = "int";
+            HI.CallPassType = HoverInfo::PassType{PassMode::Value, false};
+          },
+          R"(variable foo
+
+Passed by value)",
+      },
+      {
+          [](HoverInfo &HI) {
+            HI.Kind = index::SymbolKind::Variable;
+            HI.Name = "foo";
             HI.Definition = "int foo = 3";
             HI.LocalScope = "test::Bar::";
             HI.Value = "3";