refactor: Improve InodeCache::get signature

4 files changed, 40 insertions, 60 deletions

diff --git a/src/InodeCache.cpp b/src/InodeCache.cpp
index 7610417e..db75d58b 100644
--- a/src/InodeCache.cpp
+++ b/src/InodeCache.cpp
@@ -465,22 +465,19 @@ InodeCache::available(int fd)
   return fd_is_on_known_to_work_file_system(fd);
 }
 
-bool
-InodeCache::get(const std::string& path,
-                ContentType type,
-                Digest& file_digest,
-                HashSourceCodeResult* return_value)
+std::optional<HashSourceCodeResult>
+InodeCache::get(const std::string& path, ContentType type, Digest& file_digest)
 {
   if (!initialize()) {
-    return false;
+    return std::nullopt;
   }
 
   Digest key_digest;
   if (!hash_inode(path, type, key_digest)) {
-    return false;
+    return std::nullopt;
   }
 
-  bool found = false;
+  std::optional<HashSourceCodeResult> result;
   const bool success = with_bucket(key_digest, [&](const auto bucket) {
     for (uint32_t i = 0; i < k_num_entries; ++i) {
       if (bucket->entries[i].key_digest == key_digest) {
@@ -491,28 +488,25 @@ InodeCache::get(const std::string& path,
         }
 
         file_digest = bucket->entries[0].file_digest;
-        if (return_value) {
-          *return_value =
-            HashSourceCodeResult::from_bitmask(bucket->entries[0].return_value);
-        }
-        found = true;
+        result =
+          HashSourceCodeResult::from_bitmask(bucket->entries[0].return_value);
         break;
       }
     }
   });
   if (!success) {
-    return false;
+    return std::nullopt;
   }
 
   if (m_config.debug()) {
-    LOG("Inode cache {}: {}", found ? "hit" : "miss", path);
-    if (found) {
+    LOG("Inode cache {}: {}", result ? "hit" : "miss", path);
+    if (result) {
       ++m_sr->hits;
     } else {
       ++m_sr->misses;
     }
   }
-  return found;
+  return result;
 }
 
 bool
diff --git a/src/InodeCache.hpp b/src/InodeCache.hpp
index eea3b49d..98276a34 100644
--- a/src/InodeCache.hpp
+++ b/src/InodeCache.hpp
@@ -25,6 +25,7 @@
 
 #include <cstdint>
 #include <functional>
+#include <optional>
 #include <string>
 
 class Config;
@@ -77,10 +78,8 @@ public:
   //
   // Returns true if saved values could be retrieved from the cache, false
   // otherwise.
-  bool get(const std::string& path,
-           ContentType type,
-           Digest& file_digest,
-           HashSourceCodeResult* return_value = nullptr);
+  std::optional<HashSourceCodeResult>
+  get(const std::string& path, ContentType type, Digest& file_digest);
 
   // Put hash digest and return value from a successful call to do_hash_file()
   // in hashutil.cpp.
diff --git a/src/hashutil.cpp b/src/hashutil.cpp
index f241903d..1b019f9e 100644
--- a/src/hashutil.cpp
+++ b/src/hashutil.cpp
@@ -187,9 +187,9 @@ do_hash_file(const Context& ctx,
     check_temporal_macros ? InodeCache::ContentType::checked_for_temporal_macros
                           : InodeCache::ContentType::raw;
   if (ctx.config.inode_cache()) {
-    HashSourceCodeResult result;
-    if (ctx.inode_cache.get(path, content_type, digest, &result)) {
-      return result;
+    const auto result = ctx.inode_cache.get(path, content_type, digest);
+    if (result) {
+      return *result;
     }
   }
 #else
diff --git a/unittest/test_InodeCache.cpp b/unittest/test_InodeCache.cpp
index e60a0733..34bd0034 100644
--- a/unittest/test_InodeCache.cpp
+++ b/unittest/test_InodeCache.cpp
@@ -77,16 +77,13 @@ TEST_CASE("Test disabled")
   InodeCache inode_cache(config, util::Duration(0));
 
   Digest digest;
-  HashSourceCodeResult return_value;
 
-  CHECK(!inode_cache.get("a",
-                         InodeCache::ContentType::checked_for_temporal_macros,
-                         digest,
-                         &return_value));
+  CHECK(!inode_cache.get(
+    "a", InodeCache::ContentType::checked_for_temporal_macros, digest));
   CHECK(!inode_cache.put("a",
                          InodeCache::ContentType::checked_for_temporal_macros,
                          digest,
-                         return_value));
+                         HashSourceCodeResult()));
   CHECK(inode_cache.get_hits() == -1);
   CHECK(inode_cache.get_misses() == -1);
   CHECK(inode_cache.get_errors() == -1);
@@ -103,12 +100,9 @@ TEST_CASE("Test lookup nonexistent")
   util::write_file("a", "");
 
   Digest digest;
-  HashSourceCodeResult return_value;
 
-  CHECK(!inode_cache.get("a",
-                         InodeCache::ContentType::checked_for_temporal_macros,
-                         digest,
-                         &return_value));
+  CHECK(!inode_cache.get(
+    "a", InodeCache::ContentType::checked_for_temporal_macros, digest));
   CHECK(inode_cache.get_hits() == 0);
   CHECK(inode_cache.get_misses() == 1);
   CHECK(inode_cache.get_errors() == 0);
@@ -129,14 +123,11 @@ TEST_CASE("Test put and lookup")
   CHECK(put(inode_cache, "a", "a text", result));
 
   Digest digest;
-  HashSourceCodeResult return_value;
-
-  CHECK(inode_cache.get("a",
-                        InodeCache::ContentType::checked_for_temporal_macros,
-                        digest,
-                        &return_value));
+  auto return_value = inode_cache.get(
+    "a", InodeCache::ContentType::checked_for_temporal_macros, digest);
+  REQUIRE(return_value);
   CHECK(digest == Hash().hash("a text").digest());
-  CHECK(return_value.to_bitmask()
+  CHECK(return_value->to_bitmask()
         == static_cast<int>(HashSourceCode::found_date));
   CHECK(inode_cache.get_hits() == 1);
   CHECK(inode_cache.get_misses() == 0);
@@ -144,10 +135,8 @@ TEST_CASE("Test put and lookup")
 
   util::write_file("a", "something else");
 
-  CHECK(!inode_cache.get("a",
-                         InodeCache::ContentType::checked_for_temporal_macros,
-                         digest,
-                         &return_value));
+  CHECK(!inode_cache.get(
+    "a", InodeCache::ContentType::checked_for_temporal_macros, digest));
   CHECK(inode_cache.get_hits() == 1);
   CHECK(inode_cache.get_misses() == 1);
   CHECK(inode_cache.get_errors() == 0);
@@ -157,12 +146,11 @@ TEST_CASE("Test put and lookup")
             "something else",
             HashSourceCodeResult(HashSourceCode::found_time)));
 
