[PATCH] D38171: Implement clang-tidy check aliases.

2017-10-27 Thread András Leitereg via Phabricator via cfe-commits
leanil added a comment. In https://reviews.llvm.org/D38171#901427, @xazax.hun wrote: > One problem to think about when we add all clang-diagnostic as "first or > second" class citizen, `checkes=*` might now enable all the warnings which > make no sense and might be surprising to the users. What

[PATCH] D38171: Implement clang-tidy check aliases.

2017-10-27 Thread András Leitereg via Phabricator via cfe-commits
leanil updated this revision to Diff 120597. leanil added a comment. > Make clang-diagnostic-* checks first-class citizens and take full control of > all diagnostics, i.e. disable all Clang diagnostics by default, and enable > the ones that correspond to the enabled clang-diagnostic checks. (As

[PATCH] D38171: Implement clang-tidy check aliases.

2017-10-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. One problem to think about when we add all clang-diagnostic as "first or second" class citizen, `checkes=*` might now enable all the warnings which make no sense and might be surprising to the users. What do you think? https://reviews.llvm.org/D38171 _

[PATCH] D38171: Implement clang-tidy check aliases.

2017-10-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Let's also summarize what do we have now and what do we want. > I also think this sounds good, though I'm not quite sure it can be done in > this patch. Anyway, we should definitely specify what we expect from > first-class citizen checks. Please correct & extend the

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-26 Thread András Leitereg via Phabricator via cfe-commits
leanil added a comment. In https://reviews.llvm.org/D38171#880021, @xazax.hun wrote: > In https://reviews.llvm.org/D38171#878808, @alexfh wrote: > > > András, that's definitely an interesting idea. However, it might be > > interesting to explore a more principled approach: > > > > 1. Make `clang

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-26 Thread András Leitereg via Phabricator via cfe-commits
leanil updated this revision to Diff 116694. leanil added a comment. Add more tests and minor fixes. https://reviews.llvm.org/D38171 Files: clang-tidy/ClangTidy.cpp clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tidy/ClangTidyDiagnosticConsumer.h clang-tidy/ClangTidyModule.h clang-t

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D38171#880025, @xazax.hun wrote: > In https://reviews.llvm.org/D38171#880022, @lebedev.ri wrote: > > > A mail [0] posted by @JonasToth is about the similar problem, i think we > > can continue there. > > > Great! Could you summarize your po

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-25 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In https://reviews.llvm.org/D38171#880022, @lebedev.ri wrote: > A mail [0] posted by @JonasToth is about the similar problem, i think we can > continue there. Great! Could you summarize your points there as well? Thanks in advance. https://reviews.llvm.org/D38171

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-25 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D38171#878814, @lebedev.ri wrote: > On a somewhat related note, since this is already talking about aliases > > I feel like the current handling of the clang-tidy check aliases needs > adjusting. > If one enables all the checks (`Checks: '*'`

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D38171#880016, @xazax.hun wrote: > In https://reviews.llvm.org/D38171#878814, @lebedev.ri wrote: > > > I feel like the current handling of the clang-tidy check aliases needs > > adjusting. > > If one enables all the checks (`Checks: '*'`),

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-25 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In https://reviews.llvm.org/D38171#878808, @alexfh wrote: > András, that's definitely an interesting idea. However, it might be > interesting to explore a more principled approach: > > 1. Make `clang-diagnostic-*` checks first-class citizens and take full > control of

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-25 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. @leanil : Could you add a test case where the warnings are explicitly disabled in the compilation command and also one where only the aliased warning is explicitly disabled? In https://reviews.llvm.org/D38171#878814, @lebedev.ri wrote: > I feel like the current handl

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-22 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. On a somewhat related note, since this is already talking about aliases I feel like the current handling of the clang-tidy check aliases needs adjusting. If one enables all the checks (`Checks: '*'`), and then disables some of them on check-by-check basis, if the dis

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-22 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. András, that's definitely an interesting idea. However, it might be interesting to explore a more principled approach: 1. Make `clang-diagnostic-*` checks first-class citizens and take full control of all diagnostics, i.e. disable all Clang diagnostics by default, and en

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thank you for working on this -- it's very nice functionality! Comment at: clang-tidy/cert/CERTTidyModule.cpp:79 + void addWarningCheckAliases( + llvm::DenseMap &WarningCheckAliases) { +WarningCheckAliases.insert( You sh

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-22 Thread András Leitereg via Phabricator via cfe-commits
leanil created this revision. Herald added a subscriber: JDevlieghere. Clang warnings can be mapped to virtual check names as aliases. The mappings can be set in the appropriate ClangTidyModules. If the virtual check is enabled, then - the corresponding warning options get passed to clang, and