Skip to content

Commit 5b2eb0d

Browse files
tbaederrtbaederr
authored
[clang][bytecode][NFC] Add FullExpression scopes (#170705)
And use them instead of the extending decl. This is close to what the current interpreter is doing. This is NFC right now but fixes a problem I encountered while looking into the expansion statement stuff.
1 parent cc5b07c commit 5b2eb0d

File tree

2 files changed

+45
-63
lines changed

2 files changed

+45
-63
lines changed

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

Lines changed: 40 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ static std::optional<bool> getBoolValue(const Expr *E) {
4040
template <class Emitter> class DeclScope final : public LocalScope<Emitter> {
4141
public:
4242
DeclScope(Compiler<Emitter> *Ctx, const ValueDecl *VD)
43-
: LocalScope<Emitter>(Ctx, VD), Scope(Ctx->P),
43+
: LocalScope<Emitter>(Ctx), Scope(Ctx->P),
4444
OldInitializingDecl(Ctx->InitializingDecl) {
4545
Ctx->InitializingDecl = VD;
4646
Ctx->InitStack.push_back(InitLink::Decl(VD));
@@ -2137,8 +2137,7 @@ bool Compiler<Emitter>::visitCallArgs(ArrayRef<const Expr *> Args,
21372137
}
21382138

21392139
UnsignedOrNone LocalIndex =
2140-
allocateLocal(std::move(Source), Arg->getType(),
2141-
/*ExtendingDecl=*/nullptr, ScopeKind::Call);
2140+
allocateLocal(std::move(Source), Arg->getType(), ScopeKind::Call);
21422141
if (!LocalIndex)
21432142
return false;
21442143

@@ -2451,7 +2450,7 @@ bool Compiler<Emitter>::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E) {
24512450
// and the RHS is our SubExpr.
24522451
for (size_t I = 0; I != Size; ++I) {
24532452
ArrayIndexScope<Emitter> IndexScope(this, I);
2454-
LocalScope<Emitter> BS(this);
2453+
LocalScope<Emitter> BS(this, ScopeKind::FullExpression);
24552454

24562455
if (!this->visitArrayElemInit(I, SubExpr, SubExprT))
24572456
return false;
@@ -2903,7 +2902,7 @@ bool Compiler<Emitter>::VisitCompoundAssignOperator(
29032902

29042903
template <class Emitter>
29052904
bool Compiler<Emitter>::VisitExprWithCleanups(const ExprWithCleanups *E) {
2906-
LocalScope<Emitter> ES(this);
2905+
LocalScope<Emitter> ES(this, ScopeKind::FullExpression);
29072906
const Expr *SubExpr = E->getSubExpr();
29082907

29092908
return this->delegate(SubExpr) && ES.destroyLocals(E);
@@ -2926,9 +2925,7 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr(
29262925
// When we're initializing a global variable *or* the storage duration of
29272926
// the temporary is explicitly static, create a global variable.
29282927
OptPrimType SubExprT = classify(SubExpr);
2929-
bool IsStatic = E->getStorageDuration() == SD_Static;
2930-
if (IsStatic) {
2931-
2928+
if (E->getStorageDuration() == SD_Static) {
29322929
UnsignedOrNone GlobalIndex = P.createGlobal(E);
29332930
if (!GlobalIndex)
29342931
return false;
@@ -2955,12 +2952,16 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr(
29552952
return this->emitInitGlobalTempComp(TempDecl, E);
29562953
}
29572954

2955+
ScopeKind VarScope = E->getStorageDuration() == SD_FullExpression
2956+
? ScopeKind::FullExpression
2957+
: ScopeKind::Block;
2958+
29582959
// For everyhing else, use local variables.
29592960
if (SubExprT) {
29602961
bool IsConst = SubExpr->getType().isConstQualified();
29612962
bool IsVolatile = SubExpr->getType().isVolatileQualified();
2962-
unsigned LocalIndex = allocateLocalPrimitive(
2963-
E, *SubExprT, IsConst, IsVolatile, E->getExtendingDecl());
2963+
unsigned LocalIndex =
2964+
allocateLocalPrimitive(E, *SubExprT, IsConst, IsVolatile, VarScope);
29642965
if (!this->VarScope->LocalsAlwaysEnabled &&
29652966
!this->emitEnableLocal(LocalIndex, E))
29662967
return false;
@@ -2975,9 +2976,10 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr(
29752976

29762977
if (!this->checkLiteralType(SubExpr))
29772978
return false;
2979+
29782980
const Expr *Inner = E->getSubExpr()->skipRValueSubobjectAdjustments();
29792981
if (UnsignedOrNone LocalIndex =
2980-
allocateLocal(E, Inner->getType(), E->getExtendingDecl())) {
2982+
allocateLocal(E, Inner->getType(), VarScope)) {
29812983
InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex));
29822984

29832985
if (!this->VarScope->LocalsAlwaysEnabled &&
@@ -4269,7 +4271,8 @@ template <class Emitter> bool Compiler<Emitter>::visit(const Expr *E) {
42694271

42704272
// Create local variable to hold the return value.
42714273
if (!E->isGLValue() && !canClassify(E->getType())) {
4272-
UnsignedOrNone LocalIndex = allocateLocal(stripDerivedToBaseCasts(E));
4274+
UnsignedOrNone LocalIndex = allocateLocal(
4275+
stripDerivedToBaseCasts(E), QualType(), ScopeKind::FullExpression);
42734276
if (!LocalIndex)
42744277
return false;
42754278

@@ -4626,9 +4629,11 @@ bool Compiler<Emitter>::emitConst(const APSInt &Value, const Expr *E) {
46264629
}
46274630

46284631
template <class Emitter>
4629-
unsigned Compiler<Emitter>::allocateLocalPrimitive(
4630-
DeclTy &&Src, PrimType Ty, bool IsConst, bool IsVolatile,
4631-
const ValueDecl *ExtendingDecl, ScopeKind SC, bool IsConstexprUnknown) {
4632+
unsigned Compiler<Emitter>::allocateLocalPrimitive(DeclTy &&Src, PrimType Ty,
4633+
bool IsConst,
4634+
bool IsVolatile,
4635+
ScopeKind SC,
4636+
bool IsConstexprUnknown) {
46324637
// FIXME: There are cases where Src.is<Expr*>() is wrong, e.g.
46334638
// (int){12} in C. Consider using Expr::isTemporaryObject() instead
46344639
// or isa<MaterializeTemporaryExpr>().
@@ -4639,16 +4644,12 @@ unsigned Compiler<Emitter>::allocateLocalPrimitive(
46394644
Scope::Local Local = this->createLocal(D);
46404645
if (auto *VD = dyn_cast_if_present<ValueDecl>(Src.dyn_cast<const Decl *>()))
46414646
Locals.insert({VD, Local});
4642-
if (ExtendingDecl)
4643-
VarScope->addExtended(Local, ExtendingDecl);
4644-
else
4645-
VarScope->addForScopeKind(Local, SC);
4647+
VarScope->addForScopeKind(Local, SC);
46464648
return Local.Offset;
46474649
}
46484650

46494651
template <class Emitter>
46504652
UnsignedOrNone Compiler<Emitter>::allocateLocal(DeclTy &&Src, QualType Ty,
4651-
const ValueDecl *ExtendingDecl,
46524653
ScopeKind SC,
46534654
bool IsConstexprUnknown) {
46544655
const ValueDecl *Key = nullptr;
@@ -4676,10 +4677,7 @@ UnsignedOrNone Compiler<Emitter>::allocateLocal(DeclTy &&Src, QualType Ty,
46764677
Scope::Local Local = this->createLocal(D);
46774678
if (Key)
46784679
Locals.insert({Key, Local});
4679-
if (ExtendingDecl)
4680-
VarScope->addExtended(Local, ExtendingDecl);
4681-
else
4682-
VarScope->addForScopeKind(Local, SC);
4680+
VarScope->addForScopeKind(Local, SC);
46834681
return Local.Offset;
46844682
}
46854683

@@ -4731,7 +4729,7 @@ const Function *Compiler<Emitter>::getFunction(const FunctionDecl *FD) {
47314729

47324730
template <class Emitter>
47334731
bool Compiler<Emitter>::visitExpr(const Expr *E, bool DestroyToplevelScope) {
4734-
LocalScope<Emitter> RootScope(this);
4732+
LocalScope<Emitter> RootScope(this, ScopeKind::FullExpression);
47354733

47364734
// If we won't destroy the toplevel scope, check for memory leaks first.
47374735
if (!DestroyToplevelScope) {
@@ -4825,7 +4823,7 @@ bool Compiler<Emitter>::visitDeclAndReturn(const VarDecl *VD, const Expr *Init,
48254823
LS.destroyLocals() && this->emitCheckAllocations(VD);
48264824
}
48274825

4828-
LocalScope<Emitter> VDScope(this, VD);
4826+
LocalScope<Emitter> VDScope(this);
48294827
if (!this->visitVarDecl(VD, Init, /*Toplevel=*/true))
48304828
return false;
48314829

@@ -4936,7 +4934,7 @@ Compiler<Emitter>::visitVarDecl(const VarDecl *VD, const Expr *Init,
49364934
if (VarT) {
49374935
unsigned Offset = this->allocateLocalPrimitive(
49384936
VD, *VarT, VD->getType().isConstQualified(),
4939-
VD->getType().isVolatileQualified(), nullptr, ScopeKind::Block,
4937+
VD->getType().isVolatileQualified(), ScopeKind::Block,
49404938
IsConstexprUnknown);
49414939

49424940
if (!Init)
@@ -4956,7 +4954,7 @@ Compiler<Emitter>::visitVarDecl(const VarDecl *VD, const Expr *Init,
49564954
}
49574955
// Local composite variables.
49584956
if (UnsignedOrNone Offset = this->allocateLocal(
4959-
VD, VD->getType(), nullptr, ScopeKind::Block, IsConstexprUnknown)) {
4957+
VD, VD->getType(), ScopeKind::Block, IsConstexprUnknown)) {
49604958
if (!Init)
49614959
return true;
49624960

@@ -5671,19 +5669,24 @@ bool Compiler<Emitter>::visitReturnStmt(const ReturnStmt *RS) {
56715669
}
56725670

56735671
template <class Emitter> bool Compiler<Emitter>::visitIfStmt(const IfStmt *IS) {
5672+
LocalScope<Emitter> IfScope(this);
5673+
56745674
auto visitChildStmt = [&](const Stmt *S) -> bool {
56755675
LocalScope<Emitter> SScope(this);
56765676
if (!visitStmt(S))
56775677
return false;
56785678
return SScope.destroyLocals();
56795679
};
5680-
if (auto *CondInit = IS->getInit())
5680+
5681+
if (auto *CondInit = IS->getInit()) {
56815682
if (!visitStmt(CondInit))
56825683
return false;
5684+
}
56835685

5684-
if (const DeclStmt *CondDecl = IS->getConditionVariableDeclStmt())
5686+
if (const DeclStmt *CondDecl = IS->getConditionVariableDeclStmt()) {
56855687
if (!visitDeclStmt(CondDecl))
56865688
return false;
5689+
}
56875690

56885691
// Save ourselves compiling some code and the jumps, etc. if the condition is
56895692
// stataically known to be either true or false. We could look at more cases
@@ -5707,8 +5710,11 @@ template <class Emitter> bool Compiler<Emitter>::visitIfStmt(const IfStmt *IS) {
57075710
if (!this->emitInv(IS))
57085711
return false;
57095712
} else {
5713+
LocalScope<Emitter> CondScope(this, ScopeKind::FullExpression);
57105714
if (!this->visitBool(IS->getCond()))
57115715
return false;
5716+
if (!CondScope.destroyLocals())
5717+
return false;
57125718
}
57135719

57145720
if (!this->maybeEmitDeferredVarInit(IS->getConditionVariable()))
@@ -5736,6 +5742,9 @@ template <class Emitter> bool Compiler<Emitter>::visitIfStmt(const IfStmt *IS) {
57365742
this->emitLabel(LabelEnd);
57375743
}
57385744

5745+
if (!IfScope.destroyLocals())
5746+
return false;
5747+
57395748
return true;
57405749
}
57415750

@@ -6333,7 +6342,7 @@ bool Compiler<Emitter>::compileConstructor(const CXXConstructorDecl *Ctor) {
63336342
InitLinkScope<Emitter> InitScope(this, InitLink::This());
63346343
for (const auto *Init : Ctor->inits()) {
63356344
// Scope needed for the initializers.
6336-
LocalScope<Emitter> Scope(this);
6345+
LocalScope<Emitter> Scope(this, ScopeKind::FullExpression);
63376346

63386347
const Expr *InitExpr = Init->getInit();
63396348
if (const FieldDecl *Member = Init->getMember()) {

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

Lines changed: 5 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ struct VarCreationState {
104104
bool notCreated() const { return !S; }
105105
};
106106

107-
enum class ScopeKind { Call, Block };
107+
enum class ScopeKind { Block, FullExpression, Call };
108108

109109
/// Compilation context for expressions.
110110
template <class Emitter>
@@ -330,13 +330,11 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
330330
/// Creates a local primitive value.
331331
unsigned allocateLocalPrimitive(DeclTy &&Decl, PrimType Ty, bool IsConst,
332332
bool IsVolatile = false,
333-
const ValueDecl *ExtendingDecl = nullptr,
334333
ScopeKind SC = ScopeKind::Block,
335334
bool IsConstexprUnknown = false);
336335

337336
/// Allocates a space storing a local given its type.
338337
UnsignedOrNone allocateLocal(DeclTy &&Decl, QualType Ty = QualType(),
339-
const ValueDecl *ExtendingDecl = nullptr,
340338
ScopeKind = ScopeKind::Block,
341339
bool IsConstexprUnknown = false);
342340
UnsignedOrNone allocateTemporary(const Expr *E);
@@ -476,9 +474,8 @@ extern template class Compiler<EvalEmitter>;
476474
/// Scope chain managing the variable lifetimes.
477475
template <class Emitter> class VariableScope {
478476
public:
479-
VariableScope(Compiler<Emitter> *Ctx, const ValueDecl *VD,
480-
ScopeKind Kind = ScopeKind::Block)
481-
: Ctx(Ctx), Parent(Ctx->VarScope), ValDecl(VD), Kind(Kind) {
477+
VariableScope(Compiler<Emitter> *Ctx, ScopeKind Kind = ScopeKind::Block)
478+
: Ctx(Ctx), Parent(Ctx->VarScope), Kind(Kind) {
482479
if (Parent)
483480
this->LocalsAlwaysEnabled = Parent->LocalsAlwaysEnabled;
484481
Ctx->VarScope = this;
@@ -489,28 +486,6 @@ template <class Emitter> class VariableScope {
489486
virtual void addLocal(Scope::Local Local) {
490487
llvm_unreachable("Shouldn't be called");
491488
}
492-
493-
void addExtended(const Scope::Local &Local, const ValueDecl *ExtendingDecl) {
494-
// Walk up the chain of scopes until we find the one for ExtendingDecl.
495-
// If there is no such scope, attach it to the parent one.
496-
VariableScope *P = this;
497-
while (P) {
498-
if (P->ValDecl == ExtendingDecl) {
499-
P->addLocal(Local);
500-
return;
501-
}
502-
P = P->Parent;
503-
if (!P)
504-
break;
505-
}
506-
507-
// Use the parent scope.
508-
if (this->Parent)
509-
this->Parent->addLocal(Local);
510-
else
511-
this->addLocal(Local);
512-
}
513-
514489
/// Like addExtended, but adds to the nearest scope of the given kind.
515490
void addForScopeKind(const Scope::Local &Local, ScopeKind Kind) {
516491
VariableScope *P = this;
@@ -523,6 +498,7 @@ template <class Emitter> class VariableScope {
523498
if (!P)
524499
break;
525500
}
501+
526502
// Add to this scope.
527503
this->addLocal(Local);
528504
}
@@ -542,17 +518,14 @@ template <class Emitter> class VariableScope {
542518
Compiler<Emitter> *Ctx;
543519
/// Link to the parent scope.
544520
VariableScope *Parent;
545-
const ValueDecl *ValDecl = nullptr;
546521
ScopeKind Kind;
547522
};
548523

549524
/// Generic scope for local variables.
550525
template <class Emitter> class LocalScope : public VariableScope<Emitter> {
551526
public:
552527
LocalScope(Compiler<Emitter> *Ctx, ScopeKind Kind = ScopeKind::Block)
553-
: VariableScope<Emitter>(Ctx, nullptr, Kind) {}
554-
LocalScope(Compiler<Emitter> *Ctx, const ValueDecl *VD)
555-
: VariableScope<Emitter>(Ctx, VD) {}
528+
: VariableScope<Emitter>(Ctx, Kind) {}
556529

557530
/// Emit a Destroy op for this scope.
558531
~LocalScope() override {

0 commit comments

Comments
 (0)