Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add std flag #1266

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add std flag #1266

wants to merge 5 commits into from

Conversation

heytdep
Copy link
Contributor

@heytdep heytdep commented Apr 26, 2024

What

Adds a new feature and disables allocator when std flag is set.

Why

Allows to use the std while having a custom allocator.

Known limitations

N/A

@@ -48,7 +48,7 @@
compile_error!("'testutils' feature is not supported on 'wasm' target");

// When used in a no_std contract, provide a panic handler as one is required.
#[cfg(all(not(feature = "alloc"), target_family = "wasm"))]
#[cfg(all(not(feature = "alloc"), not(feature = "std"), target_family = "wasm"))]
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be breaking for someone, can't it? No-one would have the new feature enabled, so handle_panic may disappear suddenly. Wouldn't it be safer to add a feature that disables handle_panic on-demand?

@leighmcculloch
Copy link
Member

leighmcculloch commented May 3, 2024

There's a variety of situations that we could support, and so I'm not sure using 'std' as the name of this feature is most appropriate because of the possible variety of interpretations.

Here's a matrix of configurations that could be supported. Only the two configurations with text in there are the ones currently supported. The text in the boxes is the feature enabled to get those things. Today we have:

no allocator sdks allocator custom allocator libstd allocator
sdk panic handler default alloc
custom panic handler
libstd panic handler

If this PR is merged, the result would be support for:

no allocator sdks allocator custom allocator libstd allocator
sdk panic handler default alloc
custom panic handler std
libstd panic handler

If we wanted to support all these things, we could add the following features:

no allocator sdks allocator custom allocator libstd allocator
sdk panic handler default alloc alloc-custom alloc-std
custom panic handler panic-handler-custom
libstd panic handler panic-handler-std

To that end, I think this PR's new feature could be named panic-handler-custom, because the feature is disabling the built-in panic handler, and leaving it up to the developer to provide the panic handler in some way, either by importing a lib that contains one like libstd, or providing their own.

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