From f4ebe8d5d9e8f6033e78faaebeefc0eaffc639c5 Mon Sep 17 00:00:00 2001 From: Michael Vandeberg Date: Fri, 12 Jun 2026 13:45:46 -0600 Subject: [PATCH] refactor(resolver): replace resolver_results wrapper with a type alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolver_results was a shared_ptr> wrapper that gave O(1) copies and an identity-based operator==. The equality ops were never used, and the only by-value sink (connect) must own the range for coroutine safety regardless, so an rvalue is moved in cheaply either way. Make resolver_results an alias for std::vector: deletes the wrapper boilerplate and exposes the full vector interface. The one regression — deep-copying an lvalue you want to retain — is documented on resolve() and connect(), which point callers at std::move or the iterator-based connect overloads. Closes #123. --- .../ROOT/pages/3.tutorials/3c.dns-lookup.adoc | 5 +- .../ROOT/pages/4.guide/4j.resolver.adoc | 34 +++-- include/boost/corosio/connect.hpp | 9 +- .../detail/iocp/win_resolver_service.hpp | 3 +- .../detail/posix/posix_resolver_service.hpp | 3 +- .../boost/corosio/native/native_resolver.hpp | 4 + include/boost/corosio/resolver.hpp | 6 + include/boost/corosio/resolver_results.hpp | 119 ++---------------- 8 files changed, 53 insertions(+), 130 deletions(-) diff --git a/doc/modules/ROOT/pages/3.tutorials/3c.dns-lookup.adoc b/doc/modules/ROOT/pages/3.tutorials/3c.dns-lookup.adoc index 793906fcf..342c1cea1 100644 --- a/doc/modules/ROOT/pages/3.tutorials/3c.dns-lookup.adoc +++ b/doc/modules/ROOT/pages/3.tutorials/3c.dns-lookup.adoc @@ -85,8 +85,9 @@ capy::task do_lookup( == Understanding Results -The resolver returns a `resolver_results` object containing `resolver_entry` -elements. Each entry provides: +The resolver returns a `resolver_results` — an alias for +`std::vector` — containing one `resolver_entry` per resolved +endpoint. Each entry provides: * `get_endpoint()` — The resolved endpoint (address + port) * `host_name()` — The queried hostname diff --git a/doc/modules/ROOT/pages/4.guide/4j.resolver.adoc b/doc/modules/ROOT/pages/4.guide/4j.resolver.adoc index 79b792148..37811a511 100644 --- a/doc/modules/ROOT/pages/4.guide/4j.resolver.adoc +++ b/doc/modules/ROOT/pages/4.guide/4j.resolver.adoc @@ -117,7 +117,14 @@ auto [ec, results] = co_await r.resolve("127.0.0.1", "8080", flags); == Working with Results -The `resolver_results` class is a range of `resolver_entry` objects: +`resolver_results` is an alias for `std::vector`, so it +supports the full `std::vector` interface (iteration, `size()`, `empty()`, +indexing, and so on): + +[source,cpp] +---- +using resolver_results = std::vector; +---- [source,cpp] ---- @@ -134,23 +141,14 @@ for (auto const& entry : results) } ---- -=== resolver_results Interface - -[source,cpp] ----- -class resolver_results -{ -public: - using iterator = /* ... */; - using const_iterator = /* ... */; - - std::size_t size() const; - bool empty() const; - - const_iterator begin() const; - const_iterator end() const; -}; ----- +[NOTE] +==== +Copying a `resolver_results` deep-copies every entry, and each entry owns +two `std::string` query names. When handing the results to a sink that +takes the range by value — such as `corosio::connect` — pass an rvalue +(`std::move(results)`) or use the iterator-based overload +(`connect(s, results.begin(), results.end())`) to avoid the copy. +==== === resolver_entry Interface diff --git a/include/boost/corosio/connect.hpp b/include/boost/corosio/connect.hpp index d106fa576..36025ae56 100644 --- a/include/boost/corosio/connect.hpp +++ b/include/boost/corosio/connect.hpp @@ -105,6 +105,11 @@ connect(Socket& s, Iter begin, Iter end, ConnectCondition cond); @param endpoints A range of candidate endpoints. Taken by value so temporaries (e.g. `resolver_results` returned from `resolver::resolve`) remain alive for the coroutine's lifetime. + Because the range is owned by the coroutine, passing an lvalue + copies it; since `resolver_results` is a + `std::vector`, that is a deep copy of every entry. + Pass an rvalue (`std::move(results)`) or use the iterator overload + (`connect(s, results.begin(), results.end())`) to avoid the copy. @return An awaitable completing with `capy::io_result`: @@ -153,7 +158,9 @@ connect(Socket& s, Range endpoints) @param s The socket to connect. See the non-condition overload for requirements. - @param endpoints A range of candidate endpoints. + @param endpoints A range of candidate endpoints, taken by value. See + the non-condition overload for the deep-copy caveat when passing + an lvalue `resolver_results`. @param cond A predicate invocable with `(std::error_code const&, typename Socket::endpoint_type const&)` returning a value contextually convertible to `bool`. diff --git a/include/boost/corosio/native/detail/iocp/win_resolver_service.hpp b/include/boost/corosio/native/detail/iocp/win_resolver_service.hpp index 00896ed72..bebddc42b 100644 --- a/include/boost/corosio/native/detail/iocp/win_resolver_service.hpp +++ b/include/boost/corosio/native/detail/iocp/win_resolver_service.hpp @@ -1,6 +1,7 @@ // // Copyright (c) 2025 Vinnie Falco (vinnie.falco@gmail.com) // Copyright (c) 2026 Steve Gerbino +// Copyright (c) 2026 Michael Vandeberg // // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) @@ -203,7 +204,7 @@ convert_results( } } - return resolver_results(std::move(entries)); + return entries; } } // namespace resolver_detail diff --git a/include/boost/corosio/native/detail/posix/posix_resolver_service.hpp b/include/boost/corosio/native/detail/posix/posix_resolver_service.hpp index 8505cd0b2..97b29572e 100644 --- a/include/boost/corosio/native/detail/posix/posix_resolver_service.hpp +++ b/include/boost/corosio/native/detail/posix/posix_resolver_service.hpp @@ -1,5 +1,6 @@ // // Copyright (c) 2026 Steve Gerbino +// Copyright (c) 2026 Michael Vandeberg // // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) @@ -161,7 +162,7 @@ posix_resolver_detail::convert_results( } } - return resolver_results(std::move(entries)); + return entries; } inline std::error_code diff --git a/include/boost/corosio/native/native_resolver.hpp b/include/boost/corosio/native/native_resolver.hpp index bd248eb45..c2a75f52d 100644 --- a/include/boost/corosio/native/native_resolver.hpp +++ b/include/boost/corosio/native/native_resolver.hpp @@ -1,5 +1,6 @@ // // Copyright (c) 2026 Steve Gerbino +// Copyright (c) 2026 Michael Vandeberg // // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) @@ -191,6 +192,9 @@ class native_resolver : public resolver @param service The service name or port string. @return An awaitable yielding `io_result`. + + @note `resolver_results` is an alias for `std::vector`; + copying it deep-copies every entry. See @ref resolver::resolve. */ auto resolve(std::string_view host, std::string_view service) { diff --git a/include/boost/corosio/resolver.hpp b/include/boost/corosio/resolver.hpp index ad27a7ec5..062022e2e 100644 --- a/include/boost/corosio/resolver.hpp +++ b/include/boost/corosio/resolver.hpp @@ -1,6 +1,7 @@ // // Copyright (c) 2025 Vinnie Falco (vinnie.falco@gmail.com) // Copyright (c) 2026 Steve Gerbino +// Copyright (c) 2026 Michael Vandeberg // // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) @@ -351,6 +352,11 @@ class BOOST_COROSIO_DECL resolver : public io_object @return An awaitable that completes with `io_result`. + @note `resolver_results` is an alias for `std::vector`. + Copying it deep-copies every entry (each owns two `std::string`s); + move it (`std::move(results)`) or pass iterators when handing it to + a by-value sink such as @ref connect. + @par Example @code auto [ec, results] = co_await r.resolve("www.example.com", "https"); diff --git a/include/boost/corosio/resolver_results.hpp b/include/boost/corosio/resolver_results.hpp index e3d744d23..e1310e87d 100644 --- a/include/boost/corosio/resolver_results.hpp +++ b/include/boost/corosio/resolver_results.hpp @@ -1,5 +1,6 @@ // // Copyright (c) 2025 Vinnie Falco (vinnie.falco@gmail.com) +// Copyright (c) 2026 Michael Vandeberg // // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) @@ -13,8 +14,6 @@ #include #include -#include -#include #include #include #include @@ -80,115 +79,21 @@ class resolver_entry /** A range of entries produced by a resolver. - This class holds the results of a DNS resolution query. - It provides a range interface for iterating over the - resolved endpoints. + This is an alias for `std::vector`: a contiguous, + owning range of the endpoints resolved by a query. It supports the + full `std::vector` interface (iteration, `size()`, `empty()`, etc.). + + @note Copying a `resolver_results` deep-copies every entry, and each + entry owns two `std::string`s (the host and service names). When you + want to hand a result to a sink that takes the range by value — such + as `corosio::connect` — pass an rvalue (`std::move(results)`) or use + the iterator-based `connect` overloads to avoid the copy. @par Thread Safety Distinct objects: Safe.@n - Shared objects: Safe (immutable after construction). + Shared objects: Unsafe. */ -class resolver_results -{ -public: - /// The entry type. - using value_type = resolver_entry; - - /// Const reference to an entry. - using const_reference = value_type const&; - - /// Reference to an entry (always const). - using reference = const_reference; - - /// Const iterator over entries. - using const_iterator = std::vector::const_iterator; - - /// Iterator over entries (always const). - using iterator = const_iterator; - - /// Signed difference type. - using difference_type = std::ptrdiff_t; - - /// Unsigned size type. - using size_type = std::size_t; - -private: - std::shared_ptr> entries_; - -public: - /// Construct an empty results range. - resolver_results() = default; - - /** Construct from a vector of entries. - - @param entries The resolved entries. - */ - explicit resolver_results(std::vector entries) - : entries_( - std::make_shared>(std::move(entries))) - { - } - - /// Return the number of entries. - size_type size() const noexcept - { - return entries_ ? entries_->size() : 0; - } - - /// Check if the results are empty. - bool empty() const noexcept - { - return !entries_ || entries_->empty(); - } - - /// Return an iterator to the first entry. - const_iterator begin() const noexcept - { - if (entries_) - return entries_->begin(); - return std::vector::const_iterator(); - } - - /// Return an iterator past the last entry. - const_iterator end() const noexcept - { - if (entries_) - return entries_->end(); - return std::vector::const_iterator(); - } - - /// Return an iterator to the first entry. - const_iterator cbegin() const noexcept - { - return begin(); - } - - /// Return an iterator past the last entry. - const_iterator cend() const noexcept - { - return end(); - } - - /// Swap with another results object. - void swap(resolver_results& other) noexcept - { - entries_.swap(other.entries_); - } - - /// Test for equality. - friend bool - operator==(resolver_results const& a, resolver_results const& b) noexcept - { - return a.entries_ == b.entries_; - } - - /// Test for inequality. - friend bool - operator!=(resolver_results const& a, resolver_results const& b) noexcept - { - return !(a == b); - } -}; +using resolver_results = std::vector; /** The result of a reverse DNS resolution.