st: fix CI - Expected analyzer false positive via std::get_if

clang-analyzer (LLVM 18 + libstdc++) cannot model the std::variant
discriminant through std::get<1>, so it treats the Error returned by
Expected::error() as having an uninitialized `result` field and flags
every Error copy/move that originates from it (reported at Result.h:107,
the Error copy/move ctor). std::get_if<1> - already used without
complaint by operator-> - returns a pointer the analyzer models as
defined, so error() now reads through it, and value() routes through
error() instead of raw std::get<1>. get_if also better fits the noexcept
accessor (std::get can throw bad_variant_access).

Revert the ineffective `Result result{}` default-init in the classic
Result.h header (the analyzer flags the copy, not default construction;
the real fix is above), restoring that header to pristine. Suppress an
unrelated NewDeleteLeaks false positive on a move-only mapper in
FutureTest.

Author: merlimat

include/pulsar/Result.h

@@ -105,7 +105,7 @@ PULSAR_PUBLIC const char* strResult(Result result);
 PULSAR_PUBLIC std::ostream& operator<<(std::ostream& s, pulsar::Result result);
 
 struct PULSAR_PUBLIC Error {
-    Result result{};  // defaults to ResultOk; avoids an uninitialized field on default construction
+    Result result;
     std::string message;
 };
 

include/pulsar/st/Expected.h

@@ -124,15 +124,15 @@ class [[nodiscard]] Expected {
      * this aborts. Use `operator bool` + `operator*` for the non-throwing path.
      */
     const T& value() const& {
-        if (!has_value()) PULSAR_ST_THROW(ClientException(std::get<1>(storage_)));
+        if (!has_value()) PULSAR_ST_THROW(ClientException(error()));
         return std::get<0>(storage_);
     }
     T& value() & {
-        if (!has_value()) PULSAR_ST_THROW(ClientException(std::get<1>(storage_)));
+        if (!has_value()) PULSAR_ST_THROW(ClientException(error()));
         return std::get<0>(storage_);
     }
     T&& value() && {
-        if (!has_value()) PULSAR_ST_THROW(ClientException(std::get<1>(storage_)));
+        if (!has_value()) PULSAR_ST_THROW(ClientException(error()));
         return std::get<0>(std::move(storage_));
     }
 
@@ -170,9 +170,9 @@ class [[nodiscard]] Expected {
      * @pre `!has_value()`. Behaviour is undefined if this holds a value.
      * @return a reference to the contained `Error`.
      */
-    const Error& error() const& noexcept { return std::get<1>(storage_); }
+    const Error& error() const& noexcept { return *std::get_if<1>(&storage_); }
     /** @copydoc error() const& */
-    Error& error() & noexcept { return std::get<1>(storage_); }
+    Error& error() & noexcept { return *std::get_if<1>(&storage_); }
 
     /**
      * Return the contained value, or @p fallback if this holds an error.

tests/st/FutureTest.cc

@@ -131,8 +131,13 @@ TEST(FutureTest, testThenApplyVoidMapper) {
 TEST(FutureTest, testThenApplyMoveOnlyMapper) {
     Promise<int> promise;
     auto bonus = std::make_unique<int>(100);
+    // The unique_ptr is moved into the mapper, which the Future's shared state owns and
+    // frees when the chain is destroyed; the analyzer can't trace that through
+    // std::function, so it false-positives a leak.
+    // NOLINTBEGIN(clang-analyzer-cplusplus.NewDeleteLeaks)
     Future<int> mapped =
         promise.getFuture().thenApply([b = std::move(bonus)](const int& x) { return x + *b; });
+    // NOLINTEND(clang-analyzer-cplusplus.NewDeleteLeaks)
     promise.setValue(5);
     auto r = mapped.get();
     ASSERT_TRUE(r);