[PATCH] D125769: [clang-tidy] Introduce the WarnIntoHeaders option to modernize-deprecated-headers

[Richard via Phabricator via cfe-commits]

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