aaron.ballman added inline comments.
================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:37-39
+ IncludeDirective(SourceLocation Loc_, CharSourceRange Range_,
+ StringRef Filename_)
+ : Loc(Loc_), Range(Range_), Filename(Filename_) {}
----------------
Please don't suffix with an underscore -- you can actually drop the underscore
here (we allow constructor arguments to hide the names of class members).
================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:48-49
+
+ typedef std::vector<IncludeDirective> FileIncludes;
+ std::map<clang::FileID, FileIncludes> IncludeDirectives;
+
----------------
Eugene.Zelenko wrote:
> Please use using and run Clang-tidy modernize.
Are you sure that `vector` and `map` are the best data types to use, rather
than LLVM ADTs?
Also you can drop the elaborated type specifier for `FileID`.
================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:69
+
+ for (auto &Bucket : IncludeDirectives) {
+ auto &FileDirectives = Bucket.second;
----------------
`const auto &`
================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:74
+ for (const auto &Include : FileDirectives) {
+ // Emit a warning.
+ auto D = Check.diag(Include.Loc, "Restricted include found.");
----------------
This comment doesn't really add much value.
================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:75
+ // Emit a warning.
+ auto D = Check.diag(Include.Loc, "Restricted include found.");
+
----------------
Diagnostics are not complete sentences, so you should remove the punctuation
and capitalization here.
================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.h:32
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ std::vector<std::string> &getRestrictedIncludes() {
+ return RestrictedIncludes;
----------------
Function can be `const` and return a `const` ref.
================
Comment at: test/clang-tidy/fuchsia-restrict-includes.cpp:13
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Restricted include found.
+// CHECK-FIXES-NOT: #include <s.h>
----------------
I think this could use some edge case tests for awful things people can do:
```
#define foo <stdio.h>
#include foo
# /* hahaha */ include /* oops */ foo
```
https://reviews.llvm.org/D43778
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
