llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> The code and the comment are correct, but they also applied to the conditional operator condition, which they shouldn't. --- Full diff: https://github.com/llvm/llvm-project/pull/201777.diff 2 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+11-10) - (modified) clang/test/AST/ByteCode/cxx20.cpp (+17) ``````````diff diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 732a9e227430c..f5c7700b9cde7 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -2834,16 +2834,6 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator( 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 - // taken and only destroy the varibles of the active branch. This is what the - // "enabled" flags on local variables are used for. - llvm::SaveAndRestore LAAA(this->VarScope->LocalsAlwaysEnabled, - /*NewValue=*/false); bool IsBcpCall = false; if (const auto *CE = dyn_cast<CallExpr>(Condition->IgnoreParenCasts()); CE && CE->getBuiltinCallee() == Builtin::BI__builtin_constant_p) { @@ -2871,6 +2861,17 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator( return false; } + // 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 + // taken and only destroy the varibles of the active branch. This is what the + // "enabled" flags on local variables are used for. + llvm::SaveAndRestore LAAA(this->VarScope->LocalsAlwaysEnabled, + /*NewValue=*/false); + if (!this->jumpFalse(LabelFalse, E)) return false; if (!this->delegate(TrueExpr)) diff --git a/clang/test/AST/ByteCode/cxx20.cpp b/clang/test/AST/ByteCode/cxx20.cpp index d3e6265a9cbe7..a5a3a3346dad4 100644 --- a/clang/test/AST/ByteCode/cxx20.cpp +++ b/clang/test/AST/ByteCode/cxx20.cpp @@ -1262,6 +1262,23 @@ namespace ConditionalTemporaries { } static_assert(foo(false)== 13); static_assert(foo(true)== 12); + + struct S { + int *p; + constexpr S() { p = new int(); } + constexpr ~S() { delete p; } + }; + + constexpr bool func(S &s1, S s2) { + return true; + } + + constexpr bool test() { + S s1; + func(s1, {}) ? void() : void(); + return true; + } + static_assert(test()); } namespace PointerCmp { `````````` </details> https://github.com/llvm/llvm-project/pull/201777 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
