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

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked an inline comment as done.
erichkeane added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:1962-1964
+  QualType TransformElaboratedType(TypeLocBuilder &TLB, ElaboratedTypeLoc TL) {
+    return TransformType(TLB, TL.getNamedTypeLoc());
+  }
----------------
rsmith wrote:
> Removing the qualifier seems like it might work out OK in practice, given that we've already resolved the name to a specific declaration, but removing the elaborated type keyword (eg, `struct X` -> `X`) seems wrong to me.
> 
> If the problem is that expanding the typedef is breaking the invariants of `NestedNameSpecifier`, I'd be concerned that there are other cases where we're breaking those invariants in addition to this one. For example, do we have the same problem with `DependentNameType`?
> 
> Perhaps we should be special-casing transform of nested name specifiers instead. In your example below, it would seem more correct to transform `typename STy::Child` -> `typename PR46111::template S<T>::Child`. But that doesn't work in general; consider `typedef struct PR46111::S<T> STy;` for example.
> 
> One possible option would be to change `TransformTypedefType` to wrap its resulting type in a `TypeofType` type. That would give us a consistent transformation pattern: `typename STy::Child` -> `typename __typeof(<substituted typedef>)::Child`. But it's a bit hacky. It would be cleaner in some sense for `TransformTypedefType` to actually produce a new `TypedefDecl` (as if by instantiating the member typedef), and to produce a new `TypedefType` referring to the transformed typedef declaration.
> 
> Out of the available options, transforming the typedef declaration is, I think, my favored choice so far. (In the absence of a better `DeclContext` for it, I think we could set its `DeclContext` to be the `CXXDeductionGuideDecl`. That would at least have the right template parameters etc. in scope. We'll need to delay parenting it until after we create the deduction guide, perhaps by initially creating it as a child of the TUDecl then reparenting it, like we do for function parameters.)
> If the problem is that expanding the typedef is breaking the invariants of NestedNameSpecifier, I'd be concerned that there are other cases where we're breaking those invariants in addition to this one. For example, do we have the same problem with DependentNameType?

I'm not sure, this is the only one I saw with my reproducer.  I've tried to get DependentNameType to cause an issue, but I cannot come up with a reproducer that has this issue, which is likely due to my lack of knowledge on deduction guides.

>It would be cleaner in some sense for TransformTypedefType to actually produce a new TypedefDecl (as if by instantiating the member typedef), and to produce a new TypedefType referring to the transformed typedef declaration.

I'll look into this.  It doesn't SOUND too difficult to deal with the parent management, though I'm having trouble figuring out how to get this to transform, nor which TypeSouceInfo to use to construct it.

Presumably, it should be an anonymous typdef with empty location data, but the rest isn't particularly clear.  If you have suggestions, I'd definitely appreciate them, otherwise I'll continue trying to get a functional patch together.





Repository:
  rC Clang

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

https://reviews.llvm.org/D80743