[PATCH] D80743: (PR46111) Desugar Elaborated types in Deduction Guides.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D80743#2074121 <https://reviews.llvm.org/D80743#2074121>, @erichkeane wrote:

> @rsmith I think this implements what you've suggested?


Yes, thanks.

> This seems to 'work' for a small subset of works, but it doesn't properly register the typedef to the LocalInstantiationScope, so the normal template instantiation (like here https://github.com/llvm/llvm-project/blob/master/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp#L156) ends up hitting the 'findInstantiationOf' assert here: https://github.com/llvm/llvm-project/blob/master/clang/lib/Sema/SemaTemplateInstantiate.cpp#L3564

Hm. Yes, that'd be a problem. To fix that, we'll need to transform the typedef declarations as part of transforming the deduction guide. Roughly-speaking, we've transformed

  template<typename T> struct A {
    using U = X1<T>;
    A(X2<U>);
  };

into something like

  template<typename T> A(X2<U>) -> A<T> {
    using U = X1<T>;
  };

... and the problem is that we need to substitute into the new `using U =` typedef before we form a use of `U` when producing the substituted type of the alias declaration.

There are a couple of ways we could approach this:

1. when we start transforming a deduction guide, we can walk its `TypedefDecl` children, and instantiate those into new typedef declarations immediately, so that later references to them work
2. when we reach the reference to the typedef declaration (in `FindInstantiatedDecl`, when we see a declaration whose decl context is a deduction guide), instantiate the declaration then

These are distinguishable by an example such as:

  template<typename T> struct Type { using type = typename T::type; };
  struct B { using type = int; };
  template<typename T = B> struct A {
    using type = Type<T>::type;
    A(T, typename T::type, type); // #1
    A(...); // #2
  };
  A a(1, 2, 3);

For which option 1 would result in a hard error when substituting T=int into the injected typedef for #1, but option 2 would accept, because substitution stops at the 'typename T::type' without ever reaching the typedef.

For that reason I think option 2 is the way to go. We already have some logic for this in `FindInstantiatedDecl`; search there for `NeedInstantiate` and try extending it from local classes and enums to also cover typedefs whose decl context is a deduction guide. (You'll need a matching change in `findInstantiationOf` to return `nullptr` in that case. This code doesn't seem very well factored...)



================
Comment at: clang/lib/Sema/SemaTemplate.cpp:1969-1970
+        SemaRef.getASTContext(),
+        SemaRef.getASTContext().getTranslationUnitDecl(), SourceLocation(),
+        SourceLocation(), TL.getTypedefNameDecl()->getIdentifier(), TSI);
+    MaterializedTypedefs.push_back(Decl);
----------------
I think it would be a good idea to retain the source location from the original typedef (and to produce a `TypeAliasDecl` if that's what we started with); if any diagnostics end up referring to this typedef, we will want them to point to the one in the class template (and describe it as a "typedef declaration" or "alias declaration" as appropriate).


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

https://reviews.llvm.org/D80743