[clang-tools-extra] 6a3cffc - [clangd] Disable printing of Value for tag-types on hover

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-06-24T17:32:19+02:00
New Revision: 6a3cffce3e8076d0608daf951575d5f4d6a333a6

URL: https://github.com/llvm/llvm-project/commit/6a3cffce3e8076d0608daf951575d5f4d6a333a6
DIFF: https://github.com/llvm/llvm-project/commit/6a3cffce3e8076d0608daf951575d5f4d6a333a6.diff

LOG: [clangd] Disable printing of Value for tag-types on hover

Summary: This is both confusing and crashy.

Reviewers: sammccall

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

Tags: #clang

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 2861d63d4d00..3d5d002f8ded 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -341,7 +341,10 @@ llvm::Optional<std::string> printExprValue(const Expr *E,
       T->isFunctionReferenceType())
     return llvm::None;
   // Attempt to evaluate. If expr is dependent, evaluation crashes!
-  if (E->isValueDependent() || !E->EvaluateAsRValue(Constant, Ctx))
+  if (E->isValueDependent() || !E->EvaluateAsRValue(Constant, Ctx) ||
+      // Disable printing for record-types, as they are usually confusing and
+      // might make clang crash while printing the expressions.
+      Constant.Val.isStruct() || Constant.Val.isUnion())
     return llvm::None;
 
   // Show enums symbolically, not numerically like APValue::printPretty().
@@ -353,7 +356,7 @@ llvm::Optional<std::string> printExprValue(const Expr *E,
       if (ECD->getInitVal() == Val)
         return llvm::formatv("{0} ({1})", ECD->getNameAsString(), Val).str();
   }
-  return Constant.Val.getAsString(Ctx, E->getType());
+  return Constant.Val.getAsString(Ctx, T);
 }
 
 llvm::Optional<std::string> printExprValue(const SelectionTree::Node *N,

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index dc818ea66193..c6c0daa6380e 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -696,6 +696,18 @@ class Foo {})cpp";
          HI.Parameters->back().Name = "v";
          HI.AccessSpecifier = "public";
        }},
+      {// Field type initializer.
+       R"cpp(
+          struct X { int x = 2; };
+          X ^[[x]];
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.Name = "x";
+         HI.Kind = index::SymbolKind::Variable;
+         HI.NamespaceScope = "";
+         HI.Definition = "X x";
+         HI.Type = "struct X";
+       }},
   };
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Code);
@@ -978,13 +990,14 @@ TEST(Hover, All) {
             HI.LocalScope = "Foo::";
             HI.Type = "int";
             HI.Definition = "int x";
-            HI.Value = "{1}";
+            // FIXME: Initializer for x is a DesignatedInitListExpr, hence it is
+            // of struct type and omitted.
           }},
       {
           R"cpp(// Field, field designator
-            struct Foo { int x; };
+            struct Foo { int x; int y; };
             int main() {
-              Foo bar = { .^[[x]] = 2 };
+              Foo bar = { .^[[x]] = 2, .y = 2 };
             }
           )cpp",
           [](HoverInfo &HI) {
@@ -994,7 +1007,6 @@ TEST(Hover, All) {
             HI.LocalScope = "Foo::";
             HI.Type = "int";
             HI.Definition = "int x";
-            HI.Value = "{2}";
           }},
       {
           R"cpp(// Method call
@@ -1592,7 +1604,6 @@ TEST(Hover, All) {
             HI.LocalScope = "test::";
             HI.Type = "struct Test &&";
             HI.Definition = "Test &&test = {}";
-            HI.Value = "{}";
           }},
       {
           R"cpp(// auto on alias
@@ -1651,7 +1662,6 @@ TEST(Hover, All) {
             HI.NamespaceScope = "";
             HI.Name = "foo";
             HI.Type = "cls<cls<cls<int>>>";
-            HI.Value = "{}";
           }},
       {
           R"cpp(// type of nested templates.