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.

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.

cfe-commits mailing list

Reply via email to