llvmorg-github-actions[bot] wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Christian Kandeler (ckandeler)

<details>
<summary>Changes</summary>

`HeuristicResolver` already falls back to a template parameter's default 
argument when there is nothing else it can do with the parameter, but it only 
did so when it met the parameter directly. Two common ways of arriving at one 
were missed, both of which occur in `std::vector`:

- through a dependent name — libstdc++ has `typedef _Alloc allocator_type;`, so 
`std::vector&lt;T&gt;::allocator_type` resolves to a bare template parameter;
- through the sugar of the member typedef itself, e.g. when it is the declared 
return type of a member function.

Note that neither can be done on a canonicalized type: a canonical 
`TemplateTypeParmType` does not retain its `TemplateTypeParmDecl` (see 
`ASTContext::getTemplateTypeParmType()`), so there is no default argument left 
to consult. The typedef's underlying type as written is used instead.

The second commit addresses a related gap on the same example: a member 
re-exported from a dependent base class by a using declaration, such as 
libstdc++'s `using _Base::get_allocator;`, is found as an 
`UnresolvedUsingValueDecl`, which has no function type of its own. 
`resolveTypeOfCallExpr()` could therefore not determine the type of a call to 
it. It now resolves such a declaration to what it names, using the existing 
`resolveUsingValueDecl()`.

Together these make code completion work for, for example:

```c++
template &lt;class T&gt; void f(std::vector&lt;T&gt; &amp;v) {
  typename std::vector&lt;T&gt;::allocator_type al = v.get_allocator();
  al. // now offers std::allocator's members
}
```

The third commit adds tests for two cases the heuristic still does not handle, 
so that they are recorded and will be noticed if that ever changes: a template 
parameter with no default argument at all, and a parameter whose default 
argument is not the one we end up looking at, because looking a name up in a 
base class's primary template discards the arguments the derived class passes 
to it.

---
Full diff: https://github.com/llvm/llvm-project/pull/223667.diff


2 Files Affected:

- (modified) clang/lib/Sema/HeuristicResolver.cpp (+61-17) 
- (modified) clang/unittests/Sema/HeuristicResolverTest.cpp (+130) 


``````````diff
diff --git a/clang/lib/Sema/HeuristicResolver.cpp 
b/clang/lib/Sema/HeuristicResolver.cpp
index 8a799a1b46436..29ece2de5fbe8 100644
--- a/clang/lib/Sema/HeuristicResolver.cpp
+++ b/clang/lib/Sema/HeuristicResolver.cpp
@@ -124,6 +124,30 @@ TemplateName getReferencedTemplateName(const Type *T) {
   return TemplateName();
 }
 
+// If `T` is a template parameter with a default argument, return that default
+// argument, otherwise return a null QualType.
+// We can't do anything useful with a template parameter itself (e.g. we cannot
+// look up member names inside it), so where one turns up, using its default
+// argument is a reasonable heuristic: it's what the parameter will be bound to
+// unless the instantiation site says otherwise.
+// Note that `T` must not be canonicalized: a canonical TemplateTypeParmType
+// does not retain its TemplateTypeParmDecl, and so has no default argument to
+// offer.
+QualType getDefaultTemplateArgument(QualType T) {
+  // Use getAs() rather than a dyn_cast, so that we see through sugar such as a
+  // member typedef naming the parameter (e.g. `typedef A allocator_type;`).
+  const auto *TTPT = T.isNull() ? nullptr : T->getAs<TemplateTypeParmType>();
+  if (!TTPT)
+    return QualType();
+  const auto *TTPD = TTPT->getDecl();
+  if (!TTPD || !TTPD->hasDefaultArgument())
+    return QualType();
+  const auto &DefaultArg = TTPD->getDefaultArgument().getArgument();
+  if (DefaultArg.getKind() != TemplateArgument::Type)
+    return QualType();
+  return DefaultArg.getAsType();
+}
+
 // Helper function for HeuristicResolver::resolveDependentMember()
 // which takes a possibly-dependent type `T` and heuristically
 // resolves it to a CXXRecordDecl in which we can try name lookup.
