Update CI to clang-tidy 9 and fix warnings

.circleci/config.yml

@@ -89,8 +89,7 @@ references:
           # They aren't set automatically unfortunately
           sudo ln -s /usr/bin/$CC /usr/bin/clang
           sudo ln -s /usr/bin/$CXX /usr/bin/clang++
-          sudo ln -s /usr/bin/clang-tidy-8 /usr/bin/clang-tidy
+          sudo ln -s /usr/bin/clang-tidy-9 /usr/bin/clang-tidy
-          sudo ln -s /usr/bin/run-clang-tidy-8.py /usr/bin/run-clang-tidy.py
         fi
 
         # Setup build cache
@@ -652,10 +651,10 @@ jobs:
     - store_artifacts:
         path: /tmp/clang-tidy-fixes
     environment:
-      CC: clang-8
+      CC: clang-9
-      CXX: clang++-8
+      CXX: clang++-9
       BUILD_TOOLSET: clang
-      APT_COMPILER_PACKAGE: "clang-8 clang-tidy-8"
+      APT_COMPILER_PACKAGE: "clang-9 clang-tidy-9"
 
 workflows:
   version: 2

.clang-tidy

@@ -2,6 +2,7 @@
 # TODO Enable (some of) the explicitly disabled checks. Possibly needs helper types from gsl library or similar to enable full cppcoreguidelines.
 # TODO Enable more checks (google-*, hicpp-*, llvm-*, modernize-*, mpi-*, performance-*, readability-*)
 # TODO Maybe just enable * and disable a list instead?
+# TODO Check if there's new checks in clang-tidy-9 and potentially enable them
 Checks: |
   clang-diagnostic-*,
   clang-analyzer-*,

run-clang-tidy.sh

@@ -19,4 +19,4 @@ cat compile_commands.json|jq "map(select(.file | test(\"^$(realpath ${0%/*})/(sr
 rm compile_commands.json
 mv compile_commands2.json compile_commands.json
 
-run-clang-tidy.py -j${NUMCORES} -quiet -header-filter "$(realpath ${0%/*})/(src|test)/.*" $@
+run-clang-tidy-9.py -j${NUMCORES} -quiet -header-filter "$(realpath ${0%/*})/(src|test)/.*" $@

src/cpp-utils/crypto/kdf/SCryptParameters.h

@@ -26,6 +26,10 @@ namespace cpputils {
         SCryptParameters(SCryptParameters &&rhs) = default;
 
         SCryptParameters &operator=(const SCryptParameters &rhs) {
+            if (this == &rhs) {
+                return *this;
+            }
+
             _salt = rhs._salt.copy();
             _N = rhs._N;
             _r = rhs._r;

src/cpp-utils/either.h

@@ -47,7 +47,12 @@ namespace cpputils {
         }
 
         //TODO Try allowing copy-assignment when Left/Right types are std::is_convertible
+        // NOLINTNEXTLINE(cert-oop54-cpp)
         either<Left, Right> &operator=(const either<Left, Right> &rhs) noexcept(noexcept(std::declval<either<Left, Right>>()._construct_left(rhs._left)) && noexcept(std::declval<either<Left, Right>>()._construct_right(rhs._right))) {
+            if (this == &rhs) {
+                return *this;
+            }
+
             _destruct();
             _side = rhs._side;
             if (_side == Side::left) {
@@ -59,6 +64,10 @@ namespace cpputils {
         }
 
         either<Left, Right> &operator=(either<Left, Right> &&rhs) noexcept(noexcept(std::declval<either<Left, Right>>()._construct_left(std::move(rhs._left))) && noexcept(std::declval<either<Left, Right>>()._construct_right(std::move(rhs._right)))) {
+            if (this == &rhs) {
+                return *this;
+            }
+
             _destruct();
             _side = rhs._side;
             if (_side == Side::left) {

src/cpp-utils/value_type/ValueType.h

@@ -70,8 +70,8 @@ public:
         return *this;
     }
     constexpr IdValueType& operator=(const IdValueType& rhs) noexcept(noexcept(*std::declval<UnderlyingType*>() = rhs.value_)) {
-        value_ = rhs.value_;
+        // NOLINTNEXTLINE(cppcoreguidelines-c-copy-assignment-signature,misc-unconventional-assign-operator)
-        return *this;
+        return operator=(IdValueType(rhs));
     }
 
 protected:

src/cryfs-cli/VersionChecker.cpp

@@ -37,6 +37,7 @@ namespace cryfs_cli {
         if (warnings == none) {
             return none;
         }
+        // NOLINTNEXTLINE(bugprone-branch-clone)
         BOOST_FOREACH(const ptree::value_type &v, *warnings) {
             if(v.first == version) {
                 return v.second.get_value<std::string>();

src/cryfs/impl/filesystem/cachingfsblobstore/DirBlobRef.h

@@ -98,11 +98,11 @@ public:
         return _base->AppendChildrenTo(result);
     }
 
-    const blockstore::BlockId &blockId() const {
+    const blockstore::BlockId &blockId() const override {
         return _base->blockId();
     }
 
-    fspp::num_bytes_t lstat_size() const {
+    fspp::num_bytes_t lstat_size() const override {
         return _base->lstat_size();
     }
 

src/cryfs/impl/filesystem/cachingfsblobstore/FileBlobRef.h

@@ -36,11 +36,11 @@ public:
         return _base->flush();
     }
 
-    const blockstore::BlockId &blockId() const {
+    const blockstore::BlockId &blockId() const override {
         return _base->blockId();
     }
 
-    fspp::num_bytes_t lstat_size() const {
+    fspp::num_bytes_t lstat_size() const override {
         return _base->lstat_size();
     }
 

src/cryfs/impl/filesystem/cachingfsblobstore/SymlinkBlobRef.h

@@ -20,11 +20,11 @@ public:
         return _base->target();
     }
 
-    const blockstore::BlockId &blockId() const {
+    const blockstore::BlockId &blockId() const override {
         return _base->blockId();
     }
 
-    fspp::num_bytes_t lstat_size() const {
+    fspp::num_bytes_t lstat_size() const override {
         return _base->lstat_size();
     }
 

test/blockstore/implementations/caching/cache/testutils/CopyableMovableValueType.h

@@ -9,6 +9,7 @@ public:
   CopyableMovableValueType(const CopyableMovableValueType &rhs): CopyableMovableValueType(rhs._value) {
     ++numCopyConstructorCalled;
   }
+  // NOLINTNEXTLINE(cert-oop54-cpp)
   CopyableMovableValueType &operator=(const CopyableMovableValueType &rhs) {
     _value = rhs._value;
     ++numCopyConstructorCalled;