Skip to content

Conversation

@kevinheavey
Copy link
Contributor

@kevinheavey kevinheavey commented Feb 10, 2025

This function is only used here and in solana-bpf-loader program. When used here, it doesn't need num-traits. I have opened a PR in the Agave repo to copy the num-traits version into solana-bpf-loader program anza-xyz/agave#4888

The num-traits dep is causing pinocchio to copy-paste a significant amount of code.

Since the function is hidden this is not a breaking change.

@kevinheavey kevinheavey changed the title make is_nonoverlapping private and remove num-traits dep make is_nonoverlapping private and remove num-traits dep from program-memory Feb 10, 2025
@febo
Copy link
Contributor

febo commented Feb 13, 2025

We could also add the missing unsafe markers – that way we can replicate the implementation from pinocchio here.

@kevinheavey
Copy link
Contributor Author

We could also add the missing unsafe markers

Where?

@kevinheavey
Copy link
Contributor Author

Oh guessing you mean these ones:

__This function is incorrectly missing an `unsafe` declaration.__

Let's leave those for another PR

Copy link
Collaborator

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

This makes sense to me, thanks for the contribution! Just a suggestion for the test.

And since this is a breaking change anyway, we can mark those functions unsafe as febo recommended in a separate PR, then release a new major version.

Co-authored-by: Jon C <[email protected]>
Copy link
Collaborator

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Thanks!

@joncinque joncinque merged commit da624c3 into anza-xyz:master Mar 13, 2025
20 checks passed
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.

3 participants