Skip to content

[Clang] Reconsider the timing of instantiation of local constexpr lambdas #98758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 45 additions & 10 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#include "clang/Sema/Template.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLForwardCompat.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/ConvertUTF.h"
Expand Down Expand Up @@ -5340,20 +5341,31 @@ bool Sema::CheckCXXDefaultArgExpr(SourceLocation CallLoc, FunctionDecl *FD,

struct ImmediateCallVisitor : public RecursiveASTVisitor<ImmediateCallVisitor> {
const ASTContext &Context;
ImmediateCallVisitor(const ASTContext &Ctx) : Context(Ctx) {}
llvm::SmallPtrSetImpl<const FunctionDecl *> *ReferencedFunctions;

ImmediateCallVisitor(const ASTContext &Ctx,
llvm::SmallPtrSetImpl<const FunctionDecl *>
*ReferencedFunctions = nullptr)
: Context(Ctx), ReferencedFunctions(ReferencedFunctions) {}
Comment on lines +5346 to +5349
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are going to reuse this object, we should rename it - or maybe we need a separate class

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A separate class would avoid the pointer to SmallPtrSetImpl, which is not great


bool HasImmediateCalls = false;
bool shouldVisitImplicitCode() const { return true; }

bool VisitCallExpr(CallExpr *E) {
if (const FunctionDecl *FD = E->getDirectCallee())
if (const FunctionDecl *FD = E->getDirectCallee()) {
HasImmediateCalls |= FD->isImmediateFunction();
if (ReferencedFunctions)
ReferencedFunctions->insert(FD);
}
return RecursiveASTVisitor<ImmediateCallVisitor>::VisitStmt(E);
}

bool VisitCXXConstructExpr(CXXConstructExpr *E) {
if (const FunctionDecl *FD = E->getConstructor())
if (const FunctionDecl *FD = E->getConstructor()) {
HasImmediateCalls |= FD->isImmediateFunction();
if (ReferencedFunctions)
ReferencedFunctions->insert(FD);
}
return RecursiveASTVisitor<ImmediateCallVisitor>::VisitStmt(E);
}

Expand Down Expand Up @@ -16983,6 +16995,30 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
SmallVector<PartialDiagnosticAt, 8> Notes;
EvalResult.Diag = &Notes;

// Check if the expression refers to local functions yet to be instantiated.
// If so, instantiate them now, as the constant evaluation requires the
// function definition.
if (!PendingLocalImplicitInstantiations.empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why we check for not having pending instantiations ?

llvm::SmallPtrSet<const FunctionDecl *, 4> ReferencedFunctions;
ImmediateCallVisitor V(getASTContext(), &ReferencedFunctions);
V.TraverseStmt(E);

auto Pred = [&](PendingImplicitInstantiation Pair) {
ValueDecl *VD = Pair.first;
return isa<FunctionDecl>(VD) &&
ReferencedFunctions.contains(cast<FunctionDecl>(VD));
};
// Workaround: A lambda with captures cannot be copy-assigned, which is
// required by llvm::make_filter_range().
llvm::function_ref<bool(PendingImplicitInstantiation)> PredRef = Pred;

auto R =
llvm::make_filter_range(PendingLocalImplicitInstantiations, PredRef);
LocalEagerInstantiationScope InstantiateReferencedLocalFunctions(*this);
PendingLocalImplicitInstantiations = {R.begin(), R.end()};
InstantiateReferencedLocalFunctions.perform();
}

// Try to evaluate the expression, and produce diagnostics explaining why it's
// not a constant expression as a side-effect.
bool Folded =
Expand Down Expand Up @@ -17938,17 +17974,16 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func,

if (FirstInstantiation || TSK != TSK_ImplicitInstantiation ||
Func->isConstexpr()) {
if (Func->isConstexpr())
if (isa<CXXRecordDecl>(Func->getDeclContext()) &&
cast<CXXRecordDecl>(Func->getDeclContext())->isLocalClass() &&
CodeSynthesisContexts.size())
PendingLocalImplicitInstantiations.push_back(
std::make_pair(Func, PointOfInstantiation));
Comment on lines +17977 to +17981
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would benefit from a comment

else if (Func->isConstexpr())
// Do not defer instantiations of constexpr functions, to avoid the
// expression evaluator needing to call back into Sema if it sees a
// call to such a function.
InstantiateFunctionDefinition(PointOfInstantiation, Func);
else if (isa<CXXRecordDecl>(Func->getDeclContext()) &&
cast<CXXRecordDecl>(Func->getDeclContext())
->isLocalClass() &&
CodeSynthesisContexts.size())
PendingLocalImplicitInstantiations.push_back(
std::make_pair(Func, PointOfInstantiation));
else {
Func->setInstantiationIsPending(true);
PendingInstantiations.push_back(
Expand Down
26 changes: 26 additions & 0 deletions clang/test/SemaTemplate/instantiate-local-class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -532,4 +532,30 @@ int main() {

} // namespace GH35052

namespace GH98526 {

template <typename F> struct recursive_lambda {
template <typename... Args> auto operator()(Args &&...args) const {
return fn(*this, args...);
}
F fn;
};

template <typename F> recursive_lambda(F) -> recursive_lambda<F>;

struct Tree {
Tree *left, *right;
};

int sumSize(Tree *tree) {
auto accumulate =
recursive_lambda{[&](auto &self_fn, Tree *element_node) -> int {
return 1 + self_fn(tree->left) + self_fn(tree->right);
}};

return accumulate(tree);
}

} // namespace GH98526

#endif
Loading