LegalizeAdulthood added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h:63 std::vector<detail::IncludeMarker> IncludesToBeProcessed; + bool WarnIntoHeaders; }; ---------------- 1) How is this different from the clang-tidy option that specifies whether or not fixits are applied to header files? As an owner of a code base, I would know which header files are included from C source files and I would set my header-file regex (honestly, not a fan of a regex for that option; I'd prefer white/black lists, but that's another discussion) to exclude header files that are known to be included in C source files. 2) Assuming that the header-file regex is somehow insufficient to cover this scenario, I like the functionality but the name of this option feels "off". (Naming things is hard.) Elsewhere we have options that say `HeaderFile` not `Headers` and `Into` just doesn't sound like the way normal conversation would state the situation. Something like `CheckHeaderFile` would be more consistent with existing options. ================ Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp:2-4 // Copy the 'mylib.h' to a directory under the build directory. This is // required, since the relative order of the emitted diagnostics depends on the // absolute file paths which is sorted by clang-tidy prior emitting. ---------------- IMO, all of this hackery is simply ducking the issue, which is that `check_clang_tidy.py` doesn't have proper support for validating fixits applied to header files. See https://reviews.llvm.org/D17482 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125769/new/ https://reviews.llvm.org/D125769 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
