From 399dd4b8bc38043cc95fe991da9c0808c180df44 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Mon, 20 Jun 2022 15:48:37 +0530 Subject: [PATCH 01/20] Go module path updated so that it can be installed. --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 72ad487..ebbff95 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/go-python/gopy +module github.com/rudderlabs/gopy go 1.15 From f6c26ce05481505f09268df90415cdbc18e76f1d Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Mon, 20 Jun 2022 16:02:03 +0530 Subject: [PATCH 02/20] Replacing gopy URL so that it works in forks too. --- go.mod | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go.mod b/go.mod index ebbff95..5243577 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,11 @@ module github.com/rudderlabs/gopy go 1.15 require ( + github.com/go-python/gopy v0.4.2 // indirect github.com/gonuts/commander v0.1.0 github.com/gonuts/flag v0.1.0 github.com/pkg/errors v0.9.1 golang.org/x/tools v0.1.11-0.20220413170336-afc6aad76eb1 ) + +replace github.com/go-python/gopy => ../gopy From 4ea0c077f4028f44941052b2d096b696b1bdf7bf Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Mon, 20 Jun 2022 16:13:36 +0530 Subject: [PATCH 03/20] go package to test is at rudderlabs/gopy --- .github/workflows/ci.yml | 2 +- _examples/cpkg/run.go | 2 +- _examples/funcs/funcs.go | 2 +- _examples/hi/hi.go | 4 ++-- _examples/iface/iface.go | 2 +- _examples/wrapper/pywrapper/wrapper_code.go | 2 +- bind/gen.go | 2 +- cmd_build.go | 4 ++-- cmd_exe.go | 6 +++--- cmd_gen.go | 4 ++-- cmd_pkg.go | 6 +++--- gen.go | 2 +- go.mod | 3 --- main.go | 2 +- main_test.go | 16 ++++++++-------- main_windows.go | 2 +- pkgsetup.go | 2 +- 17 files changed, 30 insertions(+), 33 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b1b0b9e..a639f41 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,7 +8,7 @@ on: env: TAGS: "-tags=ci" - COVERAGE: "-coverpkg=github.com/go-python/gopy/..." + COVERAGE: "-coverpkg=github.com/rudderlabs/gopy/..." # Init() in main_test will make sure all backends are available if # GOPY_TRAVIS_CI is set GOPY_TRAVIS_CI: 1 diff --git a/_examples/cpkg/run.go b/_examples/cpkg/run.go index 85bd647..366095c 100644 --- a/_examples/cpkg/run.go +++ b/_examples/cpkg/run.go @@ -9,7 +9,7 @@ package main import ( "fmt" - "github.com/go-python/gopy/_examples/cpkg" + "github.com/rudderlabs/gopy/_examples/cpkg" ) func main() { diff --git a/_examples/funcs/funcs.go b/_examples/funcs/funcs.go index baac011..0c0fc26 100644 --- a/_examples/funcs/funcs.go +++ b/_examples/funcs/funcs.go @@ -7,7 +7,7 @@ package funcs import ( "fmt" - "github.com/go-python/gopy/_examples/cpkg" + "github.com/rudderlabs/gopy/_examples/cpkg" ) type FunStruct struct { diff --git a/_examples/hi/hi.go b/_examples/hi/hi.go index 0d62e1d..d968928 100644 --- a/_examples/hi/hi.go +++ b/_examples/hi/hi.go @@ -8,8 +8,8 @@ package hi import ( "fmt" - "github.com/go-python/gopy/_examples/cpkg" - "github.com/go-python/gopy/_examples/structs" + "github.com/rudderlabs/gopy/_examples/cpkg" + "github.com/rudderlabs/gopy/_examples/structs" ) const ( diff --git a/_examples/iface/iface.go b/_examples/iface/iface.go index 56d9863..6c8d376 100644 --- a/_examples/iface/iface.go +++ b/_examples/iface/iface.go @@ -6,7 +6,7 @@ package iface import ( - "github.com/go-python/gopy/_examples/cpkg" + "github.com/rudderlabs/gopy/_examples/cpkg" ) // Iface has a single F() method diff --git a/_examples/wrapper/pywrapper/wrapper_code.go b/_examples/wrapper/pywrapper/wrapper_code.go index 1ded41b..2b928ef 100644 --- a/_examples/wrapper/pywrapper/wrapper_code.go +++ b/_examples/wrapper/pywrapper/wrapper_code.go @@ -7,7 +7,7 @@ package pywrapper import ( "fmt" - "github.com/go-python/gopy/_examples/wrapper" + "github.com/rudderlabs/gopy/_examples/wrapper" ) type WrapperStruct struct { diff --git a/bind/gen.go b/bind/gen.go index d8c8393..40a6db8 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -89,7 +89,7 @@ static inline void gopy_err_handle() { */ import "C" import ( - "github.com/go-python/gopy/gopyh" // handler + "github.com/rudderlabs/gopy/gopyh" // handler %[6]s ) diff --git a/cmd_build.go b/cmd_build.go index 43d3c39..e933e47 100644 --- a/cmd_build.go +++ b/cmd_build.go @@ -16,7 +16,7 @@ import ( "github.com/gonuts/commander" "github.com/gonuts/flag" - "github.com/go-python/gopy/bind" + "github.com/rudderlabs/gopy/bind" ) func gopyMakeCmdBuild() *commander.Command { @@ -29,7 +29,7 @@ build generates and compiles (C)Python language bindings for Go package(s). ex: $ gopy build [options] [other-go-package...] - $ gopy build github.com/go-python/gopy/_examples/hi + $ gopy build github.com/rudderlabs/gopy/_examples/hi `, Flag: *flag.NewFlagSet("gopy-build", flag.ExitOnError), } diff --git a/cmd_exe.go b/cmd_exe.go index 22b4366..00030e7 100644 --- a/cmd_exe.go +++ b/cmd_exe.go @@ -11,7 +11,7 @@ import ( "path/filepath" "strings" - "github.com/go-python/gopy/bind" + "github.com/rudderlabs/gopy/bind" "github.com/gonuts/commander" "github.com/gonuts/flag" ) @@ -35,7 +35,7 @@ When including multiple packages, list in order of increasing dependency, and us ex: $ gopy exe [options] [other-go-package...] - $ gopy exe github.com/go-python/gopy/_examples/hi + $ gopy exe github.com/rudderlabs/gopy/_examples/hi `, Flag: *flag.NewFlagSet("gopy-exe", flag.ExitOnError), } @@ -55,7 +55,7 @@ ex: cmd.Flag.String("author", "gopy", "author name") cmd.Flag.String("email", "gopy@example.com", "author email") cmd.Flag.String("desc", "", "short description of project (long comes from README.md)") - cmd.Flag.String("url", "https://github.com/go-python/gopy", "home page for project") + cmd.Flag.String("url", "https://github.com/rudderlabs/gopy", "home page for project") cmd.Flag.Bool("no-warn", false, "suppress warning messages, which may be expected") cmd.Flag.Bool("no-make", false, "do not generate a Makefile, e.g., when called from Makefile") diff --git a/cmd_gen.go b/cmd_gen.go index 97a6238..ee0e72a 100644 --- a/cmd_gen.go +++ b/cmd_gen.go @@ -8,7 +8,7 @@ import ( "fmt" "log" - "github.com/go-python/gopy/bind" + "github.com/rudderlabs/gopy/bind" "github.com/gonuts/commander" "github.com/gonuts/flag" ) @@ -23,7 +23,7 @@ gen generates (C)Python language bindings for Go package(s). ex: $ gopy gen [options] [other-go-package...] - $ gopy gen github.com/go-python/gopy/_examples/hi + $ gopy gen github.com/rudderlabs/gopy/_examples/hi `, Flag: *flag.NewFlagSet("gopy-gen", flag.ExitOnError), } diff --git a/cmd_pkg.go b/cmd_pkg.go index ce0f437..a4ca1d6 100644 --- a/cmd_pkg.go +++ b/cmd_pkg.go @@ -11,7 +11,7 @@ import ( "path/filepath" "strings" - "github.com/go-python/gopy/bind" + "github.com/rudderlabs/gopy/bind" "github.com/gonuts/commander" "github.com/gonuts/flag" ) @@ -33,7 +33,7 @@ When including multiple packages, list in order of increasing dependency, and us ex: $ gopy pkg [options] [other-go-package...] - $ gopy pkg github.com/go-python/gopy/_examples/hi + $ gopy pkg github.com/rudderlabs/gopy/_examples/hi `, Flag: *flag.NewFlagSet("gopy-pkg", flag.ExitOnError), } @@ -52,7 +52,7 @@ ex: cmd.Flag.String("author", "gopy", "author name") cmd.Flag.String("email", "gopy@example.com", "author email") cmd.Flag.String("desc", "", "short description of project (long comes from README.md)") - cmd.Flag.String("url", "https://github.com/go-python/gopy", "home page for project") + cmd.Flag.String("url", "https://github.com/rudderlabs/gopy", "home page for project") cmd.Flag.Bool("no-warn", false, "suppress warning messages, which may be expected") cmd.Flag.Bool("no-make", false, "do not generate a Makefile, e.g., when called from Makefile") diff --git a/gen.go b/gen.go index 78e6235..88cd9ca 100644 --- a/gen.go +++ b/gen.go @@ -19,7 +19,7 @@ import ( "github.com/pkg/errors" "golang.org/x/tools/go/packages" - "github.com/go-python/gopy/bind" + "github.com/rudderlabs/gopy/bind" ) // argStr returns the full command args as a string, without path to exe diff --git a/go.mod b/go.mod index 5243577..ebbff95 100644 --- a/go.mod +++ b/go.mod @@ -3,11 +3,8 @@ module github.com/rudderlabs/gopy go 1.15 require ( - github.com/go-python/gopy v0.4.2 // indirect github.com/gonuts/commander v0.1.0 github.com/gonuts/flag v0.1.0 github.com/pkg/errors v0.9.1 golang.org/x/tools v0.1.11-0.20220413170336-afc6aad76eb1 ) - -replace github.com/go-python/gopy => ../gopy diff --git a/main.go b/main.go index a8d7dfd..d7a132a 100644 --- a/main.go +++ b/main.go @@ -15,7 +15,7 @@ import ( "github.com/gonuts/flag" "github.com/pkg/errors" - "github.com/go-python/gopy/bind" + "github.com/rudderlabs/gopy/bind" ) // BuildCfg contains command options and binding generation options diff --git a/main_test.go b/main_test.go index 12a0193..6dbdf35 100644 --- a/main_test.go +++ b/main_test.go @@ -17,7 +17,7 @@ import ( "strings" "testing" - "github.com/go-python/gopy/bind" + "github.com/rudderlabs/gopy/bind" ) var ( @@ -116,11 +116,11 @@ func TestGoPyErrors(t *testing.T) { if err != nil { t.Fatalf("could not run %v: %+v\n", strings.Join(cmd.Args, " "), err) } - contains := `--- Processing package: github.com/go-python/gopy/_examples/gopyerrors --- -ignoring python incompatible function: .func github.com/go-python/gopy/_examples/gopyerrors.NotErrorMany() (int, int): func() (int, int): gopy: second result value must be of type error: func() (int, int) -ignoring python incompatible method: gopyerrors.func (*github.com/go-python/gopy/_examples/gopyerrors.Struct).NotErrorMany() (int, string): func() (int, string): gopy: second result value must be of type error: func() (int, string) -ignoring python incompatible method: gopyerrors.func (*github.com/go-python/gopy/_examples/gopyerrors.Struct).TooMany() (int, int, string): func() (int, int, string): gopy: too many results to return: func() (int, int, string) -ignoring python incompatible function: .func github.com/go-python/gopy/_examples/gopyerrors.TooMany() (int, int, string): func() (int, int, string): gopy: too many results to return: func() (int, int, string) + contains := `--- Processing package: github.com/rudderlabs/gopy/_examples/gopyerrors --- +ignoring python incompatible function: .func github.com/rudderlabs/gopy/_examples/gopyerrors.NotErrorMany() (int, int): func() (int, int): gopy: second result value must be of type error: func() (int, int) +ignoring python incompatible method: gopyerrors.func (*github.com/rudderlabs/gopy/_examples/gopyerrors.Struct).NotErrorMany() (int, string): func() (int, string): gopy: second result value must be of type error: func() (int, string) +ignoring python incompatible method: gopyerrors.func (*github.com/rudderlabs/gopy/_examples/gopyerrors.Struct).TooMany() (int, int, string): func() (int, int, string): gopy: too many results to return: func() (int, int, string) +ignoring python incompatible function: .func github.com/rudderlabs/gopy/_examples/gopyerrors.TooMany() (int, int, string): func() (int, int, string): gopy: too many results to return: func() (int, int, string) ` if got, want := string(out), contains; !strings.Contains(got, want) { t.Fatalf("%v does not contain\n%v\n", got, want) @@ -954,8 +954,8 @@ func writeGoMod(t *testing.T, pkgDir, tstDir string) { template := ` module dummy -require github.com/go-python/gopy v0.0.0 -replace github.com/go-python/gopy => %s +require github.com/rudderlabs/gopy v0.0.0 +replace github.com/rudderlabs/gopy => %s ` contents := fmt.Sprintf(template, pkgDir) if err := ioutil.WriteFile(filepath.Join(tstDir, "go.mod"), []byte(contents), 0666); err != nil { diff --git a/main_windows.go b/main_windows.go index 9800b09..4eaf422 100644 --- a/main_windows.go +++ b/main_windows.go @@ -7,7 +7,7 @@ package main -import "github.com/go-python/gopy/bind" +import "github.com/rudderlabs/gopy/bind" const ( libExt = ".pyd" diff --git a/pkgsetup.go b/pkgsetup.go index dbc4ad1..54b7418 100644 --- a/pkgsetup.go +++ b/pkgsetup.go @@ -9,7 +9,7 @@ import ( "os" "path/filepath" - "github.com/go-python/gopy/bind" + "github.com/rudderlabs/gopy/bind" ) // 1 = pkg name, 2 = -user, 3 = version 4 = author, 5 = email, 6 = desc, 7 = url From 3e1931491427118c2e59d2d2bfafa3bcf4996ee8 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Mon, 13 Jun 2022 02:22:01 +0530 Subject: [PATCH 04/20] Added test code for multireturn. --- _examples/multireturn/multireturn.go | 91 ++++++++++++++++++++++++++++ _examples/multireturn/test.py | 52 ++++++++++++++++ main_test.go | 26 ++++++++ 3 files changed, 169 insertions(+) create mode 100644 _examples/multireturn/multireturn.go create mode 100644 _examples/multireturn/test.py diff --git a/_examples/multireturn/multireturn.go b/_examples/multireturn/multireturn.go new file mode 100644 index 0000000..72788c6 --- /dev/null +++ b/_examples/multireturn/multireturn.go @@ -0,0 +1,91 @@ +package multireturn + +import ( + "fmt" +) + +/////////////// No Return ////////////// +func NoReturnFunc() { +} + +/////////////// Single WithoutError Return ////////////// +func SingleWithoutErrorFunc() int { + return 100 +} + +/////////////// Single WithError Return ////////////// +func SingleWithErrorFunc(throwError bool) error { + if throwError { + return fmt.Errorf("Error") + } else { + return nil + } +} + +/////////////// Double WithoutError Return ////////////// +func DoubleWithoutErrorFunc() (int, int) { + return 200, 300 +} + +/////////////// Double WithError Return ////////////// +func DoubleWithErrorFunc(throwError bool) (string, error) { + if throwError { + return "400", fmt.Errorf("Error") + } else { + return "500", nil + } +} + +/////////////// Triple Returns Without Error ////////////// +func TripleWithoutErrorFunc(vargs ...int) (int, string, int) { + return 600, "700", 800 +} + +/////////////// Triple Returns With Error ////////////// +func TripleWithErrorFunc(throwError bool) (int, int, error) { + if throwError { + return 900, 1000, fmt.Errorf("Error") + } else { + return 1100, 1200, nil + } +} + +/////////////// Triple Struct Returns With Error ////////////// +type IntStrUct struct { + P int +} + +func NewIntStrUct(n int) IntStrUct { + return IntStrUct{ + P: n, + } +} + +func TripleWithStructWithErrorFunc(throwError bool) (*IntStrUct, IntStrUct, error) { + s1300 := IntStrUct{P: 1300} + s1400 := IntStrUct{P: 1400} + s1500 := IntStrUct{P: 1500} + s1600 := IntStrUct{P: 1600} + if throwError { + return &s1300, s1400, fmt.Errorf("Error") + } else { + return &s1500, s1600, nil + } +} + +/////////////// Triple Interface Returns Without Error ////////////// +type IntInterFace interface { + Number() int +} + +func (is *IntStrUct) Number() int { + return is.P +} + +func TripleWithInterfaceWithoutErrorFunc() (IntInterFace, IntStrUct, *IntStrUct) { + i1700 := IntStrUct{P: 1700} + s1800 := IntStrUct{P: 1800} + s1900 := IntStrUct{P: 1900} + + return &i1700, s1800, &s1900 +} diff --git a/_examples/multireturn/test.py b/_examples/multireturn/test.py new file mode 100644 index 0000000..0182473 --- /dev/null +++ b/_examples/multireturn/test.py @@ -0,0 +1,52 @@ +# Copyright 2018 The go-python Authors. All rights reserved. +# Use of this source code is governed by a BSD-style +# license that can be found in the LICENSE file. +import multireturn, go + +############### No Return ############## +noResult = multireturn.NoReturnFunc() +print("No Return %r" % noResult) + +############### Single WithoutError Return ############## +oneResult = multireturn.SingleWithoutErrorFunc() +print("Single WithoutError Return %r" % oneResult) + +############### Single WithError Return ############## +errorFalseResult = multireturn.SingleWithErrorFunc(False) +print("Single WithError(False) Return %r" % errorFalseResult) + +errorTrueResult = multireturn.SingleWithErrorFunc(True) +print("Single WithError(True) Return %r" % errorTrueResult) + +############### Double WithoutError Return ############## +twoResults = multireturn.DoubleWithoutErrorFunc() +print("Double WithoutError Return %r" % twoResults) + +############### Double WithError Return ############## +(value400, errorTrueResult) = multireturn.DoubleeWithErrorFunc(True) +print("Double WithError(True) Return (%r, %r)" % (value400, errorTrueResult)) + +(value500, errorFalseResult) = multireturn.DoubleWithErrorFunc(False) +print("Double WithError(False) Return (%r, %r)" % (value500, errorFalseResult)) + +############### Triple Without Error Return ############## +threeResults = multireturn.TripleWithoutErrorFunc() +print("Triple WithoutError Return %r" % threeResult) + +############### Triple With Error Return ############## +(value900, value1000, errorTrueResult) = multireturn.TripleWithErrorFunc(True) +print("Triple WithError(True) Return (%r, %r, %r)" % (value900, value1000, errorFalseResult)) + +(value1100, value1200, errorFalseResult) = multireturn.TripleWithErrorFunc(False) +print("Triple WithError(False) Return (%r, %r, %r)" % (value1100, value1200, errorFalseResult)) + +############### Triple Struct Return With Error ############## +(ptr1300, struct1400, errorTrueResult) = multireturn.TripleWithStructWithErrorFunc(True) +print("Triple WithError(True) Return (%r, %r, %r)" % (ptr1300.P, struct1400.P, errorFalseResult)) + +(value1500, value1600, errorFalseResult) = multireturn.TripleWithStructWithErrorFunc(False) +print("Triple WithError(False) Return (%r, %r, %r)" % (value1500.P, value1600.P, errorFalseResult)) + +############### Triple Interface Return Without Error ############## +(interface1700, struct1800, ptr1900) = multireturn.TripleWithInterfaceWithoutErrorFunc() +print("Triple WithError(True) Return (%r, %r, %r)" % (interface1700.P, struct1800.P, ptr1900)) diff --git a/main_test.go b/main_test.go index 6dbdf35..18ebdb7 100644 --- a/main_test.go +++ b/main_test.go @@ -49,6 +49,7 @@ var ( "_examples/cstrings": []string{"py2", "py3"}, "_examples/pkgconflict": []string{"py2", "py3"}, "_examples/variadic": []string{"py3"}, + "_examples/multireturn": []string{"py3"}, } testEnvironment = os.Environ() @@ -829,6 +830,31 @@ Type OK }) } +func TestBindMultiReturn(t *testing.T) { + // t.Parallel() + path := "_examples/multireturn" + testPkg(t, pkg{ + path: path, + lang: features[path], + cmd: "build", + extras: nil, + want: []byte(`No Return None +Single WithoutError Return 100 +Single WithError(False) Return nil +Single WithError(True) Return 'Error' +Double WithoutError Return (200,300) +Double WithError(True) Return (400, Error) +Double WithError(False) Return (500, nil) +Triple WithoutError Return (600, 700, 800) +Triple WithError(True) Return (900, 1000, Error) +Triple WithError(False) Return (1100, 1200, nil) +Triple WithError(True) Return (1300, 1400, Error) +Triple WithError(False) Return (1500, 1600, nil) +Triple WithError(True) Return (1700, 1800, 1900) +`), + }) +} + // Generate / verify SUPPORT_MATRIX.md from features map. func TestCheckSupportMatrix(t *testing.T) { var buf bytes.Buffer From d0d0d65445bdde3ee400e4c9b2ac45b1f8abf7a2 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Sun, 12 Jun 2022 22:08:16 +0530 Subject: [PATCH 05/20] First stage changes made. Next step is to change the loop to reflect original design. --- bind/bind.go | 16 ++ bind/gen.go | 18 ++- bind/gen_func.go | 283 ++++++++++++++++++--------------- bind/package.go | 8 +- bind/symbols.go | 72 ++++----- bind/types.go | 21 ++- cmd_build.go | 23 +-- cmd_exe.go | 24 +-- cmd_gen.go | 25 +-- cmd_pkg.go | 22 +-- goerr2pyex/error_translator.go | 18 +++ goerr2pyex/test.sh | 9 ++ main.go | 42 ++++- 13 files changed, 327 insertions(+), 254 deletions(-) create mode 100644 goerr2pyex/error_translator.go create mode 100644 goerr2pyex/test.sh diff --git a/bind/bind.go b/bind/bind.go index a3b7a85..fd09072 100644 --- a/bind/bind.go +++ b/bind/bind.go @@ -16,18 +16,34 @@ import ( type BindCfg struct { // output directory for bindings OutputDir string + // name of output package (otherwise name of first package is used) Name string + // code string to run in the go main() function in the cgo library Main string + // the full command args as a string, without path to exe Cmd string + // path to python interpreter VM string + // package prefix used when generating python import statements PkgPrefix string + // rename Go exported symbols to python PEP snake_case RenameCase bool + + // If set, python exceptions are not thrown. + NoPyExceptions bool + + // Path to Go module, which is to be used to translate Go errors to Python exceptions. + ModPathGoErr2PyEx string + + // If set, when a Go function returns a (value, err), python returns (value, ) tuple. + // By default, we return just value. + UsePyTuple4VE bool } // ErrorList is a list of errors diff --git a/bind/gen.go b/bind/gen.go index 40a6db8..e8f12cf 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -38,7 +38,8 @@ var WindowsOS = false // for all preambles: 1 = name of package (outname), 2 = cmdstr -// 3 = libcfg, 4 = GoHandle, 5 = CGoHandle, 6 = all imports, 7 = mainstr, 8 = exe pre C, 9 = exe pre go +// 3 = libcfg, 4 = GoHandle, 5 = CGoHandle, 6 = all imports, 7 = goerr2pyex Package path, 8 = mainstr, +// 9 = exe pre C, 10 = exe pre go const ( goPreamble = `/* cgo stubs for package %[1]s. @@ -85,17 +86,19 @@ static inline void gopy_err_handle() { PyErr_Print(); } } -%[8]s +%[9]s */ import "C" import ( "github.com/rudderlabs/gopy/gopyh" // handler + "%[7]s" // Error Translator + %[6]s ) // main doesn't do anything in lib / pkg mode, but is essential for exe mode func main() { - %[7]s + %[8]s } // initialization functions -- can be called from python after library is loaded @@ -104,7 +107,7 @@ func main() { //export GoPyInit func GoPyInit() { - %[7]s + %[8]s } // type for the handle -- int64 for speed (can switch to string) @@ -164,7 +167,7 @@ func complex128PyToGo(o *C.PyObject) complex128 { return complex(float64(v.real), float64(v.imag)) } -%[9]s +%[10]s ` goExePreambleC = ` @@ -430,6 +433,9 @@ var NoWarn = false // NoMake turns off generation of Makefiles var NoMake = false +// NoPyExceptions turns off generation of Python Exceptions +var NoPyExceptions = false + // GenPyBind generates a .go file, build.py file to enable pybindgen to create python bindings, // and wrapper .py file(s) that are loaded as the interface to the package with shadow // python-side classes @@ -603,7 +609,7 @@ func (g *pyGen) genGoPreamble() { exeprego = goExePreambleGo } g.gofile.Printf(goPreamble, g.cfg.Name, g.cfg.Cmd, libcfg, GoHandle, CGoHandle, - pkgimport, g.cfg.Main, exeprec, exeprego) + pkgimport, g.cfg.ModPathGoErr2PyEx, g.cfg.Main, exeprec, exeprego) g.gofile.Printf("\n// --- generated code for package: %[1]s below: ---\n\n", g.cfg.Name) } diff --git a/bind/gen_func.go b/bind/gen_func.go index 991165f..02b786f 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -6,6 +6,8 @@ package bind import ( "fmt" + "log" + "strconv" "go/types" "strings" ) @@ -58,13 +60,9 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { res := sig.Results() nargs := 0 nres := len(res) - - // note: this is enforced in creation of Func, in newFuncFrom - if nres > 2 { - return false - } - if nres == 2 && !fsym.err { - return false + npyres := nres + if fsym.haserr { + npyres -= 1 } var ( @@ -122,14 +120,15 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { // But given specific return types, we may want to add more // behavior to the wrapped function code gen. addFuncName := "add_checked_function" - if len(res) > 0 { - ret := res[0] - switch t := ret.GoType().(type) { - case *types.Basic: - // string return types need special memory leak patches - // to free the allocated char* - if t.Kind() == types.String { - addFuncName = "add_checked_string_function" + if npyres > 0 { + for i := 0; i < npyres; i++ { + switch t := res[i].GoType().(type) { + case *types.Basic: + // string return types need special memory leak patches + // to free the allocated char* + if t.Kind() == types.String { + addFuncName = "add_checked_string_function" + } } } } @@ -154,8 +153,9 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { } goRet := "" - nres = len(res) - if nres > 0 { + if npyres == 0 { + g.pybuild.Printf("None") + } else if npyres == 1 { ret := res[0] sret := current.symtype(ret.GoType()) if sret == nil { @@ -172,7 +172,26 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { } goRet = fmt.Sprintf("%s", sret.cgoname) } else { - g.pybuild.Printf("None") + // On Python side, we are returning PyTuple. + g.pybuild.Printf("retval('PyObject*', caller_owns_return=True)") + + // On Go side, we are returning multiple values. + for i := 0; i < npyres; i++ { + sret := current.symtype(res[i].GoType()) + if sret == nil { + panic(fmt.Errorf( + "gopy: could not find symbol for %q", + res[i].Name(), + )) + } + goRet += sret.cgoname + if i != npyres-1 { + goRet += ", " + } + } + if npyres > 1 { + goRet = "(" + goRet + ")" + } } if len(goArgs) > 0 { @@ -237,12 +256,13 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { res := sig.Results() args := sig.Params() nres := len(res) - - rvIsErr := false // set to true if the main return is an error - if nres == 1 { - ret := res[0] - if isErrorType(ret.GoType()) { - rvIsErr = true + npyres := nres + rvHasErr := false // set to true if the main return is an error + if fsym.haserr { + if NoPyExceptions { + rvHasErr = true + } else { + npyres -= 1 } } @@ -260,28 +280,33 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { } } } + if isMethod { g.gofile.Printf( `vifc, __err := gopyh.VarFromHandleTry((gopyh.CGoHandle)(_handle), "%s") if __err != nil { `, symNm) g.gofile.Indent() - if nres > 0 { - ret := res[0] - if ret.sym.zval == "" { - fmt.Printf("gopy: programmer error: empty zval zero value in symbol: %v\n", ret.sym) - } - if ret.sym.go2py != "" { - g.gofile.Printf("return %s(%s)%s\n", ret.sym.go2py, ret.sym.zval, ret.sym.go2pyParenEx) - } else { - g.gofile.Printf("return %s\n", ret.sym.zval) + if npyres > 0 { + retvals := make([]string, npyres, npyres) + for i := 0; i < npyres; i++ { + ret := res[i] + if ret.sym.zval == "" { + fmt.Printf("gopy: programmer error: empty zval zero value in symbol: %v\n", ret.sym) + } + if ret.sym.go2py != "" { + retvals[i] = ret.sym.go2py + "(" + ret.sym.zval + ")" + ret.sym.go2pyParenEx + } else { + retvals[i] = ret.sym.zval + } } + g.gofile.Printf("return %s\n", strings.Join(retvals, ", ")) } else { g.gofile.Printf("return\n") } g.gofile.Outdent() g.gofile.Printf("}\n") - } else if rvIsErr { + } else if rvHasErr { g.gofile.Printf("var __err error\n") } @@ -335,120 +360,128 @@ if __err != nil { if isMethod { mnm = sym.id + "_" + fsym.GoName() } - rvHasHandle := false - if nres > 0 { - ret := res[0] - if !rvIsErr && ret.sym.hasHandle() { - rvHasHandle = true - cvnm := ret.sym.pyPkgId(g.pkg.pkg) - g.pywrap.Printf("return %s(handle=_%s.%s(", cvnm, pkgname, mnm) - } else { - g.pywrap.Printf("return _%s.%s(", pkgname, mnm) - } - } else { - g.pywrap.Printf("_%s.%s(", pkgname, mnm) - } - hasRetCvt := false - hasAddrOfTmp := false - if nres > 0 { - ret := res[0] - switch { - case rvIsErr: - g.gofile.Printf("__err = ") - case nres == 2: - g.gofile.Printf("cret, __err := ") - case ret.sym.hasHandle() && !ret.sym.isPtrOrIface(): - hasAddrOfTmp = true - g.gofile.Printf("cret := ") - case ret.sym.go2py != "": - hasRetCvt = true - g.gofile.Printf("return %s(", ret.sym.go2py) - default: - g.gofile.Printf("return ") - } - } + log.Println(mnm) + if nres == 0 { wrapArgs = append(wrapArgs, "goRun") } - g.pywrap.Printf("%s)", strings.Join(wrapArgs, ", ")) - if rvHasHandle { - g.pywrap.Printf(")") + + pyFuncCall := fmt.Sprintf("_%s.%s(%s)", pkgname, mnm, strings.Join(wrapArgs, ", ")) + if nres > 0 { + retvars := make([]string, nres, nres) + for i := 0; i < npyres; i++ { + retvars[i] = "ret" + strconv.Itoa(i) + if res[i].sym.hasHandle() { + retvars[i] = "_" + retvars[i] + } + } + if fsym.haserr { + retvars[nres-1] = "__err" + } + + // Call upstream method and collect returns. + g.pywrap.Printf(fmt.Sprintf("%s := %s", strings.Join(retvars, ", "), pyFuncCall)) + + // ReMap handle returns from pyFuncCall. + for i := 0; i < npyres; i++ { + if res[i].sym.hasHandle() { + cvnm := res[i].sym.pyPkgId(g.pkg.pkg) + g.pywrap.Printf("ret%d = %s(handle=_ret%d)", cvnm, i, i) + retvars[i] = "ret" + strconv.Itoa(i) + } + } + + g.pywrap.Printf("return %s", strings.Join(retvars, ", ")) + } else { + g.pywrap.Printf(pyFuncCall) } - funCall := "" + g.pywrap.Printf("\n") + g.pywrap.Outdent() + + goFuncCall := "" if isMethod { if sym.isStruct() { - funCall = fmt.Sprintf("gopyh.Embed(vifc, reflect.TypeOf(%s{})).(%s).%s(%s)", nonPtrName(symNm), symNm, fsym.GoName(), strings.Join(callArgs, ", ")) + goFuncCall = fmt.Sprintf("gopyh.Embed(vifc, reflect.TypeOf(%s{})).(%s).%s(%s)", + nonPtrName(symNm), + symNm, + fsym.GoName(), + strings.Join(callArgs, ", ")) } else { - funCall = fmt.Sprintf("vifc.(%s).%s(%s)", symNm, fsym.GoName(), strings.Join(callArgs, ", ")) + goFuncCall = fmt.Sprintf("vifc.(%s).%s(%s)", + symNm, + fsym.GoName(), + strings.Join(callArgs, ", ")) } } else { - funCall = fmt.Sprintf("%s(%s)", fsym.GoFmt(), strings.Join(callArgs, ", ")) - } - if hasRetCvt { - ret := res[0] - funCall += fmt.Sprintf(")%s", ret.sym.go2pyParenEx) + goFuncCall = fmt.Sprintf("%s(%s)", + fsym.GoFmt(), + strings.Join(callArgs, ", ")) } - if nres == 0 { + if nres > 0 { + retvals := make([]string, nres, nres) + for i := 0; i < npyres; i++ { + retvals[i] = "cret" + strconv.Itoa(i) + } + if fsym.haserr { + retvals[nres-1] = "__err" + } + + // Call upstream method and collect returns. + g.gofile.Printf(fmt.Sprintf("%s := %s\n", strings.Join(retvals, ", "), goFuncCall)) + + // ReMap handle returns from pyFuncCall. + for i := 0; i < npyres; i++ { + if res[i].sym.hasHandle() && !res[i].sym.isPtrOrIface(){ + retvals[i] = "&" + retvals[i] + } + if res[i].sym.go2py != "" { + retvals[i] = fmt.Sprintf("%s(%s)%s", res[i].sym.go2py, retvals[i], res[i].sym.go2pyParenEx) + } + } + + if fsym.haserr { + g.gofile.Printf("\n") + + if rvHasErr { + retvals[npyres-1] = "estr" // NOTE: leaked string + g.gofile.Printf("var estr C.CString\n") + g.gofile.Printf("if __err != nil {\n") + g.gofile.Indent() + g.gofile.Printf("estr = C.CString(__err.Error())// NOTE: leaked string\n") // NOTE: leaked string + g.gofile.Outdent() + g.gofile.Printf("} else {\n") + g.gofile.Indent() + g.gofile.Printf("estr = C.CString(\"\")// NOTE: leaked string\n") // NOTE: leaked string + g.gofile.Outdent() + g.gofile.Printf("}\n") + } else { + g.gofile.Printf("if __err != nil {\n") + g.gofile.Indent() + g.gofile.Printf("estr := C.CString(__err.Error())// NOTE: leaked string\n") // NOTE: leaked string + g.gofile.Printf("C.PyErr_SetString(C.PyExc_RuntimeError, estr)\n") + g.gofile.Printf("C.free(unsafe.Pointer(estr))\n") // python should have converted, safe + g.gofile.Outdent() + g.gofile.Printf("}\n") + } + } + + g.gofile.Printf("return %s", strings.Join(retvals[0:npyres], ", ")) + } else { g.gofile.Printf("if boolPyToGo(goRun) {\n") g.gofile.Indent() - g.gofile.Printf("go %s\n", funCall) + g.gofile.Printf("go %s\n", goFuncCall) g.gofile.Outdent() g.gofile.Printf("} else {\n") g.gofile.Indent() - g.gofile.Printf("%s\n", funCall) + g.gofile.Printf("%s\n", goFuncCall) g.gofile.Outdent() g.gofile.Printf("}") - } else { - g.gofile.Printf("%s\n", funCall) } - if rvIsErr || nres == 2 { - g.gofile.Printf("\n") - g.gofile.Printf("if __err != nil {\n") - g.gofile.Indent() - g.gofile.Printf("estr := C.CString(__err.Error())\n") - g.gofile.Printf("C.PyErr_SetString(C.PyExc_RuntimeError, estr)\n") - if rvIsErr { - g.gofile.Printf("return estr\n") // NOTE: leaked string - } else { - g.gofile.Printf("C.free(unsafe.Pointer(estr))\n") // python should have converted, safe - ret := res[0] - if ret.sym.zval == "" { - fmt.Printf("gopy: programmer error: empty zval zero value in symbol: %v\n", ret.sym) - } - if ret.sym.go2py != "" { - g.gofile.Printf("return %s(%s)%s\n", ret.sym.go2py, ret.sym.zval, ret.sym.go2pyParenEx) - } else { - g.gofile.Printf("return %s\n", ret.sym.zval) - } - } - g.gofile.Outdent() - g.gofile.Printf("}\n") - if rvIsErr { - g.gofile.Printf("return C.CString(\"\")") // NOTE: leaked string - } else { - ret := res[0] - if ret.sym.go2py != "" { - if ret.sym.hasHandle() && !ret.sym.isPtrOrIface() { - g.gofile.Printf("return %s(&cret)%s", ret.sym.go2py, ret.sym.go2pyParenEx) - } else { - g.gofile.Printf("return %s(cret)%s", ret.sym.go2py, ret.sym.go2pyParenEx) - } - } else { - g.gofile.Printf("return cret") - } - } - } else if hasAddrOfTmp { - ret := res[0] - g.gofile.Printf("\nreturn %s(&cret)%s", ret.sym.go2py, ret.sym.go2pyParenEx) - } g.gofile.Printf("\n") g.gofile.Outdent() g.gofile.Printf("}\n") - - g.pywrap.Printf("\n") - g.pywrap.Outdent() } diff --git a/bind/package.go b/bind/package.go index 8a87df5..e300227 100644 --- a/bind/package.go +++ b/bind/package.go @@ -163,7 +163,7 @@ func (p *Package) getDoc(parent string, o types.Object) string { case *types.Func: sig := o.Type().(*types.Signature) - _, _, _, err := isPyCompatFunc(sig) + _, _, err := isPyCompatFunc(sig) if err != nil { return "" } @@ -407,12 +407,12 @@ func (p *Package) process() error { continue } ret := fct.Return() - if ret == nil { + if len(ret) == 0 || len(ret) > 1 { continue } - retptr, retIsPtr := ret.(*types.Pointer) + retptr, retIsPtr := ret[0].(*types.Pointer) - if ret == styp || (retIsPtr && retptr.Elem() == styp) { + if ret[0] == styp || (retIsPtr && retptr.Elem() == styp) { delete(funcs, name) fct.doc = p.getDoc(sname, scope.Lookup(name)) fct.ctor = true diff --git a/bind/symbols.go b/bind/symbols.go index a9249d2..6ba3bff 100644 --- a/bind/symbols.go +++ b/bind/symbols.go @@ -144,48 +144,46 @@ func isPyCompatField(f *types.Var) (*symbol, error) { return ftyp, isPyCompatVar(ftyp) } +func getPyReturnType(sig *types.Signature) (ret []types.Type, err error) { + results := sig.Results() + + if (results.Len() == 0) { + return make([]types.Type, 0, 0), nil + } else { + outCount := results.Len() + if !NoPyExceptions && isErrorType(results.At(outCount-1).Type()) { + outCount -= 1 + } + + retval := make([]types.Type, outCount, outCount) + for i := 0; i < outCount; i++ { + retval[i] = results.At(i).Type() + } + + return retval, nil + } +} + // isPyCompatFunc checks if function signature is a python-compatible function. // Returns nil if function is compatible, err message if not. // Also returns the return type of the function // haserr is true if 2nd arg is an error type, which is only // supported form of multi-return-value functions // hasfun is true if one of the args is a function signature -func isPyCompatFunc(sig *types.Signature) (ret types.Type, haserr, hasfun bool, err error) { - res := sig.Results() - - switch res.Len() { - case 2: - if !isErrorType(res.At(1).Type()) { - err = fmt.Errorf("gopy: second result value must be of type error: %s", sig.String()) - return - } - haserr = true - ret = res.At(0).Type() - case 1: - if isErrorType(res.At(0).Type()) { - haserr = true - ret = nil +func isPyCompatFunc(sig *types.Signature) (haserr, hasfun bool, err error) { + results := sig.Results() + + for i := 0; i < results.Len(); i++ { + result := results.At(i) + if i < results.Len()-1 { + if isErrorType(result.Type()) { + err = fmt.Errorf("gopy: Only last result value can be of type error: %s", sig.String()) + return + } } else { - ret = res.At(0).Type() - } - case 0: - ret = nil - default: - err = fmt.Errorf("gopy: too many results to return: %s", sig.String()) - return - } - - if ret != nil { - if err = isPyCompatType(ret); err != nil { - return - } - if _, isSig := ret.Underlying().(*types.Signature); isSig { - err = fmt.Errorf("gopy: return type is signature") - return - } - if ret.Underlying().String() == "interface{}" { - err = fmt.Errorf("gopy: return type is interface{}") - return + if isErrorType(result.Type()) { + haserr = true + } } } @@ -554,7 +552,7 @@ func (sym *symtab) addSymbol(obj types.Object) error { case *types.Func: sig := obj.Type().Underlying().(*types.Signature) - _, _, _, err := isPyCompatFunc(sig) + _, _, err := isPyCompatFunc(sig) if err == nil { sym.syms[fn] = &symbol{ gopkg: pkg, @@ -1137,7 +1135,7 @@ func (sym *symtab) addSignatureType(pkg *types.Package, obj types.Object, t type func (sym *symtab) addMethod(pkg *types.Package, obj types.Object, t types.Type, kind symkind, id, n string) error { sig := t.Underlying().(*types.Signature) - _, _, _, err := isPyCompatFunc(sig) + _, _, err := isPyCompatFunc(sig) if err != nil { if !NoWarn { fmt.Printf("ignoring python incompatible method: %v.%v: %v: %v\n", pkg.Name(), obj.String(), t.String(), err) diff --git a/bind/types.go b/bind/types.go index 1de871e..58a4e2e 100644 --- a/bind/types.go +++ b/bind/types.go @@ -369,19 +369,24 @@ type Func struct { id string doc string - ret types.Type // return type, if any - err bool // true if original go func has comma-error - ctor bool // true if this is a newXXX function - hasfun bool // true if this function has a function argument - isVariadic bool // True, if this is a variadic function. + ret []types.Type // return type, if any + haserr bool // true if original go func has comma-error + ctor bool // true if this is a newXXX function + hasfun bool // true if this function has a function argument + isVariadic bool // True, if this is a variadic function. } func newFuncFrom(p *Package, parent string, obj types.Object, sig *types.Signature) (*Func, error) { - ret, haserr, hasfun, err := isPyCompatFunc(sig) + haserr, hasfun, err := isPyCompatFunc(sig) if err != nil { return nil, err } + ret, err2 := getPyReturnType(sig) + if err2 != nil { + return nil, err2 + } + id := obj.Pkg().Name() + "_" + obj.Name() if parent != "" { id = obj.Pkg().Name() + "_" + parent + "_" + obj.Name() @@ -401,7 +406,7 @@ func newFuncFrom(p *Package, parent string, obj types.Object, sig *types.Signatu id: id, doc: p.getDoc(parent, obj), ret: ret, - err: haserr, + haserr: haserr, hasfun: hasfun, isVariadic: sig.Variadic(), }, nil @@ -452,7 +457,7 @@ func (f *Func) Signature() *Signature { return f.sig } -func (f *Func) Return() types.Type { +func (f *Func) Return() []types.Type { return f.ret } diff --git a/cmd_build.go b/cmd_build.go index e933e47..d510b9f 100644 --- a/cmd_build.go +++ b/cmd_build.go @@ -34,16 +34,13 @@ ex: Flag: *flag.NewFlagSet("gopy-build", flag.ExitOnError), } - cmd.Flag.String("vm", "python", "path to python interpreter") - cmd.Flag.String("output", "", "output directory for bindings") - cmd.Flag.String("name", "", "name of output package (otherwise name of first package is used)") - cmd.Flag.String("main", "", "code string to run in the go main() function in the cgo library") + AddCommonCmdFlags(&cmd.Flag) + + // Build Specific flags. cmd.Flag.String("package-prefix", ".", "custom package prefix used when generating import "+ "statements for generated package") - cmd.Flag.Bool("rename", false, "rename Go symbols to python PEP snake_case") cmd.Flag.Bool("symbols", true, "include symbols in output") - cmd.Flag.Bool("no-warn", false, "suppress warning messages, which may be expected") - cmd.Flag.Bool("no-make", false, "do not generate a Makefile, e.g., when called from Makefile") + return cmd } @@ -54,19 +51,11 @@ func gopyRunCmdBuild(cmdr *commander.Command, args []string) error { return err } - cfg := NewBuildCfg() - cfg.OutputDir = cmdr.Flag.Lookup("output").Value.Get().(string) - cfg.Name = cmdr.Flag.Lookup("name").Value.Get().(string) - cfg.Main = cmdr.Flag.Lookup("main").Value.Get().(string) - cfg.VM = cmdr.Flag.Lookup("vm").Value.Get().(string) - cfg.PkgPrefix = cmdr.Flag.Lookup("package-prefix").Value.Get().(string) - cfg.RenameCase = cmdr.Flag.Lookup("rename").Value.Get().(bool) - cfg.Symbols = cmdr.Flag.Lookup("symbols").Value.Get().(bool) - cfg.NoWarn = cmdr.Flag.Lookup("no-warn").Value.Get().(bool) - cfg.NoMake = cmdr.Flag.Lookup("no-make").Value.Get().(bool) + cfg := NewBuildCfg(&cmdr.Flag) bind.NoWarn = cfg.NoWarn bind.NoMake = cfg.NoMake + bind.NoPyExceptions = cfg.NoPyExceptions for _, path := range args { bpkg, err := loadPackage(path, true) // build first diff --git a/cmd_exe.go b/cmd_exe.go index 00030e7..57c2a65 100644 --- a/cmd_exe.go +++ b/cmd_exe.go @@ -40,14 +40,11 @@ ex: Flag: *flag.NewFlagSet("gopy-exe", flag.ExitOnError), } - cmd.Flag.String("vm", "python", "path to python interpreter") - cmd.Flag.String("output", "", "output directory for root of package") - cmd.Flag.String("name", "", "name of output package (otherwise name of first package is used)") - cmd.Flag.String("main", "", "code string to run in the go main() function in the cgo library "+ - "-- defaults to GoPyMainRun() but typically should be overriden") + AddCommonCmdFlags(&cmd.Flag) + + // Exe specific flags. // cmd.Flag.String("package-prefix", ".", "custom package prefix used when generating import "+ // "statements for generated package") - cmd.Flag.Bool("rename", false, "rename Go symbols to python PEP snake_case") cmd.Flag.Bool("symbols", true, "include symbols in output") cmd.Flag.String("exclude", "", "comma-separated list of package names to exclude") cmd.Flag.String("user", "", "username on https://www.pypa.io/en/latest/ for package name suffix") @@ -56,8 +53,6 @@ ex: cmd.Flag.String("email", "gopy@example.com", "author email") cmd.Flag.String("desc", "", "short description of project (long comes from README.md)") cmd.Flag.String("url", "https://github.com/rudderlabs/gopy", "home page for project") - cmd.Flag.Bool("no-warn", false, "suppress warning messages, which may be expected") - cmd.Flag.Bool("no-make", false, "do not generate a Makefile, e.g., when called from Makefile") return cmd } @@ -69,17 +64,7 @@ func gopyRunCmdExe(cmdr *commander.Command, args []string) error { return err } - cfg := NewBuildCfg() - cfg.OutputDir = cmdr.Flag.Lookup("output").Value.Get().(string) - cfg.Name = cmdr.Flag.Lookup("name").Value.Get().(string) - cfg.Main = cmdr.Flag.Lookup("main").Value.Get().(string) - cfg.VM = cmdr.Flag.Lookup("vm").Value.Get().(string) - // cfg.PkgPrefix = cmdr.Flag.Lookup("package-prefix").Value.Get().(string) - cfg.PkgPrefix = "" // doesn't make sense for exe - cfg.RenameCase = cmdr.Flag.Lookup("rename").Value.Get().(bool) - cfg.Symbols = cmdr.Flag.Lookup("symbols").Value.Get().(bool) - cfg.NoWarn = cmdr.Flag.Lookup("no-warn").Value.Get().(bool) - cfg.NoMake = cmdr.Flag.Lookup("no-make").Value.Get().(bool) + cfg := NewBuildCfg(&cmdr.Flag) var ( exclude = cmdr.Flag.Lookup("exclude").Value.Get().(string) @@ -93,6 +78,7 @@ func gopyRunCmdExe(cmdr *commander.Command, args []string) error { bind.NoWarn = cfg.NoWarn bind.NoMake = cfg.NoMake + bind.NoPyExceptions = cfg.NoPyExceptions if cfg.Name == "" { path := args[0] diff --git a/cmd_gen.go b/cmd_gen.go index ee0e72a..675de09 100644 --- a/cmd_gen.go +++ b/cmd_gen.go @@ -28,15 +28,12 @@ ex: Flag: *flag.NewFlagSet("gopy-gen", flag.ExitOnError), } - cmd.Flag.String("vm", "python", "path to python interpreter") - cmd.Flag.String("output", "", "output directory for bindings") - cmd.Flag.String("name", "", "name of output package (otherwise name of first package is used)") - cmd.Flag.String("main", "", "code string to run in the go main() function in the cgo library") + AddCommonCmdFlags(&cmd.Flag) + + // Gen specific flags. cmd.Flag.String("package-prefix", ".", "custom package prefix used when generating import "+ "statements for generated package") - cmd.Flag.Bool("rename", false, "rename Go symbols to python PEP snake_case") - cmd.Flag.Bool("no-warn", false, "suppress warning messages, which may be expected") - cmd.Flag.Bool("no-make", false, "do not generate a Makefile, e.g., when called from Makefile") + return cmd } @@ -49,19 +46,7 @@ func gopyRunCmdGen(cmdr *commander.Command, args []string) error { return err } - cfg := NewBuildCfg() - cfg.OutputDir = cmdr.Flag.Lookup("output").Value.Get().(string) - cfg.VM = cmdr.Flag.Lookup("vm").Value.Get().(string) - cfg.Name = cmdr.Flag.Lookup("name").Value.Get().(string) - cfg.Main = cmdr.Flag.Lookup("main").Value.Get().(string) - cfg.PkgPrefix = cmdr.Flag.Lookup("package-prefix").Value.Get().(string) - cfg.RenameCase = cmdr.Flag.Lookup("rename").Value.Get().(bool) - cfg.NoWarn = cmdr.Flag.Lookup("no-warn").Value.Get().(bool) - cfg.NoMake = cmdr.Flag.Lookup("no-make").Value.Get().(bool) - - if cfg.VM == "" { - cfg.VM = "python" - } + cfg := NewBuildCfg(&cmdr.Flag) bind.NoWarn = cfg.NoWarn bind.NoMake = cfg.NoMake diff --git a/cmd_pkg.go b/cmd_pkg.go index a4ca1d6..76befd0 100644 --- a/cmd_pkg.go +++ b/cmd_pkg.go @@ -38,13 +38,11 @@ ex: Flag: *flag.NewFlagSet("gopy-pkg", flag.ExitOnError), } - cmd.Flag.String("vm", "python", "path to python interpreter") - cmd.Flag.String("output", "", "output directory for root of package") - cmd.Flag.String("name", "", "name of output package (otherwise name of first package is used)") - cmd.Flag.String("main", "", "code string to run in the go GoPyInit() function in the cgo library") + AddCommonCmdFlags(&cmd.Flag) + + // Pkg specific flags. cmd.Flag.String("package-prefix", ".", "custom package prefix used when generating import "+ "statements for generated package") - cmd.Flag.Bool("rename", false, "rename Go symbols to python PEP snake_case") cmd.Flag.Bool("symbols", true, "include symbols in output") cmd.Flag.String("exclude", "", "comma-separated list of package names to exclude") cmd.Flag.String("user", "", "username on https://www.pypa.io/en/latest/ for package name suffix") @@ -53,8 +51,6 @@ ex: cmd.Flag.String("email", "gopy@example.com", "author email") cmd.Flag.String("desc", "", "short description of project (long comes from README.md)") cmd.Flag.String("url", "https://github.com/rudderlabs/gopy", "home page for project") - cmd.Flag.Bool("no-warn", false, "suppress warning messages, which may be expected") - cmd.Flag.Bool("no-make", false, "do not generate a Makefile, e.g., when called from Makefile") return cmd } @@ -66,16 +62,7 @@ func gopyRunCmdPkg(cmdr *commander.Command, args []string) error { return err } - cfg := NewBuildCfg() - cfg.OutputDir = cmdr.Flag.Lookup("output").Value.Get().(string) - cfg.Name = cmdr.Flag.Lookup("name").Value.Get().(string) - cfg.Main = cmdr.Flag.Lookup("main").Value.Get().(string) - cfg.VM = cmdr.Flag.Lookup("vm").Value.Get().(string) - cfg.PkgPrefix = cmdr.Flag.Lookup("package-prefix").Value.Get().(string) - cfg.RenameCase = cmdr.Flag.Lookup("rename").Value.Get().(bool) - cfg.Symbols = cmdr.Flag.Lookup("symbols").Value.Get().(bool) - cfg.NoWarn = cmdr.Flag.Lookup("no-warn").Value.Get().(bool) - cfg.NoMake = cmdr.Flag.Lookup("no-make").Value.Get().(bool) + cfg := NewBuildCfg(&cmdr.Flag) var ( exclude = cmdr.Flag.Lookup("exclude").Value.Get().(string) @@ -89,6 +76,7 @@ func gopyRunCmdPkg(cmdr *commander.Command, args []string) error { bind.NoWarn = cfg.NoWarn bind.NoMake = cfg.NoMake + bind.NoPyExceptions = cfg.NoPyExceptions if cfg.Name == "" { path := args[0] diff --git a/goerr2pyex/error_translator.go b/goerr2pyex/error_translator.go new file mode 100644 index 0000000..20a4ba5 --- /dev/null +++ b/goerr2pyex/error_translator.go @@ -0,0 +1,18 @@ +package goerr2pyex + +//#include +import "C" +import ( + "unsafe" +) + +func DefaultGoErrorToPyException(err error) bool { + if err != nil { + estr := C.CString(err.Error()) + C.PyErr_SetString(C.PyExc_RuntimeError, estr) + C.free(unsafe.Pointer(estr)) + return true + } else { + return false + } +} diff --git a/goerr2pyex/test.sh b/goerr2pyex/test.sh new file mode 100644 index 0000000..d9e6c43 --- /dev/null +++ b/goerr2pyex/test.sh @@ -0,0 +1,9 @@ +# The script to test build. +#env CGO_ENABLED=1 CGO_CFLAGS="-I/opt/homebrew/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/include/python3.9 -Wno-error -Wno-implicit-function-declaration -Wno-int-conversion" CGO_LDFLAGS="-L/opt/homebrew/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib -lpython3.9 -ldl -framework CoreFoundation" go build -mod=mod -buildmode=c-shared -o ./error_translator.so error_translator.go +#env CGO_ENABLED=1 CGO_CFLAGS="-I/opt/homebrew/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/include/python3.9 -Wno-error -Wno-implicit-function-declaration -Wno-int-conversion" CGO_LDFLAGS="-L/opt/homebrew/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib -lpython3.9 -ldl -framework CoreFoundation" go build -mod=mod -o ./error_translator.so error_translator.go +#env CGO_ENABLED=1 CGO_CFLAGS="-I/opt/homebrew/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/include/python3.9 -Wno-error -Wno-implicit-function-declaration -Wno-int-conversion" CGO_LDFLAGS="-L/opt/homebrew/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib -lpython3.9 -ldl -framework CoreFoundation" go build -o ./error_translator.so error_translator.go +#env CGO_ENABLED=1 CGO_CFLAGS="-I/opt/homebrew/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/include/python3.9 -Wno-error -Wno-implicit-function-declaration -Wno-int-conversion" CGO_LDFLAGS="-L/opt/homebrew/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib -lpython3.9 -ldl -framework CoreFoundation" go build error_translator.go + + +env CGO_ENABLED=1 CGO_CFLAGS="-I/opt/homebrew/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/include/python3.9 -Wno-error -Wno-implicit-function-declaration -Wno-int-conversion" CGO_LDFLAGS="-L/opt/homebrew/opt/python@3.9/Frameworks/Python.framework/Versions/3.9/lib -lpython3.9 -ldl -framework CoreFoundation" go build + diff --git a/main.go b/main.go index d7a132a..a4b87b1 100644 --- a/main.go +++ b/main.go @@ -24,19 +24,59 @@ type BuildCfg struct { // include symbols in output Symbols bool + // suppress warning messages, which may be expected NoWarn bool + // do not generate a Makefile, e.g., when called from Makefile NoMake bool } // NewBuildCfg returns a newly constructed build config -func NewBuildCfg() *BuildCfg { +func NewBuildCfg(flagSet *flag.FlagSet) *BuildCfg { var cfg BuildCfg cfg.Cmd = argStr() + + cfg.OutputDir = flagSet.Lookup("output").Value.Get().(string) + cfg.Name = flagSet.Lookup("name").Value.Get().(string) + cfg.Main = flagSet.Lookup("main").Value.Get().(string) + cfg.VM = flagSet.Lookup("vm").Value.Get().(string) + cfg.RenameCase = flagSet.Lookup("rename").Value.Get().(bool) + cfg.NoWarn = flagSet.Lookup("no-warn").Value.Get().(bool) + cfg.NoMake = flagSet.Lookup("no-make").Value.Get().(bool) + cfg.PkgPrefix = flagSet.Lookup("package-prefix").Value.Get().(string) + cfg.Symbols = flagSet.Lookup("symbols").Value.Get().(bool) + cfg.NoPyExceptions = flagSet.Lookup("no-exceptions").Value.Get().(bool) + cfg.ModPathGoErr2PyEx = flagSet.Lookup("gomod-2pyex").Value.Get().(string) + cfg.UsePyTuple4VE = flagSet.Lookup("use-pytuple-4ve").Value.Get().(bool) + + if cfg.ModPathGoErr2PyEx == "" { + cfg.ModPathGoErr2PyEx = "github.com/rudderlabs/gopy/goerr2pyex/" + } + + if cfg.VM == "" { + cfg.VM = "python" + } + return &cfg } +func AddCommonCmdFlags(flagSet *flag.FlagSet) { + flagSet.String("vm", "python", "path to python interpreter") + flagSet.String("output", "", "output directory for root of package") + flagSet.String("name", "", "name of output package (otherwise name of first package is used)") + flagSet.String("main", "", "code string to run in the go main()/GoPyInit() function(s) in the cgo library "+ + "-- defaults to GoPyMainRun() but typically should be overriden") + + flagSet.Bool("rename", false, "rename Go symbols to python PEP snake_case") + flagSet.Bool("no-warn", false, "suppress warning messages, which may be expected") + flagSet.Bool("no-make", false, "do not generate a Makefile, e.g., when called from Makefile") + + flagSet.Bool("no-exceptions", false, "Don't throw python exceptions when the last return is in error.") + flagSet.Bool("use-pytuple-4ve", false, "When Go returns (value, err) with err=nil, return (value, ) tuple to python.") + flagSet.String("gomod-2pyex", "", "Go module to use to translate Go errors to Python exceptions.") +} + func run(args []string) error { app := &commander.Command{ UsageLine: "gopy", From b5b07fe8781154b7637172c7d96bab6aabc77fe1 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Mon, 13 Jun 2022 02:11:01 +0530 Subject: [PATCH 06/20] Python code being generated sensibly. --- bind/gen_func.go | 8 ++++---- bind/package.go | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/bind/gen_func.go b/bind/gen_func.go index 02b786f..c8edd3a 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -381,13 +381,13 @@ if __err != nil { } // Call upstream method and collect returns. - g.pywrap.Printf(fmt.Sprintf("%s := %s", strings.Join(retvars, ", "), pyFuncCall)) + g.pywrap.Printf(fmt.Sprintf("%s := %s\n", strings.Join(retvars, ", "), pyFuncCall)) // ReMap handle returns from pyFuncCall. for i := 0; i < npyres; i++ { if res[i].sym.hasHandle() { cvnm := res[i].sym.pyPkgId(g.pkg.pkg) - g.pywrap.Printf("ret%d = %s(handle=_ret%d)", cvnm, i, i) + g.pywrap.Printf("ret%d = %s(handle=_ret%d)\n", i, cvnm, i) retvars[i] = "ret" + strconv.Itoa(i) } } @@ -397,7 +397,7 @@ if __err != nil { g.pywrap.Printf(pyFuncCall) } - g.pywrap.Printf("\n") + g.pywrap.Printf("\n\n") g.pywrap.Outdent() goFuncCall := "" @@ -460,7 +460,7 @@ if __err != nil { } else { g.gofile.Printf("if __err != nil {\n") g.gofile.Indent() - g.gofile.Printf("estr := C.CString(__err.Error())// NOTE: leaked string\n") // NOTE: leaked string + g.gofile.Printf("estr := C.CString(__err.Error())\n") // NOTE: freed string g.gofile.Printf("C.PyErr_SetString(C.PyExc_RuntimeError, estr)\n") g.gofile.Printf("C.free(unsafe.Pointer(estr))\n") // python should have converted, safe g.gofile.Outdent() diff --git a/bind/package.go b/bind/package.go index e300227..83720d6 100644 --- a/bind/package.go +++ b/bind/package.go @@ -246,6 +246,10 @@ func (p *Package) getDoc(parent string, o types.Object) string { paramString := strings.Join(params, ", ") resultString := strings.Join(results, ", ") + if len(results) > 1 { + resultString = "(" + resultString + ")" + } + //FIXME(sbinet): add receiver for methods? docSig := fmt.Sprintf("%s(%s) %s", o.Name(), paramString, resultString) From c57ef418be0c72a853b2704510eb096c3950b0bd Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Mon, 13 Jun 2022 09:12:16 +0530 Subject: [PATCH 07/20] Correcting npyres computation. --- bind/gen_func.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bind/gen_func.go b/bind/gen_func.go index c8edd3a..8c14773 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -61,8 +61,13 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { nargs := 0 nres := len(res) npyres := nres + rvHasErr := false // set to true if the main return is an error if fsym.haserr { - npyres -= 1 + if NoPyExceptions { + rvHasErr = true + } else { + npyres -= 1 + } } var ( From b3c7b5d5db6dcc9376ef6187be6eaf3435afeab6 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Mon, 13 Jun 2022 11:06:06 +0530 Subject: [PATCH 08/20] Verfied generated python code, atleast for smaller ret-counts. --- bind/gen_func.go | 18 ++++++++---------- bind/package.go | 13 ++++++++++++- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/bind/gen_func.go b/bind/gen_func.go index 8c14773..da5016e 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -200,14 +200,11 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { } if len(goArgs) > 0 { - gstr := strings.Join(goArgs, ", ") - g.gofile.Printf("%v) %v", gstr, goRet) + g.gofile.Printf("%v) %v", strings.Join(goArgs, ", "), goRet) - pstr := strings.Join(pyArgs, ", ") - g.pybuild.Printf(", [%v])\n", pstr) + g.pybuild.Printf(", [%v])\n", strings.Join(pyArgs, ", ")) - wstr := strings.Join(wpArgs, ", ") - g.pywrap.Printf("%v)", wstr) + g.pywrap.Printf("%v)", strings.Join(wpArgs, ", ")) } else { g.gofile.Printf(") %v", goRet) @@ -254,9 +251,6 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { pkgname := g.cfg.Name - _, gdoc, _ := extractPythonName(fsym.GoName(), fsym.Doc()) - ifchandle, gdoc := isIfaceHandle(gdoc) - sig := fsym.Signature() res := sig.Results() args := sig.Params() @@ -271,6 +265,9 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { } } + _, gdoc, _ := extractPythonName(fsym.GoName(), fsym.Doc()) + ifchandle, gdoc := isIfaceHandle(gdoc) + g.pywrap.Printf(":\n") g.pywrap.Indent() g.pywrap.Printf(`"""%s"""`, gdoc) @@ -402,8 +399,9 @@ if __err != nil { g.pywrap.Printf(pyFuncCall) } - g.pywrap.Printf("\n\n") + g.pywrap.Printf("\n") g.pywrap.Outdent() + g.pywrap.Printf("\n") goFuncCall := "" if isMethod { diff --git a/bind/package.go b/bind/package.go index 83720d6..46bc5a4 100644 --- a/bind/package.go +++ b/bind/package.go @@ -241,7 +241,18 @@ func (p *Package) getDoc(parent string, o types.Object) string { } params := parseFn(sig.Params()) - results := parseFn(sig.Results()) + + res := sig.Results() + results := parseFn(res) + + if (len(results) > 0) { + lastResult := res.At(len(results)-1) + if isErrorType(lastResult.Type()) { + if !NoPyExceptions { + results = results[0:len(results)-1] + } + } + } paramString := strings.Join(params, ", ") resultString := strings.Join(results, ", ") From d93d069b33f25d79bb04a71cbdb9aaeb396ad805 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Tue, 14 Jun 2022 17:03:06 +0530 Subject: [PATCH 09/20] Code to handle multiple return values as tuples now logically complete, even though it doesn't work currently. --- bind/gen.go | 22 ++++++++++++++ bind/gen_func.go | 79 +++++++++++++++++++++++++++++++----------------- 2 files changed, 74 insertions(+), 27 deletions(-) diff --git a/bind/gen.go b/bind/gen.go index e8f12cf..6a2bdc2 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -86,6 +86,28 @@ static inline void gopy_err_handle() { PyErr_Print(); } } + +static PyObject* Py_BuildValue1(const char *format, void* arg0) +{ + return Py_BuildValue(format, arg0); +} +static PyObject* Py_BuildValue2(const char *format, void* arg0, void* arg1) +{ + return Py_BuildValue(format, arg0, arg1); +} +static PyObject * Py_BuildValue3(const char *format, void* arg0, void* arg1, void* arg2) +{ + return Py_BuildValue(format, arg0, arg1, arg2); +} +static PyObject * Py_BuildValue4(const char *format, void* arg0, void* arg1, void* arg2, void* arg3) +{ + return Py_BuildValue(format, arg0, arg1, arg2, arg3); +} +static PyObject * Py_BuildValue5(const char *format, void* arg0, void* arg1, void* arg2, void* arg3, void* arg4) +{ + return Py_BuildValue(format, arg0, arg1, arg2, arg3, arg4); +} + %[9]s */ import "C" diff --git a/bind/gen_func.go b/bind/gen_func.go index da5016e..7caf626 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -12,6 +12,17 @@ import ( "strings" ) +func buildPyTuple(fsym *Func) bool { + npyres := len(fsym.sig.Results()) + if fsym.haserr { + if !NoPyExceptions { + npyres -= 1 + } + } + + return (npyres > 1) +} + func (g *pyGen) recurse(gotype types.Type, prefix, name string) { switch t := gotype.(type) { case *types.Basic: @@ -61,11 +72,8 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { nargs := 0 nres := len(res) npyres := nres - rvHasErr := false // set to true if the main return is an error if fsym.haserr { - if NoPyExceptions { - rvHasErr = true - } else { + if !NoPyExceptions { npyres -= 1 } } @@ -160,27 +168,16 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { goRet := "" if npyres == 0 { g.pybuild.Printf("None") - } else if npyres == 1 { - ret := res[0] - sret := current.symtype(ret.GoType()) - if sret == nil { - panic(fmt.Errorf( - "gopy: could not find symbol for %q", - ret.Name(), - )) - } - - if sret.cpyname == "PyObject*" { - g.pybuild.Printf("retval('%s', caller_owns_return=True)", sret.cpyname) - } else { - g.pybuild.Printf("retval('%s')", sret.cpyname) - } - goRet = fmt.Sprintf("%s", sret.cgoname) - } else { - // On Python side, we are returning PyTuple. + } else if buildPyTuple(fsym) { + // We are returning PyTuple*. Setup pybindgen accordingly. g.pybuild.Printf("retval('PyObject*', caller_owns_return=True)") - // On Go side, we are returning multiple values. + // On Go side, return *C.PyObject. + goRet = "unsafe.Pointer" + } else { + ownership := "" + pyrets := make([]string, npyres, npyres) + gorets := make([]string, npyres, npyres) for i := 0; i < npyres; i++ { sret := current.symtype(res[i].GoType()) if sret == nil { @@ -189,11 +186,16 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { res[i].Name(), )) } - goRet += sret.cgoname - if i != npyres-1 { - goRet += ", " + gorets[i] = sret.cgoname + pyrets[i] = "'" + sret.cpyname + "'" + if sret.cpyname == "PyObject*" { + ownership = "caller_owns_return=True" } } + + g.pybuild.Printf("retval(%s%s)", strings.Join(pyrets, ", "), ownership) + + goRet = strings.Join(gorets, ", ") if npyres > 1 { goRet = "(" + goRet + ")" } @@ -471,7 +473,30 @@ if __err != nil { } } - g.gofile.Printf("return %s", strings.Join(retvals[0:npyres], ", ")) + if buildPyTuple(fsym) { + g.gofile.Printf("\n") + formatStr := "" + for i := 0; i < npyres; i++ { + sret := current.symtype(res[i].GoType()) + if sret == nil { + panic(fmt.Errorf( + "gopy: could not find symbol for %q", + res[i].Name(), + )) + } + if sret.pyfmt == "" { + formatStr += "?" + } else { + formatStr += sret.pyfmt + } + } + g.gofile.Printf("return unsafe.Pointer(C.Py_BuildValue%d(\"%s\", %s))\n", + npyres, + formatStr, + strings.Join(retvals[0:npyres], ", ")) + } else { + g.gofile.Printf("return %s\n", strings.Join(retvals[0:npyres], ", ")) + } } else { g.gofile.Printf("if boolPyToGo(goRun) {\n") g.gofile.Indent() From 1ae769674dfb9e1330a8be1ab4fafe23bbf56dd9 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Tue, 14 Jun 2022 19:14:42 +0530 Subject: [PATCH 10/20] Both go and python code is now compiling. Automatic Test are failing. --- bind/gen.go | 17 ++--------------- bind/gen_func.go | 34 +++++++++++++++++++++++++--------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/bind/gen.go b/bind/gen.go index 6a2bdc2..3239d98 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -86,26 +86,13 @@ static inline void gopy_err_handle() { PyErr_Print(); } } - static PyObject* Py_BuildValue1(const char *format, void* arg0) { return Py_BuildValue(format, arg0); } -static PyObject* Py_BuildValue2(const char *format, void* arg0, void* arg1) -{ - return Py_BuildValue(format, arg0, arg1); -} -static PyObject * Py_BuildValue3(const char *format, void* arg0, void* arg1, void* arg2) -{ - return Py_BuildValue(format, arg0, arg1, arg2); -} -static PyObject * Py_BuildValue4(const char *format, void* arg0, void* arg1, void* arg2, void* arg3) -{ - return Py_BuildValue(format, arg0, arg1, arg2, arg3); -} -static PyObject * Py_BuildValue5(const char *format, void* arg0, void* arg1, void* arg2, void* arg3, void* arg4) +static PyObject* Py_BuildValue2(const char *format, long long arg0) { - return Py_BuildValue(format, arg0, arg1, arg2, arg3, arg4); + return Py_BuildValue(format, arg0); } %[9]s diff --git a/bind/gen_func.go b/bind/gen_func.go index 7caf626..e40d7cb 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -239,6 +239,13 @@ func isIfaceHandle(gdoc string) (bool, string) { return false, gdoc } +func isPointer(pyfmt string) bool { + if (pyfmt == "s") { + return true + } + return false +} + func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { isMethod := (sym != nil) isIface := false @@ -385,7 +392,7 @@ if __err != nil { } // Call upstream method and collect returns. - g.pywrap.Printf(fmt.Sprintf("%s := %s\n", strings.Join(retvars, ", "), pyFuncCall)) + g.pywrap.Printf(fmt.Sprintf("%s = %s\n", strings.Join(retvars, ", "), pyFuncCall)) // ReMap handle returns from pyFuncCall. for i := 0; i < npyres; i++ { @@ -475,7 +482,7 @@ if __err != nil { if buildPyTuple(fsym) { g.gofile.Printf("\n") - formatStr := "" + g.gofile.Printf("retTuple := C.PyTuple_New(%d);\n", npyres) for i := 0; i < npyres; i++ { sret := current.symtype(res[i].GoType()) if sret == nil { @@ -484,16 +491,25 @@ if __err != nil { res[i].Name(), )) } + formatStr := sret.pyfmt if sret.pyfmt == "" { - formatStr += "?" - } else { - formatStr += sret.pyfmt + formatStr = "?" + } + + buildValueFunc := "C.Py_BuildValue1" + typeCast := "unsafe.Pointer" + if !isPointer(formatStr) { + buildValueFunc = "C.Py_BuildValue2" + typeCast = "C.longlong" } + valueCall := fmt.Sprintf("%s(C.CString(\"%s\"), %s(%s))", + buildValueFunc, + formatStr, + typeCast, + retvals[i]) + g.gofile.Printf("C.PyTuple_SetItem(retTuple, %d, %s);\n", i, valueCall) } - g.gofile.Printf("return unsafe.Pointer(C.Py_BuildValue%d(\"%s\", %s))\n", - npyres, - formatStr, - strings.Join(retvals[0:npyres], ", ")) + g.gofile.Printf("return unsafe.Pointer(retTuple);") } else { g.gofile.Printf("return %s\n", strings.Join(retvals[0:npyres], ", ")) } From 05d1874d4a85bcef38e70f92773d9375e169e333 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Wed, 15 Jun 2022 15:41:39 +0530 Subject: [PATCH 11/20] Working quite a bit as expected. But, some memory corruption is causing un-expected memory freeing and crashes. --- _examples/multireturn/multireturn.go | 10 ++++++- _examples/multireturn/test.py | 41 +++++++++++++++++++--------- bind/gen_func.go | 7 ++++- main_test.go | 8 ++++-- 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/_examples/multireturn/multireturn.go b/_examples/multireturn/multireturn.go index 72788c6..444f32e 100644 --- a/_examples/multireturn/multireturn.go +++ b/_examples/multireturn/multireturn.go @@ -13,6 +13,11 @@ func SingleWithoutErrorFunc() int { return 100 } +/////////////// Single Str WithoutError Return ////////////// +func SingleStrWithoutErrorFunc(vargs ...int) string { + return "150" +} + /////////////// Single WithError Return ////////////// func SingleWithErrorFunc(throwError bool) error { if throwError { @@ -37,7 +42,10 @@ func DoubleWithErrorFunc(throwError bool) (string, error) { } /////////////// Triple Returns Without Error ////////////// -func TripleWithoutErrorFunc(vargs ...int) (int, string, int) { +func TripleWithoutErrorFunc1(vargs ...int) (int, int, int) { + return 600, 700, 800 +} +func TripleWithoutErrorFunc2(vargs ...int) (int, string, int) { return 600, "700", 800 } diff --git a/_examples/multireturn/test.py b/_examples/multireturn/test.py index 0182473..7adf420 100644 --- a/_examples/multireturn/test.py +++ b/_examples/multireturn/test.py @@ -11,34 +11,49 @@ oneResult = multireturn.SingleWithoutErrorFunc() print("Single WithoutError Return %r" % oneResult) +############### Single Str WithoutError Return ############## +oneStrResult = multireturn.SingleStrWithoutErrorFunc() +print("Single Str WithoutError Return %r" % oneStrResult) + ############### Single WithError Return ############## errorFalseResult = multireturn.SingleWithErrorFunc(False) print("Single WithError(False) Return %r" % errorFalseResult) -errorTrueResult = multireturn.SingleWithErrorFunc(True) -print("Single WithError(True) Return %r" % errorTrueResult) +try: + errorTrueResult = multireturn.SingleWithErrorFunc(True) + print("Failed to throw an exception") +except RuntimeError as ex: + print("Single WithError(True). Exception:%r" % ex) ############### Double WithoutError Return ############## twoResults = multireturn.DoubleWithoutErrorFunc() -print("Double WithoutError Return %r" % twoResults) +print("Double WithoutError Return (%r, %r)" % twoResults) ############### Double WithError Return ############## -(value400, errorTrueResult) = multireturn.DoubleeWithErrorFunc(True) -print("Double WithError(True) Return (%r, %r)" % (value400, errorTrueResult)) +try: + value400 = multireturn.DoubleWithErrorFunc(True) + print("Failed to throw an exception. Return (%r, %r)." % value400) +except RuntimeError as ex: + print("Double WithError(True). exception Return %r" % ex) -(value500, errorFalseResult) = multireturn.DoubleWithErrorFunc(False) -print("Double WithError(False) Return (%r, %r)" % (value500, errorFalseResult)) +value500 = multireturn.DoubleWithErrorFunc(False) +print("Double WithError(False) Return %r" % value500) ############### Triple Without Error Return ############## -threeResults = multireturn.TripleWithoutErrorFunc() -print("Triple WithoutError Return %r" % threeResult) +threeResults = multireturn.TripleWithoutErrorFunc1() +print("Triple WithoutError(Without String) Return (%r, %r, %r)" % threeResults) + +threeResults = multireturn.TripleWithoutErrorFunc2() +print("Triple WithoutError(With String) Return (%r, %r, %r)" % threeResults) ############### Triple With Error Return ############## -(value900, value1000, errorTrueResult) = multireturn.TripleWithErrorFunc(True) -print("Triple WithError(True) Return (%r, %r, %r)" % (value900, value1000, errorFalseResult)) +try: + (value900, value1000, errorTrueResult) = multireturn.TripleWithErrorFunc(True) +except RuntimeError as ex: + print("Triple WithError(True). exception Return %r" % ex) -(value1100, value1200, errorFalseResult) = multireturn.TripleWithErrorFunc(False) -print("Triple WithError(False) Return (%r, %r, %r)" % (value1100, value1200, errorFalseResult)) +(value1100, value1200) = multireturn.TripleWithErrorFunc(False) +print("Triple WithError(False) Return (%r, %r)" % (value1100, value1200)) ############### Triple Struct Return With Error ############## (ptr1300, struct1400, errorTrueResult) = multireturn.TripleWithStructWithErrorFunc(True) diff --git a/bind/gen_func.go b/bind/gen_func.go index e40d7cb..198523c 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -379,7 +379,7 @@ if __err != nil { } pyFuncCall := fmt.Sprintf("_%s.%s(%s)", pkgname, mnm, strings.Join(wrapArgs, ", ")) - if nres > 0 { + if npyres > 0 { retvars := make([]string, nres, nres) for i := 0; i < npyres; i++ { retvars[i] = "ret" + strconv.Itoa(i) @@ -387,10 +387,15 @@ if __err != nil { retvars[i] = "_" + retvars[i] } } + if fsym.haserr { + // Only used if npyres == nres. retvars[nres-1] = "__err" } + // May drop the error var, if it is not supposed to be returned. + retvars = retvars[0:npyres] + // Call upstream method and collect returns. g.pywrap.Printf(fmt.Sprintf("%s = %s\n", strings.Join(retvars, ", "), pyFuncCall)) diff --git a/main_test.go b/main_test.go index 18ebdb7..25488b6 100644 --- a/main_test.go +++ b/main_test.go @@ -840,12 +840,14 @@ func TestBindMultiReturn(t *testing.T) { extras: nil, want: []byte(`No Return None Single WithoutError Return 100 +Single Str WithoutError Return 150 Single WithError(False) Return nil -Single WithError(True) Return 'Error' +Single WithError(True). Exception: 'Error' Double WithoutError Return (200,300) -Double WithError(True) Return (400, Error) +Double WithError(True). Return (400, Error) Double WithError(False) Return (500, nil) -Triple WithoutError Return (600, 700, 800) +Triple WithoutError(Without String) Return (600, 700, 800) +Triple WithoutError(With String) Return (600, 700, 800) Triple WithError(True) Return (900, 1000, Error) Triple WithError(False) Return (1100, 1200, nil) Triple WithError(True) Return (1300, 1400, Error) From 1e1167555ed7dd994a6928b348f5edd2ae1b73f3 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Sun, 19 Jun 2022 06:43:41 +0530 Subject: [PATCH 12/20] Tests passing once we let memory leak. Fix for memory leaks to be carefully enabled. --- _examples/multireturn/multireturn.go | 5 +++- _examples/multireturn/test.py | 35 +++++++++++++--------- bind/gen.go | 45 +++++++++++++++++----------- bind/gen_func.go | 10 +++++-- main_test.go | 25 ++++++++-------- 5 files changed, 74 insertions(+), 46 deletions(-) diff --git a/_examples/multireturn/multireturn.go b/_examples/multireturn/multireturn.go index 444f32e..8edb52d 100644 --- a/_examples/multireturn/multireturn.go +++ b/_examples/multireturn/multireturn.go @@ -28,9 +28,12 @@ func SingleWithErrorFunc(throwError bool) error { } /////////////// Double WithoutError Return ////////////// -func DoubleWithoutErrorFunc() (int, int) { +func DoubleWithoutErrorFunc1() (int, int) { return 200, 300 } +func DoubleWithoutErrorFunc2() (string, string) { + return "200", "300" +} /////////////// Double WithError Return ////////////// func DoubleWithErrorFunc(throwError bool) (string, error) { diff --git a/_examples/multireturn/test.py b/_examples/multireturn/test.py index 7adf420..709c831 100644 --- a/_examples/multireturn/test.py +++ b/_examples/multireturn/test.py @@ -16,25 +16,28 @@ print("Single Str WithoutError Return %r" % oneStrResult) ############### Single WithError Return ############## -errorFalseResult = multireturn.SingleWithErrorFunc(False) -print("Single WithError(False) Return %r" % errorFalseResult) +errorFalseResult1 = multireturn.SingleWithErrorFunc(False) +print("Single WithError(False) Return %r" % errorFalseResult1) try: - errorTrueResult = multireturn.SingleWithErrorFunc(True) + errorTrueResult1 = multireturn.SingleWithErrorFunc(True) print("Failed to throw an exception") except RuntimeError as ex: - print("Single WithError(True). Exception:%r" % ex) + print("Single WithError(True). Exception: %r" % ex) ############### Double WithoutError Return ############## -twoResults = multireturn.DoubleWithoutErrorFunc() -print("Double WithoutError Return (%r, %r)" % twoResults) +twoResults = multireturn.DoubleWithoutErrorFunc1() +print("Double WithoutError(Without String) Return (%r, %r)" % twoResults) + +twoResults = multireturn.DoubleWithoutErrorFunc2() +print("Double WithoutError(With String) Return (%r, %r)" % twoResults) ############### Double WithError Return ############## try: value400 = multireturn.DoubleWithErrorFunc(True) print("Failed to throw an exception. Return (%r, %r)." % value400) except RuntimeError as ex: - print("Double WithError(True). exception Return %r" % ex) + print("Double WithError(True). Exception: %r" % ex) value500 = multireturn.DoubleWithErrorFunc(False) print("Double WithError(False) Return %r" % value500) @@ -48,20 +51,24 @@ ############### Triple With Error Return ############## try: - (value900, value1000, errorTrueResult) = multireturn.TripleWithErrorFunc(True) + (value900, value1000) = multireturn.TripleWithErrorFunc(True) + print("Triple WithError(True) Return (%r, %r, %r)" % (value900, value1000)) except RuntimeError as ex: - print("Triple WithError(True). exception Return %r" % ex) + print("Triple WithError(True) Exception: %r" % ex) (value1100, value1200) = multireturn.TripleWithErrorFunc(False) print("Triple WithError(False) Return (%r, %r)" % (value1100, value1200)) ############### Triple Struct Return With Error ############## -(ptr1300, struct1400, errorTrueResult) = multireturn.TripleWithStructWithErrorFunc(True) -print("Triple WithError(True) Return (%r, %r, %r)" % (ptr1300.P, struct1400.P, errorFalseResult)) +try: + (ptr1300, struct1400) = multireturn.TripleWithStructWithErrorFunc(True) + print("Triple WithError(True) Return (%r, %r)" % (ptr1300.P, struct1400.P)) +except RuntimeError as ex: + print("Triple WithError(True) Exception: %r" % ex) -(value1500, value1600, errorFalseResult) = multireturn.TripleWithStructWithErrorFunc(False) -print("Triple WithError(False) Return (%r, %r, %r)" % (value1500.P, value1600.P, errorFalseResult)) +(value1500, value1600) = multireturn.TripleWithStructWithErrorFunc(False) +print("Triple WithError(False) Return (%r, %r)" % (value1500.P, value1600.P)) ############### Triple Interface Return Without Error ############## (interface1700, struct1800, ptr1900) = multireturn.TripleWithInterfaceWithoutErrorFunc() -print("Triple WithError(True) Return (%r, %r, %r)" % (interface1700.P, struct1800.P, ptr1900)) +print("Triple WithoutError() Return (%r, %r, %r)" % (interface1700.Number(), struct1800.P, ptr1900.P)) diff --git a/bind/gen.go b/bind/gen.go index 3239d98..bcb14ce 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -86,13 +86,17 @@ static inline void gopy_err_handle() { PyErr_Print(); } } -static PyObject* Py_BuildValue1(const char *format, void* arg0) +static PyObject* Py_BuildValue1(char *format, void* arg0) { - return Py_BuildValue(format, arg0); + PyObject *retval = Py_BuildValue(format, arg0); + free(format); + return retval; } -static PyObject* Py_BuildValue2(const char *format, long long arg0) +static PyObject* Py_BuildValue2(char *format, long long arg0) { - return Py_BuildValue(format, arg0); + PyObject *retval = Py_BuildValue(format, arg0); + free(format); + return retval; } %[9]s @@ -241,19 +245,22 @@ class CheckedFunction(Function): failure_cleanup = self._failure_cleanup or None self.before_call.write_error_check(check, failure_cleanup) -def add_checked_function(mod, name, retval, params, failure_expression='', *a, **kw): - fn = CheckedFunction(name, retval, params, *a, **kw) - fn.set_failure_expression(failure_expression) - mod._add_function_obj(fn) - return fn - -def add_checked_string_function(mod, name, retval, params, failure_expression='', *a, **kw): - fn = CheckedFunction(name, retval, params, *a, **kw) - fn.set_failure_cleanup('if (retval != NULL) free(retval);') - fn.after_call.add_cleanup_code('free(retval);') - fn.set_failure_expression(failure_expression) - mod._add_function_obj(fn) - return fn +def add_checked_function_generator(pyTupleBuilt, retvals_to_free): + if not pyTupleBuilt: + if retvals_to_free: + assert(len(retvals_to_free) == 1) + assert(retvals_to_free[0] == 0) + def rv_format(format_str, rv): + return format_str.format("PyTuple_GetItem(retval, {0})".format(rv)) + def add_checked_function(mod, name, retval, params, failure_expression='', *a, **kw): + fn = CheckedFunction(name, retval, params, *a, **kw) + #TODO: Figure out how to free allocated variables. Stop leaking memory. + #fn.set_failure_cleanup('\n'.join([rv_format('if ({0} != NULL) free({0});', rv) for rv in retvals_to_free])) + #fn.after_call.add_cleanup_code('\n'.join([rv_format('free({0});', rv) for rv in retvals_to_free])) + fn.set_failure_expression(failure_expression) + mod._add_function_obj(fn) + return fn + return add_checked_function mod = Module('_%[1]s') mod.add_include('"%[1]s_go.h"') @@ -384,6 +391,10 @@ build: # generated %[1]s.py python wrapper imports this c-code package %[9]s $(GCC) %[1]s.c %[6]s %[1]s_go$(LIBEXT) -o _%[1]s$(LIBEXT) $(CFLAGS) $(LDFLAGS) -fPIC --shared -w + +halfbuild: + $(GOBUILD) -buildmode=c-shared -o %[1]s_go$(LIBEXT) %[1]s.go + $(GCC) %[1]s.c %[6]s %[1]s_go$(LIBEXT) -o _%[1]s$(LIBEXT) $(CFLAGS) $(LDFLAGS) -fPIC --shared -w ` diff --git a/bind/gen_func.go b/bind/gen_func.go index 198523c..3dbccb1 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -132,7 +132,7 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { // a function that adds function calls with exception checking. // But given specific return types, we may want to add more // behavior to the wrapped function code gen. - addFuncName := "add_checked_function" + retvalsToFree := make([]string, 0, npyres) if npyres > 0 { for i := 0; i < npyres; i++ { switch t := res[i].GoType().(type) { @@ -140,11 +140,16 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { // string return types need special memory leak patches // to free the allocated char* if t.Kind() == types.String { - addFuncName = "add_checked_string_function" + retvalsToFree = append(retvalsToFree, strconv.Itoa(i)) } } } } + pyTupleBuilt := "True" + if !buildPyTuple(fsym) { + pyTupleBuilt = "False" + } + addFuncName := "add_checked_function_generator(" + pyTupleBuilt + ", [" + strings.Join(retvalsToFree, ", ") + "])" switch { case isMethod: @@ -506,6 +511,7 @@ if __err != nil { if !isPointer(formatStr) { buildValueFunc = "C.Py_BuildValue2" typeCast = "C.longlong" + formatStr = "L" } valueCall := fmt.Sprintf("%s(C.CString(\"%s\"), %s(%s))", buildValueFunc, diff --git a/main_test.go b/main_test.go index 25488b6..03198eb 100644 --- a/main_test.go +++ b/main_test.go @@ -840,19 +840,20 @@ func TestBindMultiReturn(t *testing.T) { extras: nil, want: []byte(`No Return None Single WithoutError Return 100 -Single Str WithoutError Return 150 -Single WithError(False) Return nil -Single WithError(True). Exception: 'Error' -Double WithoutError Return (200,300) -Double WithError(True). Return (400, Error) -Double WithError(False) Return (500, nil) +Single Str WithoutError Return '150' +Single WithError(False) Return None +Single WithError(True). Exception: RuntimeError('Error') +Double WithoutError(Without String) Return (200, 300) +Double WithoutError(With String) Return ('200', '300') +Double WithError(True). Exception: RuntimeError('Error') +Double WithError(False) Return '500' Triple WithoutError(Without String) Return (600, 700, 800) -Triple WithoutError(With String) Return (600, 700, 800) -Triple WithError(True) Return (900, 1000, Error) -Triple WithError(False) Return (1100, 1200, nil) -Triple WithError(True) Return (1300, 1400, Error) -Triple WithError(False) Return (1500, 1600, nil) -Triple WithError(True) Return (1700, 1800, 1900) +Triple WithoutError(With String) Return (600, '700', 800) +Triple WithError(True) Exception: RuntimeError('Error') +Triple WithError(False) Return (1100, 1200) +Triple WithError(True) Exception: RuntimeError('Error') +Triple WithError(False) Return (1500, 1600) +Triple WithoutError() Return (1700, 1800, 1900) `), }) } From 14d81b4e8acfc87c5ad24dcc1a379dd4f7104b3d Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Thu, 23 Jun 2022 12:08:36 +0530 Subject: [PATCH 13/20] Ignoring functions returning functions. --- _examples/multireturn/multireturn.go | 8 ++++++++ _examples/multireturn/test.py | 3 +++ bind/symbols.go | 4 ++++ bind/utils.go | 5 +++++ 4 files changed, 20 insertions(+) diff --git a/_examples/multireturn/multireturn.go b/_examples/multireturn/multireturn.go index 8edb52d..8c8a8f1 100644 --- a/_examples/multireturn/multireturn.go +++ b/_examples/multireturn/multireturn.go @@ -100,3 +100,11 @@ func TripleWithInterfaceWithoutErrorFunc() (IntInterFace, IntStrUct, *IntStrUct) return &i1700, s1800, &s1900 } + +//// Function returning function ///// +type FunctionType func(input int) int +func FunctionReturningFunction() FunctionType { + return func(input int) int{ + return input + } +} diff --git a/_examples/multireturn/test.py b/_examples/multireturn/test.py index 709c831..e885a86 100644 --- a/_examples/multireturn/test.py +++ b/_examples/multireturn/test.py @@ -72,3 +72,6 @@ ############### Triple Interface Return Without Error ############## (interface1700, struct1800, ptr1900) = multireturn.TripleWithInterfaceWithoutErrorFunc() print("Triple WithoutError() Return (%r, %r, %r)" % (interface1700.Number(), struct1800.P, ptr1900.P)) + +############## Function Returning Functions ignored ############## +assert("FunctionReturningFunction" not in dir(multireturn)) diff --git a/bind/symbols.go b/bind/symbols.go index 6ba3bff..99db6fe 100644 --- a/bind/symbols.go +++ b/bind/symbols.go @@ -184,6 +184,10 @@ func isPyCompatFunc(sig *types.Signature) (haserr, hasfun bool, err error) { if isErrorType(result.Type()) { haserr = true } + if isFuncType(result.Type()) { + err = fmt.Errorf("gopy: Result values of type function are not supported: %s", sig.String()) + return + } } } diff --git a/bind/utils.go b/bind/utils.go index a78fe4f..ab2f158 100644 --- a/bind/utils.go +++ b/bind/utils.go @@ -24,6 +24,11 @@ func isErrorType(typ types.Type) bool { return typ == types.Universe.Lookup("error").Type() } +func isFuncType(typ types.Type) bool { + _, ok := typ.Underlying().(*types.Signature) + return ok +} + func isStringer(obj types.Object) bool { switch obj := obj.(type) { case *types.Func: From 2cb06f830a2fcc2c3ed00bd27a93af02c637fa8d Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Thu, 23 Jun 2022 13:02:50 +0530 Subject: [PATCH 14/20] __err was getting reused in a way which declarations difficult. Fixed. --- bind/gen_func.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bind/gen_func.go b/bind/gen_func.go index 3dbccb1..c70e0c5 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -448,7 +448,7 @@ if __err != nil { retvals[i] = "cret" + strconv.Itoa(i) } if fsym.haserr { - retvals[nres-1] = "__err" + retvals[nres-1] = "__err_ret" } // Call upstream method and collect returns. @@ -470,9 +470,9 @@ if __err != nil { if rvHasErr { retvals[npyres-1] = "estr" // NOTE: leaked string g.gofile.Printf("var estr C.CString\n") - g.gofile.Printf("if __err != nil {\n") + g.gofile.Printf("if __err_ret != nil {\n") g.gofile.Indent() - g.gofile.Printf("estr = C.CString(__err.Error())// NOTE: leaked string\n") // NOTE: leaked string + g.gofile.Printf("estr = C.CString(__err_ret.Error())// NOTE: leaked string\n") // NOTE: leaked string g.gofile.Outdent() g.gofile.Printf("} else {\n") g.gofile.Indent() @@ -480,9 +480,9 @@ if __err != nil { g.gofile.Outdent() g.gofile.Printf("}\n") } else { - g.gofile.Printf("if __err != nil {\n") + g.gofile.Printf("if __err_ret != nil {\n") g.gofile.Indent() - g.gofile.Printf("estr := C.CString(__err.Error())\n") // NOTE: freed string + g.gofile.Printf("estr := C.CString(__err_ret.Error())\n") // NOTE: freed string g.gofile.Printf("C.PyErr_SetString(C.PyExc_RuntimeError, estr)\n") g.gofile.Printf("C.free(unsafe.Pointer(estr))\n") // python should have converted, safe g.gofile.Outdent() From 1a54a50801816a488f2212f12ed710b0010a266a Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Thu, 23 Jun 2022 19:51:26 +0530 Subject: [PATCH 15/20] Fixed a few code generation error for methods. --- bind/gen_func.go | 117 +++++++++++++++++++++++++---------------------- 1 file changed, 63 insertions(+), 54 deletions(-) diff --git a/bind/gen_func.go b/bind/gen_func.go index c70e0c5..f689dee 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -251,6 +251,67 @@ func isPointer(pyfmt string) bool { return false } +func (g *pyGen)generateReturn(buildPyTuple bool, npyres int, results []*Var, retvals *[]string) { + g.gofile.Printf("\n") + valueCalls := make([]string, npyres, npyres) + for i := 0; i < npyres; i++ { + result := results[i] + sret := current.symtype(result.GoType()) + if sret == nil { + panic(fmt.Errorf( + "gopy: could not find symbol for %q", + result.Name(), + )) + } + formatStr := sret.pyfmt + if sret.pyfmt == "" { + formatStr = "?" + } + + retval := "" + if retvals != nil { + retval = (*retvals)[i] + } else if result.sym.zval != "" { + retval = result.sym.zval + } else { + fmt.Printf("gopy: programmer error: empty zval zero value in symbol: %v\n", result.sym) + } + + if result.sym.go2py != "" { + retval = result.sym.go2py + "(" + retval + ")" + result.sym.go2pyParenEx + } + + if (buildPyTuple) { + buildValueFunc := "C.Py_BuildValue1" + typeCast := "unsafe.Pointer" + if !isPointer(formatStr) { + buildValueFunc = "C.Py_BuildValue2" + typeCast = "C.longlong" + formatStr = "L" + } + valueCalls[i] = fmt.Sprintf("%s(C.CString(\"%s\"), %s(%s))", + buildValueFunc, + formatStr, + typeCast, + retval) + } else { + valueCalls[i] = retval + } + } + + if npyres == 0 { + g.gofile.Printf("return\n") + } else if (buildPyTuple) { + g.gofile.Printf("retTuple := C.PyTuple_New(%d);\n", npyres) + for i := 0; i < npyres; i++ { + g.gofile.Printf("C.PyTuple_SetItem(retTuple, %d, %s);\n", i, valueCalls[i]) + } + g.gofile.Printf("return unsafe.Pointer(retTuple);") + } else { + g.gofile.Printf("return %s\n", strings.Join(valueCalls, ", ")) + } +} + func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { isMethod := (sym != nil) isIface := false @@ -303,23 +364,7 @@ func (g *pyGen) genFuncBody(sym *symbol, fsym *Func) { if __err != nil { `, symNm) g.gofile.Indent() - if npyres > 0 { - retvals := make([]string, npyres, npyres) - for i := 0; i < npyres; i++ { - ret := res[i] - if ret.sym.zval == "" { - fmt.Printf("gopy: programmer error: empty zval zero value in symbol: %v\n", ret.sym) - } - if ret.sym.go2py != "" { - retvals[i] = ret.sym.go2py + "(" + ret.sym.zval + ")" + ret.sym.go2pyParenEx - } else { - retvals[i] = ret.sym.zval - } - } - g.gofile.Printf("return %s\n", strings.Join(retvals, ", ")) - } else { - g.gofile.Printf("return\n") - } + g.generateReturn(buildPyTuple(fsym), npyres, res, nil) g.gofile.Outdent() g.gofile.Printf("}\n") } else if rvHasErr { @@ -459,9 +504,6 @@ if __err != nil { if res[i].sym.hasHandle() && !res[i].sym.isPtrOrIface(){ retvals[i] = "&" + retvals[i] } - if res[i].sym.go2py != "" { - retvals[i] = fmt.Sprintf("%s(%s)%s", res[i].sym.go2py, retvals[i], res[i].sym.go2pyParenEx) - } } if fsym.haserr { @@ -490,40 +532,7 @@ if __err != nil { } } - if buildPyTuple(fsym) { - g.gofile.Printf("\n") - g.gofile.Printf("retTuple := C.PyTuple_New(%d);\n", npyres) - for i := 0; i < npyres; i++ { - sret := current.symtype(res[i].GoType()) - if sret == nil { - panic(fmt.Errorf( - "gopy: could not find symbol for %q", - res[i].Name(), - )) - } - formatStr := sret.pyfmt - if sret.pyfmt == "" { - formatStr = "?" - } - - buildValueFunc := "C.Py_BuildValue1" - typeCast := "unsafe.Pointer" - if !isPointer(formatStr) { - buildValueFunc = "C.Py_BuildValue2" - typeCast = "C.longlong" - formatStr = "L" - } - valueCall := fmt.Sprintf("%s(C.CString(\"%s\"), %s(%s))", - buildValueFunc, - formatStr, - typeCast, - retvals[i]) - g.gofile.Printf("C.PyTuple_SetItem(retTuple, %d, %s);\n", i, valueCall) - } - g.gofile.Printf("return unsafe.Pointer(retTuple);") - } else { - g.gofile.Printf("return %s\n", strings.Join(retvals[0:npyres], ", ")) - } + g.generateReturn(buildPyTuple(fsym), npyres, res, &retvals); } else { g.gofile.Printf("if boolPyToGo(goRun) {\n") g.gofile.Indent() From 9350594db2e3991710cbbe79cf097023dda83b19 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Thu, 23 Jun 2022 20:57:00 +0530 Subject: [PATCH 16/20] Python None now mapped to GoLang nil. --- bind/gen_func.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bind/gen_func.go b/bind/gen_func.go index f689dee..12cd488 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -396,12 +396,12 @@ if __err != nil { switch { case arg.sym.goname == "interface{}": if ifchandle { - wrapArgs = append(wrapArgs, fmt.Sprintf("%s.handle", anm)) + wrapArgs = append(wrapArgs, fmt.Sprintf("(-1 if %s == None else %s.handle)", anm, anm)) } else { wrapArgs = append(wrapArgs, anm) } case arg.sym.hasHandle(): - wrapArgs = append(wrapArgs, fmt.Sprintf("%s.handle", anm)) + wrapArgs = append(wrapArgs, fmt.Sprintf("(-1 if %s == None else %s.handle)", anm, anm)) default: wrapArgs = append(wrapArgs, anm) } From ea0d913fe8908f6bea27bec760e840e559229b65 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Tue, 28 Jun 2022 12:49:59 +0530 Subject: [PATCH 17/20] TestGovet and TestGofmt now passing. --- SUPPORT_MATRIX.md | 2 + _examples/cpkg/run.go | 1 + _examples/multireturn/multireturn.go | 7 +- _examples/variadic/variadic.go | 16 +-- bind/bind.go | 2 +- bind/gen.go | 2 +- bind/gen_func.go | 40 +++---- bind/package.go | 6 +- bind/symbols.go | 2 +- cmd_build.go | 163 +++++++++++++++------------ cmd_exe.go | 2 +- cmd_gen.go | 2 +- cmd_pkg.go | 2 +- goerr2pyex/error_translator.go | 2 +- main_darwin.go | 1 + main_test.go | 11 +- main_unix.go | 1 + main_unix_test.go | 1 + 18 files changed, 148 insertions(+), 115 deletions(-) diff --git a/SUPPORT_MATRIX.md b/SUPPORT_MATRIX.md index 5e473e0..bd5e1ec 100644 --- a/SUPPORT_MATRIX.md +++ b/SUPPORT_MATRIX.md @@ -17,6 +17,7 @@ _examples/hi | no | yes _examples/iface | no | yes _examples/lot | yes | yes _examples/maps | yes | yes +_examples/multireturn | no | yes _examples/named | yes | yes _examples/osfile | yes | yes _examples/pkgconflict | yes | yes @@ -29,4 +30,5 @@ _examples/sliceptr | yes | yes _examples/slices | yes | yes _examples/structs | yes | yes _examples/unicode | no | yes +_examples/variadic | no | yes _examples/vars | yes | yes diff --git a/_examples/cpkg/run.go b/_examples/cpkg/run.go index 366095c..5fc61ac 100644 --- a/_examples/cpkg/run.go +++ b/_examples/cpkg/run.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build ignore // +build ignore package main diff --git a/_examples/multireturn/multireturn.go b/_examples/multireturn/multireturn.go index 8c8a8f1..55a6e43 100644 --- a/_examples/multireturn/multireturn.go +++ b/_examples/multireturn/multireturn.go @@ -103,8 +103,9 @@ func TripleWithInterfaceWithoutErrorFunc() (IntInterFace, IntStrUct, *IntStrUct) //// Function returning function ///// type FunctionType func(input int) int + func FunctionReturningFunction() FunctionType { - return func(input int) int{ - return input - } + return func(input int) int { + return input + } } diff --git a/_examples/variadic/variadic.go b/_examples/variadic/variadic.go index dce6447..1766d6b 100644 --- a/_examples/variadic/variadic.go +++ b/_examples/variadic/variadic.go @@ -1,7 +1,7 @@ package variadic /////////////// Non Variadic ////////////// -func NonVariFunc(arg1 int, arg2 []int, arg3 int) int{ +func NonVariFunc(arg1 int, arg2 []int, arg3 int) int { total := arg1 for _, num := range arg2 { total += num @@ -12,7 +12,7 @@ func NonVariFunc(arg1 int, arg2 []int, arg3 int) int{ } /////////////// Variadic Over Int ////////////// -func VariFunc(vargs ...int) int{ +func VariFunc(vargs ...int) int { total := 0 for _, num := range vargs { total += num @@ -26,15 +26,15 @@ type IntStrUct struct { } func NewIntStrUct(n int) IntStrUct { - return IntStrUct { - p:n, + return IntStrUct{ + p: n, } -} +} -func VariStructFunc(vargs ...IntStrUct) int{ +func VariStructFunc(vargs ...IntStrUct) int { total := 0 for _, inst := range vargs { - total += inst.p + total += inst.p } return total } @@ -48,7 +48,7 @@ func (is *IntStrUct) Number() int { return is.p } -func VariInterFaceFunc(vargs ...IntInterFace) int{ +func VariInterFaceFunc(vargs ...IntInterFace) int { total := 0 for _, inst := range vargs { total += inst.Number() diff --git a/bind/bind.go b/bind/bind.go index fd09072..32b8690 100644 --- a/bind/bind.go +++ b/bind/bind.go @@ -38,7 +38,7 @@ type BindCfg struct { // If set, python exceptions are not thrown. NoPyExceptions bool - // Path to Go module, which is to be used to translate Go errors to Python exceptions. + // Path to Go module, which is to be used to translate Go errors to Python exceptions. ModPathGoErr2PyEx string // If set, when a Go function returns a (value, err), python returns (value, ) tuple. diff --git a/bind/gen.go b/bind/gen.go index bcb14ce..c151748 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -453,7 +453,7 @@ var NoWarn = false // NoMake turns off generation of Makefiles var NoMake = false -// NoPyExceptions turns off generation of Python Exceptions +// NoPyExceptions turns off generation of Python Exceptions var NoPyExceptions = false // GenPyBind generates a .go file, build.py file to enable pybindgen to create python bindings, diff --git a/bind/gen_func.go b/bind/gen_func.go index 12cd488..1ff4a29 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -6,9 +6,9 @@ package bind import ( "fmt" + "go/types" "log" "strconv" - "go/types" "strings" ) @@ -111,7 +111,7 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { } } - if i!=nargs-1 || !fsym.isVariadic { + if i != nargs-1 || !fsym.isVariadic { wpArgs = append(wpArgs, anm) } } @@ -245,13 +245,13 @@ func isIfaceHandle(gdoc string) (bool, string) { } func isPointer(pyfmt string) bool { - if (pyfmt == "s") { + if pyfmt == "s" { return true } return false } -func (g *pyGen)generateReturn(buildPyTuple bool, npyres int, results []*Var, retvals *[]string) { +func (g *pyGen) generateReturn(buildPyTuple bool, npyres int, results []*Var, retvals *[]string) { g.gofile.Printf("\n") valueCalls := make([]string, npyres, npyres) for i := 0; i < npyres; i++ { @@ -263,7 +263,7 @@ func (g *pyGen)generateReturn(buildPyTuple bool, npyres int, results []*Var, ret result.Name(), )) } - formatStr := sret.pyfmt + formatStr := sret.pyfmt if sret.pyfmt == "" { formatStr = "?" } @@ -278,10 +278,10 @@ func (g *pyGen)generateReturn(buildPyTuple bool, npyres int, results []*Var, ret } if result.sym.go2py != "" { - retval = result.sym.go2py + "(" + retval + ")" + result.sym.go2pyParenEx + retval = result.sym.go2py + "(" + retval + ")" + result.sym.go2pyParenEx } - if (buildPyTuple) { + if buildPyTuple { buildValueFunc := "C.Py_BuildValue1" typeCast := "unsafe.Pointer" if !isPointer(formatStr) { @@ -301,7 +301,7 @@ func (g *pyGen)generateReturn(buildPyTuple bool, npyres int, results []*Var, ret if npyres == 0 { g.gofile.Printf("return\n") - } else if (buildPyTuple) { + } else if buildPyTuple { g.gofile.Printf("retTuple := C.PyTuple_New(%d);\n", npyres) for i := 0; i < npyres; i++ { g.gofile.Printf("C.PyTuple_SetItem(retTuple, %d, %s);\n", i, valueCalls[i]) @@ -389,7 +389,7 @@ if __err != nil { default: na = anm } - if i == len(args) - 1 && fsym.isVariadic { + if i == len(args)-1 && fsym.isVariadic { na = na + "..." } callArgs = append(callArgs, na) @@ -471,20 +471,20 @@ if __err != nil { if isMethod { if sym.isStruct() { goFuncCall = fmt.Sprintf("gopyh.Embed(vifc, reflect.TypeOf(%s{})).(%s).%s(%s)", - nonPtrName(symNm), - symNm, - fsym.GoName(), - strings.Join(callArgs, ", ")) + nonPtrName(symNm), + symNm, + fsym.GoName(), + strings.Join(callArgs, ", ")) } else { goFuncCall = fmt.Sprintf("vifc.(%s).%s(%s)", - symNm, - fsym.GoName(), - strings.Join(callArgs, ", ")) + symNm, + fsym.GoName(), + strings.Join(callArgs, ", ")) } } else { goFuncCall = fmt.Sprintf("%s(%s)", - fsym.GoFmt(), - strings.Join(callArgs, ", ")) + fsym.GoFmt(), + strings.Join(callArgs, ", ")) } if nres > 0 { @@ -501,7 +501,7 @@ if __err != nil { // ReMap handle returns from pyFuncCall. for i := 0; i < npyres; i++ { - if res[i].sym.hasHandle() && !res[i].sym.isPtrOrIface(){ + if res[i].sym.hasHandle() && !res[i].sym.isPtrOrIface() { retvals[i] = "&" + retvals[i] } } @@ -532,7 +532,7 @@ if __err != nil { } } - g.generateReturn(buildPyTuple(fsym), npyres, res, &retvals); + g.generateReturn(buildPyTuple(fsym), npyres, res, &retvals) } else { g.gofile.Printf("if boolPyToGo(goRun) {\n") g.gofile.Indent() diff --git a/bind/package.go b/bind/package.go index 46bc5a4..044e2e2 100644 --- a/bind/package.go +++ b/bind/package.go @@ -245,11 +245,11 @@ func (p *Package) getDoc(parent string, o types.Object) string { res := sig.Results() results := parseFn(res) - if (len(results) > 0) { - lastResult := res.At(len(results)-1) + if len(results) > 0 { + lastResult := res.At(len(results) - 1) if isErrorType(lastResult.Type()) { if !NoPyExceptions { - results = results[0:len(results)-1] + results = results[0 : len(results)-1] } } } diff --git a/bind/symbols.go b/bind/symbols.go index 99db6fe..20be4c6 100644 --- a/bind/symbols.go +++ b/bind/symbols.go @@ -147,7 +147,7 @@ func isPyCompatField(f *types.Var) (*symbol, error) { func getPyReturnType(sig *types.Signature) (ret []types.Type, err error) { results := sig.Results() - if (results.Len() == 0) { + if results.Len() == 0 { return make([]types.Type, 0, 0), nil } else { outCount := results.Len() diff --git a/cmd_build.go b/cmd_build.go index d510b9f..d6a1547 100644 --- a/cmd_build.go +++ b/cmd_build.go @@ -105,8 +105,6 @@ func runBuild(mode bind.BuildMode, cfg *BuildCfg) error { return err } - pycfg, err := bind.GetPythonConfig(cfg.VM) - if mode == bind.ModeExe { of, err := os.Create(buildname + ".h") // overwrite existing fmt.Fprintf(of, "typedef uint8_t bool;\n") @@ -144,38 +142,13 @@ func runBuild(mode bind.BuildMode, cfg *BuildCfg) error { } } else { - buildLib := buildname + libExt - extext := libExt - if runtime.GOOS == "windows" { - extext = ".pyd" - } - if pycfg.ExtSuffix != "" { - extext = pycfg.ExtSuffix - } - modlib := "_" + cfg.Name + extext - - // build the go shared library upfront to generate the header - // needed by our generated cpython code - args := []string{"build", "-mod=mod", "-buildmode=c-shared"} - if !cfg.Symbols { - // These flags will omit the various symbol tables, thereby - // reducing the final size of the binary. From https://golang.org/cmd/link/ - // -s Omit the symbol table and debug information - // -w Omit the DWARF symbol table - args = append(args, "-ldflags=-s -w") - } - args = append(args, "-o", buildLib, ".") - fmt.Printf("go %v\n", strings.Join(args, " ")) - cmd = exec.Command("go", args...) - cmdout, err = cmd.CombinedOutput() + + // build extension with go + c + env, args, err := getBuildArgsAndEnv(cfg) if err != nil { - fmt.Printf("cmd had error: %v output:\n%v\n", err, string(cmdout)) + fmt.Printf("error building environment: %v\n", err) return err } - // update the output name to the one with the ABI extension - args[len(args)-2] = modlib - // we don't need this initial lib because we are going to relink - os.Remove(buildLib) // generate c code fmt.Printf("%v build.py\n", cfg.VM) @@ -196,48 +169,6 @@ func runBuild(mode bind.BuildMode, cfg *BuildCfg) error { } } - cflags := strings.Fields(strings.TrimSpace(pycfg.CFlags)) - cflags = append(cflags, "-fPIC", "-Ofast") - if include, exists := os.LookupEnv("GOPY_INCLUDE"); exists { - cflags = append(cflags, "-I"+filepath.ToSlash(include)) - } - - ldflags := strings.Fields(strings.TrimSpace(pycfg.LdFlags)) - if !cfg.Symbols { - ldflags = append(ldflags, "-s") - } - if lib, exists := os.LookupEnv("GOPY_LIBDIR"); exists { - ldflags = append(ldflags, "-L"+filepath.ToSlash(lib)) - } - if libname, exists := os.LookupEnv("GOPY_PYLIB"); exists { - ldflags = append(ldflags, "-l"+filepath.ToSlash(libname)) - } - - removeEmpty := func(src []string) []string { - o := make([]string, 0, len(src)) - for _, v := range src { - if v == "" { - continue - } - o = append(o, v) - } - return o - } - - cflags = removeEmpty(cflags) - ldflags = removeEmpty(ldflags) - - cflagsEnv := fmt.Sprintf("CGO_CFLAGS=%s", strings.Join(cflags, " ")) - ldflagsEnv := fmt.Sprintf("CGO_LDFLAGS=%s", strings.Join(ldflags, " ")) - - env := os.Environ() - env = append(env, cflagsEnv) - env = append(env, ldflagsEnv) - - fmt.Println(cflagsEnv) - fmt.Println(ldflagsEnv) - - // build extension with go + c fmt.Printf("go %v\n", strings.Join(args, " ")) cmd = exec.Command("go", args...) cmd.Env = env @@ -250,3 +181,89 @@ func runBuild(mode bind.BuildMode, cfg *BuildCfg) error { return err } + +func getBuildArgsAndEnv(cfg *BuildCfg) (args []string, env []string, err error) { + buildname := cfg.Name + "_go" + buildLib := buildname + libExt + + var pycfg bind.PyConfig + pycfg, err = bind.GetPythonConfig(cfg.VM) + if err != nil { + fmt.Printf("error creating pycfg: %r\n", err) + return args, env, err + } + + extext := libExt + if runtime.GOOS == "windows" { + extext = ".pyd" + } + if pycfg.ExtSuffix != "" { + extext = pycfg.ExtSuffix + } + modlib := "_" + cfg.Name + extext + + // build the go shared library upfront to generate the header + // needed by our generated cpython code + args = []string{"build", "-mod=mod", "-buildmode=c-shared"} + if !cfg.Symbols { + // These flags will omit the various symbol tables, thereby + // reducing the final size of the binary. From https://golang.org/cmd/link/ + // -s Omit the symbol table and debug information + // -w Omit the DWARF symbol table + args = append(args, "-ldflags=-s -w") + } + args = append(args, "-o", buildLib, ".") + fmt.Printf("go %v\n", strings.Join(args, " ")) + cmd := exec.Command("go", args...) + cmdout, err := cmd.CombinedOutput() + if err != nil { + fmt.Printf("cmd had error: %v output:\n%v\n", err, string(cmdout)) + return args, env, err + } + // update the output name to the one with the ABI extension + args[len(args)-2] = modlib + // we don't need this initial lib because we are going to relink + os.Remove(buildLib) + + cflags := strings.Fields(strings.TrimSpace(pycfg.CFlags)) + cflags = append(cflags, "-fPIC", "-Ofast") + if include, exists := os.LookupEnv("GOPY_INCLUDE"); exists { + cflags = append(cflags, "-I"+filepath.ToSlash(include)) + } + + ldflags := strings.Fields(strings.TrimSpace(pycfg.LdFlags)) + if !cfg.Symbols { + ldflags = append(ldflags, "-s") + } + if lib, exists := os.LookupEnv("GOPY_LIBDIR"); exists { + ldflags = append(ldflags, "-L"+filepath.ToSlash(lib)) + } + if libname, exists := os.LookupEnv("GOPY_PYLIB"); exists { + ldflags = append(ldflags, "-l"+filepath.ToSlash(libname)) + } + + removeEmpty := func(src []string) []string { + o := make([]string, 0, len(src)) + for _, v := range src { + if v == "" { + continue + } + o = append(o, v) + } + return o + } + + cflags = removeEmpty(cflags) + ldflags = removeEmpty(ldflags) + + cflagsEnv := fmt.Sprintf("CGO_CFLAGS=%s", strings.Join(cflags, " ")) + ldflagsEnv := fmt.Sprintf("CGO_LDFLAGS=%s", strings.Join(ldflags, " ")) + + env = os.Environ() + env = append(env, cflagsEnv) + env = append(env, ldflagsEnv) + + fmt.Println(cflagsEnv) + fmt.Println(ldflagsEnv) + return args, env, err +} diff --git a/cmd_exe.go b/cmd_exe.go index 57c2a65..0064f1c 100644 --- a/cmd_exe.go +++ b/cmd_exe.go @@ -11,9 +11,9 @@ import ( "path/filepath" "strings" - "github.com/rudderlabs/gopy/bind" "github.com/gonuts/commander" "github.com/gonuts/flag" + "github.com/rudderlabs/gopy/bind" ) // python packaging links: diff --git a/cmd_gen.go b/cmd_gen.go index 675de09..660cc0c 100644 --- a/cmd_gen.go +++ b/cmd_gen.go @@ -8,9 +8,9 @@ import ( "fmt" "log" - "github.com/rudderlabs/gopy/bind" "github.com/gonuts/commander" "github.com/gonuts/flag" + "github.com/rudderlabs/gopy/bind" ) func gopyMakeCmdGen() *commander.Command { diff --git a/cmd_pkg.go b/cmd_pkg.go index 76befd0..38d92e9 100644 --- a/cmd_pkg.go +++ b/cmd_pkg.go @@ -11,9 +11,9 @@ import ( "path/filepath" "strings" - "github.com/rudderlabs/gopy/bind" "github.com/gonuts/commander" "github.com/gonuts/flag" + "github.com/rudderlabs/gopy/bind" ) // python packaging links: diff --git a/goerr2pyex/error_translator.go b/goerr2pyex/error_translator.go index 20a4ba5..1174408 100644 --- a/goerr2pyex/error_translator.go +++ b/goerr2pyex/error_translator.go @@ -14,5 +14,5 @@ func DefaultGoErrorToPyException(err error) bool { return true } else { return false - } + } } diff --git a/main_darwin.go b/main_darwin.go index 8697476..5edc920 100644 --- a/main_darwin.go +++ b/main_darwin.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build darwin // +build darwin package main diff --git a/main_test.go b/main_test.go index 03198eb..c320faf 100644 --- a/main_test.go +++ b/main_test.go @@ -61,11 +61,20 @@ func init() { } func TestGovet(t *testing.T) { + buildCmd := gopyMakeCmdBuild() + buildCfg := NewBuildCfg(&buildCmd.Flag) + buildCfg.VM = testBackends["py3"] + buildArgs, buildEnv, err := getBuildArgsAndEnv(buildCfg) + if err != nil { + t.Fatalf("error building env:%v.\n Args:%v.\n Env:%v\n", err, buildArgs, buildEnv) + } + cmd := exec.Command("go", "vet", "./...") buf := new(bytes.Buffer) cmd.Stdout = buf cmd.Stderr = buf - err := cmd.Run() + cmd.Env = buildEnv + err = cmd.Run() if err != nil { t.Fatalf("error running %s:\n%s\n%v", "go vet", string(buf.Bytes()), err) } diff --git a/main_unix.go b/main_unix.go index fcd1871..1acf66f 100644 --- a/main_unix.go +++ b/main_unix.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build (linux && !android) || dragonfly || openbsd // +build linux,!android dragonfly openbsd package main diff --git a/main_unix_test.go b/main_unix_test.go index 8d0733a..0d709f7 100644 --- a/main_unix_test.go +++ b/main_unix_test.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build !windows // +build !windows package main From 9020e61e067afd17580ad7d6971ac5215269e9fb Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Tue, 28 Jun 2022 13:27:44 +0530 Subject: [PATCH 18/20] Crash in gen command fixed. --- cmd_build.go | 1 + cmd_exe.go | 1 + cmd_pkg.go | 1 + main.go | 1 - 4 files changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd_build.go b/cmd_build.go index d6a1547..7ab4361 100644 --- a/cmd_build.go +++ b/cmd_build.go @@ -52,6 +52,7 @@ func gopyRunCmdBuild(cmdr *commander.Command, args []string) error { } cfg := NewBuildCfg(&cmdr.Flag) + cfg.Symbols = cmdr.Flag.Lookup("symbols").Value.Get().(bool) bind.NoWarn = cfg.NoWarn bind.NoMake = cfg.NoMake diff --git a/cmd_exe.go b/cmd_exe.go index 0064f1c..1f0dd6d 100644 --- a/cmd_exe.go +++ b/cmd_exe.go @@ -65,6 +65,7 @@ func gopyRunCmdExe(cmdr *commander.Command, args []string) error { } cfg := NewBuildCfg(&cmdr.Flag) + cfg.Symbols = cmdr.Flag.Lookup("symbols").Value.Get().(bool) var ( exclude = cmdr.Flag.Lookup("exclude").Value.Get().(string) diff --git a/cmd_pkg.go b/cmd_pkg.go index 38d92e9..ba91330 100644 --- a/cmd_pkg.go +++ b/cmd_pkg.go @@ -63,6 +63,7 @@ func gopyRunCmdPkg(cmdr *commander.Command, args []string) error { } cfg := NewBuildCfg(&cmdr.Flag) + cfg.Symbols = cmdr.Flag.Lookup("symbols").Value.Get().(bool) var ( exclude = cmdr.Flag.Lookup("exclude").Value.Get().(string) diff --git a/main.go b/main.go index a4b87b1..6c722bd 100644 --- a/main.go +++ b/main.go @@ -45,7 +45,6 @@ func NewBuildCfg(flagSet *flag.FlagSet) *BuildCfg { cfg.NoWarn = flagSet.Lookup("no-warn").Value.Get().(bool) cfg.NoMake = flagSet.Lookup("no-make").Value.Get().(bool) cfg.PkgPrefix = flagSet.Lookup("package-prefix").Value.Get().(string) - cfg.Symbols = flagSet.Lookup("symbols").Value.Get().(bool) cfg.NoPyExceptions = flagSet.Lookup("no-exceptions").Value.Get().(bool) cfg.ModPathGoErr2PyEx = flagSet.Lookup("gomod-2pyex").Value.Get().(string) cfg.UsePyTuple4VE = flagSet.Lookup("use-pytuple-4ve").Value.Get().(bool) From a44f1e522bd3234666387e28e3c9e1da44cd076a Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Tue, 28 Jun 2022 14:06:59 +0530 Subject: [PATCH 19/20] gopyerrors is not applicable any more. --- _examples/gopyerrors/gopyerrors.go | 29 ----------------------------- _examples/gopyerrors/test.py | 13 ------------- main_test.go | 28 ---------------------------- 3 files changed, 70 deletions(-) delete mode 100644 _examples/gopyerrors/gopyerrors.go delete mode 100644 _examples/gopyerrors/test.py diff --git a/_examples/gopyerrors/gopyerrors.go b/_examples/gopyerrors/gopyerrors.go deleted file mode 100644 index fbec63d..0000000 --- a/_examples/gopyerrors/gopyerrors.go +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2020 The go-python Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// package gopyerrors contains functions that generate error -// messages from gopy itself. -package gopyerrors - -func NotErrorMany() (int, int) { - return 0, 0 -} - -func TooMany() (int, int, string) { - return 0, 1, "Hi" -} - -func OK() (int, error) { - return 0, nil -} - -type Struct struct{} - -func (s *Struct) NotErrorMany() (int, string) { - return 0, "Hi" -} - -func (s *Struct) TooMany() (int, int, string) { - return 0, 1, "Hi" -} diff --git a/_examples/gopyerrors/test.py b/_examples/gopyerrors/test.py deleted file mode 100644 index eb6e169..0000000 --- a/_examples/gopyerrors/test.py +++ /dev/null @@ -1,13 +0,0 @@ -# Copyright 2020 go-python Authors. All rights reserved. -# Use of this source code is governed by a BSD-style -# license that can be found in the LICENSE file. - -# py2/py3 compat -from __future__ import print_function - -import gopyerrors - -# This is empty, its only purpose is to have a test that catches -# errors generated by the gopy itself. - -print("OK") diff --git a/main_test.go b/main_test.go index c320faf..0a3a1a5 100644 --- a/main_test.go +++ b/main_test.go @@ -109,34 +109,6 @@ func TestGofmt(t *testing.T) { } } -func TestGoPyErrors(t *testing.T) { - pyvm := testBackends["py3"] - workdir, err := ioutil.TempDir("", "gopy-") - if err != nil { - t.Fatalf("could not create workdir: %v\n", err) - } - t.Logf("pyvm: %s making work dir: %s\n", pyvm, workdir) - defer os.RemoveAll(workdir) - - curPkgPath := reflect.TypeOf(pkg{}).PkgPath() - fpath := filepath.Join(curPkgPath, "_examples/gopyerrors") - cmd := exec.Command("go", "run", ".", "gen", "-vm="+pyvm, "-output="+workdir, fpath) - t.Logf("running: %v\n", cmd.Args) - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("could not run %v: %+v\n", strings.Join(cmd.Args, " "), err) - } - contains := `--- Processing package: github.com/rudderlabs/gopy/_examples/gopyerrors --- -ignoring python incompatible function: .func github.com/rudderlabs/gopy/_examples/gopyerrors.NotErrorMany() (int, int): func() (int, int): gopy: second result value must be of type error: func() (int, int) -ignoring python incompatible method: gopyerrors.func (*github.com/rudderlabs/gopy/_examples/gopyerrors.Struct).NotErrorMany() (int, string): func() (int, string): gopy: second result value must be of type error: func() (int, string) -ignoring python incompatible method: gopyerrors.func (*github.com/rudderlabs/gopy/_examples/gopyerrors.Struct).TooMany() (int, int, string): func() (int, int, string): gopy: too many results to return: func() (int, int, string) -ignoring python incompatible function: .func github.com/rudderlabs/gopy/_examples/gopyerrors.TooMany() (int, int, string): func() (int, int, string): gopy: too many results to return: func() (int, int, string) -` - if got, want := string(out), contains; !strings.Contains(got, want) { - t.Fatalf("%v does not contain\n%v\n", got, want) - } -} - func TestHi(t *testing.T) { // t.Parallel() path := "_examples/hi" From 35cda9d47171a0fb402ad1b6705098aaabfe4d30 Mon Sep 17 00:00:00 2001 From: Nishant Sharma Date: Tue, 28 Jun 2022 14:41:20 +0530 Subject: [PATCH 20/20] Test failure bug fixed. --- bind/gen_func.go | 2 +- cmd_build.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bind/gen_func.go b/bind/gen_func.go index 1ff4a29..43f267f 100644 --- a/bind/gen_func.go +++ b/bind/gen_func.go @@ -194,7 +194,7 @@ func (g *pyGen) genFuncSig(sym *symbol, fsym *Func) bool { gorets[i] = sret.cgoname pyrets[i] = "'" + sret.cpyname + "'" if sret.cpyname == "PyObject*" { - ownership = "caller_owns_return=True" + ownership = ", caller_owns_return=True" } } diff --git a/cmd_build.go b/cmd_build.go index 7ab4361..da03d2c 100644 --- a/cmd_build.go +++ b/cmd_build.go @@ -145,7 +145,7 @@ func runBuild(mode bind.BuildMode, cfg *BuildCfg) error { } else { // build extension with go + c - env, args, err := getBuildArgsAndEnv(cfg) + args, env, err := getBuildArgsAndEnv(cfg) if err != nil { fmt.Printf("error building environment: %v\n", err) return err