@@ -136,8 +160,22 @@ TagDecl 
*HeuristicResolverImpl::resolveTypeToTagDecl(QualType QT) {
   T = T->getCanonicalTypeInternal().getTypePtr();
 
   if (const auto *DNT = T->getAs<DependentNameType>()) {
-    T = resolveDeclsToType(resolveDependentNameType(DNT), Ctx)
-            .getTypePtrOrNull();
+    auto Decls = resolveDependentNameType(DNT);
+    QualType Resolved = resolveDeclsToType(Decls, Ctx);
+    // `Resolved` is canonical, so if the name refers to a member typedef of a
+    // template parameter (as `vector<T>::allocator_type` does), it has lost 
the
+    // TemplateTypeParmDecl we would need to fall back to the parameter's
+    // default argument. Consult the typedef's underlying type as written
+    // instead.
+    if (Decls.size() == 1) {
+      if (const auto *TND = dyn_cast<TypedefNameDecl>(Decls[0])) {
+        if (QualType Default =
+                getDefaultTemplateArgument(TND->getUnderlyingType());
+            !Default.isNull())
+          Resolved = Default;
+      }
+    }
+    T = Resolved.getTypePtrOrNull();
     if (!T)
       return nullptr;
     T = T->getCanonicalTypeInternal().getTypePtr();
@@ -246,19 +284,9 @@ QualType HeuristicResolverImpl::simplifyType(QualType 
Type, const Expr *E,
         }
       }
     }
