[libcxx-commits] [PATCH] D127695: WIP: clang: Implement Template Specialization Resugaring

[Matheus Izvekov via Phabricator via libcxx-commits]

mizvekov marked 2 inline comments as done.
mizvekov added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:534-540
+  QualType TransformSubstTemplateTypeParmType(TypeLocBuilder &TLB,
+                                              SubstTemplateTypeParmTypeLoc TL) {
+    QualType QT = TL.getType();
+    const SubstTemplateTypeParmType *T = TL.getTypePtr();
+    Decl *ReplacedDecl = T->getReplacedDecl();
+
+    Optional<unsigned> PackIndex = T->getPackIndex();
----------------
davrec wrote:
> davrec wrote:
> > Haven't thought this through fully, but: would the following make D128113 (storing the pack index in the STTPT or introducing a new sugar type) unnecessary?
> > ```
> > map<pair<Decl *, unsigned>, Optional<unsigned>> CurPackIndices;
> > QualType TransformSubstTemplateTypeParmType(TypeLocBuilder &TLB,
> >                                               SubstTemplateTypeParmTypeLoc TL) {
> >    QualType QT = TL.getType();
> >    const SubstTemplateTypeParmType *T = TL.getTypePtr();
> >    Decl *ReplacedDecl = T->getReplacedDecl();
> >    
> >    Optional<unsigned> &PackIndex = CurPackIndices[{ReplacedDecl, T->getIndex()}];
> >    if (!PackIndex && T->getReplacedParameter()->isParameterPack())
> >      PackIndex = 0;
> >     
> >    ...
> > 
> >    if (PackIndex)
> >      ++PackIndex;
> >      // ^ maybe reset to zero if > pack size, if we might be resugaring multiple expansions
> >    return QT;
> > }
> > ```
> Disregard above - upon further thought this does not improve the things; there still isn't enough info about the substitutions.  I.e. the issue is with substitutions, not the parameter declarations for which they are substituted.  So a sugar node wrapping the STTPTs to represent each expansion instance really is needed.  Then when we have that I think we could map from those to their current pack indices per the above to infer the pack indices.
> 
> For this sugar node, maybe we could just modify `SubstTemplateTypeParmPackType` so it is not just canonical, but can also be sugar wrapping substituted STTPTs, as opposed to introducing a new Subst* type class?
I will reply to this on https://reviews.llvm.org/D128113.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127695