Conversation
|
@phlexbot format |
|
Automatic markdownlint fixes pushed (commit 3299ec5). |
|
No automatic jsonnetfmt fixes were necessary. |
|
Automatic clang-format fixes pushed (commit 943ef76). |
|
Automatic cmake-format fixes pushed (commit ccb09dd). |
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) |
|
@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. |
Done. In commit |
e65cb92 to
e44c62a
Compare
|
@copilot apply changes based on the comments in this thread |
All three review comments addressed in
|
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #322 +/- ##
==========================================
+ Coverage 84.22% 84.28% +0.06%
==========================================
Files 128 128
Lines 3322 3322
Branches 564 564
==========================================
+ Hits 2798 2800 +2
Misses 324 324
+ Partials 200 198 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
@phlexbot clang-fix |
|
@phlexbot cmake-fix |
|
Automatic cmake-format fixes pushed (commit bcffb4a). |
|
@pcanal @knoepfel Just based on size for a Release build, the public libraries for |
|
Also, the claim is that intelligent use of symbol hiding allows more aggressive optimization within compilation units. |
Create PhlexSymbolVisibility.cmake with phlex_apply_symbol_visibility()
that generates export headers into centralized ${PROJECT_BINARY_DIR}/include,
sets CXX_VISIBILITY_PRESET hidden + VISIBILITY_INLINES_HIDDEN ON, and
installs the headers for downstream consumers.
Apply phlex_apply_symbol_visibility() to all 5 shared libraries:
phlex_utilities, phlex_model, phlex_core, phlex_configuration_internal,
run_phlex.
Annotate ~45 headers across the whole phlex namespace (including
phlex::experimental) with export macros — every non-template class and
free function with a .cpp implementation.
Also add missing includes flagged by misc-include-cleaner:
- <cstddef> and <compare> to identifier.hpp
- <cstddef> to hashing.hpp
- <string> to load_module.hpp
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Apply clang-format fixes
Export tag_invoke customization points in configuration.hpp
The Boost.JSON customization point overloads for configuration,
product_query, and experimental::identifier were hidden by the
visibility policy, causing config_test link failure:
undefined reference to
phlex::tag_invoke(boost::json::value_to_tag<phlex::configuration>...)
phlex::tag_invoke(boost::json::value_to_tag<phlex::product_query>...)
These are public API symbols (they back value_to<T>() for JSON
deserialization) that were missed in the initial visibility sweep.
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Export missing symbols causing undefined-reference link failures
Three non-template free functions had .cpp implementations but were
missing export macros, causing linker failures in test executables:
- phlex_model_EXPORT on friend operator== / operator<=> for
identifier_query in identifier.hpp (called from identifier test)
- phlex_core_EXPORT on detail::verify_name in glue.hpp (called from
template instantiations of glue<T>::provide/observe/transform/...)
- phlex_core_EXPORT on port_index_for and more_derived in message.hpp
(called from template instantiations in observer_node et al.)
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Export to_product_specifications, output_api, and product_store::more_derived
Three more non-template symbols with .cpp implementations were missing
export macros, causing linker failures across most test executables:
- phlex_model_EXPORT on to_product_specifications() in
product_specification.hpp (called from declared_transform.hpp and
declared_fold.hpp template instantiations in every test binary)
- phlex_core_EXPORT on output_api class in registration_api.hpp
(constructor called from glue<T>::output template instantiations)
- phlex_model_EXPORT on more_derived(product_store_ptr, product_store_ptr)
in product_store.hpp (distinct from the message::more_derived fixed
earlier; called directly from product_store test and from template
get_most_derived)
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Apply clang-format fixes
Add _internal companion libraries for test access and size/timing comparison
Introduce phlex_make_internal_library(target LIBRARIES ...) in
PhlexSymbolVisibility.cmake. The function:
- reads SOURCES from the already-defined public target (no duplication)
- handles generated sources (configure_file output) via src/bin dir fallback
- creates ${target}_internal SHARED using add_library (not cet_make_library)
so cetmodules never registers it for installation or package export
- propagates $<BUILD_INTERFACE:PROJECT_SOURCE_DIR> PUBLIC so consumers
such as layer_generator_internal can resolve phlex/* headers
- propagates COMPILE_DEFINITIONS and COMPILE_OPTIONS from the public target
- omits CXX_VISIBILITY_PRESET hidden so all symbols are visible by default
Applied to all four non-trivial shared libraries:
phlex_utilities, phlex_model, phlex_core, run_phlex
The _internal chain is phlex_core_internal -> phlex_model_internal ->
phlex_utilities_internal so tests linking only to phlex::core_internal
get all internal symbols transitively without loading the public .so files.
layer_generator_internal added in plugins/CMakeLists.txt (same sources,
links PRIVATE phlex_core_internal) to avoid loading both the visibility-hidden
and all-visible phlex_core variants in the same test process (ODR).
All tests in test/CMakeLists.txt and test/utilities/CMakeLists.txt switched to
the _internal variants, except:
- config_test and identifier which need phlex::configuration and thus
transitively pull in the public phlex_core/model/utilities libs; since
the relevant symbols are already exported these tests are fine on the
public libraries
- type_deduction which only requires the header-only phlex::metaprogramming
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Move export headers from include/ to include/phlex/
Generate and install phlex_X_export.hpp files under include/phlex/
instead of flat in include/, matching the layout of all other Phlex
installed headers. Update every #include "X_export.hpp" directive
in the 42 affected public headers to #include "phlex/X_export.hpp".
The target_include_directories entries are unchanged — the base paths
(PROJECT_BINARY_DIR/include for build, include for install) already
resolve correctly once the phlex/ subdirectory is part of the filename.
Co-authored-by: pcanal <5175087+pcanal@users.noreply.github.com>
Remove redundant INSTALL_INTERFACE from phlex_apply_symbol_visibility
cetmodules adds INSTALL_INTERFACE:include to every target it creates via
cet_make_library, so the explicit INSTALL_INTERFACE:include entry in
phlex_apply_symbol_visibility was a no-op duplicate. The only unique
contribution of the target_include_directories call is the
BUILD_INTERFACE to ${PROJECT_BINARY_DIR}/include, which is where
generate_export_header writes the generated phlex/X_export.hpp headers.
Co-authored-by: pcanal <5175087+pcanal@users.noreply.github.com>
Apply cmake-format fixes
Introduce option(PHLEX_HIDE_SYMBOLS ... ON) in CMakeLists.txt and wire
it through both functions in PhlexSymbolVisibility.cmake:
phlex_apply_symbol_visibility(target)
ON (default): generate export header + CXX_VISIBILITY_PRESET hidden
OFF: generate export header only; all symbols visible by default
phlex_make_internal_library(target LIBRARIES ...)
ON (default): compile separate SHARED lib with full symbol visibility
OFF: create a thin INTERFACE target wrapping the public lib; no extra
compilation or .so needed since the public lib already exports
everything
In OFF mode _internal aliases and layer_generator_internal still exist
and link correctly via the INTERFACE wrappers, so no other files change.
Benchmark tests (test/benchmarks/, test/max-parallelism/) already link
exclusively to curated public targets in both modes.
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Make layer_generator_internal conditional on PHLEX_HIDE_SYMBOLS
When PHLEX_HIDE_SYMBOLS=OFF, layer_generator already exports all symbols,
so compiling a separate .so for layer_generator_internal is unnecessary.
Use a thin INTERFACE alias instead, matching the behaviour of
phlex_make_internal_library() for the core libraries.
All plugins used by the benchmark tests (test/benchmarks/,
test/max-parallelism/) already link exclusively against curated public
targets (phlex::module, phlex::core, layer_generator), not the
_internal variants. This is unaffected by PHLEX_HIDE_SYMBOLS.
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Add PhlexOptimization.cmake: -fno-semantic-interposition, -fno-plt, PHLEX_ENABLE_IPO
The three optimizations are chosen specifically for the symbol-hiding +
external-plugin architecture:
1. -fno-semantic-interposition (GCC >= 9, Clang >= 8; when PHLEX_HIDE_SYMBOLS=ON)
Allows the compiler to inline, devirtualise, and generate direct (non-PLT)
calls for same-DSO accesses to exported functions. Safe only once the
exported-symbol surface is explicitly bounded by export macros — exactly
what PHLEX_HIDE_SYMBOLS=ON provides.
2. -fno-plt (GCC >= 7.3, Clang >= 4, ELF/Linux only)
Replaces PLT stubs with GOT-based dispatch for calls FROM a Phlex library
TO external DSOs (TBB, Boost, spdlog, ...). Removes one level of
indirection from every cross-DSO call after load-time resolution.
3. PHLEX_ENABLE_IPO (default OFF)
Sets CMAKE_INTERPROCEDURAL_OPTIMIZATION_RELEASE and _RELWITHDEBINFO
after confirming compiler support. LTO is safe with symbol hiding because
export attributes preserve the complete exported-symbol set; the linker
cannot eliminate or rename any default-visibility symbol. External plugins
compiled without LTO are unaffected.
Intentionally excluded: -ffast-math/-funsafe-math-optimizations, which can
silently break floating-point correctness in physics/numerical code.
PhlexOptimization.cmake is included from the root CMakeLists.txt.
phlex_apply_optimizations() is called after phlex_apply_symbol_visibility()
in each of the five shared library CMakeLists.txt files.
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Auto-enable PHLEX_ENABLE_IPO for Release/RelWithDebInfo build types
cmake -DCMAKE_BUILD_TYPE=Release now automatically opts into LTO without
requiring a separate -DPHLEX_ENABLE_IPO=ON. The default is derived from
CMAKE_BUILD_TYPE at first configure time:
Release / RelWithDebInfo → PHLEX_ENABLE_IPO defaults to ON
Debug / Coverage / sanitizer builds → PHLEX_ENABLE_IPO defaults to OFF
Multi-config generators (no CMAKE_BUILD_TYPE) → defaults to OFF
The option can always be overridden explicitly with -DPHLEX_ENABLE_IPO=ON|OFF.
The compile-level flags (-fno-semantic-interposition, -fno-plt) are
unchanged: they are applied unconditionally (subject to compiler/platform
support) regardless of build type, complementing CMake's per-config
CMAKE_CXX_FLAGS_RELEASE (-O3 -DNDEBUG) without duplicating it.
Documentation in the module header is updated to describe the full
build-type interaction.
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Default PHLEX_HIDE_SYMBOLS based on CMAKE_BUILD_TYPE
Mirror the PHLEX_ENABLE_IPO pattern: for Release/RelWithDebInfo builds,
default PHLEX_HIDE_SYMBOLS=ON (curated API + -fno-semantic-interposition
safe); for Debug, Coverage, and sanitizer builds, default OFF so all
symbols are visible to debuggers, profilers, and sanitizer stack traces.
cmake -DCMAKE_BUILD_TYPE=Release → PHLEX_HIDE_SYMBOLS=ON, PHLEX_ENABLE_IPO=ON
cmake -DCMAKE_BUILD_TYPE=Debug → PHLEX_HIDE_SYMBOLS=OFF, PHLEX_ENABLE_IPO=OFF
cmake (no CMAKE_BUILD_TYPE) → both OFF (conservative fallback)
Both options can still be overridden explicitly at any time.
Update PhlexOptimization.cmake header to document the combined
interaction of both options with CMAKE_BUILD_TYPE.
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Auto-adjust PHLEX_HIDE_SYMBOLS default when PHLEX_ENABLE_IPO=ON
Move PHLEX_HIDE_SYMBOLS from CMakeLists.txt into PhlexOptimization.cmake
so both options are co-located and their defaults are derived together.
PHLEX_ENABLE_IPO is now defined first; PHLEX_HIDE_SYMBOLS defaults to ON
whenever PHLEX_ENABLE_IPO=ON (in addition to Release/RelWithDebInfo):
cmake -DPHLEX_ENABLE_IPO=ON → PHLEX_HIDE_SYMBOLS defaults ON
cmake -DCMAKE_BUILD_TYPE=Release → both default ON (unchanged)
cmake (Debug / no type) → both default OFF (unchanged)
LTO achieves its full potential only with hidden symbols: without
-fvisibility=hidden the LTO optimizer must treat every exported symbol
as interposable and cannot apply -fno-semantic-interposition. Making
IPO automatically pull in symbol hiding as its default ensures the optimal
combination is selected whenever LTO is requested.
The per-target compile flags in phlex_apply_optimizations() are unchanged:
they already self-adjust at build time based on the final option values.
The note emitted when a user explicitly overrides PHLEX_HIDE_SYMBOLS=OFF
after requesting IPO is retained (now factual rather than advisory).
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Force PHLEX_HIDE_SYMBOLS=ON when PHLEX_ENABLE_IPO=ON (not just default)
option() only sets the cache default when the variable is not yet present,
so an explicit -DPHLEX_HIDE_SYMBOLS=OFF on the command line bypassed the
defaulting logic and produced the invalid combination in CMakeCache.txt.
Fix: after both options are resolved from the cache, check for the
PHLEX_ENABLE_IPO=ON + PHLEX_HIDE_SYMBOLS=OFF combination and use
set(... FORCE) to correct PHLEX_HIDE_SYMBOLS to ON. A STATUS message
explains the forced correction. CMakeCache.txt will now always show
PHLEX_HIDE_SYMBOLS=ON when PHLEX_ENABLE_IPO=ON.
Before: cmake ... -DPHLEX_HIDE_SYMBOLS=OFF -DCMAKE_BUILD_TYPE=Release
→ CMakeCache.txt: PHLEX_ENABLE_IPO=ON, PHLEX_HIDE_SYMBOLS=OFF ✗
After: cmake ... -DPHLEX_HIDE_SYMBOLS=OFF -DCMAKE_BUILD_TYPE=Release
→ STATUS "PHLEX_HIDE_SYMBOLS forced ON because PHLEX_ENABLE_IPO=ON"
→ CMakeCache.txt: PHLEX_ENABLE_IPO=ON, PHLEX_HIDE_SYMBOLS=ON ✓
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Revert forced PHLEX_HIDE_SYMBOLS correction; make options fully independent
Revert the set(FORCE) that prevented PHLEX_ENABLE_IPO=ON +
PHLEX_HIDE_SYMBOLS=OFF: this combination is valid and essential for
benchmarking the LTO benefit with and without symbol hiding.
All four combinations are now fully supported:
PHLEX_HIDE_SYMBOLS=ON PHLEX_ENABLE_IPO=ON → LTO + -fno-semantic-interposition (maximum)
PHLEX_HIDE_SYMBOLS=OFF PHLEX_ENABLE_IPO=ON → LTO only (benchmark comparison case)
PHLEX_HIDE_SYMBOLS=ON PHLEX_ENABLE_IPO=OFF → -fno-semantic-interposition only
PHLEX_HIDE_SYMBOLS=OFF PHLEX_ENABLE_IPO=OFF → no special flags (debug/default)
Both options now default independently from CMAKE_BUILD_TYPE (ON for
Release/RelWithDebInfo, OFF otherwise) with no cross-dependency in the
defaulting logic. The per-target flags in phlex_apply_optimizations()
already self-adjust correctly to whichever combination is in effect.
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
|
@phlexbot cmake-fix |
|
Automatic cmake-format fixes pushed (commit c63bd45). |
Adds hidden-by-default symbol visibility to all five Phlex shared libraries, introduces non-installed
_internalcompanion libraries for testing internal APIs and benchmarking, and adds safe performance-oriented compiler flags (-fno-semantic-interposition,-fno-plt, LTO) all controlled by two independent build options.CMake infrastructure —
Modules/private/PhlexSymbolVisibility.cmake— two functions:phlex_apply_symbol_visibility(target)— generatesphlex/${target}_export.hppinto${PROJECT_BINARY_DIR}/include/phlex/(installed toinclude/phlex/), setsCXX_VISIBILITY_PRESET hiddenwhenPHLEX_HIDE_SYMBOLS=ON.phlex_make_internal_library(target LIBRARIES ...)— whenPHLEX_HIDE_SYMBOLS=ON: compiles a non-installed SHARED library from the same sources with default (visible) symbol visibility; whenOFF: creates a thin INTERFACE alias of the public target (no extra compilation needed since all symbols are already visible).PhlexOptimization.cmake— definesPHLEX_HIDE_SYMBOLS,PHLEX_ENABLE_IPO, andphlex_apply_optimizations(target):-fno-semantic-interposition— applied only whenPHLEX_HIDE_SYMBOLS=ON; safe once the exported-symbol surface is bounded by export macros.-fno-plt— applied on ELF platforms unconditionally; reduces cross-DSO dispatch overhead.PHLEX_ENABLE_IPO— enables LTO for Release/RelWithDebInfo viaCMAKE_INTERPROCEDURAL_OPTIMIZATION_RELEASE.Option defaults and interactions
CMAKE_BUILD_TYPEPHLEX_HIDE_SYMBOLSPHLEX_ENABLE_IPORelease/RelWithDebInfoONONDebug/ sanitizerOFFOFFOFFOFFAll four
ON/OFFcombinations are explicitly valid and independently overridable. In particular,PHLEX_HIDE_SYMBOLS=OFF PHLEX_ENABLE_IPO=ONis intentional — it enables benchmarking LTO benefit independently of symbol hiding:Export macro coverage
~45 headers annotated across all five libraries. Every class and non-template free function with a
.cppimplementation carriesphlex_X_EXPORT. Template-only code is left unmarked. Boost.JSONtag_invokecustomization points forconfiguration,product_query, andidentifierare explicitly exported._internal library chain
phlex_utilities_internalphlex::utilities_internalphlex_model_internalphlex::model_internalphlex_utilities_internalphlex_core_internalphlex::core_internalphlex_model_internalrun_phlex_internalphlex_core_internalTests link to
phlex::core_internaland get the full symbol set transitively.layer_generator_internalfollows the samePHLEX_HIDE_SYMBOLSconditional inplugins/CMakeLists.txt. Benchmark tests (test/benchmarks/,test/max-parallelism/) link exclusively to public curated targets in both modes.Self-contained header fixes
Added
<cstddef>tohashing.hppandidentifier.hpp,<compare>toidentifier.hpp,<string>toload_module.hpp.💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.