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

Order of generated closure wrappers+adapters not stable across builds #2302

Closed
cloneable opened this issue Sep 6, 2020 · 5 comments · Fixed by #2304
Closed

Order of generated closure wrappers+adapters not stable across builds #2302

cloneable opened this issue Sep 6, 2020 · 5 comments · Fixed by #2304
Labels

Comments

@cloneable
Copy link

Describe the Bug

When I rebuild my project the order of closure wrappers and their adapters is not stable.

Expected Behavior

Would be great if the functions could be sorted or the order otherwise be kept stable, so the file won't show up as changed.

@cloneable cloneable added the bug label Sep 6, 2020
@cloneable
Copy link
Author

Here's a diff of a fresh build without changes to code.

diff --git a/web/wasmgame.js b/web/wasmgame.js
index 544ca64..4c175f4 100644
--- a/web/wasmgame.js
+++ b/web/wasmgame.js
@@ -199,11 +199,11 @@ function makeMutClosure(arg0, arg1, dtor, f) {
     return real;
 }
 function __wbg_adapter_16(arg0, arg1, arg2) {
-    wasm.wasm_bindgen__convert__closures__invoke1_mut__h39699eace1fc5f4d(arg0, arg1, arg2);
+    wasm.wasm_bindgen__convert__closures__invoke1_mut__h2d4dbb90fa43e44a(arg0, arg1, addHeapObject(arg2));
 }
 
 function __wbg_adapter_19(arg0, arg1, arg2) {
-    wasm.wasm_bindgen__convert__closures__invoke1_mut__h2d4dbb90fa43e44a(arg0, arg1, addHeapObject(arg2));
+    wasm.wasm_bindgen__convert__closures__invoke1_mut__h39699eace1fc5f4d(arg0, arg1, arg2);
 }
 
 function handleError(f) {
@@ -432,11 +432,11 @@ async function init(input) {
     imports.wbg.__wbg_drawImage_bbf7a4f3f839531f = handleError(function(arg0, arg1, arg2, arg3) {
         getObject(arg0).drawImage(getObject(arg1), arg2, arg3);
     });
-    imports.wbg.__wbindgen_closure_wrapper568 = function(arg0, arg1, arg2) {
+    imports.wbg.__wbindgen_closure_wrapper591 = function(arg0, arg1, arg2) {
         var ret = makeMutClosure(arg0, arg1, 22, __wbg_adapter_19);
         return addHeapObject(ret);
     };
-    imports.wbg.__wbindgen_closure_wrapper591 = function(arg0, arg1, arg2) {
+    imports.wbg.__wbindgen_closure_wrapper568 = function(arg0, arg1, arg2) {
         var ret = makeMutClosure(arg0, arg1, 22, __wbg_adapter_16);
         return addHeapObject(ret);
     };
diff --git a/web/wasmgame_bg.wasm b/web/wasmgame_bg.wasm
index 4310be6..ca52492 100644
--- a/web/wasmgame_bg.wasm
+++ b/web/wasmgame_bg.wasm
@@ -65,8 +65,8 @@
   (import "wbg" "__wbg_closePath_932dac3dc951538b" (func (;38;) (type 2)))
   (import "wbg" "__wbg_stroke_b60b281027593a65" (func (;39;) (type 2)))
   (import "wbg" "__wbg_drawImage_bbf7a4f3f839531f" (func (;40;) (type 15)))
-  (import "wbg" "__wbindgen_closure_wrapper568" (func (;41;) (type 1)))
-  (import "wbg" "__wbindgen_closure_wrapper591" (func (;42;) (type 1)))
+  (import "wbg" "__wbindgen_closure_wrapper591" (func (;41;) (type 1)))
+  (import "wbg" "__wbindgen_closure_wrapper568" (func (;42;) (type 1)))
   (func (;43;) (type 4) (param i32) (result i32)
     (local i32 i32 i32 i32 i32 i32 i32 i32 i64)
     block  ;; label = @1
@@ -3664,7 +3664,7 @@
                                                             local.get 3
                                                             i32.const 1048688
                                                             i32.const 27
-                                                            call 42
+                                                            call 41
                                                             local.set 1
                                                             local.get 2
                                                             i32.load offset=8
@@ -10787,7 +10787,7 @@
               local.get 2
               i32.const 1053164
               i32.const 20
-              call 41
+              call 42
               local.set 4
               i32.const 1055488
               i64.load align=4

@cloneable cloneable changed the title Generated closure wrappers+adapters not stable across builds Order of generated closure wrappers+adapters not stable across builds Sep 6, 2020
@alexcrichton
Copy link
Contributor

Thanks for the report! This is definitely a bug in wasm-bindgen. Would you be able to share code that reproduces this issue? (minimized from the original source is fine too)

@cloneable
Copy link
Author

cloneable commented Sep 8, 2020

I noticed this while working on my toy wasmgame. I tagged the latest commit with "wasm-bindgen-issue-2302":
https://github.com/cloneable/wasmgame/tree/wasm-bindgen-issue-2302

Here's the code with the two closures:
https://github.com/cloneable/wasmgame/blob/wasm-bindgen-issue-2302/src/game.rs#L32-L64

Please don't judge my Rust code too harshly. I'm just a noob :)

I'll try to minimize this tomorrow.

alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue Sep 8, 2020
This commit fixes an issue of nondeterministic JS generation when
closures were used, updating a few iterations of hash maps to iterate in
a sorted manner rather than via the raw internal order.

Closes rustwasm#2302
@alexcrichton
Copy link
Contributor

Thanks! No worries about minimizing, that was enough for me to get a fix

@cloneable
Copy link
Author

Thanks a lot, Alex!

alexcrichton added a commit that referenced this issue Sep 9, 2020
* Fix two cases of non-deterministic iteration

This commit fixes an issue of nondeterministic JS generation when
closures were used, updating a few iterations of hash maps to iterate in
a sorted manner rather than via the raw internal order.

Closes #2302

* Reformat web-sys with latest proc-macro
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants