VitaNuo marked 8 inline comments as done.
VitaNuo added a comment.
Thanks for the comments! I'll re-assign the review to Haojian for now, so that
we can hopefully make more progress this week.
================
Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:48
+void IncludeCleanerCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
----------------
kadircet wrote:
> in theory this is not enough to disable the check for objc++, it'll have both
> objc and cplusplus set. so we should check `ObjC` is false (there's no need
> to disable it for `c`, right?)
>
> nit: we can also override ` isLanguageVersionSupported` to disable check for
> non-c++.
Ah thanks for the tip with `isLanguageVersionSupported`, doing that now.
================
Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:67
+ for (const auto &Header : IgnoreHeaders)
+ IgnoreHeadersRegex.push_back(llvm::Regex{Header});
+ return IgnoreHeadersRegex;
----------------
kadircet wrote:
> let's make sure we're only going to match suffixes by appending `$` to the
> `Header` here.
>
> also before pushing into `IgnoreHeadersRegex` can you verify the regex
> `isValid` and report a diagnostic via `configurationDiag` if regex is invalid.
Thanks, good ideas!
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst:15
+
+ A comma-separated list of header files that should be ignored during the
+ analysis. The option allows for regular expressions. E.g., `foo/.*` disables
----------------
kadircet wrote:
>
You probably wanted to say `insertion/removal` instead of `inclusion/insertion`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148793/new/
https://reviews.llvm.org/D148793
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
