[PATCH] D73723: [clangd][Hover] Handle uninstantiated default args

2020-01-31 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62330 tests passed, 0 failed 
and 838 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73723



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


[PATCH] D73723: [clangd][Hover] Handle uninstantiated default args

2020-01-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c903d0373ff: [clangd][Hover] Handle uninstantiated default 
args (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73723

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
@@ -1609,6 +1609,22 @@
 HI.Type = "unsigned long";
 HI.Value = "1";
   }},
+  {
+  R"cpp(
+  template 
+  void foo(const T& = T()) {
+[[f^oo]]<>(3);
+  })cpp",
+  [](HoverInfo ) {
+HI.Name = "foo";
+HI.Kind = index::SymbolKind::Function;
+HI.Type = "void (const int &)";
+HI.ReturnType = "void";
+HI.Parameters = {
+{std::string("const int &"), llvm::None, std::string("T()")}};
+HI.Definition = "template <> void foo(const int &)";
+HI.NamespaceScope = "";
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -250,6 +250,20 @@
   });
 }
 
+// Default argument might exist but be unavailable, in the case of unparsed
+// arguments for example. This function returns the default argument if it is
+// available.
+const Expr *getDefaultArg(const ParmVarDecl *PVD) {
+  // Default argument can be unparsed or uninstatiated. For the former we
+  // can't do much, as token information is only stored in Sema and not
+  // attached to the AST node. For the latter though, it is safe to proceed as
+  // the expression is still valid.
+  if (!PVD->hasDefaultArg() || PVD->hasUnparsedDefaultArg())
+return nullptr;
+  return PVD->hasUninstantiatedDefaultArg() ? 
PVD->getUninstantiatedDefaultArg()
+: PVD->getDefaultArg();
+}
+
 // Populates Type, ReturnType, and Parameters for function-like decls.
 void fillFunctionTypeAndParams(HoverInfo , const Decl *D,
const FunctionDecl *FD,
@@ -269,10 +283,10 @@
 }
 if (!PVD->getName().empty())
   P.Name = PVD->getNameAsString();
-if (PVD->hasDefaultArg()) {
+if (const Expr *DefArg = getDefaultArg(PVD)) {
   P.Default.emplace();
   llvm::raw_string_ostream Out(*P.Default);
-  PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
+  DefArg->printPretty(Out, nullptr, Policy);
 }
   }
 


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1609,6 +1609,22 @@
 HI.Type = "unsigned long";
 HI.Value = "1";
   }},
+  {
+  R"cpp(
+  template 
+  void foo(const T& = T()) {
+[[f^oo]]<>(3);
+  })cpp",
+  [](HoverInfo ) {
+HI.Name = "foo";
+HI.Kind = index::SymbolKind::Function;
+HI.Type = "void (const int &)";
+HI.ReturnType = "void";
+HI.Parameters = {
+{std::string("const int &"), llvm::None, std::string("T()")}};
+HI.Definition = "template <> void foo(const int &)";
+HI.NamespaceScope = "";
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -250,6 +250,20 @@
   });
 }
 
+// Default argument might exist but be unavailable, in the case of unparsed
+// arguments for example. This function returns the default argument if it is
+// available.
+const Expr *getDefaultArg(const ParmVarDecl *PVD) {
+  // Default argument can be unparsed or uninstatiated. For the former we
+  // can't do much, as token information is only stored in Sema and not
+  // attached to the AST node. For the latter though, it is safe to proceed as
+  // the expression is still valid.
+  if (!PVD->hasDefaultArg() || PVD->hasUnparsedDefaultArg())
+return nullptr;
+  return PVD->hasUninstantiatedDefaultArg() ? PVD->getUninstantiatedDefaultArg()
+: PVD->getDefaultArg();
+}
+
 // Populates Type, ReturnType, and Parameters for function-like decls.
 void fillFunctionTypeAndParams(HoverInfo , const Decl *D,
 

[PATCH] D73723: [clangd][Hover] Handle uninstantiated default args

2020-01-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 241707.
kadircet marked an inline comment as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73723

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
@@ -1609,6 +1609,22 @@
 HI.Type = "unsigned long";
 HI.Value = "1";
   }},
+  {
+  R"cpp(
+  template 
+  void foo(const T& = T()) {
+[[f^oo]]<>(3);
+  })cpp",
+  [](HoverInfo ) {
+HI.Name = "foo";
+HI.Kind = index::SymbolKind::Function;
+HI.Type = "void (const int &)";
+HI.ReturnType = "void";
+HI.Parameters = {
+{std::string("const int &"), llvm::None, std::string("T()")}};
+HI.Definition = "template <> void foo(const int &)";
+HI.NamespaceScope = "";
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -250,6 +250,20 @@
   });
 }
 
+// Default argument might exist but be unavailable, in the case of unparsed
+// arguments for example. This function returns the default argument if it is
+// available.
+const Expr *getDefaultArg(const ParmVarDecl *PVD) {
+  // Default argument can be unparsed or uninstatiated. For the former we
+  // can't do much, as token information is only stored in Sema and not
+  // attached to the AST node. For the latter though, it is safe to proceed as
+  // the expression is still valid.
+  if (!PVD->hasDefaultArg() || PVD->hasUnparsedDefaultArg())
+return nullptr;
+  return PVD->hasUninstantiatedDefaultArg() ? 
PVD->getUninstantiatedDefaultArg()
+: PVD->getDefaultArg();
+}
+
 // Populates Type, ReturnType, and Parameters for function-like decls.
 void fillFunctionTypeAndParams(HoverInfo , const Decl *D,
const FunctionDecl *FD,
@@ -269,10 +283,10 @@
 }
 if (!PVD->getName().empty())
   P.Name = PVD->getNameAsString();
