Skip to content

Export the implementations of the interfaces that are exported - #463

Merged
martin-s-a merged 2 commits into
mainfrom
export-the-polymorphic-implementations
Sep 20, 2026
Merged

martin-s-a merged 2 commits into
mainfrom
export-the-polymorphic-implementations

Conversation

@martin-s-a

Copy link
Copy Markdown
Collaborator

Four classes carry REXLIB_API on their methods but not on the class itself,
so their typeinfo never leaves the shared object:

  • thread_pool_executor and synchronous_executor
  • direct_image_reader_provider and caching_image_reader_provider

Their interfaces, executor and image_reader_provider, are both
REXLIB_API. The implementations are not, which leaves each hierarchy
exported halfway.

Why it has never shown

The library builds with hidden visibility, and a polymorphic class emits its
typeinfo in the translation unit that defines its first out-of-line virtual —
inside the library. Without REXLIB_API on the class, that symbol is HIDDEN
and nothing outside can link against it.

Ordinary C++ use never asks for it: constructing one of these, calling through
it, destroying it, all go through the methods, and those are exported one by
one. It only bites something that reaches for RTTI across the boundary, which
is what a language binding does — pybind11 calls typeid() on every type it
registers.

It surfaced in gigabit-clowns/rexlib-python#157, as a load-time failure of the
extension with no build warning ahead of it:

ImportError: _binding.cpython-314-x86_64-linux-gnu.so:
undefined symbol: _ZTIN6rexlib20thread_pool_executorE

which demangles to typeinfo for rexlib::thread_pool_executor.

The change, and the evidence

One keyword on each of the four classes. Measured on
thread_pool_executor.cpp, compiled with the same
-fvisibility=hidden -fvisibility-inlines-hidden the build uses:

_ZTIN6rexlib20thread_pool_executorE
before HIDDEN
after DEFAULT

The symbol was emitted either way; what changed is whether it crosses the
boundary.

image_source and image_batch_source are left alone deliberately. Neither
is polymorphic, so their typeinfo is a weak symbol any consumer emits for
itself, and nothing is missing.

The per-method REXLIB_API on these classes is now redundant but left in
place: image_read_format_manager and image_reader_provider both already
carry the class attribute and repeat it on members, so removing it here would
be a larger diff against a convention this does not settle.

🤖 Generated with Claude Code

The library builds with hidden visibility, and a polymorphic class emits its
typeinfo where its first out-of-line virtual is defined, which is inside the
library. Without REXLIB_API on the class that typeinfo stays hidden, so a
consumer that reaches for RTTI on one of these finds nothing to link against.

Their interfaces, executor and image_reader_provider, already carry it; the
implementations did not, which makes each hierarchy exported halfway. Ordinary
use never notices, since it needs no RTTI, but anything building a type map
over the boundary does.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@martin-s-a martin-s-a added the bug Something isn't working label Sep 20, 2026
@martin-s-a martin-s-a self-assigned this Sep 20, 2026
@martin-s-a
martin-s-a enabled auto-merge (squash) September 20, 2026 19:04
oierlauzi
oierlauzi previously approved these changes Sep 20, 2026
A class carrying the dll interface may not repeat it on its members: MSVC
rejects that with C2487, which the first commit hit on every member of the
four. An exported class here does not repeat it — completion and
image_reader_provider both leave their members bare — so these follow.

The std members a dll-interface class holds then warn with C4251, and
REXLIB_STD_MEMBER_INTERFACE silences each one where it sits.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@oierlauzi oierlauzi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be needed, as the constructor and destructor of these classes has REXLIB_API. If there is an obscure reason for having it in the class declaration, remove it from the constructor, destructor and other member methods. Also add REXLIB_STD_MEMBER to the implementation pointer to avoid MSVC warnings

@martin-s-a

Copy link
Copy Markdown
Collaborator Author

Both of those are in b877466, which landed about half a minute before the
review — the member REXLIB_API is gone from all four and the implementation
pointers carry REXLIB_STD_MEMBER_INTERFACE. MSVC forced exactly that: with
the attribute on the class it rejects repeating it on members with C2487, and
then C4251 arrives for the std members.

On whether the class attribute is needed at all — it is, and the reason is
that it is not a method that is missing.

An exported constructor and destructor let a caller build one and destroy it,
which is all ordinary C++ use asks for. Typeinfo is a separate symbol, and
for a polymorphic class it is emitted in the translation unit that defines the
first out-of-line virtual, carrying the class's visibility rather than any
member's. Under -fvisibility=hidden and with no attribute on the class, it
is emitted HIDDEN and nothing outside the library can link it.

That is what broke, and it broke at load time rather than at build time:

ImportError: _binding.cpython-314-x86_64-linux-gnu.so:
undefined symbol: _ZTIN6rexlib20thread_pool_executorE

_ZTIN6rexlib20thread_pool_executorE demangles to
typeinfo for rexlib::thread_pool_executor — no method is missing from that
link, only the type's identity. pybind11 asks for it because it calls
typeid() on every type it registers, to key its type map.

Measured on thread_pool_executor.cpp, compiled with the same
-fvisibility=hidden -fvisibility-inlines-hidden the build uses, changing
nothing but the one keyword:

_ZTIN6rexlib20thread_pool_executorE
members REXLIB_API, class bare HIDDEN
class REXLIB_API DEFAULT

The symbol is emitted either way, which is why nm shows it in both and only
its visibility tells them apart.

image_source and image_batch_source are deliberately untouched: neither is
polymorphic, so their typeinfo is a weak symbol each consumer emits for
itself and nothing has to cross.

@sonarqubecloud

Copy link
Copy Markdown

@martin-s-a
martin-s-a merged commit ce67271 into main Sep 20, 2026
13 checks passed
@martin-s-a
martin-s-a deleted the export-the-polymorphic-implementations branch September 20, 2026 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants