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

[Eric Fiselier via Phabricator via cfe-commits]

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


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9377-9378
+def err_implied_comparison_category_type_not_found : Error<
+  "%0 type was not found; include <compare> before defining "
+  "or using 'operator<=>'">;
+def err_spaceship_argument_narrowing : Error<
----------------
rsmith wrote:
> This doesn't sound quite right. You can define your own `operator<=>` without including `<compare>` so long as you don't try to default it or use one of the standard comparison category kinds. Also, instead of saying "before doing X or Y", say which one the user actually did.
Ah, right. I guess this should say "cannot deduce return type of operator<=> because type %0 was not found; include <compare>"


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9384-9385
+def err_spaceship_comparison_of_void_ptr : Error<
+  "three-way comparison with void pointer %select{operand type|operand types}0 "
+  "(%1 and %2)">;
+def err_spaceship_comparison_of_invalid_comp_type : Error<
----------------
rsmith wrote:
> Why are you diagnosing this case as an error?
[expr.spaceship]p9 seems to require it. The composite pointer type is not a function/member/null pointer type, neither is it a object pointer type, so the program is ill-formed.

Unless I'm missing something.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9391
+def err_std_compare_type_invalid_member : Error<
+  "member '%1' of %0 is ill-formed">;
+def note_spaceship_operand_not_cce : Note<
----------------
rsmith wrote:
> What do you mean by "ill-formed" here?
> 
> I think what you really mean is "we don't understand how to deal with this definition of that member", so maybe we should just fold this and the prior diagnostic into something like "sorry, this standard library implementation of std::whatever is not supported"?
That's exactly what I'm trying to say. 'll clean the diagnostic up. Thanks!


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9393
+def note_spaceship_operand_not_cce : Note<
+  "argument is not a constant expression">;
 } // end of sema component.
----------------
rsmith wrote:
> Is this really useful? I would think almost all the cases where you'd hit the "cannot be narrowed" error, this diagnostic and the explanation of why the operand is not constant would be meaningless noise, because it was never meant to be constant, and the problem is simply that you are trying to three-way compare integers of mixed signedness.
I'm struggling to answer that question myself. The case I was thinking of that I wanted to help the user out with is:

```
auto cmp_sentinal(long val) {
  int SENTINAL = 0;
  return SENTINAL <=> val; // error, would be OK if SENTINAL were const.
}
```

I'll remove these diagnostics for now, and hopefully improve them in a follow up patch, if that's OK with you?


https://reviews.llvm.org/D45476