[clangd] Show values of more expressions on hover

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

clang-tools-extra/clangd/Hover.cpp

@@ -16,6 +16,7 @@
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyPrinter.h"
@@ -239,6 +240,46 @@ void fillFunctionTypeAndParams(HoverInfo &HI, const Decl *D,
   // FIXME: handle variadics.
 }
 
+llvm::Optional<std::string> printExprValue(const Expr *E, const ASTContext &Ctx) {
+  Expr::EvalResult Constant;
+  // Evaluating [[foo]]() as "&foo" isn't useful, and prevents us walking up
+  // to the enclosing call.
+  QualType T = E->getType();
+  if (T->isFunctionType() || T->isFunctionPointerType() ||
+      T->isFunctionReferenceType())
+    return llvm::None;
+  // Attempt to evaluate. If expr is dependent, evaluation crashes!
+  if (E->isValueDependent() || !E->EvaluateAsRValue(Constant, Ctx))
+    return llvm::None;
+
+  // Show enums symbolically, not numerically like APValue::printPretty().
+  if (T->isEnumeralType() && Constant.Val.getInt().getMinSignedBits() <= 64) {
+    // Compare to int64_t to avoid bit-width match requirements.
+    int64_t Val = Constant.Val.getInt().getExtValue();
+    for (const EnumConstantDecl *ECD :
+         T->castAs<EnumType>()->getDecl()->enumerators())
+      if (ECD->getInitVal() == Val)
+        return llvm::formatv("{0} ({1})", ECD->getNameAsString(), Val).str();
+  }
+  return Constant.Val.getAsString(Ctx, E->getType());
+}
+
+llvm::Optional<std::string> printExprValue(const SelectionTree::Node *N,
+                                           const ASTContext &Ctx) {
+  for (; N; N = N->Parent) {
+    // Try to evaluate the first evaluable enclosing expression.
+    if (const Expr *E = N->ASTNode.get<Expr>()) {
+      if (auto Val = printExprValue(E, Ctx))
+        return Val;
+    } else if (N->ASTNode.get<Decl>() || N->ASTNode.get<Stmt>()) {
+      // Refuse to cross certain non-exprs. (TypeLoc are OK as part of Exprs).
+      // This tries to ensure we're showing a value related to the cursor.
+      break;
+    }
+  }
+  return llvm::None;
+}
+
 /// Generate a \p Hover object given the declaration \p D.
 HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
   HoverInfo HI;
@@ -282,18 +323,9 @@ HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
   }
 
   // Fill in value with evaluated initializer if possible.
-  // FIXME(kadircet): Also set Value field for expressions like "sizeof" and
-  // function calls.
   if (const auto *Var = dyn_cast<VarDecl>(D)) {
-    if (const Expr *Init = Var->getInit()) {
-      Expr::EvalResult Result;
-      if (!Init->isValueDependent() && Init->EvaluateAsRValue(Result, Ctx)) {
-        HI.Value.emplace();
-        llvm::raw_string_ostream ValueOS(*HI.Value);
-        Result.Val.printPretty(ValueOS, const_cast<ASTContext &>(Ctx),
-                               Init->getType());
-      }
-    }
+    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())
@@ -381,8 +413,16 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
     if (const SelectionTree::Node *N = Selection.commonAncestor()) {
       DeclRelationSet Rel = DeclRelation::TemplatePattern | DeclRelation::Alias;
       auto Decls = targetDecl(N->ASTNode, Rel);
-      if (!Decls.empty())
+      if (!Decls.empty()) {
         HI = getHoverContents(Decls.front(), Index);
+        // Look for a close enclosing expression to show the value of.
+        if (!HI->Value)
+          HI->Value = printExprValue(N, AST.getASTContext());
+      }
+      // FIXME: support hovers for other nodes?
+      //  - certain expressions (sizeof etc)
+      //  - built-in types
+      //  - literals (esp user-defined)
     }
   }
 

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

@@ -275,6 +275,7 @@ void foo())cpp";
              {std::string("int"), std::string("T"), llvm::None},
              {std::string("bool"), std::string("B"), llvm::None},
          };
+         HI.Value = "false";
          return HI;
        }},
       // Lambda variable
@@ -443,10 +444,23 @@ void foo())cpp";
          HI.Definition = "GREEN";
          HI.Kind = SymbolKind::EnumMember;
          HI.Type = "enum Color";
-         HI.Value = "1";
+         HI.Value = "1"; // Numeric when hovering on the enumerator name.
+       }},
+      {R"cpp(
+        enum Color { RED, GREEN, };
+        Color x = GREEN;
+        Color y = [[^x]];
+       )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "x";
+         HI.NamespaceScope = "";
+         HI.Definition = "enum Color x = GREEN";
+         HI.Kind = SymbolKind::Variable;
+         HI.Type = "enum Color";
+         HI.Value = "GREEN (1)"; // Symbolic when hovering on an expression.
        }},
       // FIXME: We should use the Decl referenced, even if from an implicit
-      // instantiation. Then the scope would be Add<1, 2> and the value 3.
+      // instantiation. Then the scope would be Add<1, 2>.
       {R"cpp(
         template<int a, int b> struct Add {
           static constexpr int result = a + b;
@@ -460,6 +474,21 @@ void foo())cpp";
          HI.Type = "const int";
          HI.NamespaceScope = "";
          HI.LocalScope = "Add<a, b>::";
+         HI.Value = "3";
+       }},
+      {R"cpp(
+        constexpr int answer() { return 40 + 2; }
+        int x = [[ans^wer]]();
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "answer";
+         HI.Definition = "constexpr int answer()";
+         HI.Kind = SymbolKind::Function;
+         HI.Type = "int ()";
+         HI.ReturnType = "int";
+         HI.Parameters.emplace();
+         HI.NamespaceScope = "";
+         HI.Value = "42";
        }},
       {R"cpp(
         const char *[[ba^r]] = "1234";