From f533d2066dee6b2d4b6a050889f37bceeb5d0872 Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Thu, 16 Jan 2025 11:27:45 -0500 Subject: [PATCH] fix(arrow/cdata): move headers into parent Go package (#251) ### Rationale for this change fixes #244, also addresses the issue discovered by https://github.com/apache/arrow-adbc/pull/2449#discussion_r1915765617 ### What changes are included in this PR? Moving the `abi.h` and `helpers.h` headers up into the parent `cdata` Go package. Also adding `static inline` to the helper methods to fix undefined symbol issues when building with `CGO_CFLAGS=-g3`. ### Are these changes tested? Yes, existing unit tests will cover the changes. ### Are there any user-facing changes? No --- arrow/cdata/{arrow/c => }/abi.h | 0 arrow/cdata/cdata.go | 4 +-- arrow/cdata/cdata_allocate.go | 2 +- arrow/cdata/cdata_exports.go | 4 +-- arrow/cdata/cdata_fulltest.c | 4 +-- arrow/cdata/cdata_test_framework.go | 4 +-- arrow/cdata/exports.go | 4 +-- arrow/cdata/{arrow/c => }/helpers.h | 42 ++++++++++++++--------------- arrow/cdata/import_allocator.go | 2 +- arrow/cdata/trampoline.c | 2 +- 10 files changed, 34 insertions(+), 34 deletions(-) rename arrow/cdata/{arrow/c => }/abi.h (100%) rename arrow/cdata/{arrow/c => }/helpers.h (74%) diff --git a/arrow/cdata/arrow/c/abi.h b/arrow/cdata/abi.h similarity index 100% rename from arrow/cdata/arrow/c/abi.h rename to arrow/cdata/abi.h diff --git a/arrow/cdata/cdata.go b/arrow/cdata/cdata.go index 4361e6f8..86f2b50e 100644 --- a/arrow/cdata/cdata.go +++ b/arrow/cdata/cdata.go @@ -21,8 +21,8 @@ package cdata // implement handling of the Arrow C Data Interface. At least from a consuming side. -// #include "arrow/c/abi.h" -// #include "arrow/c/helpers.h" +// #include "abi.h" +// #include "helpers.h" // #include // int stream_get_schema(struct ArrowArrayStream* st, struct ArrowSchema* out) { return st->get_schema(st, out); } // int stream_get_next(struct ArrowArrayStream* st, struct ArrowArray* out) { return st->get_next(st, out); } diff --git a/arrow/cdata/cdata_allocate.go b/arrow/cdata/cdata_allocate.go index da0bd957..f9220802 100644 --- a/arrow/cdata/cdata_allocate.go +++ b/arrow/cdata/cdata_allocate.go @@ -19,7 +19,7 @@ package cdata // #include -// #include "arrow/c/abi.h" +// #include "abi.h" import "C" import ( diff --git a/arrow/cdata/cdata_exports.go b/arrow/cdata/cdata_exports.go index da7db6c2..ea145c01 100644 --- a/arrow/cdata/cdata_exports.go +++ b/arrow/cdata/cdata_exports.go @@ -19,8 +19,8 @@ package cdata // #include // #include // #include -// #include "arrow/c/abi.h" -// #include "arrow/c/helpers.h" +// #include "abi.h" +// #include "helpers.h" // // extern void releaseExportedSchema(struct ArrowSchema* schema); // extern void releaseExportedArray(struct ArrowArray* array); diff --git a/arrow/cdata/cdata_fulltest.c b/arrow/cdata/cdata_fulltest.c index 4291cfff..8fd724d4 100644 --- a/arrow/cdata/cdata_fulltest.c +++ b/arrow/cdata/cdata_fulltest.c @@ -23,8 +23,8 @@ #include #include #include -#include "arrow/c/abi.h" -#include "arrow/c/helpers.h" +#include "abi.h" +#include "helpers.h" #include "utils.h" int is_little_endian() diff --git a/arrow/cdata/cdata_test_framework.go b/arrow/cdata/cdata_test_framework.go index ad6b9118..d7b12f6d 100644 --- a/arrow/cdata/cdata_test_framework.go +++ b/arrow/cdata/cdata_test_framework.go @@ -22,8 +22,8 @@ package cdata // #include // #include // #include -// #include "arrow/c/abi.h" -// #include "arrow/c/helpers.h" +// #include "abi.h" +// #include "helpers.h" // // void setup_array_stream_test(const int n_batches, struct ArrowArrayStream* out); // static struct ArrowArray* get_test_arr() { diff --git a/arrow/cdata/exports.go b/arrow/cdata/exports.go index d70305cf..4cacb2a1 100644 --- a/arrow/cdata/exports.go +++ b/arrow/cdata/exports.go @@ -28,8 +28,8 @@ import ( // #include // #include -// #include "arrow/c/abi.h" -// #include "arrow/c/helpers.h" +// #include "abi.h" +// #include "helpers.h" // // typedef const char cchar_t; // extern int streamGetSchema(struct ArrowArrayStream*, struct ArrowSchema*); diff --git a/arrow/cdata/arrow/c/helpers.h b/arrow/cdata/helpers.h similarity index 74% rename from arrow/cdata/arrow/c/helpers.h rename to arrow/cdata/helpers.h index 6e4df17f..c129e15b 100644 --- a/arrow/cdata/arrow/c/helpers.h +++ b/arrow/cdata/helpers.h @@ -22,7 +22,7 @@ #include #include -#include "arrow/c/abi.h" +#include "abi.h" #define ARROW_C_ASSERT(condition, msg) \ do { \ @@ -37,12 +37,12 @@ extern "C" { #endif /// Query whether the C schema is released -inline int ArrowSchemaIsReleased(const struct ArrowSchema* schema) { +static inline int ArrowSchemaIsReleased(const struct ArrowSchema* schema) { return schema->release == NULL; } /// Mark the C schema released (for use in release callbacks) -inline void ArrowSchemaMarkReleased(struct ArrowSchema* schema) { +static inline void ArrowSchemaMarkReleased(struct ArrowSchema* schema) { schema->release = NULL; } @@ -50,7 +50,7 @@ inline void ArrowSchemaMarkReleased(struct ArrowSchema* schema) { /// /// Note `dest` must *not* point to a valid schema already, otherwise there /// will be a memory leak. -inline void ArrowSchemaMove(struct ArrowSchema* src, struct ArrowSchema* dest) { +static inline void ArrowSchemaMove(struct ArrowSchema* src, struct ArrowSchema* dest) { assert(dest != src); assert(!ArrowSchemaIsReleased(src)); memcpy(dest, src, sizeof(struct ArrowSchema)); @@ -58,7 +58,7 @@ inline void ArrowSchemaMove(struct ArrowSchema* src, struct ArrowSchema* dest) { } /// Release the C schema, if necessary, by calling its release callback -inline void ArrowSchemaRelease(struct ArrowSchema* schema) { +static inline void ArrowSchemaRelease(struct ArrowSchema* schema) { if (!ArrowSchemaIsReleased(schema)) { schema->release(schema); ARROW_C_ASSERT(ArrowSchemaIsReleased(schema), @@ -67,18 +67,18 @@ inline void ArrowSchemaRelease(struct ArrowSchema* schema) { } /// Query whether the C array is released -inline int ArrowArrayIsReleased(const struct ArrowArray* array) { +static inline int ArrowArrayIsReleased(const struct ArrowArray* array) { return array->release == NULL; } -inline int ArrowDeviceArrayIsReleased(const struct ArrowDeviceArray* array) { +static inline int ArrowDeviceArrayIsReleased(const struct ArrowDeviceArray* array) { return ArrowArrayIsReleased(&array->array); } /// Mark the C array released (for use in release callbacks) -inline void ArrowArrayMarkReleased(struct ArrowArray* array) { array->release = NULL; } +static inline void ArrowArrayMarkReleased(struct ArrowArray* array) { array->release = NULL; } -inline void ArrowDeviceArrayMarkReleased(struct ArrowDeviceArray* array) { +static inline void ArrowDeviceArrayMarkReleased(struct ArrowDeviceArray* array) { ArrowArrayMarkReleased(&array->array); } @@ -86,14 +86,14 @@ inline void ArrowDeviceArrayMarkReleased(struct ArrowDeviceArray* array) { /// /// Note `dest` must *not* point to a valid array already, otherwise there /// will be a memory leak. -inline void ArrowArrayMove(struct ArrowArray* src, struct ArrowArray* dest) { +static inline void ArrowArrayMove(struct ArrowArray* src, struct ArrowArray* dest) { assert(dest != src); assert(!ArrowArrayIsReleased(src)); memcpy(dest, src, sizeof(struct ArrowArray)); ArrowArrayMarkReleased(src); } -inline void ArrowDeviceArrayMove(struct ArrowDeviceArray* src, +static inline void ArrowDeviceArrayMove(struct ArrowDeviceArray* src, struct ArrowDeviceArray* dest) { assert(dest != src); assert(!ArrowDeviceArrayIsReleased(src)); @@ -102,7 +102,7 @@ inline void ArrowDeviceArrayMove(struct ArrowDeviceArray* src, } /// Release the C array, if necessary, by calling its release callback -inline void ArrowArrayRelease(struct ArrowArray* array) { +static inline void ArrowArrayRelease(struct ArrowArray* array) { if (!ArrowArrayIsReleased(array)) { array->release(array); ARROW_C_ASSERT(ArrowArrayIsReleased(array), @@ -110,7 +110,7 @@ inline void ArrowArrayRelease(struct ArrowArray* array) { } } -inline void ArrowDeviceArrayRelease(struct ArrowDeviceArray* array) { +static inline void ArrowDeviceArrayRelease(struct ArrowDeviceArray* array) { if (!ArrowDeviceArrayIsReleased(array)) { array->array.release(&array->array); ARROW_C_ASSERT(ArrowDeviceArrayIsReleased(array), @@ -119,20 +119,20 @@ inline void ArrowDeviceArrayRelease(struct ArrowDeviceArray* array) { } /// Query whether the C array stream is released -inline int ArrowArrayStreamIsReleased(const struct ArrowArrayStream* stream) { +static inline int ArrowArrayStreamIsReleased(const struct ArrowArrayStream* stream) { return stream->release == NULL; } -inline int ArrowDeviceArrayStreamIsReleased(const struct ArrowDeviceArrayStream* stream) { +static inline int ArrowDeviceArrayStreamIsReleased(const struct ArrowDeviceArrayStream* stream) { return stream->release == NULL; } /// Mark the C array stream released (for use in release callbacks) -inline void ArrowArrayStreamMarkReleased(struct ArrowArrayStream* stream) { +static inline void ArrowArrayStreamMarkReleased(struct ArrowArrayStream* stream) { stream->release = NULL; } -inline void ArrowDeviceArrayStreamMarkReleased(struct ArrowDeviceArrayStream* stream) { +static inline void ArrowDeviceArrayStreamMarkReleased(struct ArrowDeviceArrayStream* stream) { stream->release = NULL; } @@ -140,7 +140,7 @@ inline void ArrowDeviceArrayStreamMarkReleased(struct ArrowDeviceArrayStream* st /// /// Note `dest` must *not* point to a valid stream already, otherwise there /// will be a memory leak. -inline void ArrowArrayStreamMove(struct ArrowArrayStream* src, +static inline void ArrowArrayStreamMove(struct ArrowArrayStream* src, struct ArrowArrayStream* dest) { assert(dest != src); assert(!ArrowArrayStreamIsReleased(src)); @@ -148,7 +148,7 @@ inline void ArrowArrayStreamMove(struct ArrowArrayStream* src, ArrowArrayStreamMarkReleased(src); } -inline void ArrowDeviceArrayStreamMove(struct ArrowDeviceArrayStream* src, +static inline void ArrowDeviceArrayStreamMove(struct ArrowDeviceArrayStream* src, struct ArrowDeviceArrayStream* dest) { assert(dest != src); assert(!ArrowDeviceArrayStreamIsReleased(src)); @@ -157,7 +157,7 @@ inline void ArrowDeviceArrayStreamMove(struct ArrowDeviceArrayStream* src, } /// Release the C array stream, if necessary, by calling its release callback -inline void ArrowArrayStreamRelease(struct ArrowArrayStream* stream) { +static inline void ArrowArrayStreamRelease(struct ArrowArrayStream* stream) { if (!ArrowArrayStreamIsReleased(stream)) { stream->release(stream); ARROW_C_ASSERT(ArrowArrayStreamIsReleased(stream), @@ -165,7 +165,7 @@ inline void ArrowArrayStreamRelease(struct ArrowArrayStream* stream) { } } -inline void ArrowDeviceArrayStreamRelease(struct ArrowDeviceArrayStream* stream) { +static inline void ArrowDeviceArrayStreamRelease(struct ArrowDeviceArrayStream* stream) { if (!ArrowDeviceArrayStreamIsReleased(stream)) { stream->release(stream); ARROW_C_ASSERT(ArrowDeviceArrayStreamIsReleased(stream), diff --git a/arrow/cdata/import_allocator.go b/arrow/cdata/import_allocator.go index ccea91c8..ba5edf40 100644 --- a/arrow/cdata/import_allocator.go +++ b/arrow/cdata/import_allocator.go @@ -23,7 +23,7 @@ import ( "github.com/apache/arrow-go/v18/arrow/internal/debug" ) -// #include "arrow/c/helpers.h" +// #include "helpers.h" // #include import "C" diff --git a/arrow/cdata/trampoline.c b/arrow/cdata/trampoline.c index 364010b7..dc0451b3 100644 --- a/arrow/cdata/trampoline.c +++ b/arrow/cdata/trampoline.c @@ -16,7 +16,7 @@ #include -#include "arrow/c/abi.h" +#include "abi.h" int streamGetSchema(struct ArrowArrayStream*, struct ArrowSchema*); int streamGetNext(struct ArrowArrayStream*, struct ArrowArray*);