[PATCH] Allow per-file clang-tidy options.
Alexander Kornienko
alexfh at google.com
Mon Jun 2 15:46:36 PDT 2014
================
Comment at: clang-tidy/ClangTidy.cpp:208
@@ +207,3 @@
+
+ SmallVector<ClangTidyCheck*, 8> Checks;
+ ChecksFilter &Filter = Context.getChecksFilter();
----------------
Manuel Klimek wrote:
> Any reasons for not using a vector of unique_ptr's an moving that into the ASTConsumer?
Just didn't want to touch the createChecks method. But this isn't a serious reason. Fixed.
================
Comment at: clang-tidy/ClangTidy.cpp:216
@@ -204,1 +215,3 @@
+ Check->setContext(&Context);
+ Check->registerMatchers(&*Finder);
Check->registerPPCallbacks(Compiler);
----------------
Manuel Klimek wrote:
> Finder.get()?
It's a matter of taste. I slightly prefer &*, and &* is used two more times in this file (lines 98, 99) and in other files in clang-tidy. If you have serious reasons to prefer .get() over &*, I can change all of the usages in a separate patch, but now I would like the code to be consistent in this regard.
================
Comment at: clang-tidy/ClangTidy.cpp:269
@@ +268,3 @@
+ AnalyzerChecksEnabled =
+ AnalyzerChecksEnabled | !CheckName.startswith("debug") &&
+ Filter.isCheckEnabled(Checker);
----------------
Manuel Klimek wrote:
> Wait, this seems to have been incorrect before (| instead of logical ||), and I think we're missing parens.
Err, it's actually incorrect now (| instead of ||). And we're missing parens (to mute the compiler warning). Before this patch it was kind of correct (except that it wasn't short circuited).
Fixed.
================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:170
@@ +169,3 @@
+
+ StringRef CurrentFile;
+ std::unique_ptr<ChecksFilter> CheckFilter;
----------------
Manuel Klimek wrote:
> Any reason not to store a std::string here? Seems fragile to store a StringRef.
You're right. Changed.
================
Comment at: clang-tidy/ClangTidyOptions.h:30-31
@@ -25,1 +29,4 @@
+
+ /// \brief A list of line ranges in this file, from where the warnings may be
+ /// displayed.
std::vector<LineRange> LineRanges;
----------------
Manuel Klimek wrote:
> ... for which we show warnings.
Done.
================
Comment at: clang-tidy/ClangTidyOptions.h:35
@@ -27,2 +34,3 @@
-/// \brief Contains options for clang-tidy.
+/// \brief Global options. These options are nor stored neither read from
+/// configuration files.
----------------
Manuel Klimek wrote:
> neither... nor...
Done.
================
Comment at: clang-tidy/ClangTidyOptions.h:38-39
@@ +37,4 @@
+struct ClangTidyGlobalOptions {
+ /// \brief Output warnings from certain line ranges of certain files only. If
+ /// this list is emtpy, it won't be applied.
+ std::vector<FileFilter> LineFilter;
----------------
Manuel Klimek wrote:
> I think the second sentence doesn't help. I'd delete it.
I'd leave it, as it's describes a special case. Without special-casing the empty list, the list would be applied always, and the empty list would mean that no warnings will pass the filter.
================
Comment at: clang-tidy/ClangTidyOptions.h:76-78
@@ +75,5 @@
+class DefaultOptionsProvider : public ClangTidyOptionsProvider {
+ ClangTidyGlobalOptions GlobalOptions;
+ ClangTidyOptions DefaultOptions;
+
+public:
----------------
Manuel Klimek wrote:
> I'd vote for putting members at the end of the class.
Done.
http://reviews.llvm.org/D3979
More information about the cfe-commits
mailing list