Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 20, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 68 additions & 50 deletions clang/lib/AST/Interp/ByteCodeExprGen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,57 +241,11 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {

case CK_IntegralComplexToBoolean:
case CK_FloatingComplexToBoolean: {
PrimType ElemT = classifyComplexElementType(SubExpr->getType());
// 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:
Expand Down Expand Up @@ -2223,8 +2177,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);
}
Comment on lines +2182 to +2186
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AaronBallman probably knows a reason for this.

return false;
}

if (!this->visit(E))
return false;
Expand Down Expand Up @@ -3383,6 +3344,63 @@ 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).
template <class Emitter>
bool ByteCodeExprGen<Emitter>::emitRecordDestruction(const Record *R) {
assert(R);
Expand Down
1 change: 1 addition & 0 deletions clang/lib/AST/Interp/ByteCodeExprGen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
}

bool emitComplexReal(const Expr *SubExpr);
bool emitComplexBoolCast(const Expr *E);

bool emitRecordDestruction(const Record *R);
bool emitDestruction(const Descriptor *Desc);
Expand Down
21 changes: 21 additions & 0 deletions clang/test/AST/Interp/c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,24 @@ void bar_0(void) {
*(int *)(&S.a) = 0; // all-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
*(int *)(&S.b) = 0; // all-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
}

/// Complex-to-bool casts.
const int A = ((_Complex double)1.0 ? 21 : 1);
_Static_assert(A == 21, ""); // pedantic-ref-warning {{GNU extension}} \
// pedantic-expected-warning {{GNU extension}}

const int CTI1 = ((_Complex double){0.0, 1.0}); // pedantic-ref-warning {{extension}} \
// pedantic-expected-warning {{extension}}
_Static_assert(CTI1 == 0, ""); // pedantic-ref-warning {{GNU extension}} \
// pedantic-expected-warning {{GNU extension}}

const _Bool CTB2 = (_Bool)(_Complex double){0.0, 1.0}; // pedantic-ref-warning {{extension}} \
// pedantic-expected-warning {{extension}}
_Static_assert(CTB2, ""); // pedantic-ref-warning {{GNU extension}} \
// pedantic-expected-warning {{GNU extension}}

const _Bool CTB3 = (_Complex double){0.0, 1.0}; // pedantic-ref-warning {{extension}} \
// pedantic-expected-warning {{extension}}
_Static_assert(CTB3, ""); // pedantic-ref-warning {{GNU extension}} \
// pedantic-expected-warning {{GNU extension}}

2 changes: 2 additions & 0 deletions clang/test/AST/Interp/complex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ constexpr int ignored() {
(int)D1;
(double)D1;
(_Complex float)I2;
(bool)D1;
(bool)I2;
return 0;
}
static_assert(ignored() == 0, "");
Expand Down