better error message when overrider cannot be matched to method#63
better error message when overrider cannot be matched to method#63jll63 merged 3 commits intoboostorg:developfrom
Conversation
|
An automated preview of the documentation is available at https://63.openmethod.prtest3.cppalliance.org/libs/openmethod/doc/html/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-02-28 16:10:31 UTC |
There was a problem hiding this comment.
Pull request overview
This PR improves the compile-time diagnostic emitted when BOOST_OPENMETHOD_OVERRIDE cannot be matched to a corresponding method signature, and adds/adjusts CMake-based tests to validate behavior and make example tests runnable via ctest without a prior build.
Changes:
- Improve
BOOST_OPENMETHOD_OVERRIDE“method not found” error via a SFINAE-based locator with a targetedstatic_assert. - Register a new compile-fail test for the new diagnostic message.
- Update shared library example CMake/CTest wiring to build targets via fixtures and run executables from a known output directory.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
test/CMakeLists.txt |
Adds a new compile-fail test registration for the updated diagnostic. |
include/boost/openmethod/macros.hpp |
Introduces SFINAE detection and a clearer static_assert message when an overrider can’t be matched to a method. |
doc/modules/ROOT/examples/shared_libs/CMakeLists.txt |
Adds CTest build fixtures and output directory settings for shared-lib examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| openmethod_compile_fail_test( | ||
| compile_fail_override_method_not_found "cannot find 'speak' method that accepts the same arguments as the overrider") |
There was a problem hiding this comment.
This compile-fail test target references a source file compile_fail_override_method_not_found.cpp, but that file is not present under test/ in this PR. As-is, CMake configuration/build will fail because add_library(... "${testname}.cpp") points to a non-existent file. Please add the missing test/compile_fail_override_method_not_found.cpp (and ensure it triggers the new diagnostic), or remove this test registration.
| openmethod_compile_fail_test( | |
| compile_fail_override_method_not_found "cannot find 'speak' method that accepts the same arguments as the overrider") |
| add_test(NAME boost_openmethod-static | ||
| COMMAND "${shared_libs_output_dir}/boost_openmethod-static") | ||
| set_tests_properties(boost_openmethod-static PROPERTIES | ||
| FIXTURES_REQUIRED boost_openmethod-static-fixture) |
There was a problem hiding this comment.
The test command hardcodes the executable path as ${shared_libs_output_dir}/boost_openmethod-static. This is not robust on Windows (missing .exe) and can also break on multi-config generators where outputs may be placed under a per-config subdirectory. Prefer using $<TARGET_FILE:boost_openmethod-static> (and similarly for the other executables) so CTest runs the actual built artifact regardless of platform/config.
| add_test(NAME boost_openmethod-dynamic | ||
| COMMAND "${shared_libs_output_dir}/boost_openmethod-dynamic") | ||
| set_tests_properties(boost_openmethod-dynamic PROPERTIES | ||
| FIXTURES_REQUIRED "boost_openmethod-shared-fixture;boost_openmethod-dynamic-fixture") |
There was a problem hiding this comment.
This test command uses a manually constructed path ${shared_libs_output_dir}/boost_openmethod-dynamic, which can fail on platforms/configurations where the executable filename includes an extension (e.g., .exe) or is placed in a config-specific output directory. Use $<TARGET_FILE:boost_openmethod-dynamic> to make the test invocation generator- and platform-independent.
| add_test(NAME boost_openmethod-indirect | ||
| COMMAND "${shared_libs_output_dir}/boost_openmethod-indirect") | ||
| set_tests_properties(boost_openmethod-indirect PROPERTIES | ||
| FIXTURES_REQUIRED boost_openmethod-indirect-fixture) |
There was a problem hiding this comment.
The test command uses ${shared_libs_output_dir}/boost_openmethod-indirect, which may not exist as-is on Windows (extension) or with multi-config output layouts. Using $<TARGET_FILE:boost_openmethod-indirect> will ensure CTest executes the correct built binary.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #63 +/- ##
===========================================
+ Coverage 92.03% 92.66% +0.63%
===========================================
Files 40 43 +3
Lines 2838 3110 +272
Branches 1354 1544 +190
===========================================
+ Hits 2612 2882 +270
- Misses 187 188 +1
- Partials 39 40 +1 see 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
No description provided.