Skip to content

Commit dd2bf3b

Browse files
tbaederrtbaederr
committed
[clang][Interp] Redo variable (re)visiting
Depending on the circumstances we visit variables in, we need to be careful about when to destroy their temporaries and whether to emit a Ret op at all or not.
1 parent f0c7505 commit dd2bf3b

File tree

8 files changed

+87
-61
lines changed

8 files changed

+87
-61
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class ByteCodeEmitter {
4646
/// Methods implemented by the compiler.
4747
virtual bool visitFunc(const FunctionDecl *E) = 0;
4848
virtual bool visitExpr(const Expr *E) = 0;
49-
virtual bool visitDecl(const VarDecl *E, bool ConstantContext) = 0;
49+
virtual bool visitDeclAndReturn(const VarDecl *E, bool ConstantContext) = 0;
5050

5151
/// Emits jumps.
5252
bool jumpTrue(const LabelTy &Label);

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

Lines changed: 71 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ namespace clang {
2525
namespace interp {
2626

2727
/// Scope used to handle temporaries in toplevel variable declarations.
28-
template <class Emitter> class DeclScope final : public VariableScope<Emitter> {
28+
template <class Emitter> class DeclScope final : public LocalScope<Emitter> {
2929
public:
3030
DeclScope(Compiler<Emitter> *Ctx, const ValueDecl *VD)
31-
: VariableScope<Emitter>(Ctx, nullptr), Scope(Ctx->P, VD),
31+
: LocalScope<Emitter>(Ctx, VD), Scope(Ctx->P, VD),
3232
OldGlobalDecl(Ctx->GlobalDecl),
3333
OldInitializingDecl(Ctx->InitializingDecl) {
3434
Ctx->GlobalDecl = Context::shouldBeGloballyIndexed(VD);
@@ -3462,40 +3462,52 @@ template <class Emitter> bool Compiler<Emitter>::visitExpr(const Expr *E) {
34623462
return false;
34633463
}
34643464

3465-
/// Toplevel visitDecl().
3465+
template <class Emitter>
3466+
VarCreationState Compiler<Emitter>::visitDecl(const VarDecl *VD) {
3467+
3468+
auto R = this->visitVarDecl(VD, /*Toplevel=*/true);
3469+
3470+
if (R.notCreated())
3471+
return R;
3472+
3473+
if (R)
3474+
return true;
3475+
3476+
if (!R && Context::shouldBeGloballyIndexed(VD)) {
3477+
if (auto GlobalIndex = P.getGlobal(VD)) {
3478+
Block *GlobalBlock = P.getGlobal(*GlobalIndex);
3479+
GlobalInlineDescriptor &GD =
3480+
*reinterpret_cast<GlobalInlineDescriptor *>(GlobalBlock->rawData());
3481+
3482+
GD.InitState = GlobalInitState::InitializerFailed;
3483+
GlobalBlock->invokeDtor();
3484+
}
3485+
}
3486+
3487+
return R;
3488+
}
3489+
3490+
/// Toplevel visitDeclAndReturn().
34663491
/// We get here from evaluateAsInitializer().
34673492
/// We need to evaluate the initializer and return its value.
34683493
template <class Emitter>
3469-
bool Compiler<Emitter>::visitDecl(const VarDecl *VD, bool ConstantContext) {
3470-
assert(!VD->isInvalidDecl() && "Trying to constant evaluate an invalid decl");
3471-
3494+
bool Compiler<Emitter>::visitDeclAndReturn(const VarDecl *VD,
3495+
bool ConstantContext) {
34723496
std::optional<PrimType> VarT = classify(VD->getType());
34733497

34743498
// We only create variables if we're evaluating in a constant context.
34753499
// Otherwise, just evaluate the initializer and return it.
34763500
if (!ConstantContext) {
3477-
DeclScope<Emitter> LocalScope(this, VD);
3501+
DeclScope<Emitter> LS(this, VD);
34783502
if (!this->visit(VD->getAnyInitializer()))
34793503
return false;
3480-
return this->emitRet(VarT.value_or(PT_Ptr), VD);
3481-
}
3482-
3483-
// If we've seen the global variable already and the initializer failed,
3484-
// just return false immediately.
3485-
if (std::optional<unsigned> Index = P.getGlobal(VD)) {
3486-
const Pointer &Ptr = P.getPtrGlobal(*Index);
3487-
const GlobalInlineDescriptor &GD =
3488-
*reinterpret_cast<const GlobalInlineDescriptor *>(
3489-
Ptr.block()->rawData());
3490-
if (GD.InitState == GlobalInitState::InitializerFailed)
3491-
return false;
3504+
return this->emitRet(VarT.value_or(PT_Ptr), VD) && LS.destroyLocals();
34923505
}
34933506

3494-
// Create and initialize the variable.
3507+
LocalScope<Emitter> VDScope(this, VD);
34953508
if (!this->visitVarDecl(VD, /*Toplevel=*/true))
34963509
return false;
34973510

3498-
// Get a pointer to the variable
34993511
if (Context::shouldBeGloballyIndexed(VD)) {
35003512
auto GlobalIndex = P.getGlobal(VD);
35013513
assert(GlobalIndex); // visitVarDecl() didn't return false.
@@ -3535,7 +3547,7 @@ bool Compiler<Emitter>::visitDecl(const VarDecl *VD, bool ConstantContext) {
35353547
return false;
35363548
}
35373549

3538-
return true;
3550+
return VDScope.destroyLocals();
35393551
}
35403552

35413553
template <class Emitter>
@@ -3589,26 +3601,34 @@ VarCreationState Compiler<Emitter>::visitVarDecl(const VarDecl *VD, bool Topleve
35893601

35903602
return !Init || (checkDecl() && initGlobal(*GlobalIndex));
35913603
} else {
3592-
VariableScope<Emitter> LocalScope(this, VD);
35933604
InitLinkScope<Emitter> ILS(this, InitLink::Decl(VD));
35943605

35953606
if (VarT) {
35963607
unsigned Offset = this->allocateLocalPrimitive(
35973608
VD, *VarT, VD->getType().isConstQualified());
35983609
if (Init) {
3599-
// Compile the initializer in its own scope.
3600-
ExprScope<Emitter> Scope(this);
3601-
if (!this->visit(Init))
3602-
return false;
3603-
3604-
return this->emitSetLocal(*VarT, Offset, VD);
3610+
// If this is a toplevel declaration, create a scope for the
3611+
// initializer.
3612+
if (Toplevel) {
3613+
ExprScope<Emitter> Scope(this);
3614+
if (!this->visit(Init))
3615+
return false;
3616+
return this->emitSetLocal(*VarT, Offset, VD);
3617+
} else {
3618+
if (!this->visit(Init))
3619+
return false;
3620+
return this->emitSetLocal(*VarT, Offset, VD);
3621+
}
36053622
}
36063623
} else {
3607-
if (std::optional<unsigned> Offset = this->allocateLocal(VD))
3608-
return !Init || this->visitLocalInitializer(Init, *Offset);
3624+
if (std::optional<unsigned> Offset = this->allocateLocal(VD)) {
3625+
if (!Init)
3626+
return true;
3627+
3628+
return this->visitLocalInitializer(Init, *Offset);
3629+
}
36093630
return false;
36103631
}
3611-
36123632
return true;
36133633
}
36143634

@@ -4074,7 +4094,7 @@ bool Compiler<Emitter>::visitCompoundStmt(const CompoundStmt *S) {
40744094
for (const auto *InnerStmt : S->body())
40754095
if (!visitStmt(InnerStmt))
40764096
return false;
4077-
return true;
4097+
return Scope.destroyLocals();
40784098
}
40794099

40804100
template <class Emitter>
@@ -5010,6 +5030,18 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
50105030
}
50115031
}
50125032

5033+
// In case we need to re-visit a declaration.
5034+
auto revisit = [&](const VarDecl *VD) -> bool {
5035+
auto VarState = this->visitDecl(VD);
5036+
5037+
if (VarState.notCreated())
5038+
return true;
5039+
if (!VarState)
5040+
return false;
5041+
// Retry.
5042+
return this->visitDeclRef(D, E);
5043+
};
5044+
50135045
// Handle lambda captures.
50145046
if (auto It = this->LambdaCaptures.find(D);
50155047
It != this->LambdaCaptures.end()) {
@@ -5020,12 +5052,8 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
50205052
return this->emitGetPtrThisField(Offset, E);
50215053
} else if (const auto *DRE = dyn_cast<DeclRefExpr>(E);
50225054
DRE && DRE->refersToEnclosingVariableOrCapture()) {
5023-
if (const auto *VD = dyn_cast<VarDecl>(D); VD && VD->isInitCapture()) {
5024-
if (!this->visitVarDecl(cast<VarDecl>(D)))
5025-
return false;
5026-
// Retry.
5027-
return this->visitDeclRef(D, E);
5028-
}
5055+
if (const auto *VD = dyn_cast<VarDecl>(D); VD && VD->isInitCapture())
5056+
return revisit(VD);
50295057
}
50305058

50315059
if (D != InitializingDecl) {
@@ -5044,28 +5072,14 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
50445072
// Visit local const variables like normal.
50455073
if ((VD->hasGlobalStorage() || VD->isLocalVarDecl() ||
50465074
VD->isStaticDataMember()) &&
5047-
typeShouldBeVisited(VD->getType())) {
5048-
auto VarState = this->visitVarDecl(VD, true);
5049-
if (VarState.notCreated())
5050-
return true;
5051-
if (!VarState)
5052-
return false;
5053-
// Retry.
5054-
return this->visitDeclRef(VD, E);
5055-
}
5075+
typeShouldBeVisited(VD->getType()))
5076+
return revisit(VD);
50565077
}
50575078
} else {
50585079
if (const auto *VD = dyn_cast<VarDecl>(D);
50595080
VD && VD->getAnyInitializer() &&
5060-
VD->getType().isConstant(Ctx.getASTContext()) && !VD->isWeak()) {
5061-
auto VarState = this->visitVarDecl(VD, true);
5062-
if (VarState.notCreated())
5063-
return true;
5064-
if (!VarState)
5065-
return false;
5066-
// Retry.
5067-
return this->visitDeclRef(VD, E);
5068-
}
5081+
VD->getType().isConstant(Ctx.getASTContext()) && !VD->isWeak())
5082+
return revisit(VD);
50695083
}
50705084
}
50715085

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,10 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
212212
protected:
213213
bool visitStmt(const Stmt *S);
214214
bool visitExpr(const Expr *E) override;
215-
bool visitDecl(const VarDecl *VD, bool ConstantContext) override;
216215
bool visitFunc(const FunctionDecl *F) override;
217216

217+
bool visitDeclAndReturn(const VarDecl *VD, bool ConstantContext) override;
218+
218219
protected:
219220
/// Emits scope cleanup instructions.
220221
void emitCleanup();
@@ -267,6 +268,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
267268
bool delegate(const Expr *E);
268269
/// Creates and initializes a variable from the given decl.
269270
VarCreationState visitVarDecl(const VarDecl *VD, bool Toplevel = false);
271+
VarCreationState visitDecl(const VarDecl *VD);
270272
/// Visit an APValue.
271273
bool visitAPValue(const APValue &Val, PrimType ValType, const Expr *E);
272274
bool visitAPValueInitializer(const APValue &Val, const Expr *E);
@@ -496,6 +498,8 @@ template <class Emitter> class VariableScope {
496498
template <class Emitter> class LocalScope : public VariableScope<Emitter> {
497499
public:
498500
LocalScope(Compiler<Emitter> *Ctx) : VariableScope<Emitter>(Ctx, nullptr) {}
501+
LocalScope(Compiler<Emitter> *Ctx, const ValueDecl *VD)
502+
: VariableScope<Emitter>(Ctx, VD) {}
499503

500504
/// Emit a Destroy op for this scope.
501505
~LocalScope() override {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ EvaluationResult EvalEmitter::interpretDecl(const VarDecl *VD,
6868

6969
EvalResult.setSource(VD);
7070

71-
if (!this->visitDecl(VD, S.inConstantContext()) && EvalResult.empty())
71+
if (!this->visitDeclAndReturn(VD, S.inConstantContext()))
7272
EvalResult.setInvalid();
7373

7474
S.EvaluatingDecl = nullptr;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class EvalEmitter : public SourceMapper {
5555

5656
/// Methods implemented by the compiler.
5757
virtual bool visitExpr(const Expr *E) = 0;
58-
virtual bool visitDecl(const VarDecl *VD, bool ConstantContext) = 0;
58+
virtual bool visitDeclAndReturn(const VarDecl *VD, bool ConstantContext) = 0;
5959
virtual bool visitFunc(const FunctionDecl *F) = 0;
6060

6161
/// Emits jumps.

‎clang/test/C/C23/n3017.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %clang_cc1 -verify -fsyntax-only --embed-dir=%S/Inputs -std=c2x %s -Wno-constant-logical-operand
2+
// RUN: %clang_cc1 -verify -fsyntax-only --embed-dir=%S/Inputs -std=c2x %s -Wno-constant-logical-operand -fexperimental-new-constant-interpreter
23

34
/* WG14 N3017: full
45
* #embed - a scannable, tooling-friendly binary resource inclusion mechanism

‎clang/test/CodeGenCXX/no-const-init-cxx2a.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -std=c++2a | FileCheck %s
2+
// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -std=c++2a -fexperimental-new-constant-interpreter | FileCheck %s
23

34
// CHECK-DAG: @p = {{.*}} null
45
// CHECK-DAG: @_ZGR1p_ = {{.*}} null

‎clang/test/SemaCXX/warn-unused-variables.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++11 %s
33
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++14 %s
44
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify -std=gnu++17 %s
5+
6+
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify %s -fexperimental-new-constant-interpreter
7+
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++11 %s -fexperimental-new-constant-interpreter
8+
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++14 %s -fexperimental-new-constant-interpreter
9+
// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify -std=gnu++17 %s -fexperimental-new-constant-interpreter
10+
511
template<typename T> void f() {
612
T t;
713
t = 17;

0 commit comments

Comments
 (0)