This patch makes it possible for clang-tidy clients to provide
different options for different translation units. The option, which doesn't
make sense to be file-dependent, was moved to a separate ClangTidyGlobalOptions
struct. Added parsing of ClangTidyOptions.
Details
Diff Detail
Event Timeline
Added a comment and an assertion, replaced a couple of |= with definitely short cirtuited versions.
clang-tidy/ClangTidy.cpp | ||
---|---|---|
206 | Any reasons for not using a vector of unique_ptr's an moving that into the ASTConsumer? | |
214 | Finder.get()? | |
268 | Wait, this seems to have been incorrect before (| instead of logical ||), and I think we're missing parens. | |
clang-tidy/ClangTidyDiagnosticConsumer.h | ||
177 | Any reason not to store a std::string here? Seems fragile to store a StringRef. | |
clang-tidy/ClangTidyOptions.h | ||
30–31 | ... for which we show warnings. | |
34–43 | neither... nor... | |
37–38 | I think the second sentence doesn't help. I'd delete it. | |
75–77 | I'd vote for putting members at the end of the class. |
Addressed review comments.
clang-tidy/ClangTidy.cpp | ||
---|---|---|
206 | Just didn't want to touch the createChecks method. But this isn't a serious reason. Fixed. | |
214 | 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. | |
268 | 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. | |
clang-tidy/ClangTidyDiagnosticConsumer.h | ||
177 | You're right. Changed. | |
clang-tidy/ClangTidyOptions.h | ||
30–31 | Done. | |
34–43 | Done. | |
37–38 | 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. | |
75–77 | Done. |
lg
clang-tidy/ClangTidy.cpp | ||
---|---|---|
214 | I'm probably in the "more explicit" camp here - &* will work on anything that's deref'able, .get() will only work on smart-pointer-compatible types. I like the fact that I am able to tell whether &* is just a no-op after a refactoring/replacement that hasn't been noticed (for example, after changing a reference to a pointer variable), or whether it's an intentional retrieval of the pointer. I don't care too much, so feel free to leave it in. | |
clang-tidy/ClangTidyOptions.h | ||
37–38 | Interesting. My default interpretation of a filter is "for all filter, take out what it matches", which, if applied with an empty filter, doesn't change anything. |
Any reasons for not using a vector of unique_ptr's an moving that into the ASTConsumer?