[PATCH] D158006: [Clang][WIP]Experimental implementation of data member packs in dependent context.

[Zenong Zhang via Phabricator via cfe-commits]

SlaterLatiao marked 4 inline comments as done.
SlaterLatiao added inline comments.


================
Comment at: clang/lib/Sema/TreeTransform.h:4171
 
+      if (CXXDependentScopeMemberExpr *MemberExpr =
+              dyn_cast<CXXDependentScopeMemberExpr>(Pattern)) {
----------------
dblaikie wrote:
> Might be worth pulling out this new code as a separate function - the `continue` at the end is many lines away from the start or end of the loop, making control flow a bit hard to follow (probably the existing code could do with some of this massaging even before/regardless of the new code you're adding here)
Made the logic a separate function and added a comment to explain the `continue`. 


================
Comment at: clang/lib/Sema/TreeTransform.h:4198-4199
+              MemberExpr->getMemberNameInfo().getInfo());
+          TemplateArgumentListInfo TemplateArgs = TemplateArgumentListInfo();
+          MemberExpr->copyTemplateArgumentsInto(TemplateArgs);
+          auto *ExpandedMemberExpr = CXXDependentScopeMemberExpr::Create(
----------------
denik wrote:
> Why is this in a loop?
Each field need to have its own `MemberAccessExpression`. This loop iterates through the expanded fields.


================
Comment at: clang/lib/Sema/TreeTransform.h:4208
+
+          Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), Arg);
+          ExprResult Out = getDerived().TransformExpr(ExpandedMemberExpr);
----------------
denik wrote:
> Can we add a check that Arg never exceeds the number of arguments in the full expansion (is it Expansion->getNumExpansions()?)
`Expansion->getNumExpansions()` returns the number of expansions only when we know it. In this case it returns `std::nullopt`.
A more legit way to check `Arg` is to search for the specialization of  the struct based on the template args of the current context, but I didn't find a proper way to achieve that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158006