[clang][Interp] Start supporting complex types

[tbaeder]

Summary

Trying a different approach here where I split the implementation into smaller patches so they are easier to review.

[tbaeder]

Completely forgot to add the tests to the patch, oops.

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

2103

Member calls as well?

clang/lib/AST/Interp/Context.cpp

95–96

Hmmm, this seems surprising to me, I would have assumed that _Complex needed to be a primitive type so that we could perform typical math on it rather than treat it as a pair of values.

clang/test/AST/Interp/complex.cpp

8

Can you add tests for _Complex int as well? (That's also covered by isAnyComplexType())

[tbaeder]

clang/lib/AST/Interp/Context.cpp

95–96

I was going to add special opcodes for this case (or add one opcode for complex/vector/bitint/... things to do everything?).

If we don't do it this way we'd have to add isAnyComplexType() checks everywhere so we can figure out if we need to look at one or two values, don't we?

clang/test/AST/Interp/complex.cpp

8

There's a FIXME comment above in EvalEmitter.cpp so returning those doesn't work yet (but is otherwise not problematic I think.)

[aaron.ballman]

clang/lib/AST/Interp/Context.cpp

95–96

Hmm I think it depends on whether we consider the type to be primitive or not, maybe? I can see why it's not primitive -- it's comprised of two other primitive types. So that could be reasonable. But it's also primitive in that you need both of those values in order for the type to be meaningful for any operation (e.g., we don't want to accidentally split the values; we're always going to want to pass a complex value as "one thing", right). Buttttttt then again, if we treat it as not being primitive, then perhaps that gives us more optimization opportunities around things like "is an array of complex values an array of structs or a struct of arrays" kind of situations where we can decide to handle it differently depending on the situation.

So I guess I'm not certain of whether this is wrong or right.

clang/test/AST/Interp/complex.cpp

8

Hmmm, why the FIXME instead of just making the complex int type? (Alternatively, should we not use isAnyComplexType() and wait to add complex int support later?)

[tbaeder]

Add support for integer complex types as well.

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

2103

Yes, and some others, but I've only added the minimum for now to keep the patch small.

clang/test/AST/Interp/complex.cpp

8

Added int support, I just wasn't sure about the INT_TYPE_SWITCH.

[aaron.ballman]

I don't think the _BitInt test case should block this patch if that turns out to cause problems.

clang/lib/AST/Interp/EvalEmitter.cpp

224–226

The switch is fully covered with an unreachable, and the outer level returns false anyway.

clang/lib/AST/Interp/PrimType.h

108 ↗

(On Diff #509021)

Oh boy, this is going to be interesting, isn't it?

consteval _Complex _BitInt(12) UhOh() { return (_Complex _BitInt(12))1; }
consteval _Complex _BitInt(18) Why() { return (_Complex _BitInt(18))1; }

static_assert(UhOh() + Why() == 2);

[tbaeder]

clang/lib/AST/Interp/PrimType.h

108 ↗

(On Diff #509021)

_BitInt isn't supported in the new interpreter at all right now, so this just gets rejected.

Apart from that... is a complex bitint really something that should be supported? Not just in the new interpreter but generally?

[aaron.ballman]

LGTM but perhaps we should add an expected-failing test case for the _BitInt situation just so we don't forget about it.

clang/lib/AST/Interp/PrimType.h

108 ↗

(On Diff #509021)

_BitInt isn't supported in the new interpreter at all right now, so this just gets rejected.

Well, that's sort of good then! :-D We'll have to deal with _BitInt at some point, so maybe we can add this as a test case with expected failures and a fixit comment so we don't forget about it?

Apart from that... is a complex bitint really something that should be supported? Not just in the new interpreter but generally?

I don't see why not; we support complex integer types and _BitInt is an integer type. We support _Complex from C in C++ and we support _BitInt from C in C++, so it seems reasonable to expect _Complex _BitInt to work.

[tbaeder]

clang/lib/AST/Interp/PrimType.h

108 ↗

(On Diff #509021)

I don't see why not; we support complex integer types and _BitInt is an integer type. We support _Complex from C in C++ and we support _BitInt from C in C++, so it seems reasonable to expect _Complex _BitInt to work.

My immediate reaction to something like _Complex is "this is stupid, this belongs in user code". For floating-point values it at least makes sense from a mathematical POV I guess. But complex ints is already weird and complex arbitrary-width integers? What's the use case? _Complex bool is rejected as well after all.

108 ↗

(On Diff #509021)

We'll have to deal with _BitInt at some point, so maybe we can add this as a test case with expected failures and a fixit comment so we don't forget about it?

It's running into an assertion for the test case, so I added it commented-out.

[aaron.ballman]

clang/lib/AST/Interp/PrimType.h

108 ↗

(On Diff #509021)

My immediate reaction to something like _Complex is "this is stupid, this belongs in user code". For floating-point values it at least makes sense from a mathematical POV I guess. But complex ints is already weird and complex arbitrary-width integers? What's the use case? _Complex bool is rejected as well after all.

How would you explain this?

_Complex int32_t Val; // OK
_Complex _BitInt(32) OtherVal; // Not OK

The use case is the same as for _Complex int, just with getting to pick the width you want to use, which users can already do for some specific widths. Neither is a particularly strong motivation (to me anyway!), but I can't see why we'd allow a 32-bit integer but not a 32-bit (et al) precise integer.

[tbaeder]

clang/lib/AST/Interp/PrimType.h

108 ↗

(On Diff #509021)

Sure, the bitint one is just a bit crazier, I was hoping we could draw the line a little earlier and save ourselves some headaches, but this doesn't seem to be a problem in the current interpreter.

With constexpr being in c++ now and c++20 having enough to implement complex numbers in user code anyway, are there any plans to deprecate _Complex in c++20 onward (not sure about C2x)? That doesn't help me of course but it would maybe give me more hope for the future :)

[aaron.ballman]

clang/lib/AST/Interp/PrimType.h

108 ↗

(On Diff #509021)

C++ has std::complex as the official API for complex numbers (http://eel.is/c++draft/complex.numbers), but we expose _Complex in C++ as an extension because it makes it easier to ensure you're getting the same ABI between C and C++ with that datatype. (That said, I've always had the impression that std::complex and _Complex are intended to be ABI compatible in practice, but there's nothing in the C++ standard that requires it.) I'm not certain that we'd want to deprecate support for _Complex in C++ without plenty of investigation into whether it's used in practice or not.

As for deprecating _Complex in C, I'd be surprised but stranger things have happened.

clang/lib/AST/Interp/ByteCodeExprGen.cpp

if (T->isArrayType()) {
     for (const Expr *Init : E->inits()) {
       if (!this->visitArrayElemInit(ElementIndex, Init))
         return false;
       ++ElementIndex;
     }
     return true;
   }
 
+  if (T->isAnyComplexType()) {
+    unsigned InitIndex = 0;
+    for (const Expr *Init : E->inits()) {
+      PrimType InitT = classifyPrim(Init->getType());
+
+      if (!this->visit(Init))
+        return false;
+
+      if (!this->emitInitElem(InitT, InitIndex, E))
+        return false;
+      ++InitIndex;
+    }
+    assert(InitIndex == 2);
+    return true;
+  }
+
   return false;
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitCXXParenListInitExpr(
     const CXXParenListInitExpr *E) {
   if (DiscardResult) {
     for (const Expr *Init : E->getInitExprs()) {
Record *ByteCodeExprGen<Emitter>::getRecord(QualType Ty) {
   return nullptr;
 }
 
 template <class Emitter>
 Record *ByteCodeExprGen<Emitter>::getRecord(const RecordDecl *RD) {
   return P.getOrCreateRecord(RD);
 }
 
 template <class Emitter>
 const Function *ByteCodeExprGen<Emitter>::getFunction(const FunctionDecl *FD) {
   return Ctx.getOrCreateFunction(FD);
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *E) {
   ExprScope<Emitter> RootScope(this);
   if (!visit(E))
return dereference(
         },
         [this, E](PrimType T) {
           return DiscardResult ? this->emitPop(T, E) : true;
         });
   case UO_Not:    // ~x
     if (!this->visit(SubExpr))
       return false;
     return DiscardResult ? this->emitPop(*T, E) : this->emitComp(*T, E);
-  case UO_Real:   // __real x
-  case UO_Imag:   // __imag x
+  case UO_Real: { // __real x
+    assert(!T);
+    if (!this->visit(SubExpr))
+      return false;
+    if (!this->emitConstUint8(0, E))
+      return false;
+    return this->emitArrayElemPtrPopUint8(E);
+  }
+  case UO_Imag: { // __imag x
+    assert(!T);
+    if (!this->visit(SubExpr))
+      return false;
+    if (!this->emitConstUint8(1, E))
+      return false;
+    return this->emitArrayElemPtrPopUint8(E);
+  }
   case UO_Extension:
     return this->delegate(SubExpr);
   case UO_Coawait:
     assert(false && "Unhandled opcode");
   }
 
   return false;
 }

clang/lib/AST/Interp/Context.cpp

 }
 
 const LangOptions &Context::getLangOpts() const { return Ctx.getLangOpts(); }
 
 std::optional<PrimType> Context::classify(QualType T) const {
   if (T->isBooleanType())
     return PT_Bool;
 
+  if (T->isAnyComplexType())
+    return std::nullopt;
+
   if (T->isSignedIntegerOrEnumerationType()) {
     switch (Ctx.getIntWidth(T)) {
     case 64:
       return PT_Sint64;
     case 32:
       return PT_Sint32;
     case 16:
       return PT_Sint16;

clang/lib/AST/Interp/EvalEmitter.cpp

if (const auto *AT = Ty->getAsArrayTypeUnsafe()) {
         if (std::optional<PrimType> T = Ctx.classify(ElemTy)) {
           TYPE_SWITCH(*T, Ok &= ReturnValue<T>(EP.deref<T>(), Slot));
         } else {
           Ok &= Composite(ElemTy, EP.narrow(), Slot);
         }
       }
       return Ok;
     }
+
+    // Complex types.
+    if (const auto *CT = Ty->getAs<ComplexType>()) {
+      QualType ElemTy = CT->getElementType();
+      std::optional<PrimType> ElemT = Ctx.classify(ElemTy);
+      assert(ElemT);
+
+      if (ElemTy->isIntegerType()) {
+        INT_TYPE_SWITCH(*ElemT, {
+          auto V1 = Ptr.atIndex(0).deref<T>();
+          auto V2 = Ptr.atIndex(1).deref<T>();
+          Result = APValue(V1.toAPSInt(), V2.toAPSInt());
+          return true;
+        });
+      } else if (ElemTy->isFloatingType()) {
+        Result = APValue(Ptr.atIndex(0).deref<Floating>().getAPFloat(),
+                         Ptr.atIndex(1).deref<Floating>().getAPFloat());
+        return true;
+      }
+      return false;
+    }
     llvm_unreachable("invalid value to return");
   };
 
   // Return the composite type.
   const auto &Ptr = S.Stk.pop<Pointer>();
   return Composite(Ptr.getType(), Ptr, Result);
 }
 

clang/test/AST/Interp/complex.cpp

+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+// expected-no-diagnostics
+// ref-no-diagnostics
+
+constexpr _Complex double z1 = {1.0, 2.0};
+static_assert(__real(z1) == 1.0, "");
+static_assert(__imag(z1) == 2.0, "");
+
+constexpr double setter() {
+  _Complex float d = {1.0, 2.0};
+
+  __imag(d) = 4.0;
+  return __imag(d);
+}
+static_assert(setter() == 4, "");
+
+constexpr _Complex double getter() {
+  return {1.0, 3.0};
+}
+constexpr _Complex double D = getter();
+static_assert(__real(D) == 1.0, "");
+static_assert(__imag(D) == 3.0, "");
+
+
+constexpr _Complex int I1 = {1, 2};
+static_assert(__real(I1) == 1, "");
+static_assert(__imag(I1) == 2, "");
+
+
+/// FIXME: This should work in the new interpreter as well.
+// constexpr _Complex _BitInt(8) A = 0;// = {4};