proxy-types: Remove requirement for return types to be default-constructible #337

pull ryanofsky wants to merge 4 commits into bitcoin-core:master from ryanofsky:pr/noreturn changing 8 files +392 −103
  1. ryanofsky commented at 10:09 PM on August 7, 2026: collaborator

    Improve clientInvoke to avoid requirement that return types are default-constructible. This lets https://github.com/bitcoin/bitcoin/pull/10102 return types like PartiallySignedTransaction, WalletAddress, util::Result that don't have default constructors, without needing to wrap them or give them default constructors.

    Also remove requirement that return types are copyable or movable, and add tests for this, and improve documentation for ReadDest classes to describe how they are supposed to be passed to ReadField callers and used inside CustomReadField implementations to deal with different types including non-movable, non-default-constructible ones.

  2. proxy: eliminate default-constructor and move requirements for IPC return types
    Previously the generated client declared a default-constructed result variable
    and passed a reference to it into clientInvoke, requiring the return type to be
    default-constructible. Restructure the client return path so the value is
    constructed directly in place:
    
    - clientInvoke takes the return type and result Accessor as explicit template
      arguments (defaulting to void). It deserializes the result with ReadField
      into a ReadDestTemp, so the value is built from constructor arguments without
      a default constructor.
    - Propagate the value as a prvalue: IterateFieldsHelper::handleChain now returns
      decltype(auto), and the movable path uses C++17 guaranteed copy elision to
      construct the result in AlignedStorage.
    - For non-movable return types (no move or copy constructor), copy the capnp
      response to a flat word buffer on the event-loop thread and deserialize it on
      the client thread, returning a prvalue via guaranteed copy elision so no move
      constructor is ever needed.
    
    Add AlignedStorage<T>, a typed wrapper around an aligned byte buffer with a
    ptr() accessor, used immediately by clientInvoke and reused later by TryFinally.
    
    Extend FunctionTraits to all four PMF cv-qualifier combinations so clientInvoke
    can derive the capnp Results type from the request method pointer.
    
    The code generator emits clientInvoke<Result, Accessor>(...) for non-void
    methods and plain clientInvoke(...) for void methods.
    
    Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
    e8f72cfa5f
  3. proxy: eliminate move-constructor requirements for IPC return types
    Restructure result handling on the server side to eliminate
    move-constructor requirements for IPC return types. (Client side
    move-constructor requirements were removed in the previous commit.)
    
    - Add 3-arg TryFinally(fn, after, consume) overload in util.h that stores
      fn()'s return value via placement new from prvalue (C++17 guaranteed copy
      elision) and passes it by reference to consume(), then destroys it.
      This avoids ever needing to move the result.
    - Merge ServerRet into ServerCall by making ServerCall a template parameterized
      on the result Accessor (void for void methods). The result is now serialized
      inside the TryFinally after() callback, forwarding it with the invoked
      method's value category so move-only results (e.g. vector<unique_ptr<Bar>>)
      are moved rather than copied.
    - Update code generator to emit ServerCall<void>() or ServerCall<Accessor>()
      instead of Make<ServerRet, Accessor>(ServerCall()).
    
    Behavior-preserving: results are serialized exactly as before.
    
    These changes are combined because they are tightly coupled: redesigning
    TryFinally to return void forces ServerCall::invoke() to also return void,
    which breaks ServerRet's existing `auto&& result = Parent::invoke(...)` binding.
    The only clean intermediate would have ServerRet duplicate the same logic that
    ServerCall<Accessor> ends up with, so combining produces a simpler result.
    
    Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
    935dc4915c
  4. test: add Pinned<T> non-movable IPC return and exception tests
    Add Pinned<T>, a type with no default constructor and no copy or move
    operations, to exercise the non-movable IPC return path end-to-end. Its
    CustomReadField uses read_dest.construct() with a ReadDestTemp<T> argument,
    which is the only way to deserialize a type that can neither be
    default-constructed (for update()) nor moved (for a std::optional staging
    variable).
    
    Add two FooInterface methods:
    - returnPinned returns Pinned<std::vector<int>> by value, verifying the client
      can retrieve a non-movable return value.
    - throwPinned throws Pinned<std::vector<int>> via $Proxy.exception, verifying
      the exception path constructs a non-movable value as a prvalue.
    
    Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
    09e20bf272
  5. doc: document ReadField destination types and ReadDestTemp
    Replace the terse ReadDestEmplace comment with structured documentation of the
    three ReadField destination types (ReadDestEmplace, ReadDestUpdate, and the
    ReadDestTemp() helper), covering:
    
    - The contract for CustomReadField implementors: return decltype(auto) and
      forward the construct()/update() return value, which is easy to miss because
      most callers ignore it but is load-bearing when ReadDestTemp() is used.
    - The contract for emplace callbacks and the three return-type cases (container
      emplace, ReadDestTemp() prvalue, and the vector<bool> reference-like proxy).
    - When ReadDestTemp() is merely convenient versus strictly necessary, with the
      Pinned<T> return-value and nested-CustomReadField cases as examples.
    - Return-value notes on each construct()/update() method and an inline note
      flagging the placement-new exception-safety gap in ReadDestUpdate::construct.
    
    Comment-only change.
    
    Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
    aa55cddad4
  6. DrahtBot commented at 10:09 PM on August 7, 2026: none

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

Contributors

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/libmultiprocess. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-08-23 22:30 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me