Enum support - #114
Conversation
Add an `enums` module config key (plus `use_all_enums`/CPPWG_ALL) so a plain namespace-scope enum can be wrapped directly, instead of only reaching Python as the sole member of a struct via the class writer's struct-enum special case. Mirrors the free-function path end to end: a new CppEnumInfo resolves the decl via source_ns.enumerations(); a new CppEnumWrapperWriter emits py::enum_<Foo>(m, "Foo").value(...).export_values() inline in the module; the parser, module info, module writer and header collection writer gain the matching enum plumbing. Both unscoped `enum` and scoped `enum class` flow through the one path. Enums are registered before free functions and class register calls, so an enum used as a defaulted argument of a wrapped signature is already registered when pybind11 materialises that default at import time. The struct-enum special case is left untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
find_classes_in_source matched the class/struct keyword in a scoped enum (`enum class Color`), so wrapping such an enum logged a misleading "Unknown class Color". Skip a class/struct keyword directly preceded by `enum` via a fixed-width negative lookbehind (source whitespace is normalised to single spaces, so the lookbehind is reliable). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add primitives/ShapeKind.hpp: an unscoped enum ShapeKind, a scoped enum class Handedness, and a ShapeClassifier whose method takes ShapeKind as a defaulted argument. Wrap the two enums via the new `enums` config key and the class alongside them, and regenerate the primitives wrappers. testEnums covers ShapeKind.CIRCLE and its exported values, the scoped Handedness, and that the module imports at all with the defaulted enum argument (the import-time registration path). All example tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
Pull request overview
Adds first-class support for wrapping C++ enums in cppwg, allowing enums to be specified directly in package_info.yaml (or discovered via CPPWG_ALL) and emitted as module-scope py::enum_ registrations ahead of functions/classes to support default enum arguments at import time.
Changes:
- Introduces
enums/use_all_enumsin config parsing andModuleInfo, plus enum discovery/update wiring. - Adds
CppEnumInfoandCppEnumWrapperWriter, and injects enum registration into the module template before free functions and class register calls. - Extends tests, documentation, and the shapes example to demonstrate and validate enum wrapping (including defaulted enum args).
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_utils.py | Adds regression test ensuring scoped enums aren’t misdetected as classes. |
| tests/test_package_info_parser.py | Adds parsing tests for explicit enums: and CPPWG_ALL enums. |
| tests/test_module_writer.py | Tests enum registration ordering before free functions/classes for default-arg safety. |
| tests/test_header_collection_writer.py | Updates test stubs to include enums/use_all_enums. |
| tests/test_enum_writer.py | Adds unit tests for enum wrapper code generation. |
| tests/test_enum_info.py | Adds unit tests for CppEnumInfo construction/config application. |
| examples/shapes/wrapper/wrapper_header_collection.cppwg.hpp | Includes the new enum header in the wrapper header collection. |
| examples/shapes/wrapper/primitives/ShapeClassifier.cppwg.hpp | Adds generated wrapper header for the new example class. |
| examples/shapes/wrapper/primitives/ShapeClassifier.cppwg.cpp | Adds generated wrapper implementation using an enum default argument. |
| examples/shapes/wrapper/primitives/_pyshapes_primitives.main.cppwg.cpp | Demonstrates generated module-scope enum registrations and class registration order. |
| examples/shapes/wrapper/package_info.yaml | Demonstrates configuring enums: in a real wrapper config. |
| examples/shapes/src/py/tests/test_classes.py | Adds Python-level tests for enum wrapping and defaulted enum args. |
| examples/shapes/src/cpp/primitives/ShapeKind.hpp | Adds example unscoped + scoped enums and a class using a defaulted enum argument. |
| doc/reference.md | Documents the new enums module option and per-enum options. |
| cppwg/writers/module_writer.py | Emits enum registration blocks into the module body (before free funcs/classes). |
| cppwg/writers/header_collection_writer.py | Includes headers for explicitly configured enums and treats use_all_enums as include-all. |
| cppwg/writers/enum_writer.py | New writer to generate py::enum_ registration code. |
| cppwg/utils/utils.py | Adjusts class-finder regex to avoid treating enum class/struct as classes. |
| cppwg/templates/pybind11_default.py | Adds ${enums} to the module template and adds an enum_register template. |
| cppwg/parsers/package_info_parser.py | Parses enums / use_all_enums and constructs CppEnumInfo objects. |
| cppwg/info/module_info.py | Adds enum collection fields, discovery/update behavior, and sorting. |
| cppwg/info/enum_info.py | New info type resolving enums from pygccxml namespace declarations. |
Suppressed comments (1)
examples/shapes/src/cpp/primitives/ShapeKind.hpp:64
- This
#endifcomment should be updated to match the (non-reserved) include guard macro name so it stays consistent after renaming.
#endif // _SHAPEKIND_HPP
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.export_values() exports an enum's enumerators into the enclosing (module) scope. That is correct only for unscoped enums; for a scoped enum (enum class / enum struct) the enumerators belong on the type, and exporting them pollutes the module scope and can collide with other names (e.g. LEFT, RIGHT). pygccxml does not expose enum scopedness, so detect it from the source text (is_scoped_enum_in_source_file) and record it on CppEnumInfo.scoped when the decl is resolved. The enum writer then closes the registration chain with .export_values() for an unscoped enum or a plain ; for a scoped one. The shapes example's scoped Handedness now omits .export_values() (its wrapper regenerated); testEnums asserts its enumerators are not module-level while the unscoped ShapeKind's still are. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Whether an enum exports its enumerators into the module scope (pybind11's .export_values()) defaults to mirroring the C++ enum kind, but this is a pybind11 choice independent of the C++: an unscoped enum can be wrapped without exporting, and a scoped one can be exported. Add a tri-state `export_values` option: unset mirrors the C++ kind (unchanged default), True/False forces it either way. The main use is setting it False on an unscoped enum to keep its values off the module scope and avoid name collisions. It is a common option (on BaseInfo), so it inherits down the info tree: setting it at the package or module level applies to every enum below, and a per-enum value overrides. Resolved via CppEnumInfo.should_export_values using hierarchy_attribute. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_SHAPEKIND_HPP starts with an underscore followed by an uppercase letter, which is reserved to the implementation. Rename it to SHAPEKIND_HPP_, matching the non-reserved guard style used by the other cells example headers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Several shapes and cells example headers used include guards of the form _NAME_HPP - a leading underscore followed by an uppercase letter, which is reserved to the implementation. Rename them to NAME_HPP_, matching the non-reserved style already used by the other example headers. Guard-only change: include guards are not part of the parsed AST, so the generated wrappers are unaffected (shapes wrappers regenerate identically). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- basics.md listed only classes and free functions; note enums as a wrappable entity (intro, "Selecting what to wrap", and the CPPWG_ALL example). - is_scoped_enum_in_source_file said .export_values() "only applies to unscoped enums"; it is the default, overridable by the export_values option - reword. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 43 changed files in this pull request and generated no new comments.
Suppressed comments (2)
cppwg/writers/header_collection_writer.py:132
- Enum headers are only added to the header-collection include list when
source_file_pathis set. However the enum config/parser (and tests) also populatesource_file; with the current logic, providing onlysource_filewon’t include the header, so the enum won’t be parsed andCppEnumInfo.update_from_ns()will fail to resolve it.
for enum_info in module_info.enum_collection:
if enum_info.source_file_path:
include_files.add(
os.path.basename(enum_info.source_file_path)
)
cppwg/info/enum_info.py:67
- The error message here says the user must set
source_file_path, but enums can also be configured withsource_file(and the parser/tests accept it). If enum headers are included viasource_filetoo, this message becomes misleading; it should mention both options (or otherwise be aligned with the supported config keys).
logger.error(
f"Could not find enum {self.name}. Set source_file_path "
"in the config so that its header is included."
)
The enum update_from_ns (declaration lookup, not-found error, scope detection) and should_export_values were exercised only by the example generator runs, not the unit suite, and the not-found branch by nothing - dropping patch coverage. Add enum_info unit tests mirroring test_free_function_info, and extend the header collection test to assert an enum's source_file_path header is included. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
codecov/patch flagged two uncovered diff spots (target 100%): the `enums: CPPWG_ALL` discovery loop in ModuleInfo.update_from_ns (exercised by no flag - the examples use explicit enums or none), and the false side of the enum `if source_file_path` branch in the header collection writer. Extend the module_info discovery test to enable use_all_enums with an in-scope and an out-of-scope enum (covering both is_decl_in_source_path branches), and add a pathless enum to the header collection test so its skip branch is taken. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The two were documented in isolation (source_file only under Class options, source_file_path only under Enum options) and never contrasted, and free functions had no options table at all. Explain the difference in the Class options section (source_file is a bare filename; source_file_path is a resolved, verified path relative to the source root), document source_file_path for classes too, and add a Free function options section noting its source_file_path is required (name_override is not applied to free functions, so it is omitted). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Previously only classes could name their header with a bare source_file; free functions and enums required the full source_file_path. But source_file is already parsed onto every entity, and a class is included by that bare filename (resolved via the build's include path) - the same works for free functions and enums. The header collection now falls back to source_file when source_file_path is unset, so either form points cppwg at the header. Document both options for free functions and enums (source_file_path takes precedence), and switch the shapes example enums to the simpler source_file form (wrappers unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 44 changed files in this pull request and generated no new comments.
Suppressed comments (2)
cppwg/info/module_info.py:300
- When
use_all_enumsdiscovers enums, the createdCppEnumInfoobjects are appended directly without settingparent.CppEnumInfo.should_export_values()relies onhierarchy_attribute("export_values"), which walks theparentchain, so module/package-levelexport_values(and any other inheritable options) won’t apply for enums discovered viaCPPWG_ALL.
enum_info = CppEnumInfo(enum_decl.name)
enum_info.module_info = self
self.enum_collection.append(enum_info)
cppwg/info/enum_info.py:67
- The error log here says only
source_file_pathwill cause an explicitly listed enum’s header to be included, butsource_fileis also supported (and the header may already be included via another wrapped entity). This message/comment is misleading and will send users toward the wrong fix.
# header is only included when source_file_path is set in the config.
logger = logging.getLogger()
logger.error(
f"Could not find enum {self.name}. Set source_file_path "
"in the config so that its header is included."
A CPPWG_ALL-discovered enum sets module_info (the backing attribute of the parent property), so export_values (and other inheritable options) resolve up the info tree - but no test asserted it. Add a regression test that an enum discovered via use_all_enums inherits a package-level export_values through the full enum -> module -> package chain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The use_all_* discovery loops set `info.module_info = self` inline before appending, which reads as if the parent is not set (it is - module_info backs the parent property). Call the existing add_class/add_free_function/add_enum helpers instead, which append and set `.parent` explicitly, matching the config-driven path and making the parenting obvious. No behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The "Could not find …" error and its comment said only source_file_path gets an explicitly listed enum/free-function header included, but source_file works too, and the header may already be included via another wrapped entity in the same header. Reword both so the message points at the right fixes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
source_ns.enumerations() also returns enums nested in a class/struct, so enums: CPPWG_ALL discovered e.g. SemLatticeType::Value and emitted it as an unqualified py::enum_<Value> that does not compile (and duplicated). Discovery now skips enums whose parent is a class/struct, wrapping only namespace-scope enums; the struct-enum special case still handles the wrapped-struct pattern. Found by running enums: CPPWG_ALL against Chaste's SemEnumerations.hpp, which has a namespace-scope enum (SemNodeRegion) alongside a struct-wrapped one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Clarify in the Enum options section that `enums` is for plain namespace-scope enums, while a struct wrapping a single enum (e.g. RelativeTo) is listed under `classes` and wrapped by the class-writer special case - listing it under `enums` fails. Note the clean CPPWG_ALL split (struct-wrappers via classes, plain enums via enums). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comments, docstrings and test data referenced Chaste types (AbstractForce, AbstractLinearPde, TetrahedralMesh, AddCellWriter<CellAgesWriter>, AbstractNode, VertexMesh, ...) that mean nothing to a cppwg reader. Use identifiers from the cppwg examples instead: shapes Shape/AbstractShape and GetAreaIn<SquareMetres> (templated method with a custom generator), cells MacroMesh<2, 2> (the defaulted trailing-arg / CastXML-collapse case) and AbstractSphericalMesh/SphericalMesh, or generic placeholders. No behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fixes #113