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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
