-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Unable to export js_name = round unless type is f64 #2338
Comments
Jake, I couldn't get your bug to reproduce with your the repo posted or in a new project? Did at some stage you have Though the compiler does give |
Hi @pauldorehill, I removed my First thing was to check for any
Which looks like just the public functions I've defined and specific imports of
|
This is a bit unfortunate, but the way linkage and symbols work with LLD and LLVM right now you can't duplicate symbols, and symbols are tied to function exports as well. The Unfortunately there's not really much that can be done about this I think. There's a similar issue with functions named If possible, can a different function be used? |
It's easy enough to post-process to change the exported function name /**
* @param {Float32Array} out
* @param {Float32Array} a
* @returns {Float32Array}
*/
export function js_round(out, a) {
try {
var ret = wasm.js_round(addHeapObject(out), addBorrowedObject(a));
return takeObject(ret);
} finally {
heap[stack_pointer++] = undefined;
}
} to /**
* @param {Float32Array} out
* @param {Float32Array} a
* @returns {Float32Array}
*/
export function round(out, a) {
try {
var ret = wasm.js_round(addHeapObject(out), addBorrowedObject(a));
return takeObject(ret);
} finally {
heap[stack_pointer++] = undefined;
}
} I would assume there would be some form of uniqueness to the internal symbols though and the |
Yeah that's another way to fix it! We can theoretically fix this as well by only applying |
@alexcrichton This is what I would like to do. Would you be open to a PR making this change? |
Wouldn't that be a breaking change? |
Yeah, this is why I was asking. What is the |
Could you elaborate?
We try not do any breaking changes until v0.3, except increasing the MSRV. The question is really what falls under breaking change and what doesn't. As far as I know that isn't something that's clearly defined. |
Sure! You can find all the details of that particular case at penrose/penrose#1284, but basically I had a function that looked something like this: use wasm_bindgen::prelude::wasm_bindgen;
#[wasm_bindgen]
pub fn foo(v: &mut [f64]) {
for x in v.iter_mut() {
*x = -*x;
}
} With
Makes sense, thanks for clarifying! |
Interesting, I don't remember in particular what caused this change, but that's the same question as above: is the interface of exported functions like this an implementation detail? If yes, then this wasn't a breaking change, if not, then this was a breaking change. |
That was #3188; the extra parameter is a
I'd argue that yes, they're supposed to be an implementation detail, but that's just my opinion; like you said, I don't think it's clearly defined. |
I looked into it further in the meantime, I feel confident now that this has to be labeled as an implementation detail. Not only did we change it quite often, but almost all of the time these functions are not usable without all the helpers defined in the JS shim. I think if users need a stable Wasm export, they should just use
@samestep I would be fine with that now. |
Describe the Bug
returns a signature mismatch of
Steps to Reproduce
lib.rs
wasm-pack build
circumvented in my repo by naming
js_round
and post-processinghttps://github.com/jramsley/gl-matrix/blob/main/src/vec2.rs#L108
Expected Behavior
Generates an exported javascript function called
round
Actual Behavior
Additional Context
First time writing something real in Rust, I may be missing something obvious.
The text was updated successfully, but these errors were encountered: