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

5 files changed, 20 insertions, 26 deletions

diff --git a/src/libexpr/eval-error.hh b/src/libexpr/eval-error.hh
index 19540d612..7114d392f 100644
--- a/src/libexpr/eval-error.hh
+++ b/src/libexpr/eval-error.hh
@@ -1,9 +1,8 @@
 #pragma once
 ///@file
 
-#include <algorithm>
-
 #include "error.hh"
+#include "types.hh"
 #include "pos-idx.hh"
 
 namespace nix {
diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh
index 4631b71eb..30badb93f 100644
--- a/src/libexpr/eval-inline.hh
+++ b/src/libexpr/eval-inline.hh
@@ -31,7 +31,7 @@ Value * EvalState::allocValue()
 #endif
 
     nrValues++;
-    return (Value *) p;
+    return static_cast<Value *>(p);
 }
 
 
@@ -54,10 +54,10 @@ Env & EvalState::allocEnv(size_t size)
         void * p = *env1AllocCache;
         *env1AllocCache = GC_NEXT(p);
         GC_NEXT(p) = nullptr;
-        env = (Env *) p;
+        env = static_cast<Env *>(p);
     } else
 #endif
-        env = (Env *) gcAllocBytes(sizeof(Env) + size * sizeof(Value *));
+        env = static_cast<Env *>(gcAllocBytes(sizeof(Env) + size * sizeof(Value *)));
 
     /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */
 
diff --git a/src/libexpr/gc-alloc.hh b/src/libexpr/gc-alloc.hh
index fc034045f..afdd7eeb0 100644
--- a/src/libexpr/gc-alloc.hh
+++ b/src/libexpr/gc-alloc.hh
@@ -120,6 +120,7 @@ inline T * gcAllocType(size_t howMany = 1)
     // However, people can and do request zero sized allocations, so we need
     // to check that neither of our multiplicands were zero before complaining
     // about it.
+    // NOLINTNEXTLINE(bugprone-sizeof-expression): yeah we only seem to alloc pointers with this. the calculation *is* correct though!
     auto checkedSz = checked::Checked<size_t>(howMany) * sizeof(T);
     size_t sz = checkedSz.valueWrapping();
     if (checkedSz.overflowed()) {
diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc
index 08d4b279b..68da254e2 100644
--- a/src/libexpr/nixexpr.cc
+++ b/src/libexpr/nixexpr.cc
@@ -11,6 +11,7 @@
 namespace nix {
 
 ExprBlackHole eBlackHole;
+Expr *eBlackHoleAddr = &eBlackHole;
 
 // FIXME: remove, because *symbols* are abstract and do not have a single
 //        textual representation; see printIdentifier()
diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index 57485aa0a..e38b11cbf 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -136,7 +136,9 @@ class ExternalValueBase
 
 std::ostream & operator << (std::ostream & str, const ExternalValueBase & v);
 
-extern ExprBlackHole eBlackHole;
+/** This is just the address of eBlackHole. It exists because eBlackHole has an
+ * incomplete type at usage sites so is not possible to cast. */
+extern Expr *eBlackHoleAddr;
 
 struct NewValueAs
 {
@@ -196,6 +198,7 @@ private:
 public:
 
     // Discount `using NewValueAs::*;`
+// NOLINTNEXTLINE(bugprone-macro-parentheses)
 #define USING_VALUETYPE(name) using name = NewValueAs::name
     USING_VALUETYPE(integer_t);
     USING_VALUETYPE(floating_t);
@@ -473,7 +476,7 @@ public:
     /// Constructs an evil thunk, whose evaluation represents infinite recursion.
     explicit Value(blackhole_t)
         : internalType(tThunk)
-        , thunk({ .env = nullptr, .expr = reinterpret_cast<Expr *>(&eBlackHole) })
+        , thunk({ .env = nullptr, .expr = eBlackHoleAddr })
     { }
 
     Value(Value const & rhs) = default;
@@ -513,7 +516,10 @@ public:
     // type() == nThunk
     inline bool isThunk() const { return internalType == tThunk; };
     inline bool isApp() const { return internalType == tApp; };
-    inline bool isBlackhole() const;
+    inline bool isBlackhole() const
+    {
+        return internalType == tThunk && thunk.expr == eBlackHoleAddr;
+    }
 
     // type() == nFunction
     inline bool isLambda() const { return internalType == tLambda; };
@@ -669,11 +675,6 @@ public:
 
     void mkStringMove(const char * s, const NixStringContext & context);
 
-    inline void mkString(const Symbol & s)
-    {
-        mkString(((const std::string &) s).c_str());
-    }
-
     void mkPath(const SourcePath & path);
 
     inline void mkPath(const char * path)
@@ -732,7 +733,11 @@ public:
         lambda.fun = f;
     }
 
-    inline void mkBlackhole();
+    inline void mkBlackhole()
+    {
+        internalType = tThunk;
+        thunk.expr = eBlackHoleAddr;
+    }
 
     void mkPrimOp(PrimOp * p);
 
@@ -832,18 +837,6 @@ public:
     }
 };
 
-
-bool Value::isBlackhole() const
-{
-    return internalType == tThunk && thunk.expr == (Expr*) &eBlackHole;
-}
-
-void Value::mkBlackhole()
-{
-    internalType = tThunk;
-    thunk.expr = (Expr*) &eBlackHole;
-}
-
 using ValueVector = GcVector<Value *>;
 using ValueMap = GcMap<Symbol, Value *>;
 using ValueVectorMap = std::map<Symbol, ValueVector>;