[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-08 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment. In D58896#1738288 , @aaron.ballman wrote: > In D58896#1738263 , @sberg wrote: > > > In D58896#1737242 , @edward-jones > > wrote: > > > > > In D58896#

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D58896#1738263 , @sberg wrote: > In D58896#1737242 , @edward-jones > wrote: > > > In D58896#1737113 , @sberg wrote: > > > > > But how about

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-08 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment. In D58896#1737242 , @edward-jones wrote: > In D58896#1737113 , @sberg wrote: > > > But how about literals like `'\x80'` where the promoted value depends on > > whether plain `char` is signed

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. int('i') is nice fixit we could emit in this case! Thanks for this idea! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58896/new/ https://reviews.llvm.org/D58896 ___ cfe-commi

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In D58896#1737242 , @edward-jones wrote: > In D58896#1737200 , @xbolva00 wrote: > > > Well, i am not sure if one twitter report is good motivation to criple > > warning. > > > The moti

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58896/new/ https://reviews.llvm.org/D58896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Edward Jones via Phabricator via cfe-commits
edward-jones added a comment. Okay I've reverted this in rG90ecfa2f5f7f . I'll make improvements and resubmit this for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5889

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. It was said Clang does not have to match gcc 1:1. Just because someone uses weird patterns and instead of using pragma push/pop diagnostic in his/her code, should we really change it in Clang? You should provide extra flag which controls this specific behaviour (and on

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Edward Jones via Phabricator via cfe-commits
edward-jones added a comment. In D58896#1737113 , @sberg wrote: > But how about literals like `'\x80'` where the promoted value depends on > whether plain `char` is signed or unsigned? If 'char' is signed and index into an array then this will typically

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Well, i am not sure if one twitter report is good motivation to criple warning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58896/new/ https://reviews.llvm.org/D58896 ___ cf

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment. But how about literals like `'\x80'` where the promoted value depends on whether plain `char` is signed or unsigned? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58896/new/ https://reviews.llvm.org/D58896 ___

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Edward Jones via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7adab7719e55: [Sema] Suppress -Wchar-subscripts if the index is a literal char (authored by edward-jones). Herald added a subscriber: simoncook. Changed prior to commit: https://reviews.llvm.org/D58896?

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-03-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D58896#1428816 , @edward-jones wrote: > In D58896#1419964 , @aaron.ballman > wrote: > > > Do

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-03-14 Thread Edward Jones via Phabricator via cfe-commits
edward-jones added a comment. In D58896#1419964 , @aaron.ballman wrote: > Do you have some evidence that the current behavior is causing a lot of false > positives in the wild? For ASCII character literals, I can sort of guess at > why people might want

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-03-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Do you have some evidence that the current behavior is causing a lot of false positives in the wild? For ASCII character literals, I can sort of guess at why people might want to do this, but for things like wide character literals, or character literals relying o

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-03-04 Thread Edward Jones via Phabricator via cfe-commits
edward-jones created this revision. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. Assume that the user knows what they're doing if they provide a char literal as an array index. This more closely matches the behavior of GCC. Repository: rC Clang https://revi