paleolimbot commented on code in PR #531:
URL: https://github.com/apache/arrow-nanoarrow/pull/531#discussion_r1645259233
##########
src/nanoarrow/nanoarrow_types.h:
##########
@@ -32,6 +32,22 @@
extern "C" {
#endif
+#if defined _WIN32 || defined __CYGWIN__
+#if defined(NANOARROW_BUILD_DLL)
+#define NANOARROW_EXPORT __declspec(dllexport)
+#elif defined(NANOARROW_CONSUME_DLL)
+#define NANOARROW_EXPORT __declspec(dllimport)
+#else
+#define NANOARROW_EXPORT
+#endif // defined(NANOARROW_BUILD_DLL)
+#else
+#if __GNUC__ >= 4
+#define NANOARROW_EXPORT __attribute__((visibility("default")))
+#else
+#define NANOARROW_EXPORT
+#endif // __GNUC__ >= 4
Review Comment:
Got it! I'd forgotten that we build shared libraries with C++ sources
(currently just the integration test, where symbol visibility doesn't really
matter, but perhaps soon nanoarrow_testing and nanoarrow_device with Metal).
Basically, we want symbols for header-only C++ things to not make it in the
final .so (only the final C FFI).
Since the GCC attribute is currently just a C++ issue scoped to the
integration test, that is probably the best place to put this (e.g., your first
commit 😬 ) and we can sort it out for other places that need this later.
A reproducer including both C and C++:
```shell
(.venv) deweydunnington@Deweys-Air-2 testvis % nm libtestlib_shared.so
--demangle
0000000000003f34 T _VisibilityDefault
0000000000003f20 t _VisibilityHidden
0000000000003eec T _VisibilityNoAttribute
0000000000003f00 t VisibilityStatic()
0000000000003f68 t VisibilityStaticInline()
0000000000003f48 T SomeInlineClass::SomeInlineMethod()
U _printf
(.venv) deweydunnington@Deweys-Air-2 testvis % g++ -c -fvisibility=hidden
testlib.cc
(.venv) deweydunnington@Deweys-Air-2 testvis % g++ -shared -o
libtestlib_shared.so testlib.o
(.venv) deweydunnington@Deweys-Air-2 testvis % nm libtestlib_shared.so
--demangle
0000000000003f40 T _VisibilityDefault
0000000000003f2c t _VisibilityHidden
0000000000003ef8 t _VisibilityNoAttribute
0000000000003f0c t VisibilityStatic()
0000000000003f74 t VisibilityStaticInline()
0000000000003f54 t SomeInlineClass::SomeInlineMethod()
U _printf
```
<details>
testlib.h
```c
#include <stdio.h>
extern "C" {
static inline void VisibilityStaticInline(void) {
printf("something");
}
void VisibilityNoAttribute(void);
__attribute__((visibility("hidden"))) void VisibilityHidden(void);
__attribute__((visibility("default"))) void VisibilityDefault(void);
}
```
testlib.cc
```c
#include <cstdio>
#include "testlib.h"
class SomeInlineClass {
public:
void SomeInlineMethod() {
VisibilityStaticInline();
}
};
static void VisibilityStatic(void) {
SomeInlineClass cls;
cls.SomeInlineMethod();
}
void VisibilityNoAttribute(void) {
VisibilityStatic();
}
__attribute__((visibility("hidden"))) void VisibilityHidden(void) {
VisibilityNoAttribute();
}
__attribute__((visibility("default"))) void VisibilityDefault(void) {
VisibilityNoAttribute();
}
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