-    if (const auto *TTPT = dyn_cast_if_present<TemplateTypeParmType>(T.Type)) {
-      // We can't do much useful with a template parameter (e.g. we cannot look
-      // up member names inside it). However, if the template parameter has a
-      // default argument, as a heuristic we can replace T with the default
-      // argument type.
-      if (const auto *TTPD = TTPT->getDecl()) {
-        if (TTPD->hasDefaultArgument()) {
-          const auto &DefaultArg = TTPD->getDefaultArgument().getArgument();
-          if (DefaultArg.getKind() == TemplateArgument::Type) {
-            return {DefaultArg.getAsType()};
-          }
-        }
-      }
+    if (QualType Default = getDefaultTemplateArgument(T.Type);
+        !Default.isNull()) {
+      return {Default};
     }
 
     // Similarly, heuristically replace a template template parameter with its
@@ -358,8 +386,24 @@ QualType 
HeuristicResolverImpl::resolveTypeOfCallExpr(const CallExpr *CE) {
   // resolveExprToType(CE->getCallee()) would bail in the case of multiple
   // overloads, as it can't produce a single type for them. We can be more
   // permissive here, and allow multiple overloads with a common return type.
-  std::vector<const NamedDecl *> CalleeDecls =
-      resolveExprToDecls(CE->getCallee());
+  std::vector<const NamedDecl *> CalleeDecls;
+  for (const NamedDecl *D : resolveExprToDecls(CE->getCallee())) {
+    // The callee may be re-exported from a dependent base class by a using
+    // declaration, e.g. libstdc++'s `vector` has `using 
_Base::get_allocator;`.
+    // Such a declaration has no function type of its own, so replace it with
+    // what it names. That may be an overload set, but a conflicting return 
type
+    // within it is handled below just as it is between two distinct callee
+    // declarations, so simply flatten it in. Only one level is looked through;
+    // a using declaration naming another one is not resolved.
+    if (const auto *UUVD = dyn_cast<UnresolvedUsingValueDecl>(D)) {
+      auto Underlying = resolveUsingValueDecl(UUVD);
+      CalleeDecls.insert(CalleeDecls.end(), Underlying.begin(),
+                         Underlying.end());
+      continue;
+    }
+    CalleeDecls.push_back(D);
+  }
+
   QualType CommonReturnType;
   for (const NamedDecl *CalleeDecl : CalleeDecls) {
     QualType CalleeType = resolveDeclToType(CalleeDecl, Ctx);
diff --git a/clang/unittests/Sema/HeuristicResolverTest.cpp 
b/clang/unittests/Sema/HeuristicResolverTest.cpp
index c592e74c41a95..e4720b840c64b 100644
--- a/clang/unittests/Sema/HeuristicResolverTest.cpp
+++ b/clang/unittests/Sema/HeuristicResolverTest.cpp
@@ -580,6 +580,136 @@ TEST(HeuristicResolver, 
MemberExpr_DefaultTemplateArgument_Recursive) {
       cxxMethodDecl(hasName("foo")).bind("output"));
 }
 
+TEST(HeuristicResolver, MemberExpr_DefaultTemplateArgument_MemberTypedef) {
+  std::string Code = R"cpp(
+    struct Default {
+      void foo();
+    };
+    template <typename T, typename A = Default>
+    struct S {
+      typedef A type;
+    };
+    template <typename T>
+    void bar() {
+      typename S<T>::type t;
+      t.foo();
+    }
+  )cpp";
+  // Test resolution of "foo" in "t.foo()", where the type of "t" resolves
+  // to the template parameter "A" via the member typedef "type".
+  expectResolution(
+      Code, &HeuristicResolver::resolveMemberExpr,
+      cxxDependentScopeMemberExpr(hasMemberName("foo")).bind("input"),
+      cxxMethodDecl(hasName("foo")).bind("output"));
+}
+
+TEST(HeuristicResolver, MemberExpr_DefaultTemplateArgument_ReturnType) {
+  std::string Code = R"cpp(
+    struct Default {
+      void foo();
+    };
+    template <typename T, typename A = Default>
+    struct S {
+      typedef A type;
+      type get();
+    };
+    template <typename T>
+    void bar(S<T> s) {
+      s.get().foo();
+    }
+  )cpp";
+  // Test resolution of "foo" in "s.get().foo()", where the return type of
+  // "get()" resolves to the template parameter "A" via the member typedef
+  // "type".
+  expectResolution(
+      Code, &HeuristicResolver::resolveMemberExpr,
+      cxxDependentScopeMemberExpr(hasMemberName("foo")).bind("input"),
+      cxxMethodDecl(hasName("foo")).bind("output"));
+}
+
+TEST(HeuristicResolver, 
MemberExpr_MemberTypedefWithoutDefaultTemplateArgument) {
+  std::string Code = R"cpp(
+    template <typename T>
+    struct S {
+      typedef T type;
+    };
+    template <typename T>
+    void bar() {
+      typename S<T>::type t;
+      t.foo();
+    }
+  )cpp";
+  // Test that "foo" in "t.foo()" does not resolve: "S<T>::type" names S's own
+  // parameter T, which has no default argument to fall back on, and nothing
+  // else says what T will be. This is the member-typedef analogue of
+  // `std::vector<T>::value_type`.
+  expectResolution(
+      Code, &HeuristicResolver::resolveMemberExpr,
+      cxxDependentScopeMemberExpr(hasMemberName("foo")).bind("input"));
+}
+
+TEST(HeuristicResolver, MemberExpr_CallOfUsingDeclFromDependentBase) {
+  std::string Code = R"cpp(
+    struct Result {
+      void foo();
+    };
+    template <typename T>
+    struct Base {
+      Result get();
+    };
+    template <typename T>
+    struct Derived : Base<T> {
+      typedef Base<T> _Base;
+      using _Base::get;
+    };
+    template <typename T>
+    void bar(Derived<T> d) {
+      d.get().foo();
+    }
+  )cpp";
+  // Test resolution of "foo" in "d.get().foo()", where "get" is found as a
+  // using declaration naming a member of a dependent base class.
+  expectResolution(
+      Code, &HeuristicResolver::resolveMemberExpr,
+      cxxDependentScopeMemberExpr(hasMemberName("foo")).bind("input"),
+      cxxMethodDecl(hasName("foo")).bind("output"));
+}
+
+TEST(HeuristicResolver, MemberExpr_DefaultTemplateArgumentNotSeenByBase) {
+  std::string Code = R"cpp(
+    struct Result {
+      void foo();
+    };
+    template <typename T, typename A>
+    struct Base {
+      typedef A type;
+      type get();
+    };
+    template <typename T, typename A = Result>
+    struct Derived : Base<T, A> {
+      typedef Base<T, A> _Base;
+      using _Base::get;
+    };
+    template <typename T>
+    void bar(Derived<T> d) {
+      d.get().foo();
+    }
+  )cpp";
+  // Test that "foo" in "d.get().foo()" does not resolve. Looking "get" up in
+  // Base's primary template discards the arguments Derived passes to Base, so
+  // the return type is Base's own "A", which -- unlike Derived's -- has no
+  // default argument to fall back on. Resolving this would require propagating
+  // template arguments to base classes, which this file's "look the name up in
+  // the primary template" approach does not do.
+  //
+  // This mirrors libstdc++, where vector's allocator parameter is defaulted 
but
+  // _Vector_base's is not, and is why completion after `v.get_allocator().`
+  // does not work even though `vector<T>::allocator_type` itself resolves.
+  expectResolution(
+      Code, &HeuristicResolver::resolveMemberExpr,
+      cxxDependentScopeMemberExpr(hasMemberName("foo")).bind("input"));
+}
+
 TEST(HeuristicResolver, MemberExpr_DefaultTemplateTemplateArgument) {
   std::string Code = R"cpp(
     template <typename T>

``````````

</details>


https://github.com/llvm/llvm-project/pull/223667
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to