rtrieu marked an inline comment as done.
rtrieu added inline comments.

================
Comment at: test/Sema/parentheses.c:148
+
+  (void)(x | b ? 1 : 2); // expected-warning {{operator '?:' has lower 
precedence than '|'}} expected-note 2{{place parentheses}}
+  (void)(x & b ? 1 : 2); // expected-warning {{operator '?:' has lower 
precedence than '&'}} expected-note 2{{place parentheses}}
----------------
MaskRay wrote:
> rtrieu wrote:
> > MaskRay wrote:
> > > I hope these `| ? :` `& ? :` warnings are disabled-by-default.
> > These new warnings reuse the existing parentheses warnings, which is 
> > diag::warn_precedence_conditional.  That is on by default, so this one as 
> > written is also on by default..
> I agree that 
> 
> `cond1 ? 0xf0 : 0x10 | cond2 ? 0x5 : 0x2;` is confusing and justifies a 
> warning. But **what is tested here is different**.
> 
> That is why I created D65192, because such warnings are very annoying as 
> enabled-by-default diagnostics.
> 
> I think this change will make it even harder to remove some annoying 
> -Wparentheses warnings..
I can add tests for the other case.  This one was used because it was shorter 
and easier to copy.

Would creating a parentheses subgroup (like -Wbitwise-conditional-parentheses), 
duping the warning into it, and marking it DefaultIgnore be a better 
alternative for you?


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

https://reviews.llvm.org/D66043



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

Reply via email to