[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 17 00:37:05 PDT 2018


JonasToth added inline comments.


================
Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:21
+bool IsInAbseilFile(const SourceManager& manager, SourceLocation loc){
+  if (loc.isInvalid()) {
+    return false;
----------------
You can elide the braces for single stmt ifs


================
Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24
+  }
+  auto FileEntry =
+      manager.getFileEntryForID(manager.getFileID(loc));
----------------
Please dont use `auto` here as the typ is not clear


================
Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:26
+      manager.getFileEntryForID(manager.getFileID(loc));
+  if (!FileEntry) {
+    return false;
----------------
ellide braces


================
Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:29
+  }
+  auto Filename = FileEntry->getName();
+  llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
----------------
no `auto` plz


================
Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:32
+                                              "synchronization|types|utiliy)");
+  return (RE.match(Filename));
+}
----------------
Are the parens necessary? I think `match` does return a `bool`, then you can just return it without the braces. If not, please make it a boolean with an expression like `match() > 0` or so.


https://reviews.llvm.org/D50542





More information about the cfe-commits mailing list