aaron.ballman added reviewers: dblaikie, cjdb, echristo, clang-language-wg.
aaron.ballman added a comment.

In D147844#4286500 <https://reviews.llvm.org/D147844#4286500>, @philnik wrote:

> I have to say I'm not really convinced this change is a good idea. The cases 
> it flags don't really seem in any way ambiguous/erroneous.

I think some of the cases are ambiguous while others are not. It's taken me a 
while to come up with what I think my brain is doing, maybe this happens for 
others as well. When the ternary conditional expression is a binary expression, 
my brain can figure things out very quickly when the RHS of that binary 
expression is a literal but my brain is much less confident when the RHS of 
that binary expression is an identifier. e.g., `x & 1 ? foo : bar` is easy for 
my brain to go "well, obviously the condition is not testing whether `1` is 
nonzero, so it must be testing the result of `x & 1` instead", but something 
like `x & y ? foo : bar` is far less obvious as to what the behavior is because 
`x & (y ? foo : bar)` is as reasonable a choice as `(x & y) ? foo : bar` 
without putting a lot more thought into it. However, that might be splitting 
hairs with the diagnostic functionality (esp because macros and enumerations 
are sort of like literals and sort of like identifiers, depending on the way 
you want to think of them).

Adding in some more folks for opinions on whether the proposed changes in this 
patch are an improvement or whether we want to tweak the heuristic a bit more. 
My original thinking was that all precedence situations with ternary 
conditional expressions are kind of equally confusing, but given that two folks 
have pushed back on the proposed changes, more opinions would be valuable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147844/new/

https://reviews.llvm.org/D147844

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to