Skip to content

Add borrow capability constant to Reader#243

Open
cpubot wants to merge 1 commit intomasterfrom
borrow-capability
Open

Add borrow capability constant to Reader#243
cpubot wants to merge 1 commit intomasterfrom
borrow-capability

Conversation

@cpubot
Copy link
Contributor

@cpubot cpubot commented Mar 17, 2026

This adds a constant BORROW_KINDS to the Reader trait that allows implementations to specify which BorrowKinds they support. This will allow SchemaRead implementations to vary their implementation depending on the borrow capabilities of the Reader. Conditions based on Reader::supports_borrow can be constant-folded by the compiler, and as such should have no runtime cost.

Examples of where this is useful

Cow

We don't currently support Cow, but with this functionality, we can return Cow::Borrowed for Readers that support BorrowKind::Backing.

solana-sdk's v1::Message

https://github.com/anza-xyz/solana-sdk/blob/735c03b78c60d7ae9a98c0cdccac396911881676/message/src/versions/v1/message.rs#L651-L658

If we wanted to make this SchemaRead implementation compatible with a broader set of Reader back-ends in the future, this implementation could heap-allocate the InstructionHeaders if the Reader does not support BorrowKind::Backing.

ecow

https://github.com/tanmay4l/wincode/blob/a08c2349a7e2e8ba835602de5b818c5f6c11f7b1/wincode/src/schema/external/ecow.rs#L36-L44

Similarly, this implementation could check reader.supports_borrow(BorrowKind::CallSite) rather than matching on the error.

@cpubot cpubot requested a review from kskalski March 17, 2026 18:21
///
/// Users of [`Reader`] can call [`Reader::supports_borrow`] to check if a borrow kind
/// is supported.
const BORROW_CAP: u8 = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like _CAP suffix here, because it sounds like a limit (from "hat") or capacity

How about calling it BORROW_SUPPORT, maybe BORROW_KINDS or just a full BORROW_CAPABILITY ?
Also, given that this is mask, could mention it in the name like BORROW_SUPPORT_MASK

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure -- BORROW_KINDS seems reasonable to me

/// assert!(!reader.supports_borrow(BorrowKind::Backing));
/// ```
#[inline]
fn supports_borrow(&self, kind: BorrowKind) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make it const?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not -- const functions in traits isn't yet supported. But, I looked at the assembly in a few examples, and the compiler reliably constant-folds conditions out since BORROW_CAP and kind.mask is const

@cpubot cpubot force-pushed the borrow-capability branch from 9b63298 to 11b34cc Compare March 18, 2026 16:09
@cpubot cpubot requested a review from kskalski March 18, 2026 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants