Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Switch types to use RuntimeDebug instead of gated Debug #4652

Merged
merged 3 commits into from
Jan 11, 2022

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Jan 3, 2022

This is useful for when you want to debug in wasm and enable the
force-debug feature of sp-debug-derive.

Fixes: #4651

This is useful for when you want to debug in wasm and enable the
`force-debug` feature of `sp-debug-derive`.
@bkchr bkchr added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Jan 3, 2022
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

This should have a small, but non-zero impact on the code size if I am not mistaking. The RuntimeDebug will generate a impl block for this type in wasm that's not really printing anything, whereas the std-gated Debug is literally non-existent in wasm. Can you confirm which one is the case here? Maybe the RuntimeDebug's implementation is also stripped away by the compiler?

Either way, PR is fine to me.

@bkchr
Copy link
Member Author

bkchr commented Jan 4, 2022

The RuntimeDebug will generate a impl block for this type in wasm that's not really printing anything, whereas the std-gated Debug is literally non-existent in wasm.

This is correct. RuntimeDebug will print <wasm:stripped>

@hirschenberger
Copy link
Contributor

hirschenberger commented Jan 11, 2022

Can we please land this quickly, as I need it for debugging my code? Merci.

I merged this branch into mine. It builds now but I still get a <wasm:stripped> from my log::warn!.
I'm running a benchmark with --execution=native, so it should not be stripped, right?

I want to debug a frame_system::pallet::Config::Event, which is still Debug, RuntimeDebug needs to be introduced to substrate as well?

@bkchr
Copy link
Member Author

bkchr commented Jan 11, 2022

I'm running a benchmark with --execution=native, so it should not be stripped, right?

Benchmarks are only running in wasm.

I want to debug a frame_system::pallet::Config::Event, which is still Debug, RuntimeDebug needs to be introduced to substrate as well?

If it is just Debug, there is no problem.

@bkchr bkchr merged commit a5edb8e into master Jan 11, 2022
@bkchr bkchr deleted the bkchr-switcht-to-runtime-debug branch January 11, 2022 11:28
@hirschenberger
Copy link
Contributor

I'm running a benchmark with --execution=native, so it should not be stripped, right?

Benchmarks are only running in wasm.

I want to debug a frame_system::pallet::Config::Event, which is still Debug, RuntimeDebug needs to be introduced to substrate as well?

If it is just Debug, there is no problem.

So how can I debug my event then without being stripped away?

@bkchr
Copy link
Member Author

bkchr commented Jan 11, 2022

Wizdave97 pushed a commit to ComposableFi/polkadot that referenced this pull request Feb 3, 2022
…ch#4652)

* Switch types to use `RuntimeDebug` instead of gated `Debug`

This is useful for when you want to debug in wasm and enable the
`force-debug` feature of `sp-debug-derive`.

* Fixes

* 🤦
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Runtime debugging not possible due to missing RuntimeDebug implementations
3 participants