support DSO/DLL on all platforms - #75
Merged
Merged
Conversation
… review Fixes the items the earlier review verified but cut for its 15-finding cap: - Remove the dead has_id SFINAE detector (zero users anywhere); update the CLAUDE.md line that described it as load-bearing. - Consolidate the two near-identical MSVC>19.51 has_initialize/has_finalize workaround blocks into one BOOST_OPENMETHOD_DETAIL_HAS_STATIC_FN_MSVC_SAFE macro; the maintainer #pragma message now fires once per build instead of twice. - Drop registry::policy's unused Default template parameter and the get_policy_aux specialization it fed - undocumented, and unreachable since no caller ever supplied one. Rewrite has_policy_state as mp11::mp_valid. - fast_perfect_hash: reset min_value/max_value per attempt instead of per pass. A failed attempt's stale range was leaking into the eventually- successful one, contaminating hash_range() and oversizing the vptr vector (a real correctness bug, not just cosmetic). Also fix search_error.buckets reporting double the largest table actually tried (off-by-one from the for-loop's post-loop ++M). - Remove dead registry_make_dog/registry_initialize scaffolding from test/dynamic_loading/registry.cpp (confirmed zero call sites, including via dlopen). - Replace augment_methods()'s check-then-act find()/operator[] double lookup with a single try_emplace. - extensions.cpp doc snippet: move the 3 BOOST_ASSERT self-checks out of the rendered tag::content region (still compiled and run, just not displayed), and move `using namespace boost::openmethod;` inside the snippet so the displayed code actually compiles as shown. - Remove the stale "gcc-8 doesn't like CTAD" comment in registry::finalize: gcc-8/9 can't build this library at all any more (missing full <charconv> support required elsewhere), so the justification was moot. The harmless explicit-type workaround itself is kept. - Correct CMakeLists.txt comments claiming the _exereg_* dynamic_loading variants exercise lib_registry at runtime: it's built and link-configured for reverse linkage but never actually dlopened by main.cpp, only lib_method/lib_overrider are. Deliberately left alone: the duplicated test-file preambles across test_runtime_errors_*.cpp and the custom-RTTI tests carry genuine, easy-to- miss deliberate divergences (guard conditions, array-index offsets, registry configs) that a mechanical dedup would risk silently erasing; actually wiring the _exereg_* registry lib into the runtime test (rather than just correcting the comment) is a design decision, not a mechanical fix. Verified: full ctest suite (113/113) under CMake+gcc and CMake+clang, plus the complete b2 test suite, all passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # .github/workflows/ci.yml
The library shipped export/import macros only for the predefined registries
(the DEFAULT and INDIRECT pairs, triggered by #ifdef inside
default_registry.hpp). Sharing a *custom* registry's state across a shared
library boundary required hand-writing the explicit instantiation and the
matching `extern template` declaration.
Add a general, registry-parameterized pair in macros.hpp:
BOOST_OPENMETHOD_EXPORT_REGISTRY(REGISTRY) // one owning TU
BOOST_OPENMETHOD_IMPORT_REGISTRY(REGISTRY) // every client TU
They expand to registry_state<REGISTRY::registry_type> decorated with
BOOST_SYMBOL_EXPORT/IMPORT - token for token what the predefined pairs emit.
Unlike those, they take the registry type as an argument and are used at
namespace scope, after the registry's definition.
default_registry.hpp keeps its hand-written blocks: it sits below macros.hpp
in the include layering (macros.hpp pulls in core.hpp) and so cannot use them.
Update the Custom Registries section of shared_libraries.adoc and the
registry_state doc comment to use the macros instead of the raw snippet, add
reference pages for both, and list them in ref_macros.adoc.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
test/dynamic_loading covers libraries dlopen'd at runtime. The implicit case -
a shared library linked at build time, whose static constructors run before
main - was only covered by a doc example, which asserts nothing. The new
EXPORT_REGISTRY/IMPORT_REGISTRY macros had no test at all.
Add test/implicit_shared_libraries, with one directory per variant:
default_registry/ - default_registry, shared with the DEFAULT macro pair
custom_registry/ - default_registry::with<vptr_map<>>::without<type_hash>,
shared with the new general macro pair
In both, the shared library owns and exports the registry state and the
executable imports it: the natural direction for implicit linking, since the
exe already links the lib, and the only direction b2 can express on Windows.
The library registers the Animal/Dog overriders, the executable registers Cat.
The load-bearing assertion is lib_speak(cat) == "meow" - dispatch inside the
library reaching an overrider registered by the executable, which holds only
if both modules populated the same dispatch tables.
custom_registry/registry.hpp reproduces the snippet documented in
shared_libraries.adoc, so that snippet is now covered. Since with<vptr_map<>>
removes vptr_vector implicitly (by category), static_asserts over policy_list
verify that vptr_vector and fast_perfect_hash are really gone.
Wired into both build systems: add_subdirectory in test/CMakeLists.txt (no
Boost::dll needed) and build-project in test/Jamfile. Unlike dynamic_loading's
Jamfile, <local-visibility>global is deliberately not set - it would mask the
very failure the export/import macros exist to prevent.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The static, dynamic_shared and indirect tests in the shared_libs examples were registered with add_test() but their targets were never made dependencies of `tests`. They were reachable only through ALL, via the boost_openmethod-shared-all custom target. The result depended on which target was built: building ALL (as an IDE test explorer does) worked, while the ctest workflow - `cmake --build . --target tests && ctest` - left the executables unbuilt and all three tests reported "Not Run". Two of the three usually appeared to pass only because stale binaries from an earlier ALL build were still in the tree. Add the six targets (three executables and their shared libraries) to `tests`, matching what the parent examples CMakeLists already does for the targets it declares. `tests` is created in test/CMakeLists.txt, which the top level adds before this directory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
BOOST_OPENMETHOD_EXPORT_REGISTRY expands to an explicit instantiation *definition*, of which a program may contain only one, so it must not be placed in a header: every translation unit of the owning module that included that header would emit one. Within a single translation unit that is a hard "duplicate explicit instantiation" error; across translation units it is ill-formed, no diagnostic required - ELF toolchains merge the COMDAT quietly, so the mistake hides until the owning module gains a second translation unit. BOOST_OPENMETHOD_IMPORT_REGISTRY is an `extern template` declaration and may be repeated, so it stays in the header that defines the registry, guarded so the owning module's translation units skip it. Move the export out of the custom_registry test's registry.hpp into lib.cpp, and update the guidance to match: the Custom Registries section of shared_libraries.adoc now shows the header and the .cpp separately, both reference pages say where each macro belongs, and the macros.hpp and registry_state comments do the same. The export must also precede any use of the registry in its translation unit. A use instantiates the state first, and under -fvisibility=hidden that instantiation is module-local; BOOST_SYMBOL_EXPORT on a later explicit instantiation definition does not promote it. lib.cpp therefore includes registry.hpp, emits the export, and only then includes lib.hpp. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A translation unit of the owning module that does not itself emit the export
instantiates the registry state implicitly. Under -fvisibility=hidden that copy
is module-local, and since ELF merges COMDATs at the *most restrictive*
visibility, the merged symbol becomes local: the module builds and exports
nothing, and clients fail to link with an undefined reference to
registry_state<...>::st. Every owning module with more than one translation
unit was affected, including via the predefined-registry tokens.
The fix is an exported explicit instantiation *declaration* in every
translation unit of the owner, which both suppresses implicit instantiation and
pins the symbol to default visibility, with the definition in exactly one .cpp.
For custom registries this needs no new macro: BOOST_OPENMETHOD_EXPORT_REGISTRY
expands to an explicit instantiation definition, so prefixing it with `extern`
yields the declaration. The three placements are then
// header, every client TU
BOOST_OPENMETHOD_IMPORT_REGISTRY(my_registry);
// header, every TU of the owning module
extern BOOST_OPENMETHOD_EXPORT_REGISTRY(my_registry);
// exactly one .cpp of the owning module
BOOST_OPENMETHOD_EXPORT_REGISTRY(my_registry);
The predefined registries are driven by define-before-include tokens, where
that trick is unavailable, so they get a third token:
BOOST_OPENMETHOD_DECLARE_EXPORT_{DEFAULT,INDIRECT}_REGISTRY.
Add lib2.cpp to the custom_registry test, making its owning library multi-TU so
the path is covered: removing the `extern` declaration reproduces the undefined
reference. Update the guidance in shared_libraries.adoc, the reference pages,
macros.hpp, registry_state's comment and CLAUDE.md.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
extensions.cpp was shared by all three examples, textually #included by
static_extensions.cpp and indirect_extensions.cpp, each defining a different
export/import macro first. It therefore had to guess which macro it still
needed:
#if !defined(BOOST_OPENMETHOD_EXPORT_DEFAULT_REGISTRY) &&
!defined(BOOST_OPENMETHOD_IMPORT_INDIRECT_REGISTRY)
#define BOOST_OPENMETHOD_IMPORT_DEFAULT_REGISTRY
#endif
That is exactly backwards for a doc example: the one line a reader most needs
to see - which macro this module uses, and why - was the one obscured by
preprocessor conditionals and by the file's role changing with the includer.
Give each example its own directory and its own complete set of files:
implicit_linking/ animals.hpp main.cpp extensions.cpp
dynamic_loading/ animals.hpp main.cpp extensions.cpp
indirect_vptr/ animals.hpp main.cpp extensions.cpp
Every file now spells out its macro unconditionally, with a comment saying
which module owns the state and why, and reads top to bottom. The textual
#include of a .cpp is gone, as are static_extensions.cpp and
indirect_extensions.cpp. indirect_vptr/extensions.cpp also drops Tiger and
make_tiger, which its main.cpp never used - and whose assertion named
default_registry rather than the indirect registry actually in use.
Target and test names are unchanged. Each directory has its own CMakeLists.txt
and keeps the `tests` dependency, so the examples still build under the ctest
workflow. Update the include:: paths in shared_libraries.adoc and the pointer
in CLAUDE.md.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sharing a registry's state was controlled by two different mechanisms: six
define-before-include tokens for the predefined registries
(BOOST_OPENMETHOD_{EXPORT,DECLARE_EXPORT,IMPORT}_{DEFAULT,INDIRECT}_REGISTRY),
emitted from inside default_registry.hpp, and the general
BOOST_OPENMETHOD_{EXPORT,IMPORT}_REGISTRY pair for everything else.
Remove the tokens. The general pair takes the registry as an argument, so it
serves default_registry and indirect_registry just as well:
BOOST_OPENMETHOD_EXPORT_REGISTRY(boost::openmethod::default_registry);
The macros fully qualify everything they emit, so callers never have to open
namespace boost::openmethod. default_registry.hpp no longer emits any
instantiation of its own, and the whole API is now two macros, with `extern`
turning the export into the declaration that multi-TU owning modules need.
Convert the call sites: the dynamic_loading test's registry.hpp, the
implicit_shared_libraries/default_registry test, and the three doc examples.
Each example's animals.hpp now carries the conditional
#ifdef OWNS_REGISTRY_STATE
extern BOOST_OPENMETHOD_EXPORT_REGISTRY(...);
#else
BOOST_OPENMETHOD_IMPORT_REGISTRY(...);
#endif
with the plain definition in the owning .cpp. Every one of these modules has a
single translation unit, so the `extern` declaration is not strictly required -
it is shown because it *is* required as soon as an owning module has more than
one, and the examples are what readers copy. The comment says so.
Delete the four now-obsolete reference pages and their ref_macros rows.
Also correct a claim made in an earlier commit: the export does *not* have to
precede uses of the registry within its own translation unit. Checked with
readelf under -fvisibility=hidden, the definition carries default visibility
whether it appears before or after them; the earlier failure was caused solely
by a *second* translation unit instantiating the state implicitly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… the prose The examples are included verbatim into shared_libraries.adoc, so the comments explaining the export/import convention were rendered as part of every snippet - a dozen lines of prose in animals.hpp before the reader reached a single class, repeated in all three examples. Strip them. The example sources now show only code: the shared header carries the OWNS_REGISTRY_STATE-guarded declaration, the owning .cpp defines OWNS_REGISTRY_STATE and emits the definition, and nothing else needs saying in place. The explanation moves to the surrounding prose, where it is written once: how the guard selects the declaration, that the owner emits the definition in exactly one .cpp, and a TIP recording why the `extern` declaration is shown even though each example module has a single translation unit and so does not strictly need it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Static Linking section described the arrangement but included none of the sources, so the rendered page had nothing to look at - the reader had to go find doc/modules/ROOT/examples/shared_libs/implicit_linking/ themselves. Include all three files: animals.hpp, extensions.cpp (the library, which owns the state) and main.cpp (the executable, which imports it). Since Static Linking comes first, the explanation of the OWNS_REGISTRY_STATE convention and the TIP about the `extern` declaration move here from Dynamic Linking, which now refers back to them. The two examples share a byte-identical animals.hpp, so the dynamic section no longer repeats it and just notes that ownership is reversed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The indirect_vptr example picked its registry with a compiler switch, passed by CMake as target_compile_definitions on both targets. That put a fact the reader needs - which registry this program uses - in a build file the documentation never shows, and split it from the export/import macros in animals.hpp that depend on it. Define BOOST_OPENMETHOD_DEFAULT_REGISTRY in animals.hpp instead, before it includes <boost/openmethod.hpp>, next to the export/import declaration for the same registry. One header now settles both questions for every translation unit of both modules, and no target needs a compile definition. Show that header in the Indirect Vptrs section, which until now included only main.cpp, and drop the references to the -D switch and to the long-removed INDIRECT macro pair. Verified the registry actually changes: registry_state in the built executable instantiates a policy list ending in indirect_vptr, and virtual_ptrs created before the library is loaded still dispatch correctly after it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In the rendered documentation the `meet` overrider read as dead code:
auto p = BOOST_OPENMETHOD_TYPE(
meet, (virtual_ptr<Animal>, virtual_ptr<Animal>),
std::string)::next<fn>;
return "run"; // p never used
`p` was used - by two BOOST_ASSERTs sitting between `// end::content[]` and
`// tag::content[]`, which Asciidoctor strips from the snippet. So the reader
saw a variable computed and discarded, and `next`, the very thing the code was
meant to illustrate, never appeared in the output at all.
Give it a body that calls next and uses the result:
auto base = next(a, b);
return "do not " + base + ", run";
`next(a, b)` is the member form generated by BOOST_OPENMETHOD_OVERRIDE. It
forwards to the same method<...>::next<fn> the long spelling named, and is
already what every other example uses (next.cpp, hello_world.cpp, synopsis.cpp,
rolex/, headers_namespaces/).
Remove all nine BOOST_ASSERTs from the shared_libs examples, including the
static_vptr<Carnivore> checks in make_tiger() and the one in
dynamic_loading/main.cpp that was rendered into the docs unhidden. With them
gone, every mid-function tag marker pair goes too; only the outer pair remains
in each file. Nothing machine-checks these strings - the example tests are
plain exit-code add_test()s - so the printed output is free to change.
Update the trailing annotations from `// run` to `// do not greet, run` to
match. Verified by running all three examples: the new text appears, which is
proof next() reached the base overrider - across the module boundary in every
case, since that overrider lives in the executable and the caller in the
library.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tations
BOOST_OPENMETHOD_{EXPORT,IMPORT}_REGISTRY are removed. They cost more than they
returned, and CI showed why: three gcc jobs went red with
macros.hpp: error: type attributes ignored after type is already defined
[-Werror=attributes]
BOOST_OPENMETHOD_EXPORT_REGISTRY(custom_registry);
In the translation unit that defines the state, the shared header supplied an
attributed `extern` declaration and the .cpp an attributed definition. GCC
refuses to repeat the attribute; clang accepts it silently, so every local build
and every clang job stayed green.
The rule is that the visibility attribute belongs on the *declaration* and the
definition must carry none - which one token sequence, used in both positions,
cannot express. So write the three lines directly instead:
// header, every TU of a client module
extern template struct BOOST_SYMBOL_IMPORT
boost::openmethod::registry_state<R::registry_type>;
// header, every TU of the owning module
extern template struct BOOST_SYMBOL_EXPORT
boost::openmethod::registry_state<R::registry_type>;
// exactly one .cpp of the owning module - no attribute
template struct boost::openmethod::registry_state<R::registry_type>;
They are fully qualified, so nothing has to be added to namespace
boost::openmethod. Convert every call site: the three doc examples, both
implicit_shared_libraries tests and dynamic_loading/registry.hpp. Delete the two
reference pages and their ref_macros rows; rewrite the shared_libraries.adoc
intro, Custom Registries and Indirect Vptrs sections around the instantiations,
with warnings for both failure modes - repeating the attribute (a hard error on
GCC only) and omitting the declaration in a second owner TU (a link failure
under -fvisibility=hidden only).
Also restructure the implicit_linking example so the library provides the class
hierarchy and the default behaviour while the *program* extends it: Tiger and
the run/hunt overriders move to main.cpp, the class registrations and the
catch-all greet overrider to extensions.cpp. make_tiger and its extern "C"
export/import pair disappear, since Tiger is now local to the program. The
printed output is unchanged, but next() now crosses the boundary in the other
direction - the program's overrider calls the library's.
Verified with g++-13 and clang, -Wall -Wextra -Werror -fvisibility=hidden, on
the exact files that failed; b2 toolset=gcc rebuilt from scratch; ctest 114/114;
all three examples produce the expected text.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ility
Removing the macros and documenting raw explicit instantiations did not work:
CI showed that no single spelling is portable. The two ABIs want opposite
things, and each spelling fails on one platform while compiling silently on the
other - the worst possible failure mode, and precisely what a macro should hide.
* declspec platforms (Windows, Cygwin, MinGW): `__declspec(dllexport)` and
`extern` are incompatible on an explicit instantiation. MSVC says so - C4910
- and with warnings-as-errors the build fails. Nor is the declaration needed
there: visibility is not a PE concept, so the owning module's other
translation units may instantiate the state implicitly.
* ELF and Mach-O: the attribute must be on the declaration, so that every
translation unit of the owning module pins the symbol to default visibility.
Repeating it on the definition is an error on GCC - "type attributes ignored
after type is already defined" - which clang accepts silently.
So three macros, branching on BOOST_HAS_DECLSPEC:
BOOST_OPENMETHOD_IMPORT_REGISTRY header, every TU of a client module
BOOST_OPENMETHOD_EXPORT_REGISTRY header, every TU of the owning module
BOOST_OPENMETHOD_INSTANTIATE_REGISTRY exactly one .cpp of the owning module
The owning module uses two: EXPORT in the shared header and INSTANTIATE once.
On declspec platforms EXPORT expands to nothing and INSTANTIATE carries the
dllexport; elsewhere EXPORT carries the attribute and INSTANTIATE carries none.
Each takes the registry as an argument and emits only fully qualified names, so
the same three serve default_registry, indirect_registry and user-defined
registries without opening namespace boost::openmethod.
Convert every call site, restore the reference pages (three now), and rewrite
the shared_libraries.adoc sections around the macros, documenting why they exist
and warning that EXPORT is load-bearing on ELF rather than decorative.
Verified on the files that failed CI: g++-13 and clang, -Wall -Wextra -Werror
-fvisibility=hidden, in both preprocessor branches (the declspec branch forced
with -DBOOST_HAS_DECLSPEC to check it parses and exports); b2 toolset=gcc and
toolset=clang rebuilt from scratch; ctest 114/114; examples unchanged.
This does not address the separate macos-26/arm64 runtime failure, in which the
implicit_shared_libraries tests build and then abort with "not implemented",
i.e. the modules do not share state on Mach-O. That is still open.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cherry-picked from the macos/implicit-shared-state investigation, where four CI rounds established the cause. On every libc++ target - all three macOS runners and the Linux clang-12/libc++ sanitizer job - the implicit_shared_libraries tests aborted with "not implemented", i.e. no_overrider, while every libstdc++ target passed. The library trace showed two `speak` methods where there should be one, the shared library's copy holding the Animal and Dog overriders and the executable's holding Cat, each with its own slot. So a call from the executable found no applicable overrider. augment_methods() groups the per-module copies of a method by rtti::type_index(), and std_rtti implemented that with std::type_index, i.e. std::type_info::operator==. That is only as good as the platform's RTTI uniqueness. libstdc++ falls back to comparing names, so it matched. libc++ compares uniquely-named RTTI by address, and each module has its own type_info for method<...>, so it did not. Compare names instead: std_rtti::type_index returns std::string_view of type_info::name(), which is stable across modules everywhere. The return type is a policy detail - type_index_type is decltype(rtti::type_index(0)) and the default implementation returns the type_id unchanged - so nothing else changes. Note what does NOT work, since it looks obvious: marking `method` BOOST_SYMBOL_VISIBLE so its typeinfo unifies. Clang gives a template instantiation the minimum visibility of the template and all of its arguments, so method<Id, Signature, Registry> stays hidden while the method id struct, the registry, or anything in the signature is hidden. Fixing that chain would mean decorating every type a user may name in a method signature. test_compiler.cpp keyed class_map on a raw typeid; route it through the policy, which is what decides what identifies a class. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A type_id is not unique per type - there is normally one per module - so
comparing them directly is a latent cross-module bug that a single-module build
never exhibits. Two places did:
* core.hpp: final_virtual_ptr's runtime check compared dynamic_type against
static_type directly. For an object created in another module those are
different type_ids for the same class, so the check could abort on a
perfectly valid pointer - the same failure mode as the previous commit,
waiting to happen somewhere else.
* initialize.hpp: the class dedup compared ci->type against cr.type as half of
its (type, static_vptr) key. Both entries are already in the same bucket, so
the documented intent - true duplicates only when writing through either has
the same effect - is served by the policy key plus static_vptr.
Also add a regression guard to the implicit_shared_libraries default_registry
test: it asserts that the two modules agree on the rtti policy's key for the
method, which is the invariant augment_methods() depends on. It fails loudly and
locally instead of as a no_overrider abort three assertions later. It
deliberately does not assert std::type_index equality, which is false on libc++
by design and is the whole reason for the previous commit.
Note the runtime vptr maps are unaffected: vptr_map, and vptr_vector with a hash,
are designed for several type_ids per type and get an entry per registration.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
type_index is not involved in a method call: resolve_uni, resolve_multi_first, resolve_multi_next and the vptr lookup contain no reference to it, and neither do the interop headers or the vptr/hash policies. Dispatch reads vptr[slot] and looks the vptr up by raw type_id, which vptr_map and vptr_vector-with-hash are designed to do with several type_ids per type. The other seventeen uses are in initialize.hpp, i.e. initialize() time. That left exactly one runtime site: final_virtual_ptr's debug check, which compares an object's dynamic type against its static type. It is virtual_ptr *construction*, not a call, and only compiled when the registry has runtime_checks - but the previous commit had turned a pointer comparison into a name comparison there. Test the type_ids first. Equal ids settle it, which is the common case since both come from the same module, so the check stays a pointer comparison. type_index is consulted only when they differ - precisely the cross-module case, where a raw comparison alone would abort on a valid pointer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drone build 271 (PR boostorg#75, 727ceb2) fails the implicit_shared_libraries tests on the macOS pipelines, which GitHub Actions does not cover: those use much older Xcode, and they run under UBSAN. Two different sub-checks fire, both false positives, and both for the same underlying reason - macOS/Mach-O does not deduplicate a class's typeinfo and vtable across modules, which is precisely the situation these tests set up: MacOS 10.15 / Xcode 12.2, -fsanitize=function core.hpp:2500: call to function ...thunk<...Dog...>::fn(virtual_ptr<Animal>) through pointer to incorrect function type 'const char *(*)(virtual_ptr<Animal>)' The declared type and the callee's type are the same type. Only their type_info objects differ, one per module. MacOS 12.4 / Xcode 13.4.1, -fsanitize=vptr core.hpp:146: downcast of address ... which does not point to an object of type 'Dog' It really is a Dog; it was just created in the other module. test/dynamic_loading/Jamfile already disables vptr with the same rationale, so this follows an established precedent rather than inventing one. That test also sets <local-visibility>global, which incidentally unifies the typeinfo and hides both checks; these tests deliberately do not, because hidden visibility is the whole configuration the export/import macros exist for. Disabling the two sub-checks is the narrower instrument. Put the requirements in the parent Jamfile so both variants inherit them. Gate on the UBSAN feature so nothing changes elsewhere, and gate `function` on the Clang toolsets - it is a Clang-only sub-check and GCC rejects an unknown -fno-sanitize= argument outright. Verified locally on all four combinations: clang and gcc, with and without undefined-sanitizer=norecover. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
<local-visibility>global was there to keep the shared registry state at DEFAULT
visibility so the dynamic linker would deduplicate it across DSOs. Both reasons
for it are now gone: the state is exported explicitly by
BOOST_OPENMETHOD_{EXPORT,INSTANTIATE}_REGISTRY, and class and method identity no
longer depend on typeinfo unifying across modules, since std_rtti::type_index
compares RTTI names.
Removing it is also the more honest configuration. Hidden visibility is what the
export/import macros exist for and what users get by default; forcing global
visibility in the one suite that exercises cross-module state meant that suite
was not testing the thing it exists to test. It is why dynamic_loading kept
passing on macOS while implicit_shared_libraries failed - the bug was there too,
just masked.
Building at hidden visibility does bring the two UBSAN sub-checks that the
sibling suite already suppresses, both false positives from Mach-O not
deduplicating typeinfo across modules: vptr, which was already disabled here,
and function, added now. Gate `function` on the Clang toolsets - it is
Clang-only and GCC rejects an unknown -fno-sanitize= argument.
Update the comments in the implicit_shared_libraries Jamfiles that described
dynamic_loading as setting <local-visibility>global.
Verified: b2 clang and gcc, plain and undefined-sanitizer=norecover, all four
tests each; CMake 114/114. macOS is what the Drone build is for.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The variable was left behind, expanding to nothing, when the dynamic_loading tests moved to hidden visibility. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Also drop the pointer to test/implicit_shared_libraries/custom_registry, which is a test, not a worked example for readers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jll63
marked this pull request as ready for review
July 28, 2026 00:29
The Pages deploy step has leaked in three ways, all pull_request runs. A sweep of all 210 pull_request runs across the fork and boostorg finds 28 in which the step ran and failed: 1. An outside contributor's PR to my fork runs the deploy and fails on missing permissions (Alexander Grund reported this in #boost on 2026-07-08): a fork pull_request event forces GITHUB_TOKEN read-only, so the workflow's `id-token: write` request is ignored. 2. My own PR to boostorg/openmethod does the same upstream, failing with "Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable". Had the token been writable it would have published to boostorg's Pages site. 3. My own same-repo PR gets a valid token and a valid OIDC token, and still fails: Pages refuses to deploy from a PR ref, with "Deployment failed, try again later." The guard was github.repository_owner, true for any run in the fork, so it let (1) and (3) through. 8c8a875 changed it to github.triggering_actor, true for any run I trigger, which traded (1) for (2) and left (3): one failure in the fork became 27 consecutive failures on the upstream PR. It does not even hold for (1), the case it was written for, because triggering_actor is the re-run initiator -- Grund's runs are attempt 2 with actor=Flamefire but triggering_actor=jll63, so re-running a contributor's PR re-arms the deploy. No test of who triggered the run can fix this: the blocker is the event, and deploying from a pull_request cannot work whoever asks. Skip pull_request outright, and pin github.repository (owner and repo) so pushes deploy only to my own Pages site. Push-event deploys in the fork, the path that works, are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…note The standalone pointer to <<implementation_notes>> added nothing: the paragraph right below it already links there. Reword the closing paragraph around the condition that actually matters. Keeping open-methods out of a library's public API is what makes exporting the registry state unnecessary; "only uses its own registries" described the usual symptom rather than the criterion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.