[PATCH] D13643: [Sema] Warn on ternary comparison

[Matěj Grabovský via cfe-commits]

mgrabovsky added a comment.

In http://reviews.llvm.org/D13643#265976, @aaron.ballman wrote:

> Should there be an exception to this diagnostic for code involving Boolean values? e.g.,
>
> void f(bool a, bool b, bool c) {
>
>   if (a == b == c)
>     ;
>
> }
>
> At the very least, it seems like this should also follow GCC's behavior and suggest parenthesis directly.


Interesting. This might be more complex than I had predicted. I'll try to dig into GCC's code and see what they do exactly. I can't guarantee returning sane, though.

Some thoughts: Even if `a, b, c` are Booleans, what did the user mean by `a == b == c`? Do they realise that the compiler interprets it as `(a == b) == c`? What about 'type mismatches', i.e. if `a, b` are `int`s, while `b` is `bool` – clearly the user must be aware of the interpretation above here. What if `a, b, c` are all `int`s – C90 doesn't have Booleans, so it might be intended as either.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5865
@@ -5864,1 +5864,3 @@
 
+def warn_ternary_comparison : Warning<"ternary comparisons do not work "
+  "as expected">,
----------------
aaron.ballman wrote:
> This diagnostic somewhat implies that ?: does not work as expected. I prefer GCC's wording for this:
> 
> "comparisons like 'X<=Y<=Z' do not have their mathematical meaning"
> 
> However, I would love it if we could do one step better and use the same operators the user wrote, if reasonable. ;-)
> This diagnostic somewhat implies that ?: does not work as expected.

?: is a ternary operator but it's not a comparison.

> I prefer GCC's wording for this

I wasn't sure if we wanted to be copycats, but OK.

> would love it if we could do one step better and use the same operators the user wrote

Yes, that would be nice.


http://reviews.llvm.org/D13643