[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst:32
+   A string containing a semi-colon separated list of allowed include filenames.
+   The default is an empty string, which allows all includes.
----------------
juliehockett wrote:
> aaron.ballman wrote:
> > This default seems a bit odd to me, but perhaps it's fine. What's novel is that the check is a no-op by default, so how do Fuchsia developers get the correct list? Or is there no canonical list and developers are expected to populate their own manually?
> The idea is that it's a case-by-case basis -- this may change at some point in the future if we decide there's a standard whitelist of system includes across the board, but at the moment the thought is to allow everything in some places, and use this check to limit them in others. It'll need to be populated on a case-by-case basis, since different directories will have different requirements.
So there's never a case where you need to prohibit use of all system includes? Right now, an empty string means "allow all includes" while a non-empty string means "allow only these includes", so there's no way to prohibit all includes except by listing them manually. I'm wondering whether you want to use a glob for allowed file names, which gives you extra flexibility. e.g., `*` allows all includes (default value), `foo.h` allows only <foo.h>, `experimental/*` allows all headers in the experimental directory, `cstd*` allows <cstdint> while prohibiting <stdint.h>, and the empty string will disallow all system headers.

Perhaps you don't need that level of flexibility, but for the use cases I'm imagining it seems more user friendly to be able to write `cstd*` rather than manually listing out all the C++ headers explicitly.


https://reviews.llvm.org/D43778