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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.


================
Comment at: clang-tidy/fuchsia/CMakeLists.txt:8
   OverloadedOperatorCheck.cpp
+  RestrictIncludesCheck.cpp
   StaticallyConstructedObjectsCheck.cpp
----------------
Will be good idea to be have consistent name and documentation terminology: include versus header.


================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:48
+
+  typedef std::vector<IncludeDirective> FileIncludes;
+  std::map<clang::FileID, FileIncludes> IncludeDirectives;
----------------
Please use using and run Clang-tidy modernize.


================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:70
+  for (auto &Bucket : IncludeDirectives) {
+    auto &FileDirectives = Bucket.second;
+
----------------
Please don't use auto here, since type is not obvious.


================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:80
+      unsigned ToLen =
+          std::strcspn(SM.getCharacterData(Include.Range.getBegin()), "\n") + 1;
+      auto ToRange = CharSourceRange::getCharRange(
----------------
Please include <cstring>.


================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:81
+          std::strcspn(SM.getCharacterData(Include.Range.getBegin()), "\n") + 1;
+      auto ToRange = CharSourceRange::getCharRange(
+          Include.Loc, ToLoc.getLocWithOffset(ToLen));
----------------
Please don't use auto here, since type is not obvious.


================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:101
+  Compiler.getPreprocessor().addPPCallbacks(
+      ::llvm::make_unique<RestrictedIncludesPPCallbacks>(
+          *this, Compiler.getSourceManager()));
----------------
I don't think that you should prefix llvm with ::


================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.h:25
+class RestrictIncludesCheck : public ClangTidyCheck {
+ public:
+  RestrictIncludesCheck(StringRef Name, ClangTidyContext *Context)
----------------
Please run Clang-format over code.


================
Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.h:30
+            Options.get("Includes", "::std::vector"))) {}
+  void registerPPCallbacks(CompilerInstance &Compiler) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
----------------
Please separate constructor with empty line from rest of methods.


https://reviews.llvm.org/D43778