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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to