From 5d058d81f1b4069d45de1d21f1ef5212871f8375 Mon Sep 17 00:00:00 2001
From: regnat <rg@regnat.ovh>
Date: Tue, 16 Feb 2021 18:38:27 +0100
Subject: [PATCH] Fix the caching of float and ints

Because of a dirty limit of the `std::visit(overloaded` trick, these
were silently casted to booleans
---
 src/libexpr/eval.cc       | 18 +++++++++++++-----
 src/libexpr/tree-cache.cc | 25 +++++++++----------------
 src/libexpr/tree-cache.hh | 14 +++++++++++---
 3 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 056d25516..a8addfa36 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -1135,7 +1135,7 @@ tree_cache::AttrValue cachedValueFor(const Value& v)
             valueToCache = tree_cache::unknown_t{};
             break;
         case nBool:
-            valueToCache = v.boolean;
+            valueToCache = tree_cache::wrapped_basetype<bool>{v.boolean};
             break;
         case nString:
             valueToCache = tree_cache::string_t{
@@ -1153,10 +1153,10 @@ tree_cache::AttrValue cachedValueFor(const Value& v)
             valueToCache = tree_cache::attributeSet_t{};
             break;
         case nInt:
-            valueToCache = v.integer;
+            valueToCache = tree_cache::wrapped_basetype<int64_t>{v.integer};
             break;
         case nFloat:
-            valueToCache = v.fpoint;
+            valueToCache = tree_cache::wrapped_basetype<double>{v.fpoint};
             break;
     };
     return valueToCache;
@@ -1249,8 +1249,16 @@ ValueCache::CacheResult ValueCache::getValue(Store & store, const std::vector<Sy
                 mkString(dest, s.first, context);
                 return ValueCache::CacheResult{CacheHit};
             },
-            [&](bool b) {
-                dest.mkBool(b);
+            [&](tree_cache::wrapped_basetype<bool> b) {
+                dest.mkBool(b.value);
+                return ValueCache::CacheResult{CacheHit};
+            },
+            [&](tree_cache::wrapped_basetype<int64_t> i) {
+                dest.mkInt(i.value);
+                return ValueCache::CacheResult{CacheHit};
+            },
+            [&](tree_cache::wrapped_basetype<double> d) {
+                dest.mkFloat(d.value);
                 return ValueCache::CacheResult{CacheHit};
             },
         },
diff --git a/src/libexpr/tree-cache.cc b/src/libexpr/tree-cache.cc
index e051cab7d..08fd9e59a 100644
--- a/src/libexpr/tree-cache.cc
+++ b/src/libexpr/tree-cache.cc
@@ -118,13 +118,6 @@ struct AttrDb
         });
     }
 
-    AttrId setBool(
-        AttrKey key,
-        bool b)
-    {
-        return addEntry(key, b);
-    }
-
     std::optional<AttrId> getId(const AttrKey& key)
     {
         auto state(_state->lock());
@@ -180,11 +173,11 @@ struct AttrDb
                 return {{rowId, string_t{queryAttribute.getStr(2), context}}};
             }
             case AttrType::Bool:
-                return {{rowId, queryAttribute.getInt(2) != 0}};
+                return {{rowId, wrapped_basetype<bool>{queryAttribute.getInt(2) != 0}}};
             case AttrType::Int:
-                return {{rowId, queryAttribute.getInt(2)}};
+                return {{rowId, wrapped_basetype<int64_t>{queryAttribute.getInt(2)}}};
             case AttrType::Double:
-                return {{rowId, queryAttribute.getInt(2)}};
+                return {{rowId, wrapped_basetype<double>{(double)queryAttribute.getInt(2)}}};
             case AttrType::Unknown:
                 return {{rowId, unknown_t{}}};
             case AttrType::Thunk:
@@ -359,17 +352,17 @@ const RawValue RawValue::fromVariant(const AttrValue & value)
         res.value = x.first;
         res.context = x.second;
       },
-      [&](bool x) {
+      [&](wrapped_basetype<bool> x) {
         res.type = AttrType::Bool;
-        res.value = x ? "1" : "0";
+        res.value = x.value ? "1" : "0";
       },
-      [&](int64_t x) {
+      [&](wrapped_basetype<int64_t> x) {
         res.type = AttrType::Int;
-        res.value = std::to_string(x);
+        res.value = std::to_string(x.value);
       },
-      [&](double x) {
+      [&](wrapped_basetype<double> x) {
         res.type = AttrType::Double;
-        res.value = std::to_string(x);
+        res.value = std::to_string(x.value);
       },
       [&](unknown_t x) { res.type = AttrType::Unknown; },
       [&](missing_t x) { res.type = AttrType::Missing; },
diff --git a/src/libexpr/tree-cache.hh b/src/libexpr/tree-cache.hh
index c4b7f06ea..8f40fb15b 100644
--- a/src/libexpr/tree-cache.hh
+++ b/src/libexpr/tree-cache.hh
@@ -70,6 +70,14 @@ struct unknown_t {};
 struct thunk_t {};
 struct failed_t { string error; };
 struct missing_t { Symbol attrName; };
+
+// Putting several different primitive types in an `std::variant` partially
+// breaks the `std::visit(overloaded{...` hackery because of the implicit cast
+// from one to another which breaks the exhaustiveness check.
+// So we wrap them in a trivial class just to force the disambiguation
+template<typename T>
+struct wrapped_basetype{ T value; };
+
 typedef uint64_t AttrId;
 
 typedef std::pair<AttrId, Symbol> AttrKey;
@@ -82,9 +90,9 @@ typedef std::variant<
     thunk_t,
     missing_t,
     failed_t,
-    bool,
-    int64_t,
-    double
+    wrapped_basetype<bool>,
+    wrapped_basetype<int64_t>,
+    wrapped_basetype<double>
 > AttrValue;
 
 struct RawValue {