[PATCH] D101741: [clangd] Improve resolution of static method calls in HeuristicResolver

[Nathan Ridge via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf976b9997ee5: [clangd] Improve resolution of static method 
calls in HeuristicResolver (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101741

Files:
  clang-tools-extra/clangd/HeuristicResolver.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -285,7 +285,6 @@
   void bar(A a, T t) {
 nonmember($par1[[t]]);
 a.member($par2[[t]]);
-// FIXME: This one does not work yet.
 A::static_member($par3[[t]]);
 // We don't want to arbitrarily pick between
 // "anInt" or "aDouble", so just show no hint.
@@ -294,7 +293,8 @@
 };
   )cpp",
ExpectedHint{"par1: ", "par1"},
-   ExpectedHint{"par2: ", "par2"});
+   ExpectedHint{"par2: ", "par2"},
+   ExpectedHint{"par3: ", "par3"});
 }
 
 TEST(ParameterHints, VariadicFunction) {
Index: clang-tools-extra/clangd/HeuristicResolver.cpp
===
--- clang-tools-extra/clangd/HeuristicResolver.cpp
+++ clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -16,6 +16,7 @@
 
 // Convenience lambdas for use as the 'Filter' parameter of
 // HeuristicResolver::resolveDependentMember().
+const auto NoFilter = [](const NamedDecl *D) { return true; };
 const auto NonStaticFilter = [](const NamedDecl *D) {
   return D->isCXXInstanceMember();
 };
@@ -90,6 +91,28 @@
 
 std::vector HeuristicResolver::resolveMemberExpr(
 const CXXDependentScopeMemberExpr *ME) const {
+  // If the expression has a qualifier, first try resolving the member
+  // inside the qualifier's type.
+  // Note that we cannot use a NonStaticFilter in either case, for a couple
+  // of reasons:
+  //   1. It's valid to access a static member using instance member syntax,
+  //  e.g. `instance.static_member`.
+  //   2. We can sometimes get a CXXDependentScopeMemberExpr for static
+  //  member syntax too, e.g. if `X::static_member` occurs inside
+  //  an instance method, it's represented as a CXXDependentScopeMemberExpr
+  //  with `this` as the base expression as `X` as the qualifier
+  //  (which could be valid if `X` names a base class after instantiation).
+  if (NestedNameSpecifier *NNS = ME->getQualifier()) {
+if (const Type *QualifierType = resolveNestedNameSpecifierToType(NNS)) {
+  auto Decls =
+  resolveDependentMember(QualifierType, ME->getMember(), NoFilter);
+  if (!Decls.empty())
+return Decls;
+}
+  }
+
+  // If that didn't yield any results, try resolving the member inside
+  // the expression's base type.
   const Type *BaseType = ME->getBaseType().getTypePtrOrNull();
   if (ME->isArrow()) {
 BaseType = getPointeeType(BaseType);
@@ -105,7 +128,7 @@
   BaseType = resolveExprToType(Base);
 }
   }
-  return resolveDependentMember(BaseType, ME->getMember(), NonStaticFilter);
+  return resolveDependentMember(BaseType, ME->getMember(), NoFilter);
 }
 
 std::vector HeuristicResolver::resolveDeclRefExpr(


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -285,7 +285,6 @@
   void bar(A a, T t) {
 nonmember($par1[[t]]);
 a.member($par2[[t]]);
-// FIXME: This one does not work yet.
 A::static_member($par3[[t]]);
 // We don't want to arbitrarily pick between
 // "anInt" or "aDouble", so just show no hint.
@@ -294,7 +293,8 @@
 };
   )cpp",
