-
Notifications
You must be signed in to change notification settings - Fork 3
pass compiler to policies' initialize() #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request refactors the policy initialization interface to pass a context object instead of iterator pairs, improving API consistency and enabling better access to initialization state. The changes update the initialize function signatures across policies and add proper documentation for the new InitializeContext and InitializeClass blueprints.
- Refactored policy
initializemethods to accept a context object instead of iterator pairs - Updated documentation to describe the new
InitializeContextandInitializeClassblueprints - Removed unused
option_baseinfrastructure and simplified option handling - Fixed parameter constness for
finalizemethods across policies
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_shared_virtual_ptr_value_semantics.cpp | Removed space in cast expression for style consistency |
| include/boost/openmethod/preamble.hpp | Added unspecified type, removed option_base, updated InitializeContext documentation, refactored option handling |
| include/boost/openmethod/policies/vptr_vector.hpp | Updated initialize to accept context object, improved parameter constness |
| include/boost/openmethod/policies/vptr_map.hpp | Updated initialize to accept context object, improved parameter constness |
| include/boost/openmethod/policies/fast_perfect_hash.hpp | Updated initialize to use context object, refactored trace handling to use context members |
| include/boost/openmethod/initialize.hpp | Added context methods to compiler, refactored trace handling, removed option_base checks, added MSVC workaround |
| include/boost/openmethod/core.hpp | Moved unspecified type definition to preamble.hpp |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| //! | ||
| //! @return A forward iterator to the end of a range of @ref | ||
| //! InitializeClass objects. | ||
| detail::unspecified classes_end() const; |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The InitializeContext blueprint is incomplete. It should document the has_option template member and the tr member, which are accessed in fast_perfect_hash.hpp at lines 193, 194, 226, and 227. These members are part of the actual context interface (provided by the compiler class) but are not documented in the blueprint.
| detail::unspecified classes_end() const; | |
| detail::unspecified classes_end() const; | |
| //! Checks if a given option is enabled in the context. | |
| //! | |
| //! @tparam Option The option to check. | |
| //! @return true if the option is enabled, false otherwise. | |
| template <typename Option> | |
| bool has_option() const; | |
| //! Translation or type registry object associated with the context. | |
| //! | |
| //! The exact type is unspecified and provided by the compiler class. | |
| detail::unspecified tr; |
| //! function arguments. | ||
| //! @param options Zero or more option objects. | ||
| //! @tparam Options... Zero or more option types. | ||
| //! @param ctx A Context object. |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @param ctx documentation is incorrect for the finalize method. This method does not have a ctx parameter - it only takes options. Remove the @param ctx line.
| //! @param ctx A Context object. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #44 +/- ##
==========================================
Coverage ? 93.03%
==========================================
Files ? 40
Lines ? 2713
Branches ? 1226
==========================================
Hits ? 2524
Misses ? 159
Partials ? 30
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
No description provided.