[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 ↗

(On Diff #204750)

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 ↗

(On Diff #204750)

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

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

920 ↗

(On Diff #204750)

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

956 ↗

(On Diff #204750)

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?

981 ↗

(On Diff #204750)

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 ↗

(On Diff #204750)

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

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

956 ↗

(On Diff #204750)

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.

981 ↗

(On Diff #204750)

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 ↗

(On Diff #205530)

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 ↗

(On Diff #205530)

if exists -> if available?

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

673 ↗

(On Diff #205530)

are these actually still emitted, or tests not updated?

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

956 ↗

(On Diff #204750)

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

981 ↗

(On Diff #204750)

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

[kadircet]

• Address comments

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

673 ↗

(On Diff #205530)

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

920 ↗

(On Diff #204750)

apparently this constexpr is necessary, otherwise evaluation fails.

clang-tools-extra/trunk/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/trunk/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/trunk/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) {
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;
   };