[clangd] Add Value field to HoverInfo

[kadircet]

Summary

Put a symbols value information which is deduced from initializer
expression into HoverInfo struct.

[sammccall]

clang-tools-extra/clangd/XRefs.cpp

722

do we want the initializer both here and in Definition?

I suspect we'd like to turn off one or the other when both are present.

Looking at the examples, I think it's clearer to keep a non-evaluated initializer as part of the Definition (which is mostly as-written), and only include Value if it's evaluated.

733

why not print the non-evaluated init if it's dependent?

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

917

constexpr may not be required here, I think?
At least the docs suggest evaluation ignores language semantics like this.

953

Hmm, actually I'm not sure whether this is TypeLoc vs Decl here. Rather this should be a DeclRefExpr to the VarDecl (result) in the *expanded* CXXRecordDecl.

This suggests that maybe the isValueDependent check isn't quite right - the initializer expression is value dependent as spelled, but in the instantiation it's resolved. Not sure if this is fixable. What happens if you comment out the check?

978

whoa, that's... weird. But fine, I think.

(It would be nice to elide the &[] if the index is zero, or maybe print as "1234" + 2 etc)

[kadircet]

• Address comments

clang-tools-extra/clangd/XRefs.cpp

733

not-necessary anymore, since we only generate value if it can be evaluated.

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

953

yes, and the problem lies in the IndexingAPI, inside handleDeclOccurence implicit instantiations are resolved into templated decls. therefore we lose the template params :/

we can't call expression evaluator on value dependent expressions, there is an assertion in the beginning of EvaluateAsRvalue.

978

do you think it is worth changing that in printpretty (either the defaults or via some PrintingPolicy option)?

[sammccall]

Implementation LG though please do check that the new test results look useful and not too duplicative

clang-tools-extra/clangd/XRefs.cpp

723

Maybe add a FIXME about doing this for arbitrary expressions (like sizeof and function calls), not just VarDecl?

clang-tools-extra/clangd/XRefs.h

106

if exists -> if available?

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

673

are these actually still emitted, or tests not updated?

Some of them (e.g. the lambda below!) don't seem useful

953

OK, let's leave this to later. Can you change the FIXME comment? I think it's misleading as it stands

978

Let's leave it for now and see if it actually proves annoying.

[kadircet]

• Address comments

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

673

oops, somehow I've deleted the expect line that checks the values :/

917

apparently this constexpr is necessary, otherwise evaluation fails.

clang-tools-extra/clangd/XRefs.h

struct HoverInfo {
   /// Set only for variables.
   llvm::Optional<std::string> Type;
   /// Set for functions and lambadas.
   llvm::Optional<std::string> ReturnType;
   /// Set for functions, lambdas and macros with parameters.
   llvm::Optional<std::vector<Param>> Parameters;
   /// 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;
 
   /// Produce a user-readable information.
   FormattedString present() const;
 };
 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/XRefs.cpp

else
       FD->getType().print(TypeOS, Policy);
     // FIXME: handle variadics.
   } else if (const auto *VD = dyn_cast<ValueDecl>(D)) {
     HI.Type.emplace();
     llvm::raw_string_ostream OS(*HI.Type);
     VD->getType().print(OS, Policy);
   }
 
