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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
