Conversation
Copilot
AI
changed the title
[WIP] Update branch with main and implement enhancements
Armor core libraries with symbol visibility
Feb 13, 2026
Contributor
|
@phlexbot format |
Contributor
|
Automatic markdownlint fixes pushed (commit 3299ec5). |
Contributor
|
No automatic jsonnetfmt fixes were necessary. |
Contributor
|
Automatic clang-format fixes pushed (commit 943ef76). |
Contributor
|
Automatic cmake-format fixes pushed (commit ccb09dd). |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the symbol-visibility feature branch to main and completes hidden-by-default symbol export coverage across Phlex shared libraries, while also modernizing the driver/source plumbing (data-cell indexing, providers, product queries) and refreshing tests/examples accordingly.
Changes:
- Apply
GenerateExportHeader+ hidden visibility presets and expand export-macro coverage across all shared libraries. - Replace legacy level-id/label APIs with
data_cell_index+product_query(and update tests/configs/plugins accordingly). - Add/refresh Python/plugin tooling and CI hygiene (ruff/mypy config, SARIF helper, workflow tweaks).
Reviewed changes
Copilot reviewed 287 out of 289 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| test/yielding_driver.cpp | Update test to data_cell_index driver model |
| test/vector_of_abstract_types.cpp | Update graph test to providers + "_in(layer)" queries |
| test/utilities/CMakeLists.txt | Migrate tests to cet_test |
| test/specified_label.cpp | Remove tests for deleted specified_label API |
| test/replicated.cpp | Narrow namespace usage for updated utilities |
| test/python/verify.py | Add Python observer helper for tests |
| test/python/test_phlex.py | Update Python import expectations / docs |
| test/python/sumit.py | Add numpy-based Python algorithm test module |
| test/python/source.cpp | Add C++ providers for Python tests |
| test/python/registry.py | Remove old Python registry glue |
| test/python/reducer.py | Add Python reduction workflow test module |
| test/python/pyvec.jsonnet | Add Python/numpy config fixture |
| test/python/pyreduce.jsonnet | Add Python reduction config fixture |
| test/python/pyfailure.jsonnet | Add intentional Python config failure fixture |
| test/python/pyconfig.jsonnet | Add “all config types” Python fixture |
| test/python/pyadd.jsonnet | Update Python add fixture to new driver/sources layout |
| test/python/phlexpy.py | Update cppyy bootstrap + pythonization docs |
| test/python/failing_test_wrap.sh | Add wrapper for expected-failure tests |
| test/python/driver.cpp | Remove legacy C++ driver for Python tests |
| test/python/all_config.py | Add Python-side configuration consumer |
| test/python/adder.py | Replace cppyy-defined add with pure Python + registration |
| test/provider_test.cpp | New provider-focused integration test |
| test/product_store.cpp | Extend product_store tests for size/empty & id changes |
| test/product_query.cpp | New tests for product_query + _in literal |
| test/plugins/source.cpp | Remove legacy plugin source |
| test/plugins/output.cpp | Update plugin registration macro / namespaces |
| test/plugins/module.cpp | Update plugin to product_query + new macro |
| test/plugins/ij_source.cpp | New provider plugin for i/j inputs |
| test/plugins/add.jsonnet | Update plugin job config to new driver/sources layout |
| test/plugins/CMakeLists.txt | Add provider plugin + migrate job test to cet_test |
| test/multiple_function_registration.cpp | Update multi-transform test to provider inputs / new hierarchy |
| test/mock-workflow/three_tuple_algorithm.cpp | Update workflow module registration + input index type |
| test/mock-workflow/source.cpp | Remove legacy mock-workflow source |
| test/mock-workflow/pd_fast_sim.cpp | Update plugin registration macro |
| test/mock-workflow/mock-workflow.jsonnet | Update workflow config to driver+provider layout |
| test/mock-workflow/largeant.cpp | Update plugin registration macro |
| test/mock-workflow/ion_and_scint.cpp | Update plugin registration macro |
| test/mock-workflow/id_provider.cpp | New provider plugin for “id” product |
| test/mock-workflow/event_product.libsonnet | Add helper for event-layer product specs |
| test/mock-workflow/algorithm.hpp | Update algorithm test helper to use product_query inputs |
| test/mock-workflow/SinglesGen.libsonnet | Update config keys (cpp) + product specs |
| test/mock-workflow/MC_truth_algorithm.cpp | Update input index type |
| test/mock-workflow/G4Stage2.libsonnet | Update config keys + product specs |
| test/mock-workflow/G4Stage1.libsonnet | Update config keys + product specs |
| test/mock-workflow/CMakeLists.txt | Replace source plugin + migrate test execution to cet_test |
| test/memory-checks/many_events.cpp | Update memory-check driver/source plumbing |
| test/memory-checks/CMakeLists.txt | Migrate test to cet_test + link layer_generator |
| test/max-parallelism/provide_parallelism.cpp | Add provider plugin for parallelism value |
| test/max-parallelism/check_parallelism_default.jsonnet.in | Update config to driver+provider layout |
| test/max-parallelism/check_parallelism.cpp | Remove embedded source; update to product_query input |
| test/function_registration.cpp | Update test inputs to provider registration |
| test/framework_graph.cpp | Add new graph behavior tests (exceptions, single-thread progress) |
| test/form/test_helpers.hpp | Add helpers for FORM test type map |
| test/form/form_test.jsonnet | Add form-module job config |
| test/form/data_products/CMakeLists.txt | Clean up ROOT dictionary linkage |
| test/form/CMakeLists.txt | Migrate FORM tests to cet_test + add job test |
| test/filter_impl.cpp | Add test for data_map completeness behavior |
| test/demo-giantdata/waveforms.cpp | Remove logging hooks; simplify dtors |
| test/demo-giantdata/waveform_generator_input.cpp | Remove logging hooks; simplify dtors |
| test/demo-giantdata/waveform_generator.hpp | Remove unused members/includes |
| test/demo-giantdata/waveform_generator.cpp | Remove logging hooks; simplify ctor/dtor |
| test/demo-giantdata/user_algorithms.hpp | Simplify APIs (drop unused id args) |
| test/demo-giantdata/user_algorithms.cpp | Remove logging hooks; simplify clamp/accumulate |
| test/demo-giantdata/CMakeLists.txt | Convert demo to cet_test and link generator |
| test/data_cell_index.cpp | Replace level-id tests with data-cell index hashing test |
| test/configuration.cpp | Add configuration error and product_query parsing tests |
| test/class_registration.cpp | Update test inputs to providers + product_query |
| test/cached_product_stores.cpp | Remove tests for deleted cache helper |
| test/cached_execution_source.hpp | Remove legacy cached execution source |
| test/cached_execution.cpp | Update cached execution test to providers + layer generator |
| test/benchmarks/verify_even_fibonacci_numbers.cpp | Update plugin macro + product_query input parsing |
| test/benchmarks/verify_difference.cpp | Update config parsing to product_query inputs |
| test/benchmarks/read_index.cpp | Update plugin macro + config parsing |
| test/benchmarks/read_id.cpp | Update id type + input query |
| test/benchmarks/plus_one.cpp | Update plugin macro + input layer |
| test/benchmarks/plus_101.cpp | Update plugin macro + input layer |
| test/benchmarks/last_index.cpp | Update id type + input layer |
| test/benchmarks/benchmarks_source.cpp | Remove legacy benchmark source |
| test/benchmarks/benchmarks_provider.cpp | Add provider plugin for benchmark id product |
| test/benchmarks/benchmark-09.jsonnet | Update config to driver+provider layout |
| test/benchmarks/benchmark-08.jsonnet | Update config to driver+provider layout |
| test/benchmarks/benchmark-07.jsonnet | Update config to driver+provider layout |
| test/benchmarks/benchmark-06.jsonnet | Update config to driver+provider layout |
| test/benchmarks/benchmark-05.jsonnet | Update config to driver+provider layout |
| test/benchmarks/benchmark-04.jsonnet | Update config to driver+provider layout |
| test/benchmarks/benchmark-03.jsonnet | Update config to driver+provider layout |
| test/benchmarks/benchmark-02.jsonnet | Update config to driver+provider layout |
| test/benchmarks/benchmark-01.jsonnet | Update config to driver+provider layout |
| test/benchmarks/accept_fibonacci_numbers.cpp | Update plugin macro + product_query input parsing |
| test/benchmarks/accept_even_numbers.cpp | Update plugin macro + product_query input parsing |
| test/benchmarks/accept_even_ids.cpp | Update id type + config parsing |
| test/benchmarks/CMakeLists.txt | Replace source plugin with provider + migrate job tests |
| test/adjust_config.cpp | Add tests for module config normalization (cpp/py) |
| scripts/sarif-alerts.py | Add SARIF alert printing tool |
| scripts/export_llvm_lcov.py | Add docstrings for script entrypoints |
| scripts/create_coverage_symlinks.py | Add docstrings for coverage symlink helper |
| pyproject.toml | Introduce ruff+mypy configuration |
| plugins/python/src/wrap.hpp | Add Python registration wrappers + GIL RAII |
| plugins/python/CMakeLists.txt | Add Python plugin build + optional NumPy support |
| plugins/generate_layers.cpp | Add configurable layer-generator driver plugin |
| plugins/CMakeLists.txt | Add core plugins build/install |
| phlex/utilities/stripped_name.hpp | Remove deleted utility header |
| phlex/utilities/stripped_name.cpp | Remove deleted utility implementation |
| phlex/utilities/async_driver.hpp | Switch to std::jthread + propagate driver exceptions |
| phlex/utilities/CMakeLists.txt | Migrate utilities build/install to cetmodules + visibility |
| phlex/module.hpp | Replace old registration macro with shared plugin macro helper |
| phlex/model/qualified_name.hpp | Remove deprecated qualified-name API |
| phlex/model/qualified_name.cpp | Remove deprecated qualified-name implementation |
| phlex/model/products.hpp | Switch to product_specification + export + add size/empty |
| phlex/model/products.cpp | Implement size/empty |
| phlex/model/product_specification.hpp | Add replacement for qualified-name outputs (with type id) |
| phlex/model/product_specification.cpp | Implement product_specification + helpers |
| phlex/model/product_matcher.hpp | Rename level→layer terminology + export |
| phlex/model/product_matcher.cpp | Update matcher pattern strings for layer terminology |
| phlex/model/level_id.hpp | Remove legacy level-id API |
| phlex/model/level_counter.hpp | Remove legacy level-counter API |
| phlex/model/level_counter.cpp | Remove legacy level-counter implementation |
| phlex/model/handle.hpp | Update handle to carry data_cell_index |
| phlex/model/fwd.hpp | Add data-cell index fwd decls + update model fwd set |
| phlex/model/data_layer_hierarchy.hpp | Rename/replace level hierarchy with data-layer hierarchy |
| phlex/model/data_cell_index.hpp | Add new data-cell index type |
| phlex/model/data_cell_counter.hpp | Add new counters for flush accounting |
| phlex/model/data_cell_counter.cpp | Implement new counters |
| phlex/model/CMakeLists.txt | Migrate model build/install to cetmodules + visibility |
| phlex/metaprogramming/type_deduction.hpp | Expand traits/concepts (atomic/container/tuple helpers) |
| phlex/metaprogramming/CMakeLists.txt | Migrate metaprogramming install/export to cetmodules |
| phlex/graph/serializer_node.hpp | Include-order tweak for export header |
| phlex/graph/serial_node.hpp | Include-order tweak for export header |
| phlex/graph/CMakeLists.txt | Migrate graph build/install to cetmodules + visibility |
| phlex/driver.hpp | Add unified driver plugin registration + create_next helper |
| phlex/detail/plugin_macros.hpp | Add shared plugin registration macro machinery |
| phlex/core/upstream_predicates.hpp | Rename when to experimental_when |
| phlex/core/store_counters.cpp | Update flush/store accounting to data-cell ids |
| phlex/core/specified_label.hpp | Remove deprecated specified-label API |
| phlex/core/specified_label.cpp | Remove deprecated specified-label implementation |
| phlex/core/registration_api.cpp | Rename when to experimental_when in API |
| phlex/core/registrar.hpp | Export helper used across shared-library boundary |
| phlex/core/products_consumer.hpp | Switch inputs to product_query + export |
| phlex/core/products_consumer.cpp | Switch inputs to product_query |
| phlex/core/product_query.hpp | Add product_query + _in(layer) literal + type population |
| phlex/core/product_query.cpp | Implement product_query + literal error handling |
| phlex/core/node_catalog.hpp | Export node_catalog + add provider catalog |
| phlex/core/node_catalog.cpp | Add providers to execution_counts + throw on unknown name |
| phlex/core/multiplexer.hpp | Update multiplexer finalize model for providers |
| phlex/core/message_sender.hpp | Update sender to use data-layer hierarchy + data-cell ids |
| phlex/core/message_sender.cpp | Update to new hierarchy constructor |
| phlex/core/message.hpp | Switch to product_query ports + export required symbols |
| phlex/core/message.cpp | Update to data_cell_index and new error text |
| phlex/core/input_arguments.hpp | Switch retrievers to product_query + export duplicate check |
| phlex/core/input_arguments.cpp | Update duplicate detection types |
| phlex/core/graph_proxy.hpp | Add provide() to graph proxy + namespace adjustments |
| phlex/core/glue.hpp | Add provider registration + export name verification |
| phlex/core/fwd.hpp | Move framework_driver alias to phlex::framework_driver |
| phlex/core/filter.hpp | Export filter class |
| phlex/core/end_of_message.hpp | Update to new hierarchy + export |
| phlex/core/end_of_message.cpp | Update to new hierarchy types |
| phlex/core/edge_maker.hpp | Add provider edge support + export helper |
| phlex/core/edge_maker.cpp | Implement provider edge wiring |
| phlex/core/edge_creation_policy.hpp | Switch producer matching to product_query + include types |
| phlex/core/edge_creation_policy.cpp | Add type-aware producer matching + better diagnostics |
| phlex/core/detail/maybe_predicates.hpp | Export predicate extraction + rename config key |
| phlex/core/detail/maybe_predicates.cpp | Read experimental_when from config |
| phlex/core/detail/make_algorithm_name.hpp | Export algorithm-name helper |
| phlex/core/detail/filter_impl.hpp | Export filter maps + switch to product_query |
| phlex/core/detail/filter_impl.cpp | Fix completeness logic for sparse store vectors |
| phlex/core/declared_unfold.cpp | Update unfold generator to new data-cell API |
| phlex/core/declared_transform.cpp | Update ctor arg types + improve cached-store reporting |
| phlex/core/declared_provider.cpp | Add declared provider implementation |
| phlex/core/declared_predicate.hpp | Switch to product_query + data-cell hashes + export |
| phlex/core/declared_predicate.cpp | Switch ctor arg types |
| phlex/core/declared_output.hpp | Export declared_output |
| phlex/core/declared_observer.hpp | Switch to product_query + data-cell hashes + export |
| phlex/core/declared_observer.cpp | Switch cached-hash map key type |
| phlex/core/declared_fold.cpp | Switch ctor arg types |
| phlex/core/consumer.hpp | Export consumer base |
| phlex/core/concurrency.cpp | Move concurrency constants to phlex namespace |
| phlex/core/concepts.hpp | Add provider concept |
| phlex/core/cached_product_stores.hpp | Remove deprecated store-cache helper |
| phlex/configuration.cpp | Add product_query JSON conversion + keys() helper |
| phlex/concurrency.hpp | Export concurrency + move to phlex namespace |
| phlex/app/version.hpp | Export version() across shared library boundary |
| phlex/app/run.hpp | Export run() across shared library boundary |
| phlex/app/run.cpp | Update config schema (driver/modules/sources) + better errors |
| phlex/app/phlex.cpp | Catch+print top-level exceptions in CLI |
| phlex/app/load_module.hpp | Export app API + add adjust_config + new loader functions |
| phlex/app/CMakeLists.txt | Migrate app build to cetmodules + symbol visibility |
| phlex/CMakeLists.txt | Register export set + split configuration lib + install headers |
| form/storage/CMakeLists.txt | Formatting / target definition cleanup |
| form/root_storage/root_ttree_container.cpp | Fix write semantics on destruction + add guard |
| form/root_storage/CMakeLists.txt | Formatting / target definition cleanup |
| form/mock_phlex/phlex_toy_core.hpp | Remove mock-phlex shim (no longer used) |
| form/mock_phlex/phlex_toy_core.cpp | Remove mock-phlex shim implementation |
| form/mock_phlex/phlex_toy_config.hpp | Remove mock-phlex config shim |
| form/mock_phlex/phlex_toy_config.cpp | Remove mock-phlex config shim implementation |
| form/mock_phlex/CMakeLists.txt | Remove mock-phlex library |
| form/form/form.hpp | Replace mock-phlex types with FORM-local types |
| form/README.md | Minor markdown cleanup |
| form/CMakeLists.txt | Enable PIC + add form_module plugin |
| codecov.yml | Raise coverage thresholds |
| README-dev.md | Remove developer notes file |
| Modules/private/PhlexTargetUtils.cmake | CMake formatting/cleanup |
| Modules/private/PhlexSymbolVisibility.cmake | CMake formatting/cleanup for visibility helper |
| .github/workflows/python-fix.yaml | Add workflow to auto-fix Python formatting/lint |
| .github/workflows/add-issues.yaml | Pin action by SHA + permissions hardening |
| .github/dependabot.yml | Add Dependabot for GitHub Actions |
| .github/actions/get-pr-info/action.yaml | Add composite action to resolve PR refs from comments |
| .github/actions/generate-build-matrix/action.yaml | Add composite action for dynamic build matrix |
| .github/actions/detect-relevant-changes/action.yaml | Add python file patterns + ref validation |
| .github/actions/detect-act-env/action.yaml | Add act-environment detection helper |
| .github/actions/configure-cmake/action.yaml | Refactor CMake configure args + enable color forcing |
| .github/actions/build-cmake/action.yaml | Force colored output |
| .github/actionlint.yaml | Add actionlint configuration scaffolding |
| .github/CodeQL-README.md | Markdown/formatting improvements |
| .github/CODEOWNERS | Add CODEOWNERS enforcement baseline |
| .flake8 | Remove flake8 config (ruff replaces it) |
Contributor
|
@copilot I think for maximum clarity, it would be better to reset this branch to upstream/main and then re-do the work following the current philosophy, library names, etc. Otherwise this looks like a huge body of work that would be way to much for a human to review. |
Contributor
Author
Done. In commit |
greenc-FNAL
force-pushed
the
copilot/update-branch-with-main
branch
2 times, most recently
from
e65cb92 to
e44c62a
Compare
greenc-FNAL
changed the base branch from
jules-feature-plugin-symbol-visibility
to
main
Contributor
|
@copilot apply changes based on the comments in this thread |
Contributor
Author
All three review comments addressed in
|
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #322 +/- ##
==========================================
- Coverage 85.49% 85.47% -0.03%
==========================================
Files 142 142
Lines 3586 3586
Branches 615 615
==========================================
- Hits 3066 3065 -1
Misses 310 310
- Partials 210 211 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Contributor
|
@phlexbot clang-fix |
Copilot
AI
changed the title
Armor shared libraries with hidden symbol visibility; add _internal companions for testing and comparison
Armor shared libraries with hidden symbol visibility; add _internal companions, PHLEX_HIDE_SYMBOLS toggle, and optimizer flags
Mar 10, 2026
Copilot
AI
changed the title
Armor shared libraries with hidden symbol visibility; add _internal companions, PHLEX_HIDE_SYMBOLS toggle, and optimizer flags
Armor shared libraries with hidden symbol visibility; add _internal companions, optimization flags, and PHLEX_HIDE_SYMBOLS toggle
Mar 10, 2026
Contributor
|
@phlexbot cmake-fix |
Contributor
|
Automatic cmake-format fixes pushed (commit c63bd45). |
Contributor
|
Review the full CodeQL report for details. |
Contributor
|
Review the full CodeQL report for details. |
Member
|
@greenc-FNAL, here are the two issues per discussion at today's Phlex developer meeting:
The purpose of 2 is to verify that users can still debug their code with a symbol-hidden |
Contributor
This would require that all of Phlex's libraries with hidden/exported symbols be consolidated into a single library. Otherwise, compiling one higher level library with (e.g.) |
Contributor
Demonstration:
Using configuration file: /workspaces/phlex-examples/test-throw-workflow.jsonnet
[2026-03-27 19:19:30.362] [info] Number of worker threads: 1
=== Stack trace at throw site ===
0# 0x00000000000633E9 in /workspaces/phlex-examples-build/libthrow_on_process.so
1# void std::__invoke_impl<void, void (*&)(int), int>(std::__invoke_other, void (*&)(int), int&&) in /workspaces/phlex-examples-build/libthrow_on_process.so
2# std::enable_if<is_invocable_r_v<void, void (*&)(int), int>, void>::type std::__invoke_r<void, void (*&)(int), int>(void (*&)(int), int&&) in /workspaces/phlex-examples-build/libthrow_on_process.so
3# std::_Function_handler<void (int), void (*)(int)>::_M_invoke(std::_Any_data const&, int&&) in /workspaces/phlex-examples-build/libthrow_on_process.so
4# std::function<void (int)>::operator()(int) const in /workspaces/phlex-examples-build/libthrow_on_process.so
5# void std::__invoke_impl<void, std::function<void (int)> const&, phlex::handle<int> >(std::__invoke_other, std::function<void (int)> const&, phlex::handle<int>&&) in /workspaces/phlex-examples-build/libthrow_on_process.so
6# std::__invoke_result<std::function<void (int)> const&, phlex::handle<int> >::type std::__invoke<std::function<void (int)> const&, phlex::handle<int> >(std::function<void (int)> const&, phlex::handle<int>&&) in /workspaces/phlex-examples-build/libthrow_on_process.so
7# std::invoke_result<std::function<void (int)> const&, phlex::handle<int> >::type std::invoke<std::function<void (int)> const&, phlex::handle<int> >(std::function<void (int)> const&, phlex::handle<int>&&) in /workspaces/phlex-examples-build/libthrow_on_process.so
8# void phlex::experimental::observer_node<phlex::experimental::algorithm_bits<std::function<void (int)>, void (*)(int)> >::call<0ul>(std::function<void (int)> const&, phlex::experimental::message const&, std::integer_sequence<unsigned long, 0ul>) in /workspaces/phlex-examples-build/libthrow_on_process.so
9# phlex::experimental::observer_node<phlex::experimental::algorithm_bits<std::function<void (int)>, void (*)(int)> >::observer_node(phlex::experimental::algorithm_name, unsigned long, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, tbb::detail::d2::graph&, phlex::experimental::algorithm_bits<std::function<void (int)>, void (*)(int)>, std::vector<phlex::product_query, std::allocator<phlex::product_query> >)::{lambda(phlex::experimental::message const&)#1}::operator()(phlex::experimental::message const&) const in /workspaces/phlex-examples-build/libthrow_on_process.so
10# tbb::detail::d2::function_body_leaf<phlex::experimental::message, tbb::detail::d2::continue_msg, phlex::experimental::observer_node<phlex::experimental::algorithm_bits<std::function<void (int)>, void (*)(int)> >::observer_node(phlex::experimental::algorithm_name, unsigned long, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, tbb::detail::d2::graph&, phlex::experimental::algorithm_bits<std::function<void (int)>, void (*)(int)>, std::vector<phlex::product_query, std::allocator<phlex::product_query> >)::{lambda(phlex::experimental::message const&)#1}>::operator()(phlex::experimental::message const&) in /workspaces/phlex-examples-build/libthrow_on_process.so
11# tbb::detail::d2::continue_msg std::__invoke_impl<tbb::detail::d2::continue_msg, tbb::detail::d2::function_body<phlex::experimental::message, tbb::detail::d2::continue_msg>&, phlex::experimental::message const&>(std::__invoke_other, tbb::detail::d2::function_body<phlex::experimental::message, tbb::detail::d2::continue_msg>&, phlex::experimental::message const&) in /workspaces/phlex-examples-build/libthrow_on_process.so
12# std::__invoke_result<tbb::detail::d2::function_body<phlex::experimental::message, tbb::detail::d2::continue_msg>&, phlex::experimental::message const&>::type std::__invoke<tbb::detail::d2::function_body<phlex::experimental::message, tbb::detail::d2::continue_msg>&, phlex::experimental::message const&>(tbb::detail::d2::function_body<phlex::experimental::message, tbb::detail::d2::continue_msg>&, phlex::experimental::message const&) in /workspaces/phlex-examples-build/libthrow_on_process.so
13# std::invoke_result<tbb::detail::d2::function_body<phlex::experimental::message, tbb::detail::d2::continue_msg>&, phlex::experimental::message const&>::type std::invoke<tbb::detail::d2::function_body<phlex::experimental::message, tbb::detail::d2::continue_msg>&, phlex::experimental::message const&>(tbb::detail::d2::function_body<phlex::experimental::message, tbb::detail::d2::continue_msg>&, phlex::experimental::message const&) in /workspaces/phlex-examples-build/libthrow_on_process.so
14# std::invoke_result<tbb::detail::d2::function_body<phlex::experimental::message, tbb::detail::d2::continue_msg>&, phlex::experimental::message const&>::type tbb::detail::d0::invoke<tbb::detail::d2::function_body<phlex::experimental::message, tbb::detail::d2::continue_msg>&, phlex::experimental::message const&>(tbb::detail::d2::function_body<phlex::experimental::message, tbb::detail::d2::continue_msg>&, phlex::experimental::message const&) in /workspaces/phlex-examples-build/libthrow_on_process.so
15# tbb::detail::d2::function_input<phlex::experimental::message, tbb::detail::d2::continue_msg, tbb::detail::d2::graph_policy_namespace::queueing, tbb::detail::d1::cache_aligned_allocator<phlex::experimental::message> >::apply_body_impl(phlex::experimental::message const&) in /workspaces/phlex-examples-build/libthrow_on_process.so
16# tbb::detail::d2::function_input<phlex::experimental::message, tbb::detail::d2::continue_msg, tbb::detail::d2::graph_policy_namespace::queueing, tbb::detail::d1::cache_aligned_allocator<phlex::experimental::message> >::apply_body_impl_bypass(phlex::experimental::message const&) in /workspaces/phlex-examples-build/libthrow_on_process.so
17# tbb::detail::d2::function_input_base<phlex::experimental::message, tbb::detail::d2::graph_policy_namespace::queueing, tbb::detail::d1::cache_aligned_allocator<phlex::experimental::message>, tbb::detail::d2::function_input<phlex::experimental::message, tbb::detail::d2::continue_msg, tbb::detail::d2::graph_policy_namespace::queueing, tbb::detail::d1::cache_aligned_allocator<phlex::experimental::message> > >::apply_body_bypass(phlex::experimental::message const&) in /workspaces/phlex-examples-build/libthrow_on_process.so
18# tbb::detail::d2::apply_body_task_bypass<tbb::detail::d2::function_input_base<phlex::experimental::message, tbb::detail::d2::graph_policy_namespace::queueing, tbb::detail::d1::cache_aligned_allocator<phlex::experimental::message>, tbb::detail::d2::function_input<phlex::experimental::message, tbb::detail::d2::continue_msg, tbb::detail::d2::graph_policy_namespace::queueing, tbb::detail::d1::cache_aligned_allocator<phlex::experimental::message> > >, phlex::experimental::message, tbb::detail::d2::graph_task>::call_apply_body_bypass_impl(std::integral_constant<bool, true>) in /workspaces/phlex-examples-build/libthrow_on_process.so
19# tbb::detail::d2::apply_body_task_bypass<tbb::detail::d2::function_input_base<phlex::experimental::message, tbb::detail::d2::graph_policy_namespace::queueing, tbb::detail::d1::cache_aligned_allocator<phlex::experimental::message>, tbb::detail::d2::function_input<phlex::experimental::message, tbb::detail::d2::continue_msg, tbb::detail::d2::graph_policy_namespace::queueing, tbb::detail::d1::cache_aligned_allocator<phlex::experimental::message> > >, phlex::experimental::message, tbb::detail::d2::graph_task>::call_apply_body_bypass() in /workspaces/phlex-examples-build/libthrow_on_process.so
20# tbb::detail::d2::apply_body_task_bypass<tbb::detail::d2::function_input_base<phlex::experimental::message, tbb::detail::d2::graph_policy_namespace::queueing, tbb::detail::d1::cache_aligned_allocator<phlex::experimental::message>, tbb::detail::d2::function_input<phlex::experimental::message, tbb::detail::d2::continue_msg, tbb::detail::d2::graph_policy_namespace::queueing, tbb::detail::d1::cache_aligned_allocator<phlex::experimental::message> > >, phlex::experimental::message, tbb::detail::d2::graph_task>::execute(tbb::detail::d1::execution_data&) in /workspaces/phlex-examples-build/libthrow_on_process.so
21# 0x00000000000365DE in /opt/spack-environments/phlex-ci/.spack-env/._view/jxnf3g2yrgbkdszr5ylwdmxgcnoaaoys/lib/libtbb.so.12
22# tbb::detail::r1::wait(tbb::detail::d1::wait_context&, tbb::detail::d1::task_group_context&) in /opt/spack-environments/phlex-ci/.spack-env/._view/jxnf3g2yrgbkdszr5ylwdmxgcnoaaoys/lib/libtbb.so.12
23# 0x0000000000039DF6 in /workspaces/phlex-build-demo/libphlex_core.so
24# GCC_except_table43 at phlex.cpp:?
25# 0x0000000000039D64 in /workspaces/phlex-build-demo/libphlex_core.so
26# phlex::experimental::framework_graph::execute() in /workspaces/phlex-build-demo/libphlex_core.so
27# phlex::experimental::run(boost::json::object const&, int) in /workspaces/phlex-build-demo/librun_phlex.so
28# main in /workspaces/phlex-build-demo/bin/phlex
29# 0x000000000002A1CA in /lib/x86_64-linux-gnu/libc.so.6
30# __libc_start_main in /lib/x86_64-linux-gnu/libc.so.6
31# _start in /workspaces/phlex-build-demo/bin/phlex
=================================
[2026-03-27 19:19:30.502] [error] Deliberate failure in user algorithm
[2026-03-27 19:19:30.502] [info]
Seen layers:
job
│
└ spill: 3
[2026-03-27 19:19:30.502] [info] CPU time: 0.01084s Real time: 0.14000s CPU efficiency: 7.74%
[2026-03-27 19:19:30.502] [info] Max. RSS: 21.880 MB
Deliberate failure in user algorithm
- Completed in 2.114sAI analysis:
"Frame 24" is an artefact of |
Contributor
|
@copilot Please address all open review comments with code and/or comments as necessary, before resolving them or soliciting further comment as appropriate. Additionally, please update the symbol hiding/export machinery and use thereof in order to ensure that the PR's goals are met for the code as it currently exists in the repository. |
Contributor
Author
All five open reviewer items addressed in
PR description updated to accurately reflect all of the above. |
Contributor
|
@copilot Please update the symbol hiding/export machinery and use thereof in order to ensure that the PR's goals are met for the code as it currently exists in the repository. In particular, ensure that all public API for the covered libraries is exported, and non-public API code is not. |
Contributor
Author
Done in All other public API symbols across the five libraries are confirmed covered:
|
… libraries, introduces non-installed `_internal` companion libraries for testing and comparison, and adds optimization flags that automatically exploit the visibility model.
**`Modules/private/PhlexSymbolVisibility.cmake`**
- `phlex_apply_symbol_visibility(target)` — generates `phlex/X_export.hpp` (installed to `include/phlex/`), sets `-fvisibility=hidden` when `PHLEX_HIDE_SYMBOLS=ON`
- `phlex_make_internal_library(target LIBRARIES ...)` — `PHLEX_HIDE_SYMBOLS=ON`: compiles a separate SHARED lib from the same sources with full visibility (no source duplication — reads via `get_target_property`); `PHLEX_HIDE_SYMBOLS=OFF`: thin INTERFACE alias to the public target
**`Modules/private/PhlexOptimization.cmake`**
- `phlex_apply_optimizations(target)` — applies per-target flags based on option values
- `PHLEX_HIDE_SYMBOLS` and `PHLEX_ENABLE_IPO` default `ON` for `Release`/`RelWithDebInfo`, `OFF` otherwise; all four combinations are valid for benchmarking
| `PHLEX_HIDE_SYMBOLS` | `PHLEX_ENABLE_IPO` | Flags added |
| --- | --- | --- |
| ON | ON | LTO + `-fno-semantic-interposition` + `-fno-plt` |
| OFF | ON | LTO + `-fno-plt` |
| ON | OFF | `-fno-semantic-interposition` + `-fno-plt` |
| OFF | OFF | `-fno-plt` |
`-fno-semantic-interposition` is gated on `PHLEX_HIDE_SYMBOLS=ON` — it is only safe once the exported-symbol surface is explicitly bounded. `-ffast-math` intentionally excluded.
~45 headers annotated. Every non-template class and free function with a `.cpp` implementation carries `phlex_X_EXPORT`; template/header-only code is left unannotated. Boost.JSON `tag_invoke` overloads for `configuration`, `product_query`, and `identifier` explicitly exported (required for `value_to<T>()`).
```cpp
class phlex_core_EXPORT framework_graph { ... };
phlex_core_EXPORT std::size_t port_index_for(...);
```
| Target | Alias | Transitively includes |
| --- | --- | --- |
| `phlex_utilities_internal` | `phlex::utilities_internal` | — |
| `phlex_model_internal` | `phlex::model_internal` | utilities |
| `phlex_core_internal` | `phlex::core_internal` | model, utilities |
| `run_phlex_internal` | — | core |
Tests link exclusively to `phlex::core_internal`; benchmark tests (`test/benchmarks/`, `test/max-parallelism/`) link exclusively to curated public targets — isolating the two `.so` sets per process to avoid ODR. `layer_generator_internal` follows the same `PHLEX_HIDE_SYMBOLS` conditionality as the core libs.
<!-- START COPILOT CODING AGENT TIPS -->
---
💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).
PhlexSymbolVisibility.cmake: Filter -fno-semantic-interposition from the COMPILE_OPTIONS propagated to _internal targets. Those targets are built with default (interposable) visibility, so applying the flag would violate its safety invariant and skew internal/public benchmarks. PhlexOptimization.cmake: Fix the PHLEX_HIDE_SYMBOLS option description: -fno-semantic-interposition is applied whenever PHLEX_HIDE_SYMBOLS=ON (independent of PHLEX_ENABLE_IPO). The previous text incorrectly implied it required IPO to be ON as well. phlex/core/glue.hpp: Remove phlex_core_EXPORT from the glue class template. Templates are instantiated in the consumer TU and must not carry export macros; on Windows DLLs an export macro on a template causes dllimport/dllexport mismatches. The detail::verify_name free function (which has a .cpp implementation) retains its phlex_core_EXPORT annotation. phlex/concurrency.hpp: Explicitly annotate the static data members unlimited and serial with phlex_core_EXPORT. The class-level macro provides default visibility on GCC/ELF, but explicit per-member annotation is the portable and correct approach that works consistently across toolchains. test/CMakeLists.txt: Fix ODR hazard in the filter test: previously it linked phlex::core_internal alongside layer_generator (which transitively links phlex::core), loading both libphlex_core.so and libphlex_core_internal.so in the same process. Changed to layer_generator_internal, which links phlex_core_internal, so only one copy of each symbol is loaded. Agent-Logs-Url: https://github.com/Framework-R-D/phlex/sessions/d723e796-3811-4983-9bd7-1afb6202b09d Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
resolve_in_store is a non-template free function with a .cpp implementation in product_query.cpp. It was the only symbol in the public API across all five covered libraries that was missing an export macro. All other non-template classes and free functions with .cpp implementations across phlex_utilities, phlex_model, phlex_core, phlex_configuration_internal, and run_phlex are confirmed to carry the appropriate phlex_X_EXPORT macro. Template classes and header-only code remain unannotated, as intended. Agent-Logs-Url: https://github.com/Framework-R-D/phlex/sessions/2ddd3424-36f7-4c83-9e13-e8c0140e33b0 Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
knoepfel
approved these changes
Apr 2, 2026
pcanal
reviewed
Apr 2, 2026
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.
Enforces hidden-by-default symbol visibility on all five Phlex shared libraries, introduces non-installed
_internalcompanion libraries for testing and comparison, and adds optimization flags that automatically exploit the visibility model.New CMake infrastructure
Modules/private/PhlexSymbolVisibility.cmakephlex_apply_symbol_visibility(target)— generatesphlex/X_export.hpp(installed toinclude/phlex/), sets-fvisibility=hiddenwhenPHLEX_HIDE_SYMBOLS=ONphlex_make_internal_library(target LIBRARIES ...)—PHLEX_HIDE_SYMBOLS=ON: compiles a separate SHARED lib from the same sources with full visibility (no source duplication — reads viaget_target_property);PHLEX_HIDE_SYMBOLS=OFF: thin INTERFACE alias to the public targetModules/private/PhlexOptimization.cmakephlex_apply_optimizations(target)— applies per-target flags based on option valuesPHLEX_HIDE_SYMBOLSandPHLEX_ENABLE_IPOdefaultONforRelease/RelWithDebInfo,OFFotherwise; all four combinations are valid for benchmarkingPHLEX_HIDE_SYMBOLSPHLEX_ENABLE_IPO-fno-semantic-interposition+-fno-plt-fno-plt-fno-semantic-interposition+-fno-plt-fno-plt-fno-semantic-interpositionis gated onPHLEX_HIDE_SYMBOLS=ON— it is only safe once the exported-symbol surface is explicitly bounded.-ffast-mathintentionally excluded.Export macro coverage
~45 headers annotated. Every non-template class and free function with a
.cppimplementation carriesphlex_X_EXPORT; template/header-only code is left unannotated. Boost.JSONtag_invokeoverloads forconfiguration,product_query, andidentifierexplicitly exported (required forvalue_to<T>())._internalcompanion librariesphlex_utilities_internalphlex::utilities_internalphlex_model_internalphlex::model_internalphlex_core_internalphlex::core_internalrun_phlex_internalTests link exclusively to
phlex::core_internal; benchmark tests (test/benchmarks/,test/max-parallelism/) link exclusively to curated public targets — isolating the two.sosets per process to avoid ODR.layer_generator_internalfollows the samePHLEX_HIDE_SYMBOLSconditionality as the core libs.💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.