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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/AST/ExprConstant.cpp:8829
+  return EvaluateComparisonBinaryOperator(Info, E, OnSuccess, [&]() {
+    return ExprEvaluatorBaseTy::VisitBinaryOperator(E);
+  });
----------------
EricWF wrote:
> rsmith wrote:
> > It'd be clearer to call `VisitBinCmp` rather than `VisitBinaryOperator`.
> @rsmith: OK, so I'm confused about this. Originally I had an `llvm_unreachable` that the continuation was never reached, but you suggested it was. I'm not sure how. Could you provide an example?
> 
> The precondition of calling `VisitBinCmp` is that we have a call to a builtin operator. For `<=>`,  where the composite type is either an arithmetic type, pointer type, or member pointer type (which includes enum types after conversions),  *All* of these cases should be handled before reaching the function.
> 
> Is there a control flow path I'm missing? 
What about comparisons of `_Complex` types, vector types, and any other builtin type that gets added after you commit this patch? The right thing to do (at least for now) in all of those cases is to call the base class implementation, which will deal with emitting the "sorry, I don't know how to constant-evaluate this" diagnostic.

My comment here was simply that when doing so, you should call the base-class version of the *same* function, which you now do, so that concern is addressed.


https://reviews.llvm.org/D45476