[libc-commits] [PATCH] D77281: [libc] Add libc-tidy.

[Alex Brachet via Phabricator via libc-commits]

abrachet added inline comments.


================
Comment at: libc/utils/HdrGen/PublicAPICommand.h:44
+  // spec.td to be subclassed. Hence, we do not want the record |Def|
+  // to be of more than one class type..
+  if (Classes.size() != 1)
----------------
While we're moving this, we could get rid of the extra . at the end.


================
Comment at: libc/utils/LibcTidy/LibcTidyTool.cpp:55
+
+//-- Check Callbacks --//
+class QualifiedCallCheck : public MatchFinder::MatchCallback {
----------------
What is the purpose of this and the end comment?


================
Comment at: libc/utils/LibcTidy/LibcTidyTool.cpp:66
+
+  virtual void run(const MatchFinder::MatchResult &result) {
+    const auto *funcDecl = result.Nodes.getNodeAs<FunctionDecl>("func");
----------------
Remove `virtual` and add `override`


================
Comment at: libc/utils/LibcTidy/LibcTidyTool.cpp:83
+    DiagnosticsEngine &DE = result.Context->getDiagnostics();
+    const unsigned ID = DE.getCustomDiagID(
+        DiagnosticsEngine::Error,
----------------
No point for `const`


================
Comment at: libc/utils/LibcTidy/LibcTidyTool.cpp:102-104
+  if (llvm::Error Err = functionsOrErr.takeError()) {
+    return 1;
+  }
----------------
`logAllUnhandledErrors(std::move(Err), errs())` here probably.


================
Comment at: libc/utils/LibcTidy/TableGenUtil.cpp:42
+  const auto &defsMap = records.getDefs();
+  for (auto &pair : defsMap) {
+    llvm::Record *def = pair.second.get();
----------------
We could make pair explicitly `const` 


================
Comment at: libc/utils/LibcTidy/TableGenUtil.cpp:48-50
+    for (llvm::StringRef functionName : functionNameList) {
+      funcNames.insert(std::string(functionName));
+    }
----------------
`llvm::transform(functionNameList, std::back_inserter(funcNames), [](StringRef s) { return std::string(s); });` maybe


================
Comment at: libc/utils/LibcTidy/TableGenUtil.h:19
+llvm::Expected<NameSet>
+getAllLibcFunctions(const std::string &tablegenFile,
+                    const std::string &tablgenIncludeDir);
----------------
Can these be `StringRef`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77281/new/

https://reviews.llvm.org/D77281