[clang][Interp] Handle complex values in visitBool()

[tbaederr]

In C++, we get a ComplexToBool cast, but we might not in C.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

In C++, we get a ComplexToBool cast, but we might not in C.

---

Full diff: https://github.com/llvm/llvm-project/pull/79452.diff

4 Files Affected:

• (modified) clang/lib/AST/Interp/ByteCodeExprGen.cpp (+65-53)
• (modified) clang/lib/AST/Interp/ByteCodeExprGen.h (+1)
• (modified) clang/test/AST/Interp/c.c (+4)
• (modified) clang/test/AST/Interp/complex.cpp (+2)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 24a33c32df14042..ab543bd6f3a6e16 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -231,60 +231,11 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
 
   case CK_IntegralComplexToBoolean:
   case CK_FloatingComplexToBoolean: {
-    std::optional<PrimType> ElemT =
-        classifyComplexElementType(SubExpr->getType());
-    if (!ElemT)
-      return false;
-    // We emit the expression (__real(E) != 0 || __imag(E) != 0)
-    // for us, that means (bool)E[0] || (bool)E[1]
+    if (DiscardResult)
+      return this->discard(SubExpr);
     if (!this->visit(SubExpr))
       return false;
-    if (!this->emitConstUint8(0, CE))
-      return false;
-    if (!this->emitArrayElemPtrUint8(CE))
-      return false;
-    if (!this->emitLoadPop(*ElemT, CE))
-      return false;
-    if (*ElemT == PT_Float) {
-      if (!this->emitCastFloatingIntegral(PT_Bool, CE))
-        return false;
-    } else {
-      if (!this->emitCast(*ElemT, PT_Bool, CE))
-        return false;
-    }
-
-    // We now have the bool value of E[0] on the stack.
-    LabelTy LabelTrue = this->getLabel();
-    if (!this->jumpTrue(LabelTrue))
-      return false;
-
-    if (!this->emitConstUint8(1, CE))
-      return false;
-    if (!this->emitArrayElemPtrPopUint8(CE))
-      return false;
-    if (!this->emitLoadPop(*ElemT, CE))
-      return false;
-    if (*ElemT == PT_Float) {
-      if (!this->emitCastFloatingIntegral(PT_Bool, CE))
-        return false;
-    } else {
-      if (!this->emitCast(*ElemT, PT_Bool, CE))
-        return false;
-    }
-    // Leave the boolean value of E[1] on the stack.
-    LabelTy EndLabel = this->getLabel();
-    this->jump(EndLabel);
-
-    this->emitLabel(LabelTrue);
-    if (!this->emitPopPtr(CE))
-      return false;
-    if (!this->emitConstBool(true, CE))
-      return false;
-
-    this->fallthrough(EndLabel);
-    this->emitLabel(EndLabel);
-
-    return true;
+    return this->emitComplexBoolCast(SubExpr);
   }
 
   case CK_IntegralComplexToReal:
