DmitryPolukhin added a comment. @njames93 thank you for the feedback!
Still waiting for more feedback and especially high level one about this feature in general. Does it look useful? Should it be behind a flag i.e. changing default behaviour is too dangerous? ================ Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:249-251 + if (!HeaderFilter) + HeaderFilter = + std::make_unique<llvm::Regex>(*getOptions().HeaderFilterRegex); ---------------- njames93 wrote: > This should also check if the Optional has a value This code was moved from `ClangTidyDiagnosticConsumer::getHeaderFilter` but you are right, it makes sense to add support for missing value. ================ Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:349-351 + // Skip macro from system headers. + if (!*Context.getOptions().SystemHeaders && SM.isInSystemHeader(SpellingLoc)) + return true; ---------------- njames93 wrote: > This looks suspicious, in clang-tidy land `SystemHeaders` is always set, > however outside clang-tidy it may not be. > Perhaps `getValueOr(false)` should be used to prevent any asserts. > > Also can a test be added to show this respecting the SystemHeaders setting. Running this code on bigger codebase I found undesired side-effect that it is better to report issues on system macro like NULL such issues usually from external code, not from macro itself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90835/new/ https://reviews.llvm.org/D90835 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
