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

Fixes WasmAllocator to reflect recent nightly API changes #214

Merged
merged 5 commits into from
Jun 15, 2018

Conversation

0x7CFE
Copy link
Contributor

@0x7CFE 0x7CFE commented Jun 13, 2018

No description provided.

@0x7CFE 0x7CFE added the A0-please_review Pull request needs code review. label Jun 13, 2018
@0x7CFE 0x7CFE self-assigned this Jun 13, 2018
@0x7CFE 0x7CFE requested review from gavofyork and rphmeier June 13, 2018 14:52
@@ -20,16 +20,16 @@ mod __impl {
extern crate alloc;
extern crate pwasm_libc;

use self::alloc::heap::{GlobalAlloc, Layout, Opaque};
use core::alloc::{GlobalAlloc, Layout};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line needs attention. Note, that I removed the leading self. Don't sure, how WASM and no-std stuff is linked, so it may be an issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

probably the extern crate alloc and corresponding feature is no longer necessary since this stuff is in libcore now

Copy link
Contributor

@pepyakin pepyakin Jun 13, 2018

Choose a reason for hiding this comment

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

also the feature global_allocator is also no longer needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, forgot to push the second commit.

@pepyakin
Copy link
Contributor

This is not the only allocator used in the project: wee_alloc is used in basic_add project. Unfortunately, it has not been upgraded to the latest allocator API.

extern crate pwasm_libc;

use self::alloc::heap::{GlobalAlloc, Layout, Opaque};
use core::alloc::{GlobalAlloc, Layout};
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually need feature(alloc) here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it successfully compiled without alloc and allocator_api.

@gavofyork
Copy link
Member

could do with this getting sorted... is wee_alloc one of ours?

@0x7CFE
Copy link
Contributor Author

0x7CFE commented Jun 15, 2018

@gavofyork Compilation fails because wee_alloc wasn't updated yet. My PR was merged, but version is not yet updated.

Quick workaround is to update polkadot/parachain/test-chains/basic_add/Cargo.toml:

-wee_alloc = "0.4.0"
+wee_alloc = { git = "https://github.com/rustwasm/wee_alloc", branch = "master" }

@pepyakin
Copy link
Contributor

pepyakin commented Jun 15, 2018

Nope, it isn't, but I'm sort of a reviewer.

The fix is already on master, but it is not yet released on crates.io. I probably can release it, but I'd rather leave it for the main maintainer.

@gavofyork
Copy link
Member

ok lets put the workaround into #214 and merge that

@gavofyork
Copy link
Member

we can't have our CI pipeline halted waiting on an upstream project

Revision 4e9f23f points to a master after rustwasm/wee_alloc#45 was merged
@gavofyork gavofyork added A8-looksgood and removed A0-please_review Pull request needs code review. labels Jun 15, 2018
@0x7CFE
Copy link
Contributor Author

0x7CFE commented Jun 15, 2018

All tests passed. PR may be merged.

@pepyakin pepyakin merged commit 4760b35 into master Jun 15, 2018
@pepyakin pepyakin deleted the dk-pwasm-alloc-fix branch June 15, 2018 10:10
JoshOrndorff pushed a commit to moonbeam-foundation/substrate that referenced this pull request Apr 21, 2021
* init

* mv kill author to on initialize

* rm unused const

* bump version and address grumble with assert noop import
liuchengxu pushed a commit to chainx-org/substrate that referenced this pull request Aug 23, 2021
* Use bs58 instead of our own base58
* Rewrite serde_text and serde_hex

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
liuchengxu pushed a commit to autonomys/substrate that referenced this pull request Jun 3, 2022
* glossary for farmer binary, and farming separated into its own documentation
helin6 pushed a commit to boolnetwork/substrate that referenced this pull request Jul 25, 2023
* hacky integration with jsonrpsee v2

* stray todos

* fmt

* add http support

* make test build compile

* Update src/rpc.rs

* bring back set_client

* use crates.io version jsonrpsee

* WIP: workaround for embedded subxt client (paritytech#236)

* workaround for embedded subxt client

Signed-off-by: Gregory Hill <gregorydhill@outlook.com>

* increase default channel size on subxt client

Signed-off-by: Gregory Hill <gregorydhill@outlook.com>

* remove client tests due to inference problem on From

Signed-off-by: Gregory Hill <gregorydhill@outlook.com>

* add comments for missing impls

* more verbose errors

* make subscription notifs buffer bigger

* fmt

Co-authored-by: Greg Hill <gregorydhill@outlook.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants