[clang] [clang][bytecode] Always initialize scope before conditional operator (PR #171133)

[via cfe-commits]

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