crawshaw / sqlite

Go SQLite3 driver
ISC License
571 stars 68 forks source link

Remove forward declarations to cgo exported funcs #146

Open iamcalledrob opened 5 months ago

iamcalledrob commented 5 months ago

The current source fails to build with clang on windows due to forward declarations.

It appears that cgo code within a go file shouldn't contain a forward declaration to a cgo exported function from within the same file.

This manifests as either a warning or an error on clang+windows due to dllexport. There is an open task on the golang repo to better document this: https://github.com/golang/go/issues/49721

Here's what happens if you try and build this package using clang (or zig cc) on windows.

In file included from _cgo_export.c:4:
cgo-gcc-export-header-prolog:49:34: error: redeclaration of 'go_sqlite_auth_tramp' cannot add 'dllexport' attribute
   49 | extern __declspec(dllexport) int go_sqlite_auth_tramp(GoUintptr id, int action, char* cArg1, char* cArg2, char* cDB, char* cTrigger);
      |                                  ^
auth.go:5:13: note: previous declaration is here
    5 |  extern int go_sqlite_auth_tramp(uintptr_t, int, char*, char*, char*, char*);
      |             ^
cgo-gcc-export-header-prolog:50:35: warning: redeclaration of 'func_tramp' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration]
   50 | extern __declspec(dllexport) void func_tramp(sqlite3_context* ctx, int n, sqlite3_value** valarray);
      |                                   ^
func.go:22:14: note: previous declaration is here
   22 |  extern void func_tramp(sqlite3_context*, int, sqlite3_value**);
      |              ^
cgo-gcc-export-header-prolog:51:35: warning: redeclaration of 'step_tramp' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration]
   51 | extern __declspec(dllexport) void step_tramp(sqlite3_context* ctx, int n, sqlite3_value** valarray);
      |                                   ^
func.go:23:14: note: previous declaration is here
   23 |  extern void step_tramp(sqlite3_context*, int, sqlite3_value**);
      |              ^
cgo-gcc-export-header-prolog:52:35: warning: redeclaration of 'final_tramp' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration]
   52 | extern __declspec(dllexport) void final_tramp(sqlite3_context* ctx);
      |                                   ^
func.go:24:14: note: previous declaration is here
   24 |  extern void final_tramp(sqlite3_context*);
      |              ^
cgo-gcc-export-header-prolog:54:34: warning: redeclaration of 'go_strm_w_tramp' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration]
   54 | extern __declspec(dllexport) int go_strm_w_tramp(GoUintptr pOut, char* pData, int n);
      |                                  ^
session.go:22:13: note: previous declaration is here
   22 |  extern int go_strm_w_tramp(uintptr_t, char*, int);
      |             ^
cgo-gcc-export-header-prolog:55:34: warning: redeclaration of 'go_strm_r_tramp' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration]
   55 | extern __declspec(dllexport) int go_strm_r_tramp(GoUintptr pIn, char* pData, int* pnData);
      |                                  ^
session.go:23:13: note: previous declaration is here
   23 |  extern int go_strm_r_tramp(uintptr_t, char*, int*);
      |             ^
cgo-gcc-export-header-prolog:58:35: error: redeclaration of 'log_fn' cannot add 'dllexport' attribute
   58 | extern __declspec(dllexport) void log_fn(void* _, int code, char* msg);
      |                                   ^
sqlite.go:55:14: note: previous declaration is here
   55 |  extern void log_fn(void* pArg, int code, char* msg);
      |              ^
5 warnings and 2 errors generated.

This PR:

  1. Moves the offending c trampolines into the wrappers.c/h files, which addresses this issue since there are longer forward declarations in the Go code. There is precedent for this, since it looks like this was already done for sometrampolines.
  2. Fixes a typo in the readme to use the correct CGO_LDFLAGS env var for the windows linker

Should fix the root issue in https://github.com/crawshaw/sqlite/issues/137

anacrolix commented 5 months ago

Feel free to submit over at https://github.com/go-llsqlite/crawshaw if you get no traction.

iamcalledrob commented 5 months ago

@anacrolix I submitted a PR in that repo already! https://github.com/go-llsqlite/crawshaw/pull/5