Repository navigation
Conversation
Add an Interface template parameter to ProxyMethod, ProxyMethodTraits, ProxyClientMethodTraits and ProxyServerMethodTraits, and generate ProxyMethod specializations for inherited methods as well as each interface's own methods. This lets a wrapped capnp interface inherit methods from a base interface that is wrapped by a different C++ class, or not wrapped at all. Previously the mapping from an inherited method to a C++ method came from the base interface's generated ProxyMethod<Params> specialization, so it only worked when the base interface was wrapped by a C++ base class of the derived interface's class. Each method could only have one mapping, so two wrapped interfaces inheriting from the same unwrapped base could not each supply one. Bitcoin Core needs this to split its Chain interface into stable Chain and UnstableChain capnp interfaces wrapping the same interfaces::Chain class (bitcoin/bitcoin#34981). This is a breaking change for code specializing ProxyClientMethodTraits or ProxyServerMethodTraits, which now takes the interface type as its first template argument. This change was written with Claude Opus 5.5 (1M context).
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline and AI policy for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem: Currently, interface inheritance in libmultiprocess only works if capnproto base and derived interfaces both use
$Proxy.wrapannotations and have C++ base and derived classes implementing methods of both interfaces. It is currently not possible to have derived capnproto interface that uses$Proxy.wrapinherit from a base interface that is unwrapped. This is needed for the stable/unstable Chain interface split proposed in bitcoin/bitcoin#34981. This limitation exists becauseProxyMethodtemplate class which is responsible for mapping capnproto method types to c++ method pointers currently assumes that that every capnproto method will be implemented by the wrapped base interface, and doesn't allow them to be implemented be derived classes.Solution: Pass an interface type template parameter to
ProxyMethodand to theProxyMethodTraits,ProxyClientMethodTraitsandProxyServerMethodTraitstraits classes, and have mpgen generateProxyMethodspecializations for every method an interface handles, including inherited ones. Small change toproxy.h,proxy-types.handgen.cpp, plus a test of an interface inheriting from a base interface with no C++ class.This is a potentially breaking change for code that specializes
ProxyClientMethodTraitsorProxyServerMethodTraits, which now takes the interface type as its first template argument. But Bitcoin Core master does not have any of these specializations currently, so it needs no changes. Generated code is otherwise equivalent and there are no wire protocol changes.