+  // 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),
+                               Var->getType());
+      }
+    }
+  }
+
   HI.Definition = printDefinition(D);
   return HI;
 }
 
 /// Generate a \p Hover object given the type \p T.
 static HoverInfo getHoverContents(QualType T, const Decl *D,
                                   ASTContext &ASTCtx) {
   HoverInfo HI;

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

const std::function<void(HoverInfo &)> ExpectedBuilder;
          HI.LocalScope = "(anonymous struct)::";
          HI.Name = "bar";
          HI.Kind = SymbolKind::Field;
          HI.Definition = "int bar";
          HI.Type = "int";
        }},
       // Variable with template type
       {R"cpp(
-          template <typename T, class... Ts> class Foo {};
-          Foo<int, char, bool> [[fo^o]];
+          template <typename T, class... Ts> class Foo { public: Foo(int); };
+          Foo<int, char, bool> [[fo^o]] = Foo<int, char, bool>(5);
           )cpp",
        [](HoverInfo &HI) {
          HI.NamespaceScope = "";
          HI.Name = "foo";
          HI.Kind = SymbolKind::Variable;
-         HI.Definition = "Foo<int, char, bool> foo";
+         HI.Definition = "Foo<int, char, bool> foo = Foo<int, char, bool>(5)";
          HI.Type = "Foo<int, char, bool>";
        }},
       // Implicit template instantiation
       {R"cpp(
           template <typename T> class vector{};
           [[vec^tor]]<int> foo;
           )cpp",
        [](HoverInfo &HI) {
          HI.NamespaceScope = "";
          HI.Name = "vector";
void foo())cpp";
         // Best MACRO ever.
         #define MACRO(x,y,z) void foo(x, y, z);
         [[MAC^RO]](int, double d, bool z = false);
         )cpp",
        [](HoverInfo &HI) {
          HI.Name = "MACRO", HI.Kind = SymbolKind::String,
          HI.Definition = "#define MACRO(x, y, z) void foo(x, y, z);";
        }},
+
+      // constexprs
+      {R"cpp(
+        constexpr int add(int a, int b) { return a + b; }
+        int [[b^ar]] = add(1, 2);
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "bar";
+         HI.Definition = "int bar = add(1, 2)";
+         HI.Kind = SymbolKind::Variable;
+         HI.Type = "int";
+         HI.NamespaceScope = "";
+         HI.Value = "3";
+       }},
+      {R"cpp(
+        int [[b^ar]] = sizeof(char);
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "bar";
+         HI.Definition = "int bar = sizeof(char)";
+         HI.Kind = SymbolKind::Variable;
+         HI.Type = "int";
+         HI.NamespaceScope = "";
+         HI.Value = "1";
+       }},
+      {R"cpp(
+        template<int a, int b> struct Add {
+          static constexpr int result = a + b;
+        };
+        int [[ba^r]] = Add<1, 2>::result;
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "bar";
+         HI.Definition = "int bar = Add<1, 2>::result";
+         HI.Kind = SymbolKind::Variable;
+         HI.Type = "int";
+         HI.NamespaceScope = "";
+         HI.Value = "3";
+       }},
+      // FIXME: We should use the Decl referenced, even if it comes from an
+      // implicit instantiation.
+      {R"cpp(
+        template<int a, int b> struct Add {
+          static constexpr int result = a + b;
+        };
+        int bar = Add<1, 2>::[[resu^lt]];
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "result";
+         HI.Definition = "static constexpr int result = a + b";
+         HI.Kind = SymbolKind::Property;
+         HI.Type = "const int";
+         HI.NamespaceScope = "";
+         HI.LocalScope = "Add::";
+       }},
+      {R"cpp(
+        const char *[[ba^r]] = "1234";
+        )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "bar";
+         HI.Definition = "const char *bar = \"1234\"";
+         HI.Kind = SymbolKind::Variable;
+         HI.Type = "const char *";
+         HI.NamespaceScope = "";
+         HI.Value = "&\"1234\"[0]";
+       }},
   };
   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");
     auto AST = TU.build();
for (const auto &Case : Cases) {
     EXPECT_EQ(H->Kind, Expected.Kind);
     EXPECT_EQ(H->Documentation, Expected.Documentation);
     EXPECT_EQ(H->Definition, Expected.Definition);
     EXPECT_EQ(H->Type, Expected.Type);
     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);
   }
 } // namespace clang
 
 TEST(Hover, All) {
   struct OneTest {
     StringRef Input;
     StringRef ExpectedHover;
   };