Skip to content

Commit 427c5bf

Browse files
tbaederrtbaederr
committed
Revert "[clang][Interp] Fix locals created in ExprWithCleanups"
This reverts commit 1c7673b. Unfortunately, this one is broken because de04e6c has been reverted.
1 parent ebcb04a commit 427c5bf

File tree

3 files changed

+44
-67
lines changed

3 files changed

+44
-67
lines changed

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

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,15 @@ 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, nullptr), Scope(Ctx->P, VD),
32+
: VariableScope<Emitter>(Ctx), 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+
3741
~DeclScope() { this->Ctx->GlobalDecl = OldGlobalDecl; }
3842

3943
private:
@@ -81,7 +85,8 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
8185
std::optional<PrimType> SubExprT = classify(SubExpr->getType());
8286
// Prepare storage for the result.
8387
if (!Initializing && !SubExprT) {
84-
std::optional<unsigned> LocalIndex = allocateLocal(SubExpr);
88+
std::optional<unsigned> LocalIndex =
89+
allocateLocal(SubExpr, /*IsExtended=*/false);
8590
if (!LocalIndex)
8691
return false;
8792
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -357,7 +362,8 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
357362
// We're creating a complex value here, so we need to
358363
// allocate storage for it.
359364
if (!Initializing) {
360-
std::optional<unsigned> LocalIndex = allocateLocal(CE);
365+
std::optional<unsigned> LocalIndex =
366+
allocateLocal(CE, /*IsExtended=*/true);
361367
if (!LocalIndex)
362368
return false;
363369
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -384,7 +390,8 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
384390
return this->discard(SubExpr);
385391

386392
if (!Initializing) {
387-
std::optional<unsigned> LocalIndex = allocateLocal(CE);
393+
std::optional<unsigned> LocalIndex =
394+
allocateLocal(CE, /*IsExtended=*/true);
388395
if (!LocalIndex)
389396
return false;
390397
if (!this->emitGetPtrLocal(*LocalIndex, CE))
@@ -485,7 +492,7 @@ bool ByteCodeExprGen<Emitter>::VisitImaginaryLiteral(
485492
return true;
486493

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

555562
// We need a temporary variable holding our return value.
556563
if (!Initializing) {
557-
std::optional<unsigned> ResultIndex = this->allocateLocal(BO);
564+
std::optional<unsigned> ResultIndex = this->allocateLocal(BO, false);
558565
if (!this->emitGetPtrLocal(*ResultIndex, BO))
559566
return false;
560567
}
@@ -777,7 +784,7 @@ template <class Emitter>
777784
bool ByteCodeExprGen<Emitter>::VisitComplexBinOp(const BinaryOperator *E) {
778785
// Prepare storage for result.
779786
if (!Initializing) {
780-
std::optional<unsigned> LocalIndex = allocateLocal(E);
787+
std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
781788
if (!LocalIndex)
782789
return false;
783790
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -1834,12 +1841,11 @@ bool ByteCodeExprGen<Emitter>::VisitCompoundAssignOperator(
18341841
template <class Emitter>
18351842
bool ByteCodeExprGen<Emitter>::VisitExprWithCleanups(
18361843
const ExprWithCleanups *E) {
1837-
ExprScope<Emitter> ES(this);
18381844
const Expr *SubExpr = E->getSubExpr();
18391845

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

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

18451851
template <class Emitter>
@@ -1904,8 +1910,9 @@ bool ByteCodeExprGen<Emitter>::VisitMaterializeTemporaryExpr(
19041910
return this->emitGetPtrLocal(LocalIndex, E);
19051911
} else {
19061912
const Expr *Inner = E->getSubExpr()->skipRValueSubobjectAdjustments();
1913+
19071914
if (std::optional<unsigned> LocalIndex =
1908-
allocateLocal(Inner, E->getExtendingDecl())) {
1915+
allocateLocal(Inner, /*IsExtended=*/true)) {
19091916
if (!this->emitGetPtrLocal(*LocalIndex, E))
19101917
return false;
19111918
return this->visitInitializer(SubExpr);
@@ -2112,7 +2119,8 @@ bool ByteCodeExprGen<Emitter>::VisitCXXConstructExpr(
21122119
// to allocate a variable and call the constructor and destructor.
21132120
if (DiscardResult) {
21142121
assert(!Initializing);
2115-
std::optional<unsigned> LocalIndex = allocateLocal(E);
2122+
std::optional<unsigned> LocalIndex =
2123+
allocateLocal(E, /*IsExtended=*/true);
21162124

21172125
if (!LocalIndex)
21182126
return false;
@@ -2292,7 +2300,8 @@ bool ByteCodeExprGen<Emitter>::VisitCXXScalarValueInitExpr(
22922300

22932301
if (const auto *CT = Ty->getAs<ComplexType>()) {
22942302
if (!Initializing) {
2295-
std::optional<unsigned> LocalIndex = allocateLocal(E);
2303+
std::optional<unsigned> LocalIndex =
2304+
allocateLocal(E, /*IsExtended=*/false);
22962305
if (!LocalIndex)
22972306
return false;
22982307
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -2315,7 +2324,8 @@ bool ByteCodeExprGen<Emitter>::VisitCXXScalarValueInitExpr(
23152324
if (const auto *VT = Ty->getAs<VectorType>()) {
23162325
// FIXME: Code duplication with the _Complex case above.
23172326
if (!Initializing) {
2318-
std::optional<unsigned> LocalIndex = allocateLocal(E);
2327+
std::optional<unsigned> LocalIndex =
2328+
allocateLocal(E, /*IsExtended=*/false);
23192329
if (!LocalIndex)
23202330
return false;
23212331
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -2503,7 +2513,7 @@ template <class Emitter> bool ByteCodeExprGen<Emitter>::visit(const Expr *E) {
25032513
// Create local variable to hold the return value.
25042514
if (!E->isGLValue() && !E->getType()->isAnyComplexType() &&
25052515
!classify(E->getType())) {
2506-
std::optional<unsigned> LocalIndex = allocateLocal(E);
2516+
std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/true);
25072517
if (!LocalIndex)
25082518
return false;
25092519

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

27582768
template <class Emitter>
27592769
std::optional<unsigned>
2760-
ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src,
2761-
const ValueDecl *ExtendingDecl) {
2770+
ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src, bool IsExtended) {
27622771
// Make sure we don't accidentally register the same decl twice.
27632772
if ([[maybe_unused]] const auto *VD =
27642773
dyn_cast_if_present<ValueDecl>(Src.dyn_cast<const Decl *>())) {
@@ -2791,10 +2800,7 @@ ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src,
27912800
Scope::Local Local = this->createLocal(D);
27922801
if (Key)
27932802
Locals.insert({Key, Local});
2794-
if (ExtendingDecl)
2795-
VarScope->addExtended(Local, ExtendingDecl);
2796-
else
2797-
VarScope->add(Local, false);
2803+
VarScope->add(Local, IsExtended);
27982804
return Local.Offset;
27992805
}
28002806

@@ -2829,14 +2835,14 @@ bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *E) {
28292835
if (E->getType()->isVoidType()) {
28302836
if (!visit(E))
28312837
return false;
2832-
return this->emitRetVoid(E) && RootScope.destroyLocals();
2838+
return this->emitRetVoid(E);
28332839
}
28342840

28352841
// Expressions with a primitive return type.
28362842
if (std::optional<PrimType> T = classify(E)) {
28372843
if (!visit(E))
28382844
return false;
2839-
return this->emitRet(*T, E) && RootScope.destroyLocals();
2845+
return this->emitRet(*T, E);
28402846
}
28412847

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

2860-
return RootScope.destroyLocals();
2866+
return false;
28612867
}
28622868

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

29492955
return !Init || initGlobal(*GlobalIndex);
29502956
} else {
2951-
VariableScope<Emitter> LocalScope(this, VD);
2957+
VariableScope<Emitter> LocalScope(this);
29522958
if (VarT) {
29532959
unsigned Offset = this->allocateLocalPrimitive(
29542960
VD, *VarT, VD->getType().isConstQualified());
@@ -2965,7 +2971,6 @@ bool ByteCodeExprGen<Emitter>::visitVarDecl(const VarDecl *VD) {
29652971
return !Init || this->visitLocalInitializer(Init, *Offset);
29662972
return false;
29672973
}
2968-
29692974
return true;
29702975
}
29712976

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

30483053
// Non-primitive return type. Prepare storage.
30493054
if (!Initializing && !ReturnT && !ReturnType->isVoidType()) {
3050-
std::optional<unsigned> LocalIndex = allocateLocal(E);
3055+
std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
30513056
if (!LocalIndex)
30523057
return false;
30533058
if (!this->emitGetPtrLocal(*LocalIndex, E))
@@ -3461,7 +3466,8 @@ bool ByteCodeExprGen<Emitter>::VisitComplexUnaryOperator(
34613466
std::optional<PrimType> ResT = classify(E);
34623467
auto prepareResult = [=]() -> bool {
34633468
if (!ResT && !Initializing) {
3464-
std::optional<unsigned> LocalIndex = allocateLocal(SubExpr);
3469+
std::optional<unsigned> LocalIndex =
3470+
allocateLocal(SubExpr, /*IsExtended=*/false);
34653471
if (!LocalIndex)
34663472
return false;
34673473
return this->emitGetPtrLocal(*LocalIndex, E);

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

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,7 @@ 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>
239-
allocateLocal(DeclTy &&Decl, const ValueDecl *ExtendingDecl = nullptr);
238+
std::optional<unsigned> allocateLocal(DeclTy &&Decl, bool IsExtended = false);
240239

241240
private:
242241
friend class VariableScope<Emitter>;
@@ -323,8 +322,8 @@ extern template class ByteCodeExprGen<EvalEmitter>;
323322
/// Scope chain managing the variable lifetimes.
324323
template <class Emitter> class VariableScope {
325324
public:
326-
VariableScope(ByteCodeExprGen<Emitter> *Ctx, const ValueDecl *VD)
327-
: Ctx(Ctx), Parent(Ctx->VarScope), ValDecl(VD) {
325+
VariableScope(ByteCodeExprGen<Emitter> *Ctx)
326+
: Ctx(Ctx), Parent(Ctx->VarScope) {
328327
Ctx->VarScope = this;
329328
}
330329

@@ -347,24 +346,6 @@ template <class Emitter> class VariableScope {
347346
this->Parent->addExtended(Local);
348347
}
349348

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-
368349
virtual void emitDestruction() {}
369350
virtual bool emitDestructors() { return true; }
370351
VariableScope *getParent() const { return Parent; }
@@ -374,14 +355,12 @@ template <class Emitter> class VariableScope {
374355
ByteCodeExprGen<Emitter> *Ctx;
375356
/// Link to the parent scope.
376357
VariableScope *Parent;
377-
const ValueDecl *ValDecl = nullptr;
378358
};
379359

380360
/// Generic scope for local variables.
381361
template <class Emitter> class LocalScope : public VariableScope<Emitter> {
382362
public:
383-
LocalScope(ByteCodeExprGen<Emitter> *Ctx)
384-
: VariableScope<Emitter>(Ctx, nullptr) {}
363+
LocalScope(ByteCodeExprGen<Emitter> *Ctx) : VariableScope<Emitter>(Ctx) {}
385364

386365
/// Emit a Destroy op for this scope.
387366
~LocalScope() override {
@@ -495,9 +474,16 @@ template <class Emitter> class BlockScope final : public AutoScope<Emitter> {
495474
}
496475
};
497476

477+
/// Expression scope which tracks potentially lifetime extended
478+
/// temporaries which are hoisted to the parent scope on exit.
498479
template <class Emitter> class ExprScope final : public AutoScope<Emitter> {
499480
public:
500481
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+
}
501487
};
502488

503489
template <class Emitter> class ArrayIndexScope final {

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1444,18 +1444,3 @@ 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)