async_promise_type possibly unitialized value

The std::variant for callback or future/promise with gcc-13 seems to be
triggering an uninitialized value warning. We can tell it isn't
uninitialized but the compiler seems to be thinking the 2nd type in the
variant, which is the callback type, is unitialized _when_ the promise
type, 3rd type in the variant, is the one being initialized.

Closes #153

.githooks/pre-commit

@@ -12,7 +12,7 @@ should_clang_format() {
     local result=0
 
     # Ignore the test/catch_amalgamated.hpp|cpp files
-    if [[ "$1" != *"catch_amalgamated"* ]]; then
+    if [[ "$1" != *"catch_amalgamated"* && "$1" != *"pragma"* ]]; then
         for ext in $FILE_EXTS; do
             # Otherwise, if the extension is in the array of extensions to reformat, echo 1.
             [[ "$ext" == "$extension" ]] && result=1 && break

CMakeLists.txt

@@ -30,6 +30,7 @@ message("${PROJECT_NAME} LIFT_USER_LINK_LIBRARIES = ${LIFT_USER_LINK_LIBRARIES}"
 
 set(LIBLIFTHTTP_SOURCE_FILES
     inc/lift/impl/copy_util.hpp
+    inc/lift/impl/pragma.hpp
 
     inc/lift/client.hpp src/client.cpp
     inc/lift/const.hpp

README.md

@@ -47,9 +47,8 @@ int main()
     // Debug information about any request can be added by including a callback handler for debug
     // information.  Just pass in a lambda to capture the verbose debug information.
     sync_request.debug_info_handler(
-        [](const lift::request& /*unused*/, lift::debug_info_type type, std::string_view data) {
-            std::cout << "sync_request (" << lift::to_string(type) << "): " << data;
-        });
+        [](const lift::request& /*unused*/, lift::debug_info_type type, std::string_view data)
+        { std::cout << "sync_request (" << lift::to_string(type) << "): " << data; });
 
     // Set the http method for this synchronous request.
     sync_request.method(lift::http::method::post);
@@ -96,7 +95,8 @@ int main()
     // Start the request that will be completed by callback.
     client.start_request(
         std::move(async_callback_request),
-        [](lift::request_ptr async_callback_request_returned, lift::response async_callback_response) {
+        [](lift::request_ptr async_callback_request_returned, lift::response async_callback_response)
+        {
             // This on complete callback will run on the lift::client background event loop thread.
             std::cout << "Lift status (async callback): ";
             std::cout << lift::to_string(async_callback_response.lift_status()) << "\n";

examples/benchmark.cpp

@@ -38,11 +38,11 @@ int main(int argc, char* argv[])
 {
     constexpr char   short_options[] = "c:d:t:h";
     constexpr option long_options[]  = {
-        {"help", no_argument, nullptr, 'h'},
-        {"connections", required_argument, nullptr, 'c'},
-        {"duration", required_argument, nullptr, 'd'},
-        {"threads", required_argument, nullptr, 't'},
-        {nullptr, 0, nullptr, 0}};
+         {"help", no_argument, nullptr, 'h'},
+         {"connections", required_argument, nullptr, 'c'},
+         {"duration", required_argument, nullptr, 'd'},
+         {"threads", required_argument, nullptr, 't'},
+         {nullptr, 0, nullptr, 0}};
 
     int option_index = 0;
     int opt          = 0;
@@ -109,7 +109,8 @@ int main(int argc, char* argv[])
             clients.emplace_back(std::make_unique<lift::client>());
 
             callbacks.emplace_back(
-                [&clients, &success, &error, &callbacks, i](lift::request_ptr req_ptr, lift::response response) {
+                [&clients, &success, &error, &callbacks, i](lift::request_ptr req_ptr, lift::response response)
+                {
                     if (response.lift_status() == lift::lift_status::success)
                     {
                         success.fetch_add(1, std::memory_order_relaxed);

examples/readme.cpp

@@ -14,9 +14,8 @@ int main()
     // Debug information about any request can be added by including a callback handler for debug
     // information.  Just pass in a lambda to capture the verbose debug information.
     sync_request.debug_info_handler(
-        [](const lift::request& /*unused*/, lift::debug_info_type type, std::string_view data) {
-            std::cout << "sync_request (" << lift::to_string(type) << "): " << data;
-        });
+        [](const lift::request& /*unused*/, lift::debug_info_type type, std::string_view data)
+        { std::cout << "sync_request (" << lift::to_string(type) << "): " << data; });
 
     // Set the http method for this synchronous request.
     sync_request.method(lift::http::method::post);
@@ -63,7 +62,8 @@ int main()
     // Start the request that will be completed by callback.
     client.start_request(
         std::move(async_callback_request),
-        [](lift::request_ptr async_callback_request_returned, lift::response async_callback_response) {
+        [](lift::request_ptr async_callback_request_returned, lift::response async_callback_response)
+        {
             // This on complete callback will run on the lift::client background event loop thread.
             std::cout << "Lift status (async callback): ";
             std::cout << lift::to_string(async_callback_response.lift_status()) << "\n";

inc/lift/executor.hpp

@@ -34,10 +34,10 @@ class executor
     friend request;
 
 public:
-    executor(const executor&) = delete;
-    executor(executor&&)      = delete;
+    executor(const executor&)                    = delete;
+    executor(executor&&)                         = delete;
     auto operator=(const executor&) -> executor& = delete;
-    auto operator=(executor&&) -> executor& = delete;
+    auto operator=(executor&&) -> executor&      = delete;
     ~executor();
 
 private:

inc/lift/http.hpp

@@ -1,7 +1,7 @@
 #pragma once
 
-#include <string>
 #include <cstdint>
+#include <string>
 
 #include <curl/curl.h>
 

inc/lift/impl/pragma.hpp

@@ -0,0 +1,19 @@
+#if defined(__GNUC__) && !defined(__llvm__)
+    #define DO_PRAGMA(X)                 _Pragma(#X)
+    #define DISABLE_WARNING_PUSH         DO_PRAGMA(GCC diagnostic push)
+    #define DISABLE_WARNING_POP          DO_PRAGMA(GCC diagnostic pop)
+    #define DISABLE_WARNING(warningName) DO_PRAGMA(GCC diagnostic ignored #warningName)
+
+    #define DISABLE_WARNING_MAYBE_UNINITIALIZED DISABLE_WARNING(-Wmaybe-uninitialized)
+#elif defined(__clang__)
+    #define DO_PRAGMA(X) _Pragma(#X)
+    #define DISABLE_WARNING_PUSH
+    #define DISABLE_WARNING_POP
+    #define DISABLE_WARNING(warningNumber)
+
+    #define DISABLE_WARNING_MAYBE_UNINITIALIZED
+#else
+    // unknown compiler, ignoring suppression directives
+    #define DISABLE_WARNING_PUSH
+    #define DISABLE_WARNING_POP
+#endif

inc/lift/lift_status.hpp

@@ -1,7 +1,7 @@
 #pragma once
 
-#include <string>
 #include <cstdint>
+#include <string>
 
 namespace lift
 {

inc/lift/query_builder.hpp

@@ -1,10 +1,10 @@
 #pragma once
 
+#include <cstdint>
 #include <sstream>
 #include <string>
 #include <string_view>
 #include <vector>
-#include <cstdint>
 
 namespace lift
 {

inc/lift/request.hpp

@@ -3,6 +3,7 @@
 #include "lift/header.hpp"
 #include "lift/http.hpp"
 #include "lift/impl/copy_util.hpp"
+#include "lift/impl/pragma.hpp"
 #include "lift/mime_field.hpp"
 #include "lift/resolve_host.hpp"
 #include "lift/response.hpp"
@@ -155,10 +156,10 @@ class request
         return std::make_unique<request>(std::move(url), std::move(timeout));
     }
 
-    request(const request&) = default;
-    request(request&&)      = default;
+    request(const request&)                             = default;
+    request(request&&)                                  = default;
     auto operator=(const request&) noexcept -> request& = default;
-    auto operator=(request&&) noexcept -> request& = default;
+    auto operator=(request&&) noexcept -> request&      = default;
 
     /**
      * Synchronously executes this request.
@@ -538,7 +539,12 @@ class request
      */
     auto async_future() -> async_future_type
     {
+        // gcc-13 is incorrectly thinking the 2nd type in the async_handlers_type is uninitialized.
+        // Its correct since it isn't used on this code path, but it isn't a warning/error.
+        DISABLE_WARNING_PUSH
+        DISABLE_WARNING_MAYBE_UNINITIALIZED
         m_on_complete_handler.m_object = {async_promise_type{}};
+        DISABLE_WARNING_POP
         return std::get<async_promise_type>(m_on_complete_handler.m_object.value()).get_future();
     }
 

inc/lift/resolve_host.hpp

@@ -1,7 +1,7 @@
 #pragma once
 
-#include <string>
 #include <cstdint>
+#include <string>
 
 namespace lift
 {
@@ -22,9 +22,9 @@ class resolve_host
     resolve_host(std::string resolve_host, uint16_t resolve_port, std::string resolved_ip_addr);
     ~resolve_host() = default;
 
-    resolve_host(const resolve_host&)     = default;
-    resolve_host(resolve_host&&) noexcept = default;
-    auto operator=(const resolve_host&) -> resolve_host& = default;
+    resolve_host(const resolve_host&)                        = default;
+    resolve_host(resolve_host&&) noexcept                    = default;
+    auto operator=(const resolve_host&) -> resolve_host&     = default;
     auto operator=(resolve_host&&) noexcept -> resolve_host& = default;
 
     /**

inc/lift/response.hpp

@@ -27,10 +27,10 @@ class response
     response();
     ~response() = default;
 
-    response(const response&) = default;
-    response(response&&)      = default;
+    response(const response&)                             = default;
+    response(response&&)                                  = default;
     auto operator=(const response&) noexcept -> response& = default;
-    auto operator=(response&&) noexcept -> response& = default;
+    auto operator=(response&&) noexcept -> response&      = default;
 
     /**
      * The Lift status is how the request ended up in the event loop.

inc/lift/share.hpp

@@ -42,10 +42,10 @@ class share : public std::enable_shared_from_this<share>
     explicit share(options opts);
     ~share();
 
-    share(const share&) = delete;
-    share(share&&)      = delete;
+    share(const share&)                             = delete;
+    share(share&&)                                  = delete;
     auto operator=(const share&) noexcept -> share& = delete;
-    auto operator=(share&&) noexcept -> share& = delete;
+    auto operator=(share&&) noexcept -> share&      = delete;
 
     static auto make_shared(options opts) -> std::shared_ptr<share> { return std::make_shared<share>(std::move(opts)); }
 

src/executor.cpp

@@ -73,6 +73,8 @@ auto executor::prepare() -> void
 
     switch (m_request->method())
     {
+        default:
+            /* INTENTIONAL FALLTHROUGH */
         case http::method::unknown: // default to GET on unknown/bad value.
             /* INTENTIONAL FALLTHROUGH */
         case http::method::get:
@@ -103,6 +105,8 @@ auto executor::prepare() -> void
 
     switch (m_request->version())
     {
+        default:
+            /* INTENTIONAL FALLTHROUGH */
         case http::version::unknown: // default to USE_BEST on unknown/bad value.
             /* INTENTIONAL FALLTHROUGH */
         case http::version::use_best:
@@ -223,6 +227,9 @@ auto executor::prepare() -> void
             {
                 switch (auth_type)
                 {
+                    default:
+                        // CURLAUTH_BASIC is the default per docs https://curl.se/libcurl/c/CURLOPT_PROXYAUTH.html.
+                        /* INTENTIONAL FALLTHROUGH */
                     case http_auth_type::basic:
                         auth_types |= CURLAUTH_BASIC;
                         break;
@@ -536,10 +543,9 @@ auto curl_write_data(void* buffer, size_t size, size_t nitems, void* user_ptr) -
     auto&  response     = executor_ptr->m_response;
     size_t data_length  = size * nitems;
 
-    std::copy(
-        static_cast<const char*>(buffer),
-        static_cast<const char*>(buffer) + data_length,
-        std::back_inserter(response.m_data));
+    std::string_view from{static_cast<const char*>(buffer), data_length};
+
+    std::copy(from.begin(), from.end(), std::back_inserter(response.m_data));
 
     return data_length;
 }