AVRO-4221: [C++] Allow using symbol visibility annotations also on non-Windows platforms

[stephanlachnit]

What is the purpose of the change

In GCC and clang the symbol visibility behavior of MSVC can be mirror using -fvisibility=hidden. This allows to more easily test that symbol visbility annotations work correctly and can potentially lead to smaller binaries. The default behavior on non-Windows platforms is not changed with this commit.

See also https://gcc.gnu.org/wiki/Visibility

Also fixes a tiny mistake where the _WIN32 macro was used to silence a MSVC warning where the _MSC_VER macro should have been used instead.

This also needs to be backported to 1.12.1, see mesonbuild/wrapdb#2270

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

This change added tests and can be verified as follows:

• run unit tests

Documentation

• Does this pull request introduce a new feature? no
• If yes, how is the feature documented? not applicable

[stephanlachnit]

The only thing I still need to verify that the tests actually work when compiling under Windows with MSYS2, in these logs you can see that avro::json::typeToString as a symbol is missing. I'm unsure why since the symbol is not in any headers, but I also don't see the issue of just exporting it.

[stephanlachnit]

Ok so after some testing, exporting avro::json::typeToString is required. Now this should be good to go.

/cc @martin-g @wgtmac @WillAyd how does it work with backporting? Should I open a PR again branch-1.12?

[martin-g]

/cc @martin-g @wgtmac @WillAyd how does it work with backporting? Should I open a PR again branch-1.12?

Someone will have to cherry-pick it. The best would be you - to make sure that it works as desired.

[stephanlachnit]

Someone will have to cherry-pick it. The best would be you - to make sure that it works as desired.

Sure, I'll do it then.

[martin-g]

LGTM

[stephanlachnit]

Seems like there are still some issues, e.g.:

ld.lld: error: undefined symbol: __declspec(dllimport) vtable for avro::istream

avro::istream is header-only, so it should not have the AVRO_DECL attribute.

For inlines, there are warning of similar nature:

'avro::GenericDatum::type' redeclared inline; 'dllimport' attribute ignored

I'm not quite sure what the solution is here. Probably just dropping the inline attribute as it doesn't do anything anyway.

[wgtmac]

Created apache/iceberg-cpp#421 to verify this PR.

[HuaHuaY]

It seems like I'm running into another visibility related issue.

The GenericDatum class uses std::any value_; to store some types, such as GenericRecord, and also provides a template method value, which internally calls std::any_cast to perform type conversion. My dynamic library calls value<::avro::GenericRecord>, and my dynamic library also provides an interface to receive a GenericDatum.

I write set_target_properties(xxx PROPERTIES C_VISIBILITY_PRESET hidden CXX_VISIBILITY_PRESET hidden VISIBILITY_INLINES_HIDDEN 1) to hidden visibility information in my dynamic library. Then my dynamic library internally stores a GenericRecord's typeinfo.

However, when a program using my dynamic library constructs a GenericRecord and passes it to me, std::any_cast will fail to convert the type due to inconsistency in typeinfo, even though both my library and avro are dynamic libraries.

I have no ideas on how to solve this problem, unless I remove the hidden visibility configuration.

[stephanlachnit]

It seems like I'm running into another visibility related issue.

Hm, I don't think I can help with this one.

[stephanlachnit]

I updated this with the latest test from mesonbuild/wrapdb#2270, with this one it links on all platforms and tests run also fine.