Author: Timm Bäder Date: 2024-07-09T10:43:35+02:00 New Revision: dd2bf3b840df260d794e37cc96d4498372aa08f6
URL: https://github.com/llvm/llvm-project/commit/dd2bf3b840df260d794e37cc96d4498372aa08f6 DIFF: https://github.com/llvm/llvm-project/commit/dd2bf3b840df260d794e37cc96d4498372aa08f6.diff LOG: [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. Added: Modified: clang/lib/AST/Interp/ByteCodeEmitter.h clang/lib/AST/Interp/Compiler.cpp clang/lib/AST/Interp/Compiler.h clang/lib/AST/Interp/EvalEmitter.cpp clang/lib/AST/Interp/EvalEmitter.h clang/test/C/C23/n3017.c clang/test/CodeGenCXX/no-const-init-cxx2a.cpp clang/test/SemaCXX/warn-unused-variables.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/Interp/ByteCodeEmitter.h b/clang/lib/AST/Interp/ByteCodeEmitter.h index 9a329e969f339..a19a25c2f9e8e 100644 --- a/clang/lib/AST/Interp/ByteCodeEmitter.h +++ b/clang/lib/AST/Interp/ByteCodeEmitter.h @@ -46,7 +46,7 @@ class ByteCodeEmitter { /// Methods implemented by the compiler. virtual bool visitFunc(const FunctionDecl *E) = 0; virtual bool visitExpr(const Expr *E) = 0; - virtual bool visitDecl(const VarDecl *E, bool ConstantContext) = 0; + virtual bool visitDeclAndReturn(const VarDecl *E, bool ConstantContext) = 0; /// Emits jumps. bool jumpTrue(const LabelTy &Label); diff --git a/clang/lib/AST/Interp/Compiler.cpp b/clang/lib/AST/Interp/Compiler.cpp index 613bf4af137b6..1d39d7bb9e15f 100644 --- a/clang/lib/AST/Interp/Compiler.cpp +++ b/clang/lib/AST/Interp/Compiler.cpp @@ -25,10 +25,10 @@ namespace clang { namespace interp { /// Scope used to handle temporaries in toplevel variable declarations. -template <class Emitter> class DeclScope final : public VariableScope<Emitter> { +template <class Emitter> class DeclScope final : public LocalScope<Emitter> { public: DeclScope(Compiler<Emitter> *Ctx, const ValueDecl *VD) - : VariableScope<Emitter>(Ctx, nullptr), Scope(Ctx->P, VD), + : LocalScope<Emitter>(Ctx, VD), Scope(Ctx->P, VD), OldGlobalDecl(Ctx->GlobalDecl), OldInitializingDecl(Ctx->InitializingDecl) { Ctx->GlobalDecl = Context::shouldBeGloballyIndexed(VD); @@ -3462,40 +3462,52 @@ template <class Emitter> bool Compiler<Emitter>::visitExpr(const Expr *E) { return false; } -/// Toplevel visitDecl(). +template <class Emitter> +VarCreationState Compiler<Emitter>::visitDecl(const VarDecl *VD) { + + auto R = this->visitVarDecl(VD, /*Toplevel=*/true); + + if (R.notCreated()) + return R; + + if (R) + return true; + + if (!R && Context::shouldBeGloballyIndexed(VD)) { + if (auto GlobalIndex = P.getGlobal(VD)) { + Block *GlobalBlock = P.getGlobal(*GlobalIndex); + GlobalInlineDescriptor &GD = + *reinterpret_cast<GlobalInlineDescriptor *>(GlobalBlock->rawData()); + + GD.InitState = GlobalInitState::InitializerFailed; + GlobalBlock->invokeDtor(); + } + } + + return R; +} + +/// Toplevel visitDeclAndReturn(). /// We get here from evaluateAsInitializer(). /// We need to evaluate the initializer and return its value. template <class Emitter> -bool Compiler<Emitter>::visitDecl(const VarDecl *VD, bool ConstantContext) { - assert(!VD->isInvalidDecl() && "Trying to constant evaluate an invalid decl"); - +bool Compiler<Emitter>::visitDeclAndReturn(const VarDecl *VD, + bool ConstantContext) { std::optional<PrimType> VarT = classify(VD->getType()); // We only create variables if we're evaluating in a constant context. // Otherwise, just evaluate the initializer and return it. if (!ConstantContext) { - DeclScope<Emitter> LocalScope(this, VD); + DeclScope<Emitter> LS(this, VD); if (!this->visit(VD->getAnyInitializer())) return false; - return this->emitRet(VarT.value_or(PT_Ptr), VD); - } - - // If we've seen the global variable already and the initializer failed, - // just return false immediately. - if (std::optional<unsigned> Index = P.getGlobal(VD)) { - const Pointer &Ptr = P.getPtrGlobal(*Index); - const GlobalInlineDescriptor &GD = - *reinterpret_cast<const GlobalInlineDescriptor *>( - Ptr.block()->rawData()); - if (GD.InitState == GlobalInitState::InitializerFailed) - return false; + return this->emitRet(VarT.value_or(PT_Ptr), VD) && LS.destroyLocals(); } - // Create and initialize the variable. + LocalScope<Emitter> VDScope(this, VD); if (!this->visitVarDecl(VD, /*Toplevel=*/true)) return false; - // Get a pointer to the variable if (Context::shouldBeGloballyIndexed(VD)) { auto GlobalIndex = P.getGlobal(VD); assert(GlobalIndex); // visitVarDecl() didn't return false. @@ -3535,7 +3547,7 @@ bool Compiler<Emitter>::visitDecl(const VarDecl *VD, bool ConstantContext) { return false; } - return true; + return VDScope.destroyLocals(); } template <class Emitter> @@ -3589,26 +3601,34 @@ VarCreationState Compiler<Emitter>::visitVarDecl(const VarDecl *VD, bool Topleve return !Init || (checkDecl() && initGlobal(*GlobalIndex)); } else { - VariableScope<Emitter> LocalScope(this, VD); InitLinkScope<Emitter> ILS(this, InitLink::Decl(VD)); if (VarT) { unsigned Offset = this->allocateLocalPrimitive( VD, *VarT, VD->getType().isConstQualified()); if (Init) { - // Compile the initializer in its own scope. - ExprScope<Emitter> Scope(this); - if (!this->visit(Init)) - return false; - - return this->emitSetLocal(*VarT, Offset, VD); + // If this is a toplevel declaration, create a scope for the + // initializer. + if (Toplevel) { + ExprScope<Emitter> Scope(this); + if (!this->visit(Init)) + return false; + return this->emitSetLocal(*VarT, Offset, VD); + } else { + if (!this->visit(Init)) + return false; + return this->emitSetLocal(*VarT, Offset, VD); + } } } else { - if (std::optional<unsigned> Offset = this->allocateLocal(VD)) - return !Init || this->visitLocalInitializer(Init, *Offset); + if (std::optional<unsigned> Offset = this->allocateLocal(VD)) { + if (!Init) + return true; + + return this->visitLocalInitializer(Init, *Offset); + } return false; } - return true; } @@ -4074,7 +4094,7 @@ bool Compiler<Emitter>::visitCompoundStmt(const CompoundStmt *S) { for (const auto *InnerStmt : S->body()) if (!visitStmt(InnerStmt)) return false; - return true; + return Scope.destroyLocals(); } template <class Emitter> @@ -5010,6 +5030,18 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { } } + // In case we need to re-visit a declaration. + auto revisit = [&](const VarDecl *VD) -> bool { + auto VarState = this->visitDecl(VD); + + if (VarState.notCreated()) + return true; + if (!VarState) + return false; + // Retry. + return this->visitDeclRef(D, E); + }; + // Handle lambda captures. if (auto It = this->LambdaCaptures.find(D); It != this->LambdaCaptures.end()) { @@ -5020,12 +5052,8 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { return this->emitGetPtrThisField(Offset, E); } else if (const auto *DRE = dyn_cast<DeclRefExpr>(E); DRE && DRE->refersToEnclosingVariableOrCapture()) { - if (const auto *VD = dyn_cast<VarDecl>(D); VD && VD->isInitCapture()) { - if (!this->visitVarDecl(cast<VarDecl>(D))) - return false; - // Retry. - return this->visitDeclRef(D, E); - } + if (const auto *VD = dyn_cast<VarDecl>(D); VD && VD->isInitCapture()) + return revisit(VD); } if (D != InitializingDecl) { @@ -5044,28 +5072,14 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { // Visit local const variables like normal. if ((VD->hasGlobalStorage() || VD->isLocalVarDecl() || VD->isStaticDataMember()) && - typeShouldBeVisited(VD->getType())) { - auto VarState = this->visitVarDecl(VD, true); - if (VarState.notCreated()) - return true; - if (!VarState) - return false; - // Retry. - return this->visitDeclRef(VD, E); - } + typeShouldBeVisited(VD->getType())) + return revisit(VD); } } else { if (const auto *VD = dyn_cast<VarDecl>(D); VD && VD->getAnyInitializer() && - VD->getType().isConstant(Ctx.getASTContext()) && !VD->isWeak()) { - auto VarState = this->visitVarDecl(VD, true); - if (VarState.notCreated()) - return true; - if (!VarState) - return false; - // Retry. - return this->visitDeclRef(VD, E); - } + VD->getType().isConstant(Ctx.getASTContext()) && !VD->isWeak()) + return revisit(VD); } } diff --git a/clang/lib/AST/Interp/Compiler.h b/clang/lib/AST/Interp/Compiler.h index 1405abfc1a1e2..fbeda8a0b7a4f 100644 --- a/clang/lib/AST/Interp/Compiler.h +++ b/clang/lib/AST/Interp/Compiler.h @@ -212,9 +212,10 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, protected: bool visitStmt(const Stmt *S); bool visitExpr(const Expr *E) override; - bool visitDecl(const VarDecl *VD, bool ConstantContext) override; bool visitFunc(const FunctionDecl *F) override; + bool visitDeclAndReturn(const VarDecl *VD, bool ConstantContext) override; + protected: /// Emits scope cleanup instructions. void emitCleanup(); @@ -267,6 +268,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, bool delegate(const Expr *E); /// Creates and initializes a variable from the given decl. VarCreationState visitVarDecl(const VarDecl *VD, bool Toplevel = false); + VarCreationState visitDecl(const VarDecl *VD); /// Visit an APValue. bool visitAPValue(const APValue &Val, PrimType ValType, const Expr *E); bool visitAPValueInitializer(const APValue &Val, const Expr *E); @@ -496,6 +498,8 @@ template <class Emitter> class VariableScope { template <class Emitter> class LocalScope : public VariableScope<Emitter> { public: LocalScope(Compiler<Emitter> *Ctx) : VariableScope<Emitter>(Ctx, nullptr) {} + LocalScope(Compiler<Emitter> *Ctx, const ValueDecl *VD) + : VariableScope<Emitter>(Ctx, VD) {} /// Emit a Destroy op for this scope. ~LocalScope() override { diff --git a/clang/lib/AST/Interp/EvalEmitter.cpp b/clang/lib/AST/Interp/EvalEmitter.cpp index acd1f2c673ce0..9701796fb9303 100644 --- a/clang/lib/AST/Interp/EvalEmitter.cpp +++ b/clang/lib/AST/Interp/EvalEmitter.cpp @@ -68,7 +68,7 @@ EvaluationResult EvalEmitter::interpretDecl(const VarDecl *VD, EvalResult.setSource(VD); - if (!this->visitDecl(VD, S.inConstantContext()) && EvalResult.empty()) + if (!this->visitDeclAndReturn(VD, S.inConstantContext())) EvalResult.setInvalid(); S.EvaluatingDecl = nullptr; diff --git a/clang/lib/AST/Interp/EvalEmitter.h b/clang/lib/AST/Interp/EvalEmitter.h index d1e125cae9594..338786d3dea91 100644 --- a/clang/lib/AST/Interp/EvalEmitter.h +++ b/clang/lib/AST/Interp/EvalEmitter.h @@ -55,7 +55,7 @@ class EvalEmitter : public SourceMapper { /// Methods implemented by the compiler. virtual bool visitExpr(const Expr *E) = 0; - virtual bool visitDecl(const VarDecl *VD, bool ConstantContext) = 0; + virtual bool visitDeclAndReturn(const VarDecl *VD, bool ConstantContext) = 0; virtual bool visitFunc(const FunctionDecl *F) = 0; /// Emits jumps. diff --git a/clang/test/C/C23/n3017.c b/clang/test/C/C23/n3017.c index 0d22d31baa4b7..05337351dd869 100644 --- a/clang/test/C/C23/n3017.c +++ b/clang/test/C/C23/n3017.c @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -verify -fsyntax-only --embed-dir=%S/Inputs -std=c2x %s -Wno-constant-logical-operand +// RUN: %clang_cc1 -verify -fsyntax-only --embed-dir=%S/Inputs -std=c2x %s -Wno-constant-logical-operand -fexperimental-new-constant-interpreter /* WG14 N3017: full * #embed - a scannable, tooling-friendly binary resource inclusion mechanism diff --git a/clang/test/CodeGenCXX/no-const-init-cxx2a.cpp b/clang/test/CodeGenCXX/no-const-init-cxx2a.cpp index a945c066e3bb2..2933d429c62f8 100644 --- a/clang/test/CodeGenCXX/no-const-init-cxx2a.cpp +++ b/clang/test/CodeGenCXX/no-const-init-cxx2a.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -std=c++2a | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -std=c++2a -fexperimental-new-constant-interpreter | FileCheck %s // CHECK-DAG: @p = {{.*}} null // CHECK-DAG: @_ZGR1p_ = {{.*}} null diff --git a/clang/test/SemaCXX/warn-unused-variables.cpp b/clang/test/SemaCXX/warn-unused-variables.cpp index 29e8d08d37d8c..216873aa4074b 100644 --- a/clang/test/SemaCXX/warn-unused-variables.cpp +++ b/clang/test/SemaCXX/warn-unused-variables.cpp @@ -2,6 +2,12 @@ // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++11 %s // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify=expected,cxx98-14 -std=gnu++14 %s // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify -std=gnu++17 %s + +// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify %s -fexperimental-new-constant-interpreter +// 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 +// 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 +// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify -std=gnu++17 %s -fexperimental-new-constant-interpreter + template<typename T> void f() { T t; t = 17; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits