Skip to content

Conversation

@mathetake
Copy link
Contributor

@mathetake mathetake commented Sep 30, 2020

  • add wasm-abi field in TargetSpec (defaults to the empty string)
  • change default value of -wasm-abi flag from js to the empty string
  • override wasm-abi value by -wasm-abi when the flag is set to non-empty string
  • set wasm-abi field values for wasm.json and wasi.json
    • js for wasm.json
    • generic for wasi.json

@mathetake mathetake changed the title use generic wasm abi for WASI by default add wasm_abi field in TargetSpec && set generic for WASI by default Oct 1, 2020
@mathetake mathetake marked this pull request as ready for review October 1, 2020 07:09
Signed-off-by: mathetake <takeshi@tetrate.io>
Signed-off-by: mathetake <takeshi@tetrate.io>
@mathetake mathetake changed the title add wasm_abi field in TargetSpec && set generic for WASI by default add wasm-abi field in TargetSpec && set generic for WASI by default Oct 1, 2020
builder/build.go Outdated
// Use -wasm-abi=generic to disable this behaviour.
if config.Options.WasmAbi == "js" && strings.HasPrefix(config.Triple(), "wasm") {
if config.Options.WasmAbi != "" {
config.Target.WasmAbi = config.Options.WasmAbi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not modify the config.Target field, it should contain exactly what is extracted from the JSON files.

Instead, you can either:

  • Add a getter to the config variable (*compileopts.Config), so you can simply call config.WasmAbi() to get the value.
  • Use a local variable instead.

The getter would be slightly cleaner, as it matches the pattern of other configurations (CGO_ENABLED, GC, NeedsStackObjects, etc).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added the getter in d61d49c

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mathetake thanks for the addition.

@aykevl anything else needed for us to merge this now?

Signed-off-by: mathetake <takeshi@tetrate.io>
@aykevl aykevl merged commit 9a015f4 into tinygo-org:dev Oct 3, 2020
@aykevl
Copy link
Member

aykevl commented Oct 3, 2020

No that's all. Thank you!

@mathetake mathetake deleted the wasi-wasmabi-default branch October 3, 2020 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants