[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-05-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D61288#1486034 , @xbolva00 wrote: > I am not familiar with clang-tidy’s codebase and I personally prefer good > compiler warnings than dependency on another tool (clang-tidy). I mean the > cases when diagnostic check is

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-05-01 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. I am not familiar with clang-tidy’s codebase and I personally prefer good compiler warnings than dependency on another tool (clang-tidy). I mean the cases when diagnostic check is easy to do in the compiler. And in semaexpr we have all we need and it is simple to do it

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-05-01 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D61288#1486008 , @aaron.ballman wrote: > In D61288#1486006 , @xbolva00 wrote: > > > Some coding guidelines may require switch to have always default label. > > Even if devs know that

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-05-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D61288#1486006 , @xbolva00 wrote: > Some coding guidelines may require switch to have always default label. Even > if devs know that default is not reachable, they can add default: abort(); or > assert to increase safety

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-05-01 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Some coding guidelines may require switch to have always default label. Even if devs know that default is not reachable, they can add default: abort(); or assert to increase safety (and warning will be silenced). Yes, it not suitable to be enabled by default, but I sti

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-05-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Do we have evidence of false positive vs true positive rates for this catching actual problems in real world code? I realize that this is for GCC compatibility, but because it's default-off, I'm wondering where the utilities lies. Comment at: i

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-05-01 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticIDs.h:39 DIAG_SIZE_CROSSTU = 100, - DIAG_SIZE_SEMA = 3500, + DIAG_SIZE_SEMA = 3600, DIAG_SIZE_ANALYSIS = 100

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-04-30 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added inline comments. Comment at: include/clang/Basic/DiagnosticIDs.h:39 DIAG_SIZE_CROSSTU = 100, - DIAG_SIZE_SEMA = 3500, + DIAG_SIZE_SEMA = 3600, DIAG_SIZE_ANALYSIS = 100, xbolva00 wrote: > @rsm

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-04-30 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticIDs.h:39 DIAG_SIZE_CROSSTU = 100, - DIAG_SIZE_SEMA = 3500, + DIAG_SIZE_SEMA = 3600, DIAG_SIZE_ANALYSIS = 100

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-04-29 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. GCC warns even for for that case, so this patch implements GCC's behaviour for now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61288/new/ https://reviews.llvm.org/D61288 ___ cfe-commits mailing list cfe-commits

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-04-29 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment. Is it intentional to warn even if all the cases are covered ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61288/new/ https://reviews.llvm.org/D61288 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-04-29 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 197229. xbolva00 added a comment. Removed redundant line in DiagnosticGroups CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61288/new/ https://reviews.llvm.org/D61288 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/Diagnostic

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

2019-04-29 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 created this revision. xbolva00 added reviewers: rsmith, aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D61288 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticIDs.h