ExpectedHint{"par1: ", "par1"},
-   ExpectedHint{"par2: ", "par2"});
+   ExpectedHint{"par2: ", "par2"},
+   ExpectedHint{"par3: ", "par3"});
 }
 
 TEST(ParameterHints, VariadicFunction) {
Index: clang-tools-extra/clangd/HeuristicResolver.cpp
===
--- clang-tools-extra/clangd/HeuristicResolver.cpp
+++ clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -16,6 +16,7 @@
 
 // Convenience lambdas for use as the 'Filter' parameter of
 // HeuristicResolver::resolveDependentMember().
+const auto NoFilter = [](const NamedDecl *D) { return true; };
 const auto NonStaticFilter = [](const NamedDecl *D) {
   return D->isCXXInstanceMember();
 };
@@ -90,6 +91,28 @@
 
 std::vector HeuristicResolver::resolveMemberExpr(
 const 

[PATCH] D101741: [clangd] Improve resolution of static method calls in HeuristicResolver

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Nice! Thanks for the reminder on this one, I lost track of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101741

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

[PATCH] D101741: [clangd] Improve resolution of static method calls in HeuristicResolver

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
nridge requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101741

Files:
  clang-tools-extra/clangd/HeuristicResolver.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -263,7 +263,6 @@
   void bar(A a, T t) {
 nonmember($par1[[t]]);
 a.member($par2[[t]]);
-// FIXME: This one does not work yet.
 A::static_member($par3[[t]]);
 // We don't want to arbitrarily pick between
 // "anInt" or "aDouble", so just show no hint.
@@ -272,7 +271,8 @@
 };
   )cpp",
ExpectedHint{"par1: ", "par1"},
-   ExpectedHint{"par2: ", "par2"});
+   ExpectedHint{"par2: ", "par2"},
+   ExpectedHint{"par3: ", "par3"});
 }
 
 TEST(ParameterHints, VariadicFunction) {
Index: clang-tools-extra/clangd/HeuristicResolver.cpp
===
--- clang-tools-extra/clangd/HeuristicResolver.cpp
+++ clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -16,6 +16,7 @@
 
 // Convenience lambdas for use as the 'Filter' parameter of
 // HeuristicResolver::resolveDependentMember().
+const auto NoFilter = [](const NamedDecl *D) { return true; };
 const auto NonStaticFilter = [](const NamedDecl *D) {
   return D->isCXXInstanceMember();
 };
@@ -90,6 +91,28 @@
 
 std::vector HeuristicResolver::resolveMemberExpr(
 const CXXDependentScopeMemberExpr *ME) const {
+  // If the expression has a qualifier, first try resolving the member
+  // inside the qualifier's type.
+  // Note that we cannot use a NonStaticFilter in either case, for a couple
+  // of reasons:
+  //   1. It's valid to access a static member using instance member syntax,
+  //  e.g. `instance.static_member`.
+  //   2. We can sometimes get a CXXDependentScopeMemberExpr for static
+  //  member syntax too, e.g. if `X::static_member` occurs inside
+  //  an instance method, it's represented as a CXXDependentScopeMemberExpr
+  //  with `this` as the base expression as `X` as the qualifier
+  //  (which could be valid if `X` names a base class after instantiation).
+  if (NestedNameSpecifier *NNS = ME->getQualifier()) {
+if (const Type *QualifierType = resolveNestedNameSpecifierToType(NNS)) {
+  auto Decls =
+  resolveDependentMember(QualifierType, ME->getMember(), NoFilter);
+  if (!Decls.empty())
+return Decls;
+}
+  }
+
+  // If that didn't yield any results, try resolving the member inside
+  // the expression's base type.
   const Type *BaseType = ME->getBaseType().getTypePtrOrNull();
   if (ME->isArrow()) {
 BaseType = getPointeeType(BaseType);
@@ -105,7 +128,7 @@
   BaseType = resolveExprToType(Base);
 }
   }
-  return resolveDependentMember(BaseType, ME->getMember(), NonStaticFilter);
+  return resolveDependentMember(BaseType, ME->getMember(), NoFilter);
 }
 
 std::vector HeuristicResolver::resolveDeclRefExpr(


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -263,7 +263,6 @@
   void bar(A a, T t) {
 nonmember($par1[[t]]);
 a.member($par2[[t]]);
-// FIXME: This one does not work yet.
 A::static_member($par3[[t]]);
 // We don't want to arbitrarily pick between
 // "anInt" or "aDouble", so just show no hint.
@@ -272,7 +271,8 @@
 };
   )cpp",
ExpectedHint{"par1: ", "par1"},
-   ExpectedHint{"par2: ", "par2"});
+   ExpectedHint{"par2: ", "par2"},
+   ExpectedHint{"par3: ", "par3"});
 }
 
 TEST(ParameterHints, VariadicFunction) {
Index: clang-tools-extra/clangd/HeuristicResolver.cpp
===
--- clang-tools-extra/clangd/HeuristicResolver.cpp
+++ clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -16,6 +16,7 @@
 
 // Convenience lambdas for use as the 'Filter' parameter of
 // HeuristicResolver::resolveDependentMember().
+const auto NoFilter = [](const NamedDecl *D) { return true; };
 const auto NonStaticFilter = [](const NamedDecl *D) {
   return D->isCXXInstanceMember();
 };
@@ -90,6 +91,28 @@
 
 std::vector HeuristicResolver::resolveMemberExpr(
 const CXXDependentScopeMemberExpr *ME) const {
+  // If the expression has