rsmith added a comment.
I think this is the wrong solution; I think the assertion is correct and is
observing a problem elsewhere. Imagine if we had something like:
template<typename T>
void f() {
auto a = [] (auto recurse, auto x) {
auto b = [] (auto) {
refer to local decl in a
};
if constexpr (recurse)
a(false_type(), b)
else
x(0)
}
a(true_type(), a)
}
We would then have a local instantiation scope stack that looks like:
f<int>
f<int>::a::operator()<true_type, f<int>::a>
f<int>::a::operator()<false_type, f<int>::a::operator()<true_type,
f<int>::a>::b>
f<int>::a::operator()<true_type, f<int>::a>::operator()::b::operator()<int>
... so the reference from the `true_type` version of `b` to the local decl in
`a` would find the local from the enclosing `false_type` class, which is wrong.
That's the kind of bug that I think this assertion is defending against, and it
looks like we have that bug here. (As it happens, this will never actually
cause problems, because all references from b to enclosing locals already got
substituted when instantiating the `operator()` function template, but that's
actually kind of the point -- the local instantiation scope stack is wrong.)
I think the real problem is that the local instantiation scope for the
instantiation of a generic lambda's call operator from the instantiated call
operator template should never be merged with its parent. Merging the contexts
cannot be necessary -- we can have left the parent instantiation before we
instantiate a specialization of the call operator template, so there cannot be
any references from the call operator template to enclosing locals that require
substitution. I think the bug is in `InstantiateFunctionDefinition`, somewhere
around line 4863:
// Introduce a new scope where local variable instantiations will be
// recorded, unless we're actually a member function within a local
// class, in which case we need to merge our results with the parent
// scope (of the enclosing function).
bool MergeWithParentScope = false;
if (CXXRecordDecl *Rec = dyn_cast<CXXRecordDecl>(Function->getDeclContext()))
MergeWithParentScope = Rec->isLocalClass();
LocalInstantiationScope Scope(*this, MergeWithParentScope);
While we do need the enclosing local instantiation scope when we're
instantiating a non-template member of a local class, or when we're
instantiating the definition of a member function template as a member function
template (that is, when instantiating a generic lambda to form a generic
lambda), we do not need the enclosing local instantiation scope when
re-substituting into the result of such a local substitution. That is, we do
not want a local instantiation scope if we're instantiating a function template
specialization, because the template we're instantiating already had references
to locals properly substituted. I'd try a change like this to the above code:
- MergeWithParentScope = Rec->isLocalClass();
+ MergeWithParentScope = Rec->isLocalClass() &&
!Function->isFunctionTemplateSpecialization();
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98068/new/
https://reviews.llvm.org/D98068
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
