EricWF added inline comments.
================
Comment at: lib/CodeGen/CGExprAgg.cpp:1002
+ return EmitFinalDestCopy(
+ E->getType(), CGF.MakeNaturalAlignAddrLValue(Select, E->getType()));
+}
----------------
rjmccall wrote:
> Is there something in Sema actually validating that the comparison types is
> trivially copyable? Because EmitFinalDestCopy does not work on non-trivial
> types.
>
> Also, are we certain that we're allowed to do a copy from an actual global
> variable here instead of potentially a constexpr evaluation of the variable
> reference? And if we are doing a copy, are we registering ODR-uses of all
> the variables in Sema?
>
> I don't think you should worry about trying to generate a select between the
> "actual" comparison result values. At least not yet.
There is nothing is Sema validating that these comparison types are trivially
copyable, and according to the standard they don't need to be.
I assumed we only ended up in `CGExprAgg` if the types were trivially copyable.
But I'll look into implementing this for non-trivially copyable comparison
types (although we'll probably never actually encounter them).
> Also, are we certain that we're allowed to do a copy from an actual global
> variable here instead of potentially a constexpr evaluation of the variable
> reference?
I'm not sure exactly what this means. How would I observe the difference?
>And if we are doing a copy, are we registering ODR-uses of all the variables
>in Sema?
Probably not. I'll double check that.
> I don't think you should worry about trying to generate a select between the
> "actual" comparison result values. At least not yet.
I'm not sure exactly what you mean by this.
================
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) {
----------------
rsmith wrote:
> 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.
I'll remove the comment for now then.
================
Comment at: lib/Sema/SemaExpr.cpp:9906
+ bool IsThreeWay = Opc == BO_Cmp;
+ auto IsPointerType = [](ExprResult E) {
+ QualType Ty = E.get()->getType().getNonReferenceType();
----------------
rsmith wrote:
> I'd prefer a name that captures that this also recognizes member pointer
> types.
I'm bad at naming things. Is `IsAnyPointerType` better?
https://reviews.llvm.org/D45476
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
