fix(runtime-c-api) Fix several warnings #232
Conversation
| @@ -17,23 +17,23 @@ use wasmer_runtime_core::module::{ExportIndex, ImportName}; | |||
| use wasmer_runtime_core::types::{ElementType, FuncSig, MemoryDescriptor, TableDescriptor, Type}; | |||
| use wasmer_runtime_core::units::{Bytes, Pages}; | |||
|
|
|||
| #[allow(non_camel_case_types)] | |||
| pub struct wasmer_module_t(); | |||
| #[repr(C)] | |||
There was a problem hiding this comment.
repr(C) implies no_mangle and allow(non_camel_case_types) as my tests shown. Not sure, but it has removed several warnings.
There was a problem hiding this comment.
👍, also when changes are made that may affect the header files you can run make capi from the root project. That and running tests is described in this projects README. Please run make capi for this PR if you haven’t already.
There was a problem hiding this comment.
Done. I think we have an issue 😄.
| #[allow(non_camel_case_types)] | ||
| pub struct wasmer_module_t(); | ||
| #[repr(C)] | ||
| pub struct wasmer_module_t; |
There was a problem hiding this comment.
I've removed () at the end of the structure declaration. I don't think they are required.
| @@ -180,7 +181,7 @@ pub unsafe extern "C" fn wasmer_validate( | |||
| /// and `wasmer_last_error_message` to get an error message. | |||
| #[no_mangle] | |||
| pub unsafe extern "C" fn wasmer_memory_new( | |||
| mut memory: *mut *mut wasmer_memory_t, | |||
| memory: *mut *mut wasmer_memory_t, | |||
There was a problem hiding this comment.
memory contains a mutable pointer, but the binding itself is not mutable. rustc generates a warning for that.
| @@ -301,8 +302,7 @@ pub extern "C" fn wasmer_table_grow( | |||
| #[no_mangle] | |||
| pub extern "C" fn wasmer_table_length(table: *mut wasmer_table_t) -> uint32_t { | |||
| let table = unsafe { &*(table as *mut Table) }; | |||
| let len = table.size(); | |||
| len | |||
| table.size() | |||
There was a problem hiding this comment.
Clippy suggests to return the value directly instead of the binding.
| } | ||
| for (module_name, namespace) in namespaces.into_iter() { | ||
| import_object.register(module_name, namespace); | ||
| } | ||
|
|
||
| let module = unsafe { &*(module as *mut Module) }; | ||
| let module = &*(module as *const Module); |
There was a problem hiding this comment.
I think it's not necessary to get a mutable module here. Correct me if I misunderstood the code!
| wasmer_value_tag::WASM_I64 => Type::I64, | ||
| wasmer_value_tag::WASM_F32 => Type::F32, | ||
| wasmer_value_tag::WASM_F64 => Type::F64, | ||
| _ => panic!("not implemented"), |
There was a problem hiding this comment.
If I get it correctly, the _ arm is unreachable, but we don't trust the v input, is that right? In other words, we don't assume that v is correctly built.
Hywan
force-pushed
the
fix-runtime-c-api-remove-warnings
branch
from
7675396 to
f0a467c
Compare
|
Rebased on top of |
|
Once the conflicts are resolved (I think they were caused by merging #227), and we check cbindgen we should be good to merge. |
This patch removes `unsafe { … }` blocks inside `unsafe` functions.
This patch also removes some warnings about camel case. All structures
with a C representation are automatically not mangle, and allow non
camel case.
This patch cleans several warnings where mutable bindings are declared but used only as immutable bindings. It's a little bit opinionated patch. I think it's interesting to use `const` pointers as much as possible.
This patch fixes warnings raised by Clippy.
Hywan
force-pushed
the
fix-runtime-c-api-remove-warnings
branch
from
8e17079 to
25feef7
Compare
|
Everything is alright for me 👍. |
|
I think we need to update the C source file, because some types have changed (like |
This patch fixes several warnings: Some generated by
rustc, some other generated by Clippy.I tried to generate 3 “clean” commits:
I hope those changes are OK for you. I will comment the patch a little bit to provide further help.