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

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 #170981

Author: tbaederr

clang/lib/AST/ByteCode/Compiler.cpp

@@ -2503,20 +2503,22 @@ 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
   // 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 (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) {

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.

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);
+}