treewide: fix a bunch of lints

Fixes:
- Identifiers starting with _ are prohibited
- Some driveby header dependency cleaning which wound up with doing some
  extra fixups.
- Fucking C style casts, man. C++ made these 1000% worse by letting you
  also do memory corruption with them with references.
  - Remove casts to Expr * where ExprBlackHole is an incomplete type by
    introducing an explicitly-cast eBlackHoleAddr as Expr *.
  - An incredibly illegal cast of the text bytes of the StorePath hash
    into a size_t directly. You can't DO THAT.

    Replaced with actually parsing the hash so we get 100% of the bits
    being entropy, then memcpying the start of the hash. If this shows
    up in a profile we should just make the hash parser faster with a
    lookup table or something sensible like that.
  - This horrendous bit of UB which I thankfully slapped a deprecation
    warning on, built, and it didn't trigger anywhere so it was dead
    code and I just deleted it. But holy crap you *cannot* do that.

    inline void mkString(const Symbol & s)
    {
        mkString(((const std::string &) s).c_str());
    }
- Some wrong lints. Lots of wrong macro lints, one wrong
  suspicious-sizeof lint triggered by the template being instantiated
  with only pointers, but the calculation being correct for both
  pointers and not-pointers.
- Exceptions in destructors strike again. I tried to catch the
  exceptions that might actually happen rather than all the exceptions
  imaginable. We can let the runtime hard-kill it on other exceptions
  imo.

Change-Id: I71761620846cba64d66ee7ca231b20c061e69710

9 files changed, 36 insertions, 17 deletions

