Skip to content

Commit

Permalink
[Clang][Concepts] Fix the constraint equivalence checking involving p…
Browse files Browse the repository at this point in the history
…arameter packs (#102131)

We established an instantiation scope in order for constraint
equivalence checking to properly map the uninstantiated parameters.

That mechanism mapped even packs to themselves. Consequently, parameter
packs e.g. appearing in a function call, were not expanded. So they
would end up becoming `SubstTemplateTypeParmPackType`s that circularly
depend on the canonical declaration of the function template, which is
not yet determined, hence the spurious error.

No release note as I plan to backport it to 19.

Fixes #101735

---------

Co-authored-by: cor3ntin <[email protected]>
(cherry picked from commit e6974da)
  • Loading branch information
zyn0217 authored and tru committed Sep 24, 2024
1 parent 1720219 commit 3e512ba
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 2 deletions.
26 changes: 24 additions & 2 deletions clang/lib/Sema/SemaConcept.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -969,8 +969,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 are in the process of comparing potential
// re-declarations, the canonical declaration is the declaration itself at
// this point. 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.
ScopeForParameters.MakeInstantiatedLocalArgPack(PVD);
ScopeForParameters.InstantiatedLocalPackArg(PVD, PVD);
}

std::optional<Sema::CXXThisScopeRAII> ThisScope;

Expand Down
23 changes: 23 additions & 0 deletions clang/test/SemaTemplate/concepts-out-of-line-def.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -599,3 +599,26 @@ template <class DerT>
unsigned long DerivedCollection<DerTs...>::index() {}

} // namespace GH72557

namespace GH101735 {

template <class, class>
concept True = true;

template <typename T>
class A {
template <typename... Ts>
void method(Ts&... ts)
requires requires (T t) {
{ t.method(static_cast<Ts &&>(ts)...) } -> True<void>;
};
};

template <typename T>
template <typename... Ts>
void A<T>::method(Ts&... ts)
requires requires (T t) {
{ t.method(static_cast<Ts &&>(ts)...) } -> True<void>;
} {}

}

0 comments on commit 3e512ba

Please sign in to comment.