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

fix(runtime-c-api) Change mutability of memory toconst in wasmer_memory_data_length #1335

Merged
merged 5 commits into from
Mar 28, 2020

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented Mar 26, 2020

This PR changes mutability of memory for const in wasmer_memory_data_length to be more consistent regarding the returned value of wasmer_instance_context_memory.

Fixes #1314.
Fixes wasmerio/docs.wasmer.io#43.

@Hywan Hywan added bug Something isn't working 📦 lib-c-api About wasmer-c-api labels Mar 26, 2020
Copy link
Contributor

@MarkMcCaskey MarkMcCaskey left a comment

Choose a reason for hiding this comment

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

Looks good to me!

CHANGELOG.md Outdated Show resolved Hide resolved
@Hywan
Copy link
Contributor Author

Hywan commented Mar 27, 2020

bors r+

bors bot added a commit that referenced this pull request Mar 27, 2020
1335:  fix(runtime-c-api) Change mutability of `memory` for `const` in `wasmer_memory_data_length` r=Hywan a=Hywan

This PR changes mutability of `memory` for `const` in `wasmer_memory_data_length` to be more consistent regarding the returned value of `wasmer_instance_context_memory`.

Fixes #1314. 
Fixes wasmerio/docs.wasmer.io#43.

Co-authored-by: Ivan Enderlin <[email protected]>
Co-authored-by: Ivan Enderlin <[email protected]>
@Hywan Hywan changed the title fix(runtime-c-api) Change mutability of memory for const in wasmer_memory_data_length fix(runtime-c-api) Change mutability of memory toconst in wasmer_memory_data_length Mar 27, 2020
@bors
Copy link
Contributor

bors bot commented Mar 27, 2020

Timed out

@Hywan
Copy link
Contributor Author

Hywan commented Mar 28, 2020

bors r+

bors bot added a commit that referenced this pull request Mar 28, 2020
1335:  fix(runtime-c-api) Change mutability of `memory` to`const` in `wasmer_memory_data_length` r=Hywan a=Hywan

This PR changes mutability of `memory` for `const` in `wasmer_memory_data_length` to be more consistent regarding the returned value of `wasmer_instance_context_memory`.

Fixes #1314. 
Fixes wasmerio/docs.wasmer.io#43.

1337: feat(interface-types) Better handling of i32 to usize casts r=Hywan a=Hywan

Fix #1334 

This PR introduces a new `NegativeValue` error.
This PR fixes some `usize` and `u32` types for indexes (small changes).
Finally, this PR casts `i32` to `usize` by using the `TryFrom` implementation. That way:

```rust
let pointer = to_native::<i32>(&inputs[0], instruction)? as usize;
```

becomes:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)?.try_into().map_err(|_| {
    InstructionError::new(
        instruction,
        InstructionErrorKind::NegativeValue { subject: "pointer" },
    )
})?;
```

It's more verbose, but it handles the casting correctly.

Note: Maybe we should implement `From<TryFromIntError>` for `InstructionErrorKind`? What do you think?

Edit: With `From<TryFromIntError>`, it looks like this:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)? // convert from WIT to native `i32`
    .try_into() // convert to `usize`
    .map_err(|e| (e, "pointer").into()) // convert the `TryFromIntError` to `InstructionErrorKind::NegativeValue`
    .map_err(|k| InstructionError::new(instruction, k))?; // convert to an `InstructionError`
```

It is indeed simpler. Keeping things like this.

Co-authored-by: Ivan Enderlin <[email protected]>
Co-authored-by: Ivan Enderlin <[email protected]>
@syrusakbary syrusakbary merged commit baa74d7 into wasmerio:master Mar 28, 2020
@bors
Copy link
Contributor

bors bot commented Mar 28, 2020

This PR was included in a batch that timed out, it will be automatically retried

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 📦 lib-c-api About wasmer-c-api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent use of const in memory pointer in C API Get error when compiling passing-data.c example
3 participants