rsmith added inline comments.
================
Comment at: lib/Sema/SemaExpr.cpp:9816
+ RHS = S.ImpCastExprToType(RHS.get(), Type, CK_BitCast);
+ } else {
+ // C++2a [expr.spaceship]p4
----------------
EricWF wrote:
> rsmith wrote:
> > We still need to apply the usual arithmetic conversions after converting
> > enumerations to their underlying types (eg, `<=>` on `enum E : char`
> > converts the operands first to `char` then to `int`). You could remove the
> > `else` here and make this stuff unconditional, but it's probably better to
> > sidestep the extra work and convert directly to the promoted type of the
> > enum's underlying type.
> Do we still do usual arithmetic conversions if we have two enumerations of
> the same type?
Formally, yes: "If both operands have the same enumeration type E, the operator
yields the result of converting the operands to the underlying type of E and
applying <=> to the converted operands."
The recursive application of `<=>` to the converted operands will perform the
usual arithmetic conversions.
================
Comment at: lib/Sema/SemaExpr.cpp:9794
+ // other is not, the program is ill-formed.
+ if (int Count = LHSType->isBooleanType() + RHSType->isBooleanType()) {
+ // TODO: What about bool non-narrowing cases? Like '0' or '1.
----------------
You could simplify this to `if (LHSType->isBooleanType() !=
RHSType->isBooleanType()) InvalidOperands`.
================
Comment at: lib/Sema/SemaExpr.cpp:9795
+ if (int Count = LHSType->isBooleanType() + RHSType->isBooleanType()) {
+ // TODO: What about bool non-narrowing cases? Like '0' or '1.
+ if (Count != 2) {
----------------
Missing `'`. Seems like a question to take to Herb and CWG (and maybe EWG), but
I suspect the answer will be "this does what we wanted".
Looks like P0946R0 missed this case of a difference between `<=>` and other
operators... oops.
================
Comment at: lib/Sema/SemaExpr.cpp:9829-9832
+ if (EDecl->isScoped()) {
+ S.InvalidOperands(Loc, LHS, RHS);
+ return QualType();
+ }
----------------
I don't think you need this check: the usual arithmetic conversions will fail
to find a common type if the enum is scoped.
================
Comment at: lib/Sema/SemaExpr.cpp:9881-9890
enum { StrongEquality, PartialOrdering, StrongOrdering } Ordering;
if (Type->isAnyComplexType())
Ordering = StrongEquality;
else if (Type->isFloatingType())
Ordering = PartialOrdering;
else
Ordering = StrongOrdering;
----------------
This is now over-complicated, since this is unreachable for `BO_Cmp`.
================
Comment at: lib/Sema/SemaExpr.cpp:9906
+ bool IsThreeWay = Opc == BO_Cmp;
+ auto IsPointerType = [](ExprResult E) {
+ QualType Ty = E.get()->getType().getNonReferenceType();
----------------
I'd prefer a name that captures that this also recognizes member pointer types.
================
Comment at: lib/Sema/SemaExpr.cpp:9907
+ auto IsPointerType = [](ExprResult E) {
+ QualType Ty = E.get()->getType().getNonReferenceType();
+ return Ty->isPointerType() || Ty->isMemberPointerType();
----------------
You don't need a `.getNonReferenceType()` here; expressions can't have
reference type.
https://reviews.llvm.org/D45476
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