diff --git a/src/libstore/common-protocol-impl.hh b/src/libstore/common-protocol-impl.hh
index fd1387e95..387f848ed 100644
--- a/src/libstore/common-protocol-impl.hh
+++ b/src/libstore/common-protocol-impl.hh
@@ -20,6 +20,7 @@ namespace nix {
     { \
         return LengthPrefixedProtoHelper<CommonProto, T >::read(store, conn); \
     } \
+    /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \
     TEMPLATE [[nodiscard]] WireFormatGenerator CommonProto::Serialise< T >::write(const Store & store, CommonProto::WriteConn conn, const T & t) \
     { \
         return LengthPrefixedProtoHelper<CommonProto, T >::write(store, conn, t); \
diff --git a/src/libstore/derived-path.hh b/src/libstore/derived-path.hh
index c87cf2004..c96e0df67 100644
--- a/src/libstore/derived-path.hh
+++ b/src/libstore/derived-path.hh
@@ -78,10 +78,12 @@ struct SingleDerivedPathBuilt {
     DECLARE_CMP(SingleDerivedPathBuilt);
 };
 
-using _SingleDerivedPathRaw = std::variant<
+namespace derived_path::detail {
+using SingleDerivedPathRaw = std::variant<
     DerivedPathOpaque,
     SingleDerivedPathBuilt
 >;
+}
 
 /**
  * A "derived path" is a very simple sort of expression (not a Nix
@@ -94,8 +96,8 @@ using _SingleDerivedPathRaw = std::variant<
  * - built, in which case it is a pair of a derivation path and an
  *   output name.
  */
-struct SingleDerivedPath : _SingleDerivedPathRaw {
-    using Raw = _SingleDerivedPathRaw;
+struct SingleDerivedPath : derived_path::detail::SingleDerivedPathRaw {
+    using Raw = derived_path::detail::SingleDerivedPathRaw;
     using Raw::Raw;
 
     using Opaque = DerivedPathOpaque;
@@ -201,10 +203,12 @@ struct DerivedPathBuilt {
     DECLARE_CMP(DerivedPathBuilt);
 };
 
-using _DerivedPathRaw = std::variant<
+namespace derived_path::detail {
+using DerivedPathRaw = std::variant<
     DerivedPathOpaque,
     DerivedPathBuilt
 >;
+}
 
 /**
  * A "derived path" is a very simple sort of expression that evaluates
@@ -216,8 +220,8 @@ using _DerivedPathRaw = std::variant<
  * - built, in which case it is a pair of a derivation path and some
  *   output names.
  */
-struct DerivedPath : _DerivedPathRaw {
-    using Raw = _DerivedPathRaw;
+struct DerivedPath : derived_path::detail::DerivedPathRaw {
+    using Raw = derived_path::detail::DerivedPathRaw;
     using Raw::Raw;
 
     using Opaque = DerivedPathOpaque;
diff --git a/src/libstore/length-prefixed-protocol-helper.hh b/src/libstore/length-prefixed-protocol-helper.hh
index 1475d2690..423bc77b8 100644
--- a/src/libstore/length-prefixed-protocol-helper.hh
+++ b/src/libstore/length-prefixed-protocol-helper.hh
@@ -61,9 +61,9 @@ template<class Inner, typename... Ts>
 LENGTH_PREFIXED_PROTO_HELPER(Inner, std::tuple<Ts...>);
 
 template<class Inner, typename K, typename V>
-#define _X std::map<K, V>
-LENGTH_PREFIXED_PROTO_HELPER(Inner, _X);
-#undef _X
+#define DONT_SUBSTITUTE_KV_TYPE std::map<K, V>
+LENGTH_PREFIXED_PROTO_HELPER(Inner, DONT_SUBSTITUTE_KV_TYPE);
+#undef DONT_SUBSTITUTE_KV_TYPE
 
 template<class Inner, typename T>
 std::vector<T>
diff --git a/src/libstore/path.cc b/src/libstore/path.cc
index d4b5fc0dc..82ed20285 100644
--- a/src/libstore/path.cc
+++ b/src/libstore/path.cc
@@ -112,3 +112,13 @@ PathSet Store::printStorePathSet(const StorePathSet & paths) const
 }
 
 }
+
+std::size_t std::hash<nix::StorePath>::operator()(const nix::StorePath & path) const noexcept
+{
+    // It's already a cryptographic hash of 160 bits (assuming that nobody gives us bogus ones...), so just parse it.
+    auto h = nix::Hash::parseNonSRIUnprefixed(path.hashPart(), nix::HashType::SHA1);
+    // This need not be stable across machines, so bit casting the start of it is fine.
+    size_t r;
+    memcpy(&r, h.hash, sizeof(r));
+    return r;
+}
diff --git a/src/libstore/path.hh b/src/libstore/path.hh
index 4ca6747b3..09b4710c1 100644
--- a/src/libstore/path.hh
+++ b/src/libstore/path.hh
@@ -2,8 +2,9 @@
 ///@file
 
 #include <string_view>
+#include <string>
 
-#include "types.hh"
+#include "types.hh" // IWYU pragma: keep
 
 namespace nix {
 
@@ -89,10 +90,7 @@ const std::string drvExtension = ".drv";
 namespace std {
 
 template<> struct hash<nix::StorePath> {
-    std::size_t operator()(const nix::StorePath & path) const noexcept
-    {
-        return * (std::size_t *) path.to_string().data();
-    }
+    std::size_t operator()(const nix::StorePath & path) const noexcept;
 };
 
 }
diff --git a/src/libstore/pathlocks.hh b/src/libstore/pathlocks.hh
index 7fcfa2e40..d06d031b5 100644
--- a/src/libstore/pathlocks.hh
+++ b/src/libstore/pathlocks.hh
@@ -49,8 +49,12 @@ struct FdLock
 
     ~FdLock()
     {
-        if (acquired)
-            lockFile(fd, ltNone, false);
+        try {
+            if (acquired)
+                lockFile(fd, ltNone, false);
+        } catch (SysError &) {
+            ignoreException();
+        }
     }
 };
 
diff --git a/src/libstore/serve-protocol-impl.hh b/src/libstore/serve-protocol-impl.hh
index cfb1dd574..a1d1c797f 100644
--- a/src/libstore/serve-protocol-impl.hh
+++ b/src/libstore/serve-protocol-impl.hh
@@ -20,6 +20,7 @@ namespace nix {
     { \
         return LengthPrefixedProtoHelper<ServeProto, T >::read(store, conn); \
     } \
+    /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \
     TEMPLATE [[nodiscard]] WireFormatGenerator ServeProto::Serialise< T >::write(const Store & store, ServeProto::WriteConn conn, const T & t) \
     { \
         return LengthPrefixedProtoHelper<ServeProto, T >::write(store, conn, t); \
diff --git a/src/libstore/sqlite.hh b/src/libstore/sqlite.hh
index ca021087f..5740c4e45 100644
--- a/src/libstore/sqlite.hh
+++ b/src/libstore/sqlite.hh
@@ -1,9 +1,9 @@
 #pragma once
 ///@file
 
-#include <functional>
 #include <string>
 
+#include "types.hh"
 #include "error.hh"
 
 struct sqlite3;
diff --git a/src/libstore/worker-protocol-impl.hh b/src/libstore/worker-protocol-impl.hh
index b2603b954..d99c59f84 100644
--- a/src/libstore/worker-protocol-impl.hh
+++ b/src/libstore/worker-protocol-impl.hh
@@ -20,6 +20,7 @@ namespace nix {
     { \
         return LengthPrefixedProtoHelper<WorkerProto, T >::read(store, conn); \
     } \
+    /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \
     TEMPLATE [[nodiscard]] WireFormatGenerator WorkerProto::Serialise< T >::write(const Store & store, WorkerProto::WriteConn conn, const T & t) \
     { \
         return LengthPrefixedProtoHelper<WorkerProto, T >::write(store, conn, t); \