Skip to content

Commit 2221987

Browse files
authored
[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.
1 parent 1900810 commit 2221987

File tree

3 files changed

+70
-20
lines changed

3 files changed

+70
-20
lines changed

clang/lib/AST/Interp/Compiler.cpp

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -533,10 +533,8 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
533533
// We're creating a complex value here, so we need to
534534
// allocate storage for it.
535535
if (!Initializing) {
536-
std::optional<unsigned> LocalIndex = allocateLocal(CE);
537-
if (!LocalIndex)
538-
return false;
539-
if (!this->emitGetPtrLocal(*LocalIndex, CE))
536+
unsigned LocalIndex = allocateTemporary(CE);
537+
if (!this->emitGetPtrLocal(LocalIndex, CE))
540538
return false;
541539
}
542540

@@ -671,10 +669,8 @@ bool Compiler<Emitter>::VisitImaginaryLiteral(const ImaginaryLiteral *E) {
671669
return true;
672670

673671
if (!Initializing) {
674-
std::optional<unsigned> LocalIndex = allocateLocal(E);
675-
if (!LocalIndex)
676-
return false;
677-
if (!this->emitGetPtrLocal(*LocalIndex, E))
672+
unsigned LocalIndex = allocateTemporary(E);
673+
if (!this->emitGetPtrLocal(LocalIndex, E))
678674
return false;
679675
}
680676

@@ -986,10 +982,8 @@ template <class Emitter>
986982
bool Compiler<Emitter>::VisitComplexBinOp(const BinaryOperator *E) {
987983
// Prepare storage for result.
988984
if (!Initializing) {
989-
std::optional<unsigned> LocalIndex = allocateLocal(E);
990-
if (!LocalIndex)
991-
return false;
992-
if (!this->emitGetPtrLocal(*LocalIndex, E))
985+
unsigned LocalIndex = allocateTemporary(E);
986+
if (!this->emitGetPtrLocal(LocalIndex, E))
993987
return false;
994988
}
995989

@@ -1045,10 +1039,7 @@ bool Compiler<Emitter>::VisitComplexBinOp(const BinaryOperator *E) {
10451039

10461040
if (!LHSIsComplex) {
10471041
// This is using the RHS type for the fake-complex LHS.
1048-
if (auto LHSO = allocateLocal(RHS))
1049-
LHSOffset = *LHSO;
1050-
else
1051-
return false;
1042+
LHSOffset = allocateTemporary(RHS);
10521043

10531044
if (!this->emitGetPtrLocal(LHSOffset, E))
10541045
return false;
@@ -1881,15 +1872,26 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator(
18811872
if (!this->jumpFalse(LabelFalse))
18821873
return false;
18831874

1884-
if (!this->delegate(TrueExpr))
1885-
return false;
1875+
{
1876+
LocalScope<Emitter> S(this);
1877+
if (!this->delegate(TrueExpr))
1878+
return false;
1879+
if (!S.destroyLocals())
1880+
return false;
1881+
}
1882+
18861883
if (!this->jump(LabelEnd))
18871884
return false;
18881885

18891886
this->emitLabel(LabelFalse);
18901887

1891-
if (!this->delegate(FalseExpr))
1892-
return false;
1888+
{
1889+
LocalScope<Emitter> S(this);
1890+
if (!this->delegate(FalseExpr))
1891+
return false;
1892+
if (!S.destroyLocals())
1893+
return false;
1894+
}
18931895

18941896
this->fallthrough(LabelEnd);
18951897
this->emitLabel(LabelEnd);
@@ -3549,6 +3551,27 @@ Compiler<Emitter>::allocateLocal(DeclTy &&Src, const ValueDecl *ExtendingDecl) {
35493551
return Local.Offset;
35503552
}
35513553

3554+
template <class Emitter>
3555+
unsigned Compiler<Emitter>::allocateTemporary(const Expr *E) {
3556+
QualType Ty = E->getType();
3557+
assert(!Ty->isRecordType());
3558+
3559+
Descriptor *D = P.createDescriptor(
3560+
E, Ty.getTypePtr(), Descriptor::InlineDescMD, Ty.isConstQualified(),
3561+
/*IsTemporary=*/true, /*IsMutable=*/false, /*Init=*/nullptr);
3562+
assert(D);
3563+
3564+
Scope::Local Local = this->createLocal(D);
3565+
VariableScope<Emitter> *S = VarScope;
3566+
assert(S);
3567+
// Attach to topmost scope.
3568+
while (S->getParent())
3569+
S = S->getParent();
3570+
assert(S && !S->getParent());
3571+
S->addLocal(Local);
3572+
return Local.Offset;
3573+
}
3574+
35523575
template <class Emitter>
35533576
const RecordType *Compiler<Emitter>::getRecordTy(QualType Ty) {
35543577
if (const PointerType *PT = dyn_cast<PointerType>(Ty))

clang/lib/AST/Interp/Compiler.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
297297
/// Allocates a space storing a local given its type.
298298
std::optional<unsigned>
299299
allocateLocal(DeclTy &&Decl, const ValueDecl *ExtendingDecl = nullptr);
300+
unsigned allocateTemporary(const Expr *E);
300301

301302
private:
302303
friend class VariableScope<Emitter>;

clang/test/AST/Interp/records.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1627,3 +1627,29 @@ namespace DtorDestroysFieldsAfterSelf {
16271627
static_assert(foo() == 15);
16281628
}
16291629
#endif
1630+
1631+
namespace ExprWithCleanups {
1632+
struct A { A(); ~A(); int get(); };
1633+
constexpr int get() {return false ? A().get() : 1;}
1634+
static_assert(get() == 1, "");
1635+
1636+
1637+
struct S {
1638+
int V;
1639+
constexpr S(int V) : V(V) {}
1640+
constexpr int get() {
1641+
return V;
1642+
}
1643+
};
1644+
constexpr int get(bool b) {
1645+
S a = b ? S(1) : S(2);
1646+
1647+
return a.get();
1648+
}
1649+
static_assert(get(true) == 1, "");
1650+
static_assert(get(false) == 2, "");
1651+
1652+
1653+
constexpr auto F = true ? 1i : 2i;
1654+
static_assert(F == 1i, "");
1655+
}

0 commit comments

Comments
 (0)