Revert "Fix clang-tidy warnings in Caffe2 code (pytorch#134935)"

This reverts commit 7cfd236.

Reverted pytorch#134935 on behalf of https://github.com/izaitsevfb due to breaks internal builds, caffe2 is still used internally ([comment](pytorch#134935 (comment)))

.lintrunner.toml

@@ -210,8 +210,6 @@ include_patterns = [
     'aten/src/ATen/native/nested/*.h',
     'c10/**/*.cpp',
     'c10/**/*.h',
-    'caffe2/**/*.cc',
-    'caffe2/**/*.h',
     'torch/*.h',
     'torch/csrc/*.h',
     'torch/csrc/*.cpp',

BUCK.oss

@@ -65,6 +65,7 @@ cxx_library(
         "caffe2/serialize/file_adapter.cc",
         "caffe2/serialize/inline_container.cc",
         "caffe2/serialize/istream_adapter.cc",
+        "caffe2/serialize/read_adapter_interface.cc",
     ],
     visibility = ["PUBLIC"],
     deps = [

BUILD.bazel

@@ -473,6 +473,7 @@ filegroup(
         "caffe2/serialize/file_adapter.cc",
         "caffe2/serialize/inline_container.cc",
         "caffe2/serialize/istream_adapter.cc",
+        "caffe2/serialize/read_adapter_interface.cc",
     ],
 )
 

build.bzl

@@ -34,6 +34,7 @@ def define_targets(rules):
             "caffe2/serialize/file_adapter.cc",
             "caffe2/serialize/inline_container.cc",
             "caffe2/serialize/istream_adapter.cc",
+            "caffe2/serialize/read_adapter_interface.cc",
         ],
         copts = ["-fexceptions"],
         tags = [

caffe2/core/timer.h

@@ -3,7 +3,7 @@
 
 #include <chrono>
 
-#include <c10/macros/Macros.h>
+#include "caffe2/core/common.h"
 
 namespace caffe2 {
 

caffe2/perfkernels/common_avx.cc

@@ -2,7 +2,7 @@
 // example, if your compiler did not specify -mavx, you should not provide
 // the CAFFE2_PERF_WITH_AVX macro.
 
-#include "caffe2/core/macros.h"
+#include "caffe2/core/common.h"
 
 #ifdef CAFFE2_PERF_WITH_AVX
 #ifndef __AVX__

caffe2/perfkernels/common_avx2.cc

@@ -2,7 +2,7 @@
 // example, if your compiler did not specify -mavx2, you should not provide
 // the CAFFE2_PERF_WITH_AVX2 macro.
 
-#include "caffe2/core/macros.h"
+#include "caffe2/core/common.h"
 
 #ifdef CAFFE2_PERF_WITH_AVX2
 #ifndef __AVX2__

caffe2/serialize/CMakeLists.txt

@@ -6,7 +6,8 @@ list(APPEND Caffe2_CPU_SRCS
   ${CMAKE_CURRENT_SOURCE_DIR}/inline_container.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/istream_adapter.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/file_adapter.cc
-  ${CMAKE_CURRENT_SOURCE_DIR}/crc.cc)
+  ${CMAKE_CURRENT_SOURCE_DIR}/crc.cc
+  ${CMAKE_CURRENT_SOURCE_DIR}/read_adapter_interface.cc)
 list(APPEND Caffe2_CPU_INCLUDE ${PROJECT_SOURCE_DIR}/third_party/miniz-2.1.0)
 
 set(Caffe2_CPU_TEST_SRCS ${Caffe2_CPU_TEST_SRCS} PARENT_SCOPE)

caffe2/serialize/crc_alt.h

@@ -25,7 +25,7 @@
 // using the aforementioned #defines the table is automatically fitted to your needs
 
 // uint8_t, uint32_t, int32_t
-#include <cstdint>
+#include <stdint.h>
 // size_t
 #include <cstddef>
 

caffe2/serialize/file_adapter.cc

@@ -3,11 +3,13 @@
 #include <cerrno>
 #include <cstdio>
 #include <string>
+#include "caffe2/core/common.h"
 
-namespace caffe2::serialize {
+namespace caffe2 {
+namespace serialize {
 
-FileAdapter::RAIIFile::RAIIFile(const std::string& file_name)
-    : fp_(fopen(file_name.c_str(), "rb")) {
+FileAdapter::RAIIFile::RAIIFile(const std::string& file_name) {
+  fp_ = fopen(file_name.c_str(), "rb");
   if (fp_ == nullptr) {
     auto old_errno = errno;
 #if defined(_WIN32) && (defined(__MINGW32__) || defined(_MSC_VER))
@@ -75,4 +77,5 @@ size_t FileAdapter::read(uint64_t pos, void* buf, size_t n, const char* what)
 
 FileAdapter::~FileAdapter() = default;
 
-} // namespace caffe2::serialize
+} // namespace serialize
+} // namespace caffe2

caffe2/serialize/file_adapter.h

@@ -1,11 +1,14 @@
 #pragma once
 
+#include <fstream>
+#include <memory>
 #include <c10/macros/Macros.h>
-#include <string>
 
+#include "caffe2/serialize/istream_adapter.h"
 #include "caffe2/serialize/read_adapter_interface.h"
 
-namespace caffe2::serialize {
+namespace caffe2 {
+namespace serialize {
 
 class TORCH_API FileAdapter final : public ReadAdapterInterface {
  public:
@@ -29,4 +32,5 @@ class TORCH_API FileAdapter final : public ReadAdapterInterface {
   uint64_t size_;
 };
 
-} // namespace caffe2::serialize
+} // namespace serialize
+} // namespace caffe2

caffe2/serialize/in_memory_adapter.h

@@ -1,9 +1,10 @@
 #pragma once
-#include <caffe2/serialize/read_adapter_interface.h>
-#include <sys/types.h>
 #include <cstring>
+#include <caffe2/serialize/read_adapter_interface.h>
 
-namespace caffe2::serialize {
+
+namespace caffe2 {
+namespace serialize {
 
 class MemoryReadAdapter final : public caffe2::serialize::ReadAdapterInterface {
  public:
@@ -14,18 +15,18 @@ class MemoryReadAdapter final : public caffe2::serialize::ReadAdapterInterface {
     return size_;
   }
 
-  size_t read(
-      uint64_t pos,
-      void* buf,
-      size_t n,
-      const char* what [[maybe_unused]] = "") const override {
+  size_t read(uint64_t pos, void* buf, size_t n, const char* what = "")
+      const override {
+    (void) what;
     memcpy(buf, (int8_t*)(data_) + pos, n);
     return n;
   }
 
  private:
   const void* data_;
-  off_t size_{};
+  off_t size_;
 };
 
-} // namespace caffe2::serialize
+
+} // namespace serialize
+} // namespace caffe2

caffe2/serialize/inline_container.cc

@@ -10,11 +10,15 @@
 #include <sys/types.h>
 #include <thread>
 
+#include <c10/core/Allocator.h>
+#include <c10/core/Backend.h>
 #include <c10/core/CPUAllocator.h>
+#include <c10/core/Backend.h>
 #include <c10/util/Exception.h>
 #include <c10/util/Logging.h>
 #include <c10/util/hash.h>
 
+#include "caffe2/core/common.h"
 #include "caffe2/serialize/file_adapter.h"
 #include "caffe2/serialize/inline_container.h"
 #include "caffe2/serialize/istream_adapter.h"
@@ -23,8 +27,8 @@
 #include "caffe2/serialize/versions.h"
 #include "miniz.h"
 
-
-namespace caffe2::serialize {
+namespace caffe2 {
+namespace serialize {
 constexpr c10::string_view kDebugPklSuffix(".debug_pkl");
 
 struct MzZipReaderIterWrapper {
@@ -190,7 +194,8 @@ void PyTorchStreamReader::init() {
 
   // version check
   at::DataPtr version_ptr;
-  size_t version_size = 0;
+  // NOLINTNEXTLINE(cppcoreguidelines-init-variables)
+  size_t version_size;
   if (hasRecord(".data/version")) {
     std::tie(version_ptr, version_size) = getRecord(".data/version");
   } else {
@@ -199,7 +204,7 @@ void PyTorchStreamReader::init() {
   }
   std::string version(static_cast<const char*>(version_ptr.get()), version_size);
   try {
-    version_ = std::stoll(version);
+    version_ = std::stoull(version);
   } catch (const std::invalid_argument& e) {
     CAFFE_THROW("Couldn't parse the version ",
                  version,
@@ -622,7 +627,7 @@ PyTorchStreamWriter::PyTorchStreamWriter(const std::string& file_name)
 }
 
 PyTorchStreamWriter::PyTorchStreamWriter(
-    const std::function<size_t(const void*, size_t)>& writer_func)
+    const std::function<size_t(const void*, size_t)> writer_func)
     : archive_name_("archive"),
       writer_func_(writer_func) {
   setup(archive_name_);
@@ -633,7 +638,7 @@ void PyTorchStreamWriter::setup(const string& file_name) {
   memset(ar_.get(), 0, sizeof(mz_zip_archive));
   archive_name_plus_slash_ = archive_name_ + "/"; // for writeRecord().
 
-  if (archive_name_.empty()) {
+  if (archive_name_.size() == 0) {
     CAFFE_THROW("invalid file name: ", file_name);
   }
   if (!writer_func_) {
@@ -644,7 +649,7 @@ void PyTorchStreamWriter::setup(const string& file_name) {
 
     const std::string dir_name = parentdir(file_name);
     if(!dir_name.empty()) {
-      struct stat st{};
+      struct stat st;
       bool dir_exists = (stat(dir_name.c_str(), &st) == 0 && (st.st_mode & S_IFDIR));
       TORCH_CHECK(dir_exists, "Parent directory ", dir_name, " does not exist.");
     }
@@ -701,8 +706,8 @@ void PyTorchStreamWriter::writeRecord(
       /*uncomp_size=*/0,
       /*uncomp_crc32=*/0,
       /*last_modified=*/nullptr,
-      /*user_extra_data_local=*/padding_.c_str(),
-      /*user_extra_data_local_len=*/padding_size,
+      /*user_extra_data=*/padding_.c_str(),
+      /*user_extra_data_len=*/padding_size,
       /*user_extra_data_central=*/nullptr,
       /*user_extra_data_central_len=*/0);
   valid("writing file ", name.c_str());
@@ -815,4 +820,5 @@ PyTorchStreamWriter::~PyTorchStreamWriter() {
   }
 }
 
-} // namespace caffe2::serialize
+} // namespace serialize
+} // namespace caffe2

caffe2/serialize/inline_container.h

@@ -6,6 +6,7 @@
 #include <fstream>
 #include <istream>
 #include <mutex>
+#include <ostream>
 #include <unordered_set>
 
 #include <c10/core/Allocator.h>
@@ -90,8 +91,8 @@ typedef struct mz_zip_archive mz_zip_archive;
 // model.json as the last file when writing after we have accumulated all
 // other information.
 
-
-namespace caffe2::serialize {
+namespace caffe2 {
+namespace serialize {
 
 static constexpr const char* kSerializationIdRecordName = ".data/serialization_id";
 
@@ -195,18 +196,18 @@ class TORCH_API PyTorchStreamReader final {
   std::string archive_name_;
   std::string archive_name_plus_slash_;
   std::shared_ptr<ReadAdapterInterface> in_;
-  int64_t version_{};
+  int64_t version_;
   std::mutex reader_lock_;
   bool load_debug_symbol_ = true;
   std::string serialization_id_;
-  size_t additional_reader_size_threshold_{};
+  size_t additional_reader_size_threshold_;
 };
 
 class TORCH_API PyTorchStreamWriter final {
  public:
   explicit PyTorchStreamWriter(const std::string& archive_name);
   explicit PyTorchStreamWriter(
-      const std::function<size_t(const void*, size_t)>& writer_func);
+      const std::function<size_t(const void*, size_t)> writer_func);
 
   void setMinVersion(const uint64_t version);
 
@@ -273,4 +274,5 @@ size_t getPadding(
     std::string& padding_buf);
 } // namespace detail
 
-} // namespace caffe2::serialize
+} // namespace serialize
+} // namespace caffe2

caffe2/serialize/inline_container_test.cc

@@ -5,14 +5,12 @@
 
 #include <gtest/gtest.h>
 
-#include <c10/util/Logging.h>
-#include <c10/util/irange.h>
 #include "caffe2/serialize/inline_container.h"
-#include "caffe2/serialize/istream_adapter.h"
-
+#include <c10/util/Logging.h>
+#include "c10/util/irange.h"
 
-// NOLINTBEGIN(*-narrowing-conversions)
-namespace caffe2::serialize {
+namespace caffe2 {
+namespace serialize {
 namespace {
 
 TEST(PyTorchStreamWriterAndReader, SaveAndLoad) {
@@ -21,7 +19,7 @@ TEST(PyTorchStreamWriterAndReader, SaveAndLoad) {
   std::ostringstream oss;
   // write records through writers
   PyTorchStreamWriter writer([&](const void* b, size_t n) -> size_t {
-    oss.write(static_cast<const char*>(b), static_cast<std::streamsize>(n));
+    oss.write(static_cast<const char*>(b), n);
     return oss ? n : 0;
   });
   // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init,cppcoreguidelines-avoid-magic-numbers)
@@ -30,14 +28,14 @@ TEST(PyTorchStreamWriterAndReader, SaveAndLoad) {
   std::vector<uint8_t> buf(data1.size());
 
   for (auto i : c10::irange(data1.size())) {
-    data1[i] = static_cast<char>(data1.size() - i);
+    data1[i] = data1.size() - i;
   }
   writer.writeRecord("key1", data1.data(), data1.size());
 
   // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init,cppcoreguidelines-avoid-magic-numbers)
   std::array<char, 64> data2;
   for (auto i : c10::irange(data2.size())) {
-    data2[i] = static_cast<char>(data2.size() - i);
+    data2[i] = data2.size() - i;
   }
   writer.writeRecord("key2", data2.data(), data2.size());
 
@@ -151,7 +149,7 @@ TEST(PyTorchStreamWriterAndReader, LoadWithMultiThreads) {
   PyTorchStreamReader reader(&iss);
   reader.setAdditionalReaderSizeThreshold(0);
   // before testing, sanity check
-  int64_t size1 = 0, size2 = 0, ret = 0;
+  int64_t size1, size2, ret;
   at::DataPtr data_ptr;
   std::tie(data_ptr, size1) = reader.getRecord("key1");
   std::tie(data_ptr, size2) = reader.getRecord("key2");
@@ -298,7 +296,7 @@ TEST(PytorchStreamWriterAndReader, SkipDebugRecords) {
   reader.setShouldLoadDebugSymbol(false);
   EXPECT_FALSE(reader.hasRecord("key1.debug_pkl"));
   at::DataPtr ptr;
-  size_t size = 0;
+  size_t size;
   std::tie(ptr, size) = reader.getRecord("key1.debug_pkl");
   EXPECT_EQ(size, 0);
   std::vector<uint8_t> dst(data1.size());
@@ -481,5 +479,5 @@ TEST_P(ChunkRecordIteratorTest, ChunkRead) {
 }
 
 } // namespace
-} // namespace caffe2::serialize
-// NOLINTEND(*-narrowing-conversions)
+} // namespace serialize
+} // namespace caffe2

caffe2/serialize/istream_adapter.cc

@@ -1,7 +1,8 @@
 #include "caffe2/serialize/istream_adapter.h"
 #include <c10/util/Exception.h>
 
-namespace caffe2::serialize {
+namespace caffe2 {
+namespace serialize {
 
 IStreamAdapter::IStreamAdapter(std::istream* istream) : istream_(istream) {}
 
@@ -32,6 +33,8 @@ void IStreamAdapter::validate(const char* what) const {
   }
 }
 
-IStreamAdapter::~IStreamAdapter() = default;
+// NOLINTNEXTLINE(modernize-use-equals-default)
+IStreamAdapter::~IStreamAdapter() {}
 
-} // namespace caffe2::serialize
+} // namespace serialize
+} // namespace caffe2