https://github.com/tbaederr created
https://github.com/llvm/llvm-project/pull/104418
We would otherwise try to destroy the variables from both branches after the
conditional operator was done.
However, doing so breaks the case where we create internal temporaries. For
those, add allocateTemporary(), which attaches the lifetime of the temporary to
the topmost scope. Since they never have record type, this shouldn't create any
issues.
>From 0cea714e807f4863237eaa04c221e29e3ff26ed2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com>
Date: Thu, 15 Aug 2024 07:05:09 +0200
Subject: [PATCH] [clang][Interp] Add scopes to conditional operator
subexpressions
We would otherwise try to destroy the variables from both branches after
the conditional operator was done.
However, doing so breaks the case where we create internal temporaries.
For those, add allocateTemporary(), which attaches the lifetime of the
temporary to the topmost scope. Since they never have record type, this
shouldn't create any issues.
---
clang/lib/AST/Interp/Compiler.cpp | 63 +++++++++++++++++++++----------
clang/lib/AST/Interp/Compiler.h | 1 +
clang/test/AST/Interp/records.cpp | 26 +++++++++++++
3 files changed, 70 insertions(+), 20 deletions(-)
diff --git a/clang/lib/AST/Interp/Compiler.cpp
b/clang/lib/AST/Interp/Compiler.cpp
index d32b595f9f0724..5e1f507ca2b178 100644
--- a/clang/lib/AST/Interp/Compiler.cpp
+++ b/clang/lib/AST/Interp/Compiler.cpp
@@ -533,10 +533,8 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
// We're creating a complex value here, so we need to
// allocate storage for it.
if (!Initializing) {
- std::optional<unsigned> LocalIndex = allocateLocal(CE);
- if (!LocalIndex)
- return false;
- if (!this->emitGetPtrLocal(*LocalIndex, CE))
+ unsigned LocalIndex = allocateTemporary(CE);
+ if (!this->emitGetPtrLocal(LocalIndex, CE))
return false;
}
@@ -671,10 +669,8 @@ bool Compiler<Emitter>::VisitImaginaryLiteral(const
ImaginaryLiteral *E) {
return true;
if (!Initializing) {
- std::optional<unsigned> LocalIndex = allocateLocal(E);
- if (!LocalIndex)
- return false;
- if (!this->emitGetPtrLocal(*LocalIndex, E))
+ unsigned LocalIndex = allocateTemporary(E);
+ if (!this->emitGetPtrLocal(LocalIndex, E))
return false;
}
@@ -986,10 +982,8 @@ template <class Emitter>
bool Compiler<Emitter>::VisitComplexBinOp(const BinaryOperator *E) {
// Prepare storage for result.
if (!Initializing) {
- std::optional<unsigned> LocalIndex = allocateLocal(E);
- if (!LocalIndex)
- return false;
- if (!this->emitGetPtrLocal(*LocalIndex, E))
+ unsigned LocalIndex = allocateTemporary(E);
+ if (!this->emitGetPtrLocal(LocalIndex, E))
return false;
}
@@ -1045,10 +1039,7 @@ bool Compiler<Emitter>::VisitComplexBinOp(const
BinaryOperator *E) {
if (!LHSIsComplex) {
// This is using the RHS type for the fake-complex LHS.
- if (auto LHSO = allocateLocal(RHS))
- LHSOffset = *LHSO;
- else
- return false;
+ LHSOffset = allocateTemporary(RHS);
if (!this->emitGetPtrLocal(LHSOffset, E))
return false;
@@ -1881,15 +1872,26 @@ bool
Compiler<Emitter>::VisitAbstractConditionalOperator(
if (!this->jumpFalse(LabelFalse))
return false;
- if (!this->delegate(TrueExpr))
- return false;
+ {
+ LocalScope<Emitter> S(this);
+ if (!this->delegate(TrueExpr))
+ return false;
+ if (!S.destroyLocals())
+ return false;
+ }
+
if (!this->jump(LabelEnd))
return false;
this->emitLabel(LabelFalse);
- if (!this->delegate(FalseExpr))
- return false;
+ {
+ LocalScope<Emitter> S(this);
+ if (!this->delegate(FalseExpr))
+ return false;
+ if (!S.destroyLocals())
+ return false;
+ }
this->fallthrough(LabelEnd);
this->emitLabel(LabelEnd);
@@ -3549,6 +3551,27 @@ Compiler<Emitter>::allocateLocal(DeclTy &&Src, const
ValueDecl *ExtendingDecl) {
return Local.Offset;
}
+template <class Emitter>
+unsigned Compiler<Emitter>::allocateTemporary(const Expr *E) {
+ QualType Ty = E->getType();
+ assert(!Ty->isRecordType());
+
+ Descriptor *D = P.createDescriptor(
+ E, Ty.getTypePtr(), Descriptor::InlineDescMD, Ty.isConstQualified(),
+ /*IsTemporary=*/true, /*IsMutable=*/false, /*Init=*/nullptr);
+ assert(D);
+
+ Scope::Local Local = this->createLocal(D);
+ VariableScope<Emitter> *S = VarScope;
+ assert(S);
+ // Attach to topmost scope.
+ while (S->getParent())
+ S = S->getParent();
+ assert(S && !S->getParent());
+ S->addLocal(Local);
+ return Local.Offset;
+}
+
template <class Emitter>
const RecordType *Compiler<Emitter>::getRecordTy(QualType Ty) {
if (const PointerType *PT = dyn_cast<PointerType>(Ty))
diff --git a/clang/lib/AST/Interp/Compiler.h b/clang/lib/AST/Interp/Compiler.h
index 112219c49e8bdd..74bfce5f241f28 100644
--- a/clang/lib/AST/Interp/Compiler.h
+++ b/clang/lib/AST/Interp/Compiler.h
@@ -297,6 +297,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>,
bool>,
/// Allocates a space storing a local given its type.
std::optional<unsigned>
allocateLocal(DeclTy &&Decl, const ValueDecl *ExtendingDecl = nullptr);
+ unsigned allocateTemporary(const Expr *E);
private:
friend class VariableScope<Emitter>;
diff --git a/clang/test/AST/Interp/records.cpp
b/clang/test/AST/Interp/records.cpp
index f51f9771b38aaa..7e3cf5b94518f7 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -1627,3 +1627,29 @@ namespace DtorDestroysFieldsAfterSelf {
static_assert(foo() == 15);
}
#endif
+
+namespace ExprWithCleanups {
+ struct A { A(); ~A(); int get(); };
+ constexpr int get() {return false ? A().get() : 1;}
+ static_assert(get() == 1, "");
+
+
+ struct S {
+ int V;
+ constexpr S(int V) : V(V) {}
+ constexpr int get() {
+ return V;
+ }
+ };
+ constexpr int get(bool b) {
+ S a = b ? S(1) : S(2);
+
+ return a.get();
+ }
+ static_assert(get(true) == 1, "");
+ static_assert(get(false) == 2, "");
+
+
+ constexpr auto F = true ? 1i : 2i;
+ static_assert(F == 1i, "");
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
