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.

Sign up for GitHub

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

Jump to bottom

Minor fixes for Go bindings #2676

Merged
merged 2 commits into from
Oct 30, 2023
Merged

Minor fixes for Go bindings #2676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
071f792
mod name
benozol Oct 27, 2023
97173da
insert fallthrough
benozol Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion language-bindings/go/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module gitlab.alipay-inc.com/TNT_Runtime/ant-runtime/bindings/go
module github.com/bytecodealliance/wasm-micro-runtime/language-bindings/go
Copy link
Contributor Author

Choose a reason for hiding this comment

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

when using the Go bindings in a separate project, the module name must match the project URL, and the test below must use the same URL, too

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also we put header and libraries files under github.com/bytecodealliance/wasm-micro-runtime/language-bindings/go/packaged?

Copy link
Contributor Author

@benozol benozol Oct 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The header files and the host-native library are installed under go/packaged/ by go/build.sh. But there's also the script go/samples/build.sh, which does practically the same but always puts the host-native library into go/packaged/lib/amd64, which is incorrect, e.g., on arm64. What about just removing go/samples/build.sh?

Copy link
Contributor

@wenyongh wenyongh Oct 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

How about not removing go/samples/build.sh, instead, just letting it call the go/build.sh first and then building the sample, for example:

CUR_DIR=$PWD

pushd ${CUR_DIR}/.. > /dev/null 2>&1
./build.sh
popd > /dev/null 2>& 1

cd ${CUR_DIR}
rm -f test
go build test.go
./test

Copy link
Contributor

Choose a reason for hiding this comment

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

See #2681


go 1.15

Expand Down
2 changes: 1 addition & 1 deletion language-bindings/go/samples/test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
package main

import (
"gitlab.alipay-inc.com/TNT_Runtime/ant-runtime/bindings/go/wamr"
"github.com/bytecodealliance/wasm-micro-runtime/language-bindings/go/wamr"
"fmt"
)

Expand Down
2 changes: 2 additions & 0 deletions language-bindings/go/wamr/instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,9 @@ func (self *Instance) CallFuncV(funcName string,
for i = 0; i < result_count; i++ {
switch result_types[i] {
case C.WASM_I32:
fallthrough
case C.WASM_FUNCREF:
fallthrough
Copy link
Contributor Author

Choose a reason for hiding this comment

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

switch cases in Go require an explicit fallthrough, they break by default

case C.WASM_ANYREF:
i32 := (int32)(argv[argc])
results[i] = i32
Expand Down