[clang] [Clang][Concepts] Fix the constraint equivalence checking involving parameter packs (PR #102131)
Matheus Izvekov via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 26 11:31:49 PDT 2024
================
@@ -972,8 +972,30 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
// equivalence.
LocalInstantiationScope ScopeForParameters(S);
if (auto *FD = DeclInfo.getDecl()->getAsFunction())
- for (auto *PVD : FD->parameters())
- ScopeForParameters.InstantiatedLocal(PVD, PVD);
+ for (auto *PVD : FD->parameters()) {
+ if (!PVD->isParameterPack()) {
+ ScopeForParameters.InstantiatedLocal(PVD, PVD);
+ continue;
+ }
+ // This is hacky: we're mapping the parameter pack to a size-of-1 argument
+ // to avoid building SubstTemplateTypeParmPackTypes for
+ // PackExpansionTypes. The SubstTemplateTypeParmPackType node would
+ // otherwise reference the AssociatedDecl of the template arguments, which
+ // is, in this case, the template declaration.
+ //
+ // However, as we're also calculating the redeclarations of the template,
+ // the canonical declarations thereof are actually themselves at the
+ // moment. So if we didn't expand these packs, we would end up with an
+ // incorrect profile difference because we will be profiling the
+ // canonical types!
+ //
+ // FIXME: Improve the "no-transform" machinery in FindInstantiatedDecl so
+ // that we can eliminate the Scope in the cases where the declarations are
+ // not necessarily instantiated. It would also benefit the noexcept
+ // specifier comparison.
----------------
mizvekov wrote:
I think the solution in this patch improves on the existing one, even if interim, as the idea is to not have TransformDecl change these inserted declarations at all, which we failed to do for packs. So I like it more, as it just adds to existing workarounds, instead of adding a new one.
https://github.com/llvm/llvm-project/pull/102131
More information about the cfe-commits
mailing list