[clang] 2221987 - [clang][Interp] Add scopes to conditional operator subexpressions (#104418)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 15 23:37:50 PDT 2024
Author: Timm Baeder
Date: 2024-08-16T08:37:46+02:00
New Revision: 22219873987587d8b5d793ab5dea982a0887ac7c
URL: https://github.com/llvm/llvm-project/commit/22219873987587d8b5d793ab5dea982a0887ac7c
DIFF: https://github.com/llvm/llvm-project/commit/22219873987587d8b5d793ab5dea982a0887ac7c.diff
LOG: [clang][Interp] Add scopes to conditional operator subexpressions (#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.
Added:
Modified:
clang/lib/AST/Interp/Compiler.cpp
clang/lib/AST/Interp/Compiler.h
clang/test/AST/Interp/records.cpp
Removed:
################################################################################
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, "");
+}
More information about the cfe-commits
mailing list