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

Implement *WasmAbi for array #2013

Closed
wants to merge 1 commit into from
Closed

Conversation

c410-f3r
Copy link
Contributor

@c410-f3r c410-f3r commented Feb 25, 2020

WIP

Requesting review because this is a blind attempt to implement *WasmAbi for arrays (up to 32). Sincerely, I am not aware of the consequences of these changes, let alone the possible missing places required for this feature.

@c410-f3r c410-f3r force-pushed the array-impls branch 2 times, most recently from 5a4521d to 9ed3400 Compare February 25, 2020 21:58
#[inline]
fn into_abi(self) -> Self::Abi {
WasmSlice {
ptr: self.as_ptr().into_abi(),
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is a use-after-free, right? self is dropped at the end of this function but we're returning a pointer to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hehehe. I should have paid more attention instead of simply copying and pasting. Thank you

@c410-f3r
Copy link
Contributor Author

c410-f3r commented Nov 25, 2020

@alexcrichton At the time, there were some concerns about the feasibility of this feature. Has anything changed since then?

@alexcrichton
Copy link
Contributor

Unfortunately, no, not much has changed I believe.

@c410-f3r
Copy link
Contributor Author

@alexcrichton Thank you !

With the imminent stabilization of constant generics, it is unfortunate that this feature can't be implemented.
Feel free to ping me if anything changes in the near future so I can continue the work on this PR

@c410-f3r
Copy link
Contributor Author

Sup @alexcrichton

Is it still not possible to implement this feature?

@NewDark90
Copy link

I know I need this functionality.

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