[PATCH] D146408: [clang][Interp] Start supporting complex types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/Interp/PrimType.h:108
+    switch (Expr) {                                                            \
+      TYPE_SWITCH_CASE(PT_Sint8, B)                                            \
+      TYPE_SWITCH_CASE(PT_Uint8, B)                                            \
----------------
tbaeder wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > tbaeder wrote:
> > > > aaron.ballman wrote:
> > > > > 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);
> > > > > ```
> > > > > 
> > > > `_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?
> > > > _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.
> > > 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.
> > 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.
> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146408/new/

https://reviews.llvm.org/D146408