llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> Otherwise, the scope might be (lazily) initialized in one of the arms of the conditional operator, which means the will NOT be intialized in the other arm. Fixes https://github.com/llvm/llvm-project/issues/170981 --- Full diff: https://github.com/llvm/llvm-project/pull/171133.diff 3 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+9-7) - (modified) clang/lib/AST/ByteCode/Compiler.h (+14-2) - (modified) clang/test/AST/ByteCode/cxx20.cpp (+14) ``````````diff diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 14b6bb7bf61d5..ed5493c315dd7 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -2503,6 +2503,15 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator( const Expr *TrueExpr = E->getTrueExpr(); const Expr *FalseExpr = E->getFalseExpr(); + if (std::optional<bool> BoolValue = getBoolValue(Condition)) { + if (*BoolValue) + return this->delegate(TrueExpr); + return this->delegate(FalseExpr); + } + + // Force-init the scope, which creates a InitScope op. This is necessary so + // the scope is not only initialized in one arm of the conditional operator. + this->VarScope->forceInit(); // The TrueExpr and FalseExpr of a conditional operator do _not_ create a // scope, which means the local variables created within them unconditionally // always exist. However, we need to later differentiate which branch was @@ -2510,13 +2519,6 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator( // "enabled" flags on local variables are used for. llvm::SaveAndRestore LAAA(this->VarScope->LocalsAlwaysEnabled, /*NewValue=*/false); - - if (std::optional<bool> BoolValue = getBoolValue(Condition)) { - if (*BoolValue) - return this->delegate(TrueExpr); - return this->delegate(FalseExpr); - } - bool IsBcpCall = false; if (const auto *CE = dyn_cast<CallExpr>(Condition->IgnoreParenCasts()); CE && CE->getBuiltinCallee() == Builtin::BI__builtin_constant_p) { diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h index c641af52c2811..1bd15c3d79563 100644 --- a/clang/lib/AST/ByteCode/Compiler.h +++ b/clang/lib/AST/ByteCode/Compiler.h @@ -505,6 +505,7 @@ template <class Emitter> class VariableScope { virtual bool emitDestructors(const Expr *E = nullptr) { return true; } virtual bool destroyLocals(const Expr *E = nullptr) { return true; } + virtual void forceInit() {} VariableScope *getParent() const { return Parent; } ScopeKind getKind() const { return Kind; } @@ -534,7 +535,6 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { this->Ctx->emitDestroy(*Idx, SourceInfo{}); removeStoredOpaqueValues(); } - /// Explicit destruction of local variables. bool destroyLocals(const Expr *E = nullptr) override { if (!Idx) @@ -558,10 +558,22 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { this->Ctx->Descriptors[*Idx].emplace_back(Local); } + /// Force-initialize this scope. Usually, scopes are lazily initialized when + /// the first local variable is created, but in scenarios with conditonal + /// operators, we need to ensure scope is initialized just in case one of the + /// arms will create a local and the other won't. In such a case, the + /// InitScope() op would be part of the arm that created the local. + void forceInit() override { + if (!Idx) { + Idx = static_cast<unsigned>(this->Ctx->Descriptors.size()); + this->Ctx->Descriptors.emplace_back(); + this->Ctx->emitInitScope(*Idx, {}); + } + } + bool emitDestructors(const Expr *E = nullptr) override { if (!Idx) return true; - assert(!this->Ctx->Descriptors[*Idx].empty()); // Emit destructor calls for local variables of record // type with a destructor. diff --git a/clang/test/AST/ByteCode/cxx20.cpp b/clang/test/AST/ByteCode/cxx20.cpp index ea4843e95b01f..227f34cee80ff 100644 --- a/clang/test/AST/ByteCode/cxx20.cpp +++ b/clang/test/AST/ByteCode/cxx20.cpp @@ -1211,3 +1211,17 @@ namespace DyamicCast { constexpr const X *p = &y; constexpr const Y *q = dynamic_cast<const Y*>(p); } + +namespace ConditionalTemporaries { + class F { + public: + constexpr F(int a ) {this->a = a;} + constexpr ~F() {} + int a; + }; + constexpr int foo(bool b) { + return b ? F{12}.a : F{13}.a; + } + static_assert(foo(false)== 13); + static_assert(foo(true)== 12); +} `````````` </details> https://github.com/llvm/llvm-project/pull/171133 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