-if (PVD->hasDefaultArg()) {
+if (const Expr *DefArg = getDefaultArg(PVD)) {
   P.Default.emplace();
   llvm::raw_string_ostream Out(*P.Default);
-  PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
+  DefArg->printPretty(Out, nullptr, Policy);
 }
   }
 


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1609,6 +1609,22 @@
 HI.Type = "unsigned long";
 HI.Value = "1";
   }},
+  {
+  R"cpp(
+  template 
+  void foo(const T& = T()) {
+[[f^oo]]<>(3);
+  })cpp",
+  [](HoverInfo ) {
+HI.Name = "foo";
+HI.Kind = index::SymbolKind::Function;
+HI.Type = "void (const int &)";
+HI.ReturnType = "void";
+HI.Parameters = {
+{std::string("const int &"), llvm::None, std::string("T()")}};
+HI.Definition = "template <> void foo(const int &)";
+HI.NamespaceScope = "";
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -250,6 +250,20 @@
   });
 }
 
+// Default argument might exist but be unavailable, in the case of unparsed
+// arguments for example. This function returns the default argument if it is
+// available.
+const Expr *getDefaultArg(const ParmVarDecl *PVD) {
+  // Default argument can be unparsed or uninstatiated. For the former we
+  // can't do much, as token information is only stored in Sema and not
+  // attached to the AST node. For the latter though, it is safe to proceed as
+  // the expression is still valid.
+  if (!PVD->hasDefaultArg() || PVD->hasUnparsedDefaultArg())
+return nullptr;
+  return PVD->hasUninstantiatedDefaultArg() ? PVD->getUninstantiatedDefaultArg()
+: PVD->getDefaultArg();
+}
+
 // Populates Type, ReturnType, and Parameters for function-like decls.
 void fillFunctionTypeAndParams(HoverInfo , const Decl *D,
const FunctionDecl 

[PATCH] D73723: [clangd][Hover] Handle uninstantiated default args

2020-01-31 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang-tools-extra/clangd/Hover.cpp:276
+// we only print the expression.
+if (PVD->hasDefaultArg() && !PVD->hasUnparsedDefaultArg()) {
   P.Default.emplace();

NIT: maybe move this into a helper function?
```
Expr* getDefaultArgForPresentation(ParmVarDecl*);
```
I imagine this could be useful in other contexts too.

PS: As usual, naming is not my strong side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73723



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


[PATCH] D73723: [clangd][Hover] Handle uninstantiated default args

2020-01-30 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62330 tests passed, 0 failed 
and 838 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73723



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


[PATCH] D73723: [clangd][Hover] Handle uninstantiated default args

2020-01-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Default args might exist but be unparsed or uninstantiated.
getDefaultArg asserts on those. This patch makes sure we don't crash in such
scenarios.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73723

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
@@ -1609,6 +1609,22 @@
 HI.Type = "unsigned long";
 HI.Value = "1";
   }},
+  {
+  R"cpp(
+  template 
+  void foo(const T& = T()) {
+[[f^oo]]<>(3);
+  })cpp",
+  [](HoverInfo ) {
+HI.Name = "foo";
+HI.Kind = index::SymbolKind::Function;
+HI.Type = "void (const int &)";
+HI.ReturnType = "void";
+HI.Parameters = {
+{std::string("const int &"), llvm::None, std::string("T()")}};
+HI.Definition = "template <> void foo(const int &)";
+HI.NamespaceScope = "";
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -269,10 +269,17 @@
 }
 if (!PVD->getName().empty())
   P.Name = PVD->getNameAsString();
-if (PVD->hasDefaultArg()) {
+// Default argument can be unparsed or uninstatiated. For the former we
+// can't do much as token information is only stored in Sema and not
+// attached to the AST node. For the latter though, it is safe to proceed 
as
+// we only print the expression.
+if (PVD->hasDefaultArg() && !PVD->hasUnparsedDefaultArg()) {
   P.Default.emplace();
   llvm::raw_string_ostream Out(*P.Default);
-  PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
+  const Expr *Init = PVD->hasUninstantiatedDefaultArg()
+ ? PVD->getUninstantiatedDefaultArg()
+ : PVD->getDefaultArg();
+  Init->printPretty(Out, nullptr, Policy);
 }
   }
 


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1609,6 +1609,22 @@
 HI.Type = "unsigned long";
 HI.Value = "1";
   }},
+  {
+  R"cpp(
+  template 
+  void foo(const T& = T()) {
+[[f^oo]]<>(3);
+  })cpp",
+  [](HoverInfo ) {
+HI.Name = "foo";
+HI.Kind = index::SymbolKind::Function;
+HI.Type = "void (const int &)";
+HI.ReturnType = "void";
+HI.Parameters = {
+{std::string("const int &"), llvm::None, std::string("T()")}};
+HI.Definition = "template <> void foo(const int &)";
+HI.NamespaceScope = "";
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -269,10 +269,17 @@
 }
 if (!PVD->getName().empty())
   P.Name = PVD->getNameAsString();
-if (PVD->hasDefaultArg()) {
+// Default argument can be unparsed or uninstatiated. For the former we
+// can't do much as token information is only stored in Sema and not
+// attached to the AST node. For the latter though, it is safe to proceed as
+// we only print the expression.
+if (PVD->hasDefaultArg() && !PVD->hasUnparsedDefaultArg()) {
   P.Default.emplace();
   llvm::raw_string_ostream Out(*P.Default);
-  PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
+  const Expr *Init = PVD->hasUninstantiatedDefaultArg()
+ ? PVD->getUninstantiatedDefaultArg()
+ : PVD->getDefaultArg();
+  Init->printPretty(Out, nullptr, Policy);
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits