Skip to content

Commit 1c7673b

Browse files
tbaederrtbaederr
committed
[clang][Interp] Fix locals created in ExprWithCleanups
Save the declaration we're creating a scope for (if applicable), so we can attach a local temporary variable to the right scope.
1 parent de04e6c commit 1c7673b

File tree

3 files changed

+67
-44
lines changed

3 files changed

+67
-44
lines changed

‎clang/lib/AST/Interp/ByteCodeExprGen.cpp

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,11 @@ namespace interp {
2929
template <class Emitter> class DeclScope final : public VariableScope<Emitter> {
3030
public:
3131
DeclScope(ByteCodeExprGen<Emitter> *Ctx, const ValueDecl *VD)
32-
: VariableScope<Emitter>(Ctx), Scope(Ctx->P, VD),
32+
: VariableScope<Emitter>(Ctx, nullptr), Scope(Ctx->P, VD),
3333
OldGlobalDecl(Ctx->GlobalDecl) {
3434
Ctx->GlobalDecl = Context::shouldBeGloballyIndexed(VD);
3535
}
3636

37-
void addExtended(const Scope::Local &Local) override {
38-
return this->addLocal(Local);
39-
}
40-
4137
~DeclScope() { this->Ctx->GlobalDecl = OldGlobalDecl; }
4238

4339
private:
@@ -85,8 +81,7 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
8581
std::optional<PrimType> SubExprT = classify(SubExpr->getType());
8682
// Prepare storage for the result.
8783
if (!Initializing && !SubExprT) {
88-
std::optional<unsigned> LocalIndex =
89-
allocateLocal(SubExpr, /*IsExtended=*/false);
84+
std::optional<unsigned> LocalIndex = allocateLocal(SubExpr);
9085
if (!LocalIndex)
9186
return false;
9287
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -362,8 +357,7 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
362357
// We're creating a complex value here, so we need to
363358
// allocate storage for it.
364359
if (!Initializing) {
365-
std::optional<unsigned> LocalIndex =
366-
allocateLocal(CE, /*IsExtended=*/true);
360+
std::optional<unsigned> LocalIndex = allocateLocal(CE);
367361
if (!LocalIndex)
368362
return false;
369363
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -390,8 +384,7 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
390384
return this->discard(SubExpr);
391385

392386
if (!Initializing) {
393-
std::optional<unsigned> LocalIndex =
394-
allocateLocal(CE, /*IsExtended=*/true);
387+
std::optional<unsigned> LocalIndex = allocateLocal(CE);
395388
if (!LocalIndex)
396389
return false;
397390
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -492,7 +485,7 @@ bool ByteCodeExprGen<Emitter>::VisitImaginaryLiteral(
492485
return true;
493486

494487
if (!Initializing) {
495-
std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
488+
std::optional<unsigned> LocalIndex = allocateLocal(E);
496489
if (!LocalIndex)
497490
return false;
498491
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -561,7 +554,7 @@ bool ByteCodeExprGen<Emitter>::VisitBinaryOperator(const BinaryOperator *BO) {
561554

562555
// We need a temporary variable holding our return value.
563556
if (!Initializing) {
564-
std::optional<unsigned> ResultIndex = this->allocateLocal(BO, false);
557+
std::optional<unsigned> ResultIndex = this->allocateLocal(BO);
565558
if (!this->emitGetPtrLocal(*ResultIndex, BO))
566559
return false;
567560
}
@@ -784,7 +777,7 @@ template <class Emitter>
784777
bool ByteCodeExprGen<Emitter>::VisitComplexBinOp(const BinaryOperator *E) {
785778
// Prepare storage for result.
786779
if (!Initializing) {
787-
std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
780+
std::optional<unsigned> LocalIndex = allocateLocal(E);
788781
if (!LocalIndex)
789782
return false;
790783
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -1841,11 +1834,12 @@ bool ByteCodeExprGen<Emitter>::VisitCompoundAssignOperator(
18411834
template <class Emitter>
18421835
bool ByteCodeExprGen<Emitter>::VisitExprWithCleanups(
18431836
const ExprWithCleanups *E) {
1837+
ExprScope<Emitter> ES(this);
18441838
const Expr *SubExpr = E->getSubExpr();
18451839

18461840
assert(E->getNumObjects() == 0 && "TODO: Implement cleanups");
18471841

1848-
return this->delegate(SubExpr);
1842+
return this->delegate(SubExpr) && ES.destroyLocals();
18491843
}
18501844

18511845
template <class Emitter>
@@ -1910,9 +1904,8 @@ bool ByteCodeExprGen<Emitter>::VisitMaterializeTemporaryExpr(
19101904
return this->emitGetPtrLocal(LocalIndex, E);
19111905
} else {
19121906
const Expr *Inner = E->getSubExpr()->skipRValueSubobjectAdjustments();
1913-
19141907
if (std::optional<unsigned> LocalIndex =
1915-
allocateLocal(Inner, /*IsExtended=*/true)) {
1908+
allocateLocal(Inner, E->getExtendingDecl())) {
19161909
if (!this->emitGetPtrLocal(*LocalIndex, E))
19171910
return false;
19181911
return this->visitInitializer(SubExpr);
@@ -2119,8 +2112,7 @@ bool ByteCodeExprGen<Emitter>::VisitCXXConstructExpr(
21192112
// to allocate a variable and call the constructor and destructor.
21202113
if (DiscardResult) {
21212114
assert(!Initializing);
2122-
std::optional<unsigned> LocalIndex =
2123-
allocateLocal(E, /*IsExtended=*/true);
2115+
std::optional<unsigned> LocalIndex = allocateLocal(E);
21242116

21252117
if (!LocalIndex)
21262118
return false;
@@ -2300,8 +2292,7 @@ bool ByteCodeExprGen<Emitter>::VisitCXXScalarValueInitExpr(
23002292

23012293
if (const auto *CT = Ty->getAs<ComplexType>()) {
23022294
if (!Initializing) {
2303-
std::optional<unsigned> LocalIndex =
2304-
allocateLocal(E, /*IsExtended=*/false);
2295+
std::optional<unsigned> LocalIndex = allocateLocal(E);
23052296
if (!LocalIndex)
23062297
return false;
23072298
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -2324,8 +2315,7 @@ bool ByteCodeExprGen<Emitter>::VisitCXXScalarValueInitExpr(
23242315
if (const auto *VT = Ty->getAs<VectorType>()) {
23252316
// FIXME: Code duplication with the _Complex case above.
23262317
if (!Initializing) {
2327-
std::optional<unsigned> LocalIndex =
2328-
allocateLocal(E, /*IsExtended=*/false);
2318+
std::optional<unsigned> LocalIndex = allocateLocal(E);
23292319
if (!LocalIndex)
23302320
return false;
23312321
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -2513,7 +2503,7 @@ template <class Emitter> bool ByteCodeExprGen<Emitter>::visit(const Expr *E) {
25132503
// Create local variable to hold the return value.
25142504
if (!E->isGLValue() && !E->getType()->isAnyComplexType() &&
25152505
!classify(E->getType())) {
2516-
std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/true);
2506+
std::optional<unsigned> LocalIndex = allocateLocal(E);
25172507
if (!LocalIndex)
25182508
return false;
25192509

@@ -2767,7 +2757,8 @@ unsigned ByteCodeExprGen<Emitter>::allocateLocalPrimitive(DeclTy &&Src,
27672757

27682758
template <class Emitter>
27692759
std::optional<unsigned>
2770-
ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src, bool IsExtended) {
2760+
ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src,
2761+
const ValueDecl *ExtendingDecl) {
27712762
// Make sure we don't accidentally register the same decl twice.
27722763
if ([[maybe_unused]] const auto *VD =
27732764
dyn_cast_if_present<ValueDecl>(Src.dyn_cast<const Decl *>())) {
@@ -2800,7 +2791,10 @@ ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src, bool IsExtended) {
28002791
Scope::Local Local = this->createLocal(D);
28012792
if (Key)
28022793
Locals.insert({Key, Local});
2803-
VarScope->add(Local, IsExtended);
2794+
if (ExtendingDecl)
2795+
VarScope->addExtended(Local, ExtendingDecl);
2796+
else
2797+
VarScope->add(Local, false);
28042798
return Local.Offset;
28052799
}
28062800

@@ -2835,14 +2829,14 @@ bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *E) {
28352829
if (E->getType()->isVoidType()) {
28362830
if (!visit(E))
28372831
return false;
2838-
return this->emitRetVoid(E);
2832+
return this->emitRetVoid(E) && RootScope.destroyLocals();
28392833
}
28402834

28412835
// Expressions with a primitive return type.
28422836
if (std::optional<PrimType> T = classify(E)) {
28432837
if (!visit(E))
28442838
return false;
2845-
return this->emitRet(*T, E);
2839+
return this->emitRet(*T, E) && RootScope.destroyLocals();
28462840
}
28472841

28482842
// Expressions with a composite return type.
@@ -2863,7 +2857,7 @@ bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *E) {
28632857
return this->emitRetValue(E) && RootScope.destroyLocals();
28642858
}
28652859

2866-
return false;
2860+
return RootScope.destroyLocals();
28672861
}
28682862

28692863
/// Toplevel visitDecl().
@@ -2954,7 +2948,7 @@ bool ByteCodeExprGen<Emitter>::visitVarDecl(const VarDecl *VD) {
29542948

29552949
return !Init || initGlobal(*GlobalIndex);
29562950
} else {
2957-
VariableScope<Emitter> LocalScope(this);
2951+
VariableScope<Emitter> LocalScope(this, VD);
29582952
if (VarT) {
29592953
unsigned Offset = this->allocateLocalPrimitive(
29602954
VD, *VarT, VD->getType().isConstQualified());
@@ -2971,6 +2965,7 @@ bool ByteCodeExprGen<Emitter>::visitVarDecl(const VarDecl *VD) {
29712965
return !Init || this->visitLocalInitializer(Init, *Offset);
29722966
return false;
29732967
}
2968+
29742969
return true;
29752970
}
29762971

@@ -3052,7 +3047,7 @@ bool ByteCodeExprGen<Emitter>::VisitBuiltinCallExpr(const CallExpr *E) {
30523047

30533048
// Non-primitive return type. Prepare storage.
30543049
if (!Initializing && !ReturnT && !ReturnType->isVoidType()) {
3055-
std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
3050+
std::optional<unsigned> LocalIndex = allocateLocal(E);
30563051
if (!LocalIndex)
30573052
return false;
30583053
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -3466,8 +3461,7 @@ bool ByteCodeExprGen<Emitter>::VisitComplexUnaryOperator(
34663461
std::optional<PrimType> ResT = classify(E);
34673462
auto prepareResult = [=]() -> bool {
34683463
if (!ResT && !Initializing) {
3469-
std::optional<unsigned> LocalIndex =
3470-
allocateLocal(SubExpr, /*IsExtended=*/false);
3464+
std::optional<unsigned> LocalIndex = allocateLocal(SubExpr);
34713465
if (!LocalIndex)
34723466
return false;
34733467
return this->emitGetPtrLocal(*LocalIndex, E);

‎clang/lib/AST/Interp/ByteCodeExprGen.h

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,8 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
235235
bool IsExtended = false);
236236

237237
/// Allocates a space storing a local given its type.
238-
std::optional<unsigned> allocateLocal(DeclTy &&Decl, bool IsExtended = false);
238+
std::optional<unsigned>
239+
allocateLocal(DeclTy &&Decl, const ValueDecl *ExtendingDecl = nullptr);
239240

240241
private:
241242
friend class VariableScope<Emitter>;
@@ -322,8 +323,8 @@ extern template class ByteCodeExprGen<EvalEmitter>;
322323
/// Scope chain managing the variable lifetimes.
323324
template <class Emitter> class VariableScope {
324325
public:
325-
VariableScope(ByteCodeExprGen<Emitter> *Ctx)
326-
: Ctx(Ctx), Parent(Ctx->VarScope) {
326+
VariableScope(ByteCodeExprGen<Emitter> *Ctx, const ValueDecl *VD)
327+
: Ctx(Ctx), Parent(Ctx->VarScope), ValDecl(VD) {
327328
Ctx->VarScope = this;
328329
}
329330

@@ -346,6 +347,24 @@ template <class Emitter> class VariableScope {
346347
this->Parent->addExtended(Local);
347348
}
348349

350+
void addExtended(const Scope::Local &Local, const ValueDecl *ExtendingDecl) {
351+
// Walk up the chain of scopes until we find the one for ExtendingDecl.
352+
// If there is no such scope, attach it to the parent one.
353+
VariableScope *P = this;
354+
while (P) {
355+
if (P->ValDecl == ExtendingDecl) {
356+
P->addLocal(Local);
357+
return;
358+
}
359+
P = P->Parent;
360+
if (!P)
361+
break;
362+
}
363+
364+
// Use the parent scope.
365+
addExtended(Local);
366+
}
367+
349368
virtual void emitDestruction() {}
350369
virtual bool emitDestructors() { return true; }
351370
VariableScope *getParent() const { return Parent; }
@@ -355,12 +374,14 @@ template <class Emitter> class VariableScope {
355374
ByteCodeExprGen<Emitter> *Ctx;
356375
/// Link to the parent scope.
357376
VariableScope *Parent;
377+
const ValueDecl *ValDecl = nullptr;
358378
};
359379

360380
/// Generic scope for local variables.
361381
template <class Emitter> class LocalScope : public VariableScope<Emitter> {
362382
public:
363-
LocalScope(ByteCodeExprGen<Emitter> *Ctx) : VariableScope<Emitter>(Ctx) {}
383+
LocalScope(ByteCodeExprGen<Emitter> *Ctx)
384+
: VariableScope<Emitter>(Ctx, nullptr) {}
364385

365386
/// Emit a Destroy op for this scope.
366387
~LocalScope() override {
@@ -474,16 +495,9 @@ template <class Emitter> class BlockScope final : public AutoScope<Emitter> {
474495
}
475496
};
476497

477-
/// Expression scope which tracks potentially lifetime extended
478-
/// temporaries which are hoisted to the parent scope on exit.
479498
template <class Emitter> class ExprScope final : public AutoScope<Emitter> {
480499
public:
481500
ExprScope(ByteCodeExprGen<Emitter> *Ctx) : AutoScope<Emitter>(Ctx) {}
482-
483-
void addExtended(const Scope::Local &Local) override {
484-
if (this->Parent)
485-
this->Parent->addLocal(Local);
486-
}
487501
};
488502

489503
template <class Emitter> class ArrayIndexScope final {

‎clang/test/AST/Interp/records.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1444,3 +1444,18 @@ namespace {
14441444
static_assert(waldo == 4, "");
14451445
#endif
14461446
}
1447+
1448+
1449+
namespace TemporaryWithInvalidDestructor {
1450+
#if __cplusplus >= 202002L
1451+
struct A {
1452+
bool a = true;
1453+
constexpr ~A() noexcept(false) { // both-error {{never produces a constant expression}}
1454+
throw; // both-note 2{{not valid in a constant expression}} \
1455+
// both-error {{cannot use 'throw' with exceptions disabled}}
1456+
}
1457+
};
1458+
static_assert(A().a, ""); // both-error {{not an integral constant expression}} \
1459+
// both-note {{in call to}}
1460+
#endif
1461+
}

0 commit comments

Comments
 (0)