Save build ID in a source map

[loganek]

This is based on the two proposals:

• https://github.com/WebAssembly/tool-conventions/blob/main/BuildId.md
• https://github.com/tc39/source-map/blob/main/proposals/debug-id.md

[kripken]

@dschuff I think you have been involved in the build ID discussions?

[loganek]

@kripken / @dschuff do you have any thoughts on that?

[kripken]

For a test, perhaps we can do something like

https://github.com/WebAssembly/binaryen/blob/main/test/lit/debug/source-map-smearing.wast

which has an example of printing out the emitted source map?

[kripken]

What is the + achieving here?

[loganek]

that's just a conversion to integer (otherwise it'll print an ASCII representation of this character). However, I've updated it to use an explicit cast to not confuse anybody.

[dschuff]

This makes perfect sense to me. The specific behavior of adding the build ID to the source map whenever it exists in the binary also makes sense, but I did notice that https://github.com/tc39/source-map/blob/main/proposals/debug-id.md#scope says that debug IDs are only specified for source maps with embedded source files, which emscripten doesn't support (although I think it would make sense to add that support, see also emscripten-core/emscripten#22189), and the spec "applies only to non-indexed source maps and currently specifies references only for JavaScript."
I assume that limitation is only in place because it also needs to specify the behavior of the proposed JS APIs (which would in turn require some spec to understand how to get a build ID from a wasm file analogous to how it's speced for JS. I assume it would be harmless to just include the build ID here anyway; but maybe we can do even better. Do you know if there have been any discussions in TC39 here about how this might apply to wasm?

[loganek]

For a test, perhaps we can do something like

main/test/lit/debug/source-map-smearing.wast

which has an example of printing out the emitted source map?

Great, I was looking for an example but couldn't find, so thanks for sharing. However, I've pushed a precompiled wasm file because WAT parser doesn't seem to support custom sections right now (and it feels a bit out of scope for this change) - I hope that's ok.

[kripken]

Code and test lgtm. Remaining to discuss is the point @dschuff brought up.

[loganek]

says that debug IDs are only specified for source maps with embedded source files

That's one condition, and the other one is where sources are categorically not available which I think applies here because (at least for now) WASM doesn't specify how the source code can be included in WASM file itself.

Do you know if there have been any discussions in TC39 here about how this might apply to wasm?

Hi, really sorry for the late reply. I'm not part of TC39 and I'm not aware of any discussions related to WebAssembly other than here WebAssembly/tool-conventions#133. I agree there's opportunity to do better here although I'd keep it separate from this change if possible.

[dschuff]

says that debug IDs are only specified for source maps with embedded source files

That's one condition, and the other one is where sources are categorically not available which I think applies here because (at least for now) WASM doesn't specify how the source code can be included in WASM file itself.

Ah I was thinking about the case where the sources are "available" but somehow stored separately, in which case you might want to use the build id as an identifier to link them all together separately. Or more generally it just seemed confusing why you would ever want to explicitly say that the build id should be unspecified.

Do you know if there have been any discussions in TC39 here about how this might apply to wasm?

Hi, really sorry for the late reply. I'm not part of TC39 and I'm not aware of any discussions related to WebAssembly other than here WebAssembly/tool-conventions#133. I agree there's opportunity to do better here although I'd keep it separate from this change if possible.

Yeah, definitely keeping it separate from this change is fine.