[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked 7 inline comments as done.
EricWF added inline comments.


================
Comment at: lib/AST/ExprConstant.cpp:6238-6263
+    bool VisitBinCmp(const BinaryOperator *E) {
+      using CCR = ComparisonCategoryResult;
+      const ComparisonCategoryInfo &CmpInfo =
+          Info.Ctx.CompCategories.getInfoForType(E->getType());
+
+      // Build a new version of the binary operator which returns an integer
+      // representing the ComparisonCategoryResult. Then defer to
----------------
rsmith wrote:
> I don't like this approach (inventing an expression that is invalid so that we can call into some other code to compute this result). If this is really materially better than implementing `<=>` directly here, this should be justified by comments here. But I would suspect that factoring out the common code for dealing with the various kinds of comparison from `IntExprEvaluator` and calling it from both here and there would lead to an overall better design.
Ack.

I tried to split the code out, but it wasn't easy and the result wasn't pretty. I'll go ahead and give this another go.
The problems I ran into were:
* de-duplicating error checking and argument evaluation across comparison and non-comparison operations.
* three-way and non-three-way comparison operations still return a fundamentally different type, so reporting the results still requires either representing Cmp results as an integer, or having different mechanism to report the result.

Any advice or input you have would be appreciated.


https://reviews.llvm.org/D45476