denik added inline comments.
================ Comment at: clang/lib/Sema/SemaExprMember.cpp:528-529 + if (Field->getDeclName() == NameInfo.getName()) { + if (const PackExpansionType *PET = + dyn_cast<PackExpansionType>(Field->getType())) { + isMemberPack = true; ---------------- if (isa<PackExpansionType>(Field->getType())) { ... ================ Comment at: clang/lib/Sema/SemaExprMember.cpp:1018 if (R.empty()) { + // The member is expaned from member pack if its name has `@` in it. In this + // case a failed name lookup is not an error, but represents it's at the end ---------------- expanded ================ Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3289 + // Rename the expanded fields to avoid ambiguous names in + // name lookup. The renamed fields need to be invisible to users so an + // `@` is added to the name. ---------------- nit: Maybe `inaccessible by`? ================ Comment at: clang/lib/Sema/TreeTransform.h:4177 + "trying to expand non-pack member access"); + std::string UnExpanedNameStr = + MemberExpr->getMemberNameInfo().getName().getAsString(); ---------------- Expanded ================ Comment at: clang/lib/Sema/TreeTransform.h:4198-4199 + MemberExpr->getMemberNameInfo().getInfo()); + TemplateArgumentListInfo TemplateArgs = TemplateArgumentListInfo(); + MemberExpr->copyTemplateArgumentsInto(TemplateArgs); + auto *ExpandedMemberExpr = CXXDependentScopeMemberExpr::Create( ---------------- Why is this in a loop? ================ Comment at: clang/lib/Sema/TreeTransform.h:4208 + + Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), Arg); + ExprResult Out = getDerived().TransformExpr(ExpandedMemberExpr); ---------------- Can we add a check that Arg never exceeds the number of arguments in the full expansion (is it Expansion->getNumExpansions()?) ================ Comment at: clang/test/CodeGenCXX/data_member_packs.cpp:76 + // CHECK: double @_Z3sumIJilfdEEDaDpT_(i32 noundef %ts, i64 noundef %ts1, float noundef %ts3, double noundef %ts5) + S4<int, long>{}.sum_pack<float, double>(s7); + return 0; ---------------- This is a good test case! I would also add a test for the partial expansion of the member pack. And also checking the right order of the fields. Something like: ``` template<typename T> constexpr auto foo(T t) { return t; } template<typename T, typename ... Ts> constexpr auto foo(T t, Ts ... ts) { return t + 2 * foo(ts...); } S1<int, int, int> s = {1,2,3}; static_assert(foo(s.ts...) == 17); ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158006/new/ https://reviews.llvm.org/D158006 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits