[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474149.
v1nh1shungry added a comment.

Improve the patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137650/new/

https://reviews.llvm.org/D137650

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


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
       "auto x = ^42.0i;",
       "auto x = ^42;",
       "auto x = ^nullptr;",
-      "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,11 @@
          HI.Type = "char";
          HI.Value = "65 (0x41)";
        }},
+      {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+       [](HoverInfo &HI) {
+         HI.Name = "String Literal";
+         HI.Type = "const char[14]";
+       }},
       {
           R"cpp(// Local variable
             int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -804,12 +804,23 @@
 llvm::Optional<HoverInfo> getHoverContents(const Expr *E, ParsedAST &AST,
                                            const PrintingPolicy &PP,
                                            const SymbolIndex *Index) {
+  HoverInfo HI;
+
   // 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))
+  if (isLiteral(E)) {
+    // Generate hover info for string literals showing
+    // its character's type and length
+    if (const StringLiteral *SL = dyn_cast<StringLiteral>(E)) {
+      HoverInfo::PrintedType PT;
+      PT.Type = SL->getType().getAsString(PP);
+      HI.Type = PT;
+      HI.Name = "String Literal";
+      return HI;
+    }
     return llvm::None;
+  }
 
-  HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast<CXXThisExpr>(E))
     return getThisExprHoverContents(CTE, AST.getASTContext(), PP);



Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
       "auto x = ^42.0i;",
       "auto x = ^42;",
       "auto x = ^nullptr;",
-      "auto x = ^\"asdf\";",
   };
 
   for (const auto &Test : Tests) {
@@ -1326,6 +1325,11 @@
          HI.Type = "char";
          HI.Value = "65 (0x41)";
        }},
+      {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+       [](HoverInfo &HI) {
+         HI.Name = "String Literal";
+         HI.Type = "const char[14]";
+       }},
       {
           R"cpp(// Local variable
             int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -804,12 +804,23 @@
 llvm::Optional<HoverInfo> getHoverContents(const Expr *E, ParsedAST &AST,
                                            const PrintingPolicy &PP,
                                            const SymbolIndex *Index) {
+  HoverInfo HI;
+
   // 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))
+  if (isLiteral(E)) {
+    // Generate hover info for string literals showing
+    // its character's type and length
+    if (const StringLiteral *SL = dyn_cast<StringLiteral>(E)) {
+      HoverInfo::PrintedType PT;
+      PT.Type = SL->getType().getAsString(PP);
+      HI.Type = PT;
+      HI.Name = "String Literal";
+      return HI;
+    }
     return llvm::None;
+  }
 
-  HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast<CXXThisExpr>(E))
     return getThisExprHoverContents(CTE, AST.getASTContext(), PP);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits