[clangd] Show hower info for expressions
authorKadir Cetinkaya <kadircet@google.com>
Fri, 10 Jan 2020 12:11:09 +0000 (13:11 +0100)
committerKadir Cetinkaya <kadircet@google.com>
Wed, 15 Jan 2020 14:14:37 +0000 (15:14 +0100)
Summary:
This currently populates only the Name with the expression's type and
Value if expression is evaluatable.

Fixes https://github.com/clangd/clangd/issues/56

Reviewers: sammccall

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

Tags: #clang

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

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

index 526f222..97e287d 100644 (file)
@@ -13,6 +13,7 @@
 #include "FindTarget.h"
 #include "FormattedString.h"
 #include "Logger.h"
+#include "ParsedAST.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/Type.h"
 #include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include <string>
 
@@ -410,6 +417,45 @@ HoverInfo getHoverContents(const DefinedMacro &Macro, ParsedAST &AST) {
   }
   return HI;
 }
+
+bool isLiteral(const Expr *E) {
+  // Unfortunately there's no common base Literal classes inherits from
+  // (apart from Expr), therefore this is a nasty blacklist.
+  return llvm::isa<CharacterLiteral>(E) || llvm::isa<CompoundLiteralExpr>(E) ||
+         llvm::isa<CXXBoolLiteralExpr>(E) ||
+         llvm::isa<CXXNullPtrLiteralExpr>(E) ||
+         llvm::isa<FixedPointLiteral>(E) || llvm::isa<FloatingLiteral>(E) ||
+         llvm::isa<ImaginaryLiteral>(E) || llvm::isa<IntegerLiteral>(E) ||
+         llvm::isa<StringLiteral>(E) || llvm::isa<UserDefinedLiteral>(E);
+}
+
+llvm::StringLiteral getNameForExpr(const Expr *E) {
+  // FIXME: Come up with names for `special` expressions.
+  return "expression";
+}
+
+// Generates hover info for evaluatable expressions.
+// FIXME: Support hover for literals (esp user-defined)
+llvm::Optional<HoverInfo> getHoverContents(const Expr *E, ParsedAST &AST) {
+  // 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))
+    return llvm::None;
+
+  HoverInfo HI;
+  // For expressions we currently print the type and the value, iff it is
+  // evaluatable.
+  if (auto Val = printExprValue(E, AST.getASTContext())) {
+    auto Policy =
+        printingPolicyForDecls(AST.getASTContext().getPrintingPolicy());
+    Policy.SuppressTagKeyword = true;
+    HI.Type = E->getType().getAsString(Policy);
+    HI.Value = *Val;
+    HI.Name = getNameForExpr(E);
+    return HI;
+  }
+  return llvm::None;
+}
 } // namespace
 
 llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
@@ -439,11 +485,11 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
         // Look for a close enclosing expression to show the value of.
         if (!HI->Value)
           HI->Value = printExprValue(N, AST.getASTContext());
+      } else if (const Expr *E = N->ASTNode.get<Expr>()) {
+        HI = getHoverContents(E, AST);
       }
       // FIXME: support hovers for other nodes?
-      //  - certain expressions (sizeof etc)
       //  - built-in types
-      //  - literals (esp user-defined)
     }
   }
 
@@ -469,6 +515,8 @@ markup::Document HoverInfo::present() const {
   // class `X`
   //
   // function `foo` → `int`
+  //
+  // expression : `int`
   // Note that we are making use of a level-3 heading because VSCode renders
   // level 1 and 2 headers in a huge font, see
   // https://github.com/microsoft/vscode/issues/88417 for details.
index 866ff97..54901fa 100644 (file)
@@ -601,6 +601,19 @@ TEST(Hover, NoHover) {
       R"cpp(// non-named decls don't get hover. Don't crash!
             ^static_assert(1, "");
           )cpp",
+      R"cpp(// non-evaluatable expr
+          template <typename T> void foo() {
+            (void)[[size^of]](T);
+          })cpp",
+      // literals
+      "auto x = t^rue;",
+      "auto x = '^A';",
+      "auto x = ^(int){42};",
+      "auto x = ^42.;",
+      "auto x = ^42.0i;",
+      "auto x = ^42;",
+      "auto x = ^nullptr;",
+      "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1501,6 +1514,26 @@ TEST(Hover, All) {
             HI.Name = "cls<cls<cls<int> > >";
             HI.Documentation = "type of nested templates.";
           }},
+      {
+          R"cpp(// sizeof expr
+          void foo() {
+            (void)[[size^of]](char);
+          })cpp",
+          [](HoverInfo &HI) {
+            HI.Name = "expression";
+            HI.Type = "unsigned long";
+            HI.Value = "1";
+          }},
+      {
+          R"cpp(// alignof expr
+          void foo() {
+            (void)[[align^of]](char);
+          })cpp",
+          [](HoverInfo &HI) {
+            HI.Name = "expression";
+            HI.Type = "unsigned long";
+            HI.Value = "1";
+          }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.