Added constructor return type for wasm32 target #1877
Conversation
src/ir/function.rs
Outdated
| @@ -495,7 +495,15 @@ impl FunctionSig { | |||
| } else { | |||
| ty.ret_type().ok_or(ParseError::Continue)? | |||
| }; | |||
| let ret = Item::from_ty_or_ref(ty_ret_type, cursor, None, ctx); | |||
|
|
|||
| let ret = if ctx.is_target_wasm32() && is_constructor { | |||
There was a problem hiding this comment.
Can we switch the order here, is_constructor is cheaper to check, and should be false most of the time.
Also, can we add a test for this? Should be doable passing --target in the bindgen-args like tests/headers/win32-thiscall_1_0.hpp and others do.
There was a problem hiding this comment.
Added a commit which addresses both of these comments.
| @@ -577,6 +577,14 @@ If you encounter an error missing from this list, please file an issue or a PR!" | |||
| } | |||
| } | |||
|
|
|||
| /// Returns `true` if the target architecture is wasm32 | |||
| pub fn is_target_wasm32(&self) -> bool { | |||
There was a problem hiding this comment.
This won't work if building with clang 4 or 3.9 (target_info will be null), but maybe that's fine.
There was a problem hiding this comment.
I doubt that wasm backend even existed for those versions of clang and as such I don't think what would be an issue.
There was a problem hiding this comment.
We still need to update the test expectations for these two versions, as we run CI on those. To do that you need to add the file in tests/expectations/tests/libclang-3.9 (and same for 4).
With that bit, this patch looks great, thank you!
PR to fix #1822