@@ -1906,8 +1857,15 @@ bool ByteCodeExprGen<Emitter>::visitInitializer(const Expr *E) {
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visitBool(const Expr *E) {
   std::optional<PrimType> T = classify(E->getType());
-  if (!T)
+  if (!T) {
+    // Convert complex values to bool.
+    if (E->getType()->isAnyComplexType()) {
+      if (!this->visit(E))
+        return false;
+      return this->emitComplexBoolCast(E);
+    }
     return false;
+  }
 
   if (!this->visit(E))
     return false;
@@ -2997,6 +2955,60 @@ bool ByteCodeExprGen<Emitter>::emitComplexReal(const Expr *SubExpr) {
   return true;
 }
 
+template <class Emitter>
+bool ByteCodeExprGen<Emitter>::emitComplexBoolCast(const Expr *E) {
+  assert(!DiscardResult);
+  PrimType ElemT = classifyComplexElementType(E->getType());
+  // We emit the expression (__real(E) != 0 || __imag(E) != 0)
+  // for us, that means (bool)E[0] || (bool)E[1]
+  if (!this->emitConstUint8(0, E))
+    return false;
+  if (!this->emitArrayElemPtrUint8(E))
+    return false;
+  if (!this->emitLoadPop(ElemT, E))
+    return false;
+  if (ElemT == PT_Float) {
+    if (!this->emitCastFloatingIntegral(PT_Bool, E))
+      return false;
+  } else {
+    if (!this->emitCast(ElemT, PT_Bool, E))
+      return false;
+  }
+
+  // We now have the bool value of E[0] on the stack.
+  LabelTy LabelTrue = this->getLabel();
+  if (!this->jumpTrue(LabelTrue))
+    return false;
+
+  if (!this->emitConstUint8(1, E))
+    return false;
+  if (!this->emitArrayElemPtrPopUint8(E))
+    return false;
+  if (!this->emitLoadPop(ElemT, E))
+    return false;
+  if (ElemT == PT_Float) {
+    if (!this->emitCastFloatingIntegral(PT_Bool, E))
+      return false;
+  } else {
+    if (!this->emitCast(ElemT, PT_Bool, E))
+      return false;
+  }
+  // Leave the boolean value of E[1] on the stack.
+  LabelTy EndLabel = this->getLabel();
+  this->jump(EndLabel);
+
+  this->emitLabel(LabelTrue);
+  if (!this->emitPopPtr(E))
+    return false;
+  if (!this->emitConstBool(true, E))
+    return false;
+
+  this->fallthrough(EndLabel);
+  this->emitLabel(EndLabel);
+
+  return true;
+}
+
 /// When calling this, we have a pointer of the local-to-destroy
 /// on the stack.
 /// Emit destruction of record types (or arrays of record types).
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 63ea8292b587675..aafd78adf6a5311 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -291,6 +291,7 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
   }
 
   bool emitComplexReal(const Expr *SubExpr);
+  bool emitComplexBoolCast(const Expr *E);
 
   bool emitRecordDestruction(const Descriptor *Desc);
   unsigned collectBaseOffset(const RecordType *BaseType,
diff --git a/clang/test/AST/Interp/c.c b/clang/test/AST/Interp/c.c
index 2bc3d906bcc5ef9..a46c2fd7cbe7297 100644
--- a/clang/test/AST/Interp/c.c
+++ b/clang/test/AST/Interp/c.c
@@ -85,3 +85,7 @@ const intptr_t L = (intptr_t)(&(yy->y)); // expected-error {{not a compile-time
 const ptrdiff_t m = &m + 137 - &m;
 _Static_assert(m == 137, ""); // pedantic-ref-warning {{GNU extension}} \
                               // pedantic-expected-warning {{GNU extension}}
+
+const int A =  ((_Complex double)1.0 ? 21 : 1);
+_Static_assert(A == 21, ""); // pedantic-ref-warning {{GNU extension}} \
+                             // pedantic-expected-warning {{GNU extension}}
diff --git a/clang/test/AST/Interp/complex.cpp b/clang/test/AST/Interp/complex.cpp
index 836ea552adac86d..eb8b7130122758e 100644
--- a/clang/test/AST/Interp/complex.cpp
+++ b/clang/test/AST/Interp/complex.cpp
@@ -54,6 +54,8 @@ constexpr int ignoredCast() {
   D1;
   (int)D1;
   (double)D1;
+  (bool)D1;
+  (bool)I2;
   return 0;
 }
 static_assert(ignoredCast() == 0, "");

[tbaederr]

Ping

[tbaederr]

Ping

[sethp]

LGTM!

[sethp]

This feels a little odd: I wonder if Sema opts not to insert a *ComplexToBoolean cast because "in C, we might not have Boolean"? Though I'm not sure if there's any situation where we would have _Complex but not _Bool. I know gcc opts to extend support for complex types "backwards" in time as an extension (i.e. under a different name), but I think it also does so for booleans too. Maybe clang has a similar policy?

Put another way, this feels like the sort of "deep" implicit logic (i.e. that's not apparent from the AST) which keeps tripping me up when working with the evaluator in ExprConstant; it seems to me like it should either be in Sema (i.e. there ought to be a *ComplexToBoolean cast node) or maybe one level up in Visit[Conditional/Ternary/etc.] (whatever there's an implicit boolean context).

I'm just a dog at a keyboard, though, so take my very much non-expert opinion for what it's worth.

[tbaederr]

@AaronBallman probably knows a reason for this.