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

Richard via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 19 10:07:35 PDT 2022


LegalizeAdulthood added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h:63
   std::vector<detail::IncludeMarker> IncludesToBeProcessed;
+  bool WarnIntoHeaders;
 };
----------------
whisperity wrote:
> LegalizeAdulthood wrote:
> > 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.
> I do not know the answer to question //1//, but as an owner of a code-base, I would not want to know or deal with keeping additional lists (be it file-by-file, glob expressions, or regex...) specific to what headers are includable from C and what aren't. Especially considering that every C header could be included from C++. (Note how LLVM uses `.h` instead of `.hpp` for its headers, even though >90% of our headers would never compile in C mode...) If a check is misbehaving for my codebase, I'd just simply disable that check (1 line of code) and go on with my life, instead of creating a curated list, that is //at least// 2 LoC in a config file, //and// has to be maintained down the line...
> 
> Moreover, the `-header-filter` regex and `-system-headers` are Tidy-level global flags. This means that I would either have to let go of EVERY potential diagnostic that might be placed in headers, or tinker with my environment to run multiple instances of Clang-Tidy with different configurations.
Those are all good points.  One of the weakest aspects of `clang-tidy` is the selection of files that should be analyzed and/or fixed.  Otherwise, we wouldn't need `clang-tidy/tool/run-clang-tidy.py` at all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125769/new/

https://reviews.llvm.org/D125769



More information about the cfe-commits mailing list