juliehockett added inline comments.
================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:75
+ for (const auto &Include : FileDirectives) {
+ auto D = Check.diag(Include.Loc, "restricted include found");
+
----------------
aaron.ballman wrote:
> I believe this check will be finding transitive includes, correct? If so,
> maybe this should be two diagnostics: one to say `"'%0' is prohibited from
> being included"` and a note for the transitive case to say `"file included
> transitively from here"`?
>
> Regardless of the behavior you'd like for it, we should have a test case:
> ```
> // a.h
> // Assumed that a.h is prohibited
> void f();
>
> // b.h
> #include "a.h"
>
> // c.c
> #include "b.h"
>
> int main() {
> f();
> }
> ```
It will flag, but not fix, if a disallowed file is transitively included *and*
the relevant flag is passed (e.g. -system-headers for system, or
-header-filter=.* for other. I changed the warning text for header warnings to
clarify that.
================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:83
+
+ D << FixItHint::CreateRemoval(ToRange);
+ }
----------------
aaron.ballman wrote:
> Are you okay with breaking user's code from this removal? If you remove the
> header file, but not the parts of the code that require that header file to
> be included, this FixIt will break code.
Yes, but I did add a note to the documentation to call that out.
================
Comment at: docs/clang-tidy/checks/fuchsia-restrict-includes.rst:27
+
+ A string containing a comma-separated list of header filenames to restrict.
Default is an empty string.
----------------
hokein wrote:
> The check seems do nothing with the default option.
>
> Do we have a corresponding guideline of fuchsia?
The idea is to maintain a list of disallowed headers in the local .clang-tidy
file, so it will be dependent on that. If no headers are restricted, the check
won't do anything. I'll add documentation to that effect.
https://reviews.llvm.org/D43778
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