-  CHECK(inode_cache.get("a",
-                        InodeCache::ContentType::checked_for_temporal_macros,
-                        digest,
-                        &return_value));
+  return_value = inode_cache.get(
+    "a", InodeCache::ContentType::checked_for_temporal_macros, digest);
+  REQUIRE(return_value);
   CHECK(digest == Hash().hash("something else").digest());
-  CHECK(return_value.to_bitmask()
+  CHECK(return_value->to_bitmask()
         == static_cast<int>(HashSourceCode::found_time));
   CHECK(inode_cache.get_hits() == 2);
   CHECK(inode_cache.get_misses() == 1);
@@ -209,20 +197,19 @@ TEST_CASE("Test content type")
                         HashSourceCodeResult(HashSourceCode::found_time)));
 
   Digest digest;
-  HashSourceCodeResult return_value;
 
-  CHECK(
-    inode_cache.get("a", InodeCache::ContentType::raw, digest, &return_value));
+  auto return_value =
+    inode_cache.get("a", InodeCache::ContentType::raw, digest);
+  REQUIRE(return_value);
   CHECK(digest == binary_digest);
-  CHECK(return_value.to_bitmask()
+  CHECK(return_value->to_bitmask()
         == static_cast<int>(HashSourceCode::found_date));
 
-  CHECK(inode_cache.get("a",
-                        InodeCache::ContentType::checked_for_temporal_macros,
-                        digest,
-                        &return_value));
+  return_value = inode_cache.get(
+    "a", InodeCache::ContentType::checked_for_temporal_macros, digest);
+  REQUIRE(return_value);
   CHECK(digest == code_digest);
-  CHECK(return_value.to_bitmask()
+  CHECK(return_value->to_bitmask()
         == static_cast<int>(HashSourceCode::found_time));
 }