Consolidate templated wrappers#94
Draft
kwabenantim wants to merge 6 commits into
Draft
Conversation
cppwg emitted one wrapper .hpp/.cpp pair per template instantiation, so a class
wrapped at N instantiations produced 2N files of near-identical code. Emit a
single {Class}.cppwg.hpp/.cpp per class instead, holding one shared preamble and
one register_{py_name}_class function per instantiation. The registration bodies
are unchanged (byte-identical), just co-located.
- Split the class_cpp template into class_cpp_header (the once-per-class
preamble: includes, namespace, holder, prefix code, deduplicated trampoline
return typedefs) and class_cpp_register (the per-instantiation block); class_hpp
now declares every instantiation's register function; add struct_enum_register.
- class_writer.write() builds one file pair per class, deduplicating file-scope
trampoline typedefs (so a type shared by several instantiations is not
redefined) and naming files after the class (CppClassInfo.py_name_base()).
- module_writer emits one include per class; register calls are unchanged. It
warns if two classes would collide on the same wrapper file name.
This reduces the generated file count (and the translation units re-parsing the
heavy shared headers) without changing what is registered.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Regenerate with the combined-per-class wrappers: each class's template
instantiations now share one {Class}.cppwg.hpp/.cpp instead of one pair per
instantiation. Shapes drops from 19 to 15 wrapper files, cells from 34 to 22;
the module main files include one header per class and their register calls are
unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements issue #30 by consolidating per-template-instantiation wrapper outputs into a single wrapper file pair per class (one shared .cppwg.hpp/.cpp containing multiple register_* functions), reducing generated file count while preserving explicit instantiation registration.
Changes:
- Refactors class wrapper generation to emit one shared C++ preamble per class and append one registration block per instantiation.
- Updates module generation to include one wrapper header per class (using a new
py_name_base()), while still callingregister_*for each instantiation. - Adjusts templates and tests to match the new “shared header + per-instantiation register block” structure.
Reviewed changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_class_writer.py | Updates writer-facing tests for shared cpp preamble and per-instantiation register blocks. |
| examples/shapes/wrapper/primitives/Shape.cppwg.hpp | Consolidates multiple Shape instantiation declarations into one header. |
| examples/shapes/wrapper/primitives/Shape.cppwg.cpp | Consolidates multiple Shape instantiation definitions into one cpp. |
| examples/shapes/wrapper/primitives/Shape_3.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/shapes/wrapper/primitives/Shape_3.cppwg.cpp | Removes per-instantiation cpp in favor of consolidated cpp. |
| examples/shapes/wrapper/primitives/Rectangle.cppwg.cpp | Moves typedef into per-class register block layout (generator consistency). |
| examples/shapes/wrapper/primitives/Cuboid.cppwg.cpp | Moves typedef into per-class register block layout (generator consistency). |
| examples/shapes/wrapper/primitives/_pyshapes_primitives.main.cppwg.cpp | Updates module includes to consolidated Shape wrapper header. |
| examples/shapes/wrapper/geometry/Point.cppwg.hpp | Consolidates multiple Point instantiation declarations into one header. |
| examples/shapes/wrapper/geometry/Point.cppwg.cpp | Consolidates multiple Point instantiation definitions into one cpp. |
| examples/shapes/wrapper/geometry/Point_3.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/shapes/wrapper/geometry/Point_3.cppwg.cpp | Removes per-instantiation cpp in favor of consolidated cpp. |
| examples/shapes/wrapper/geometry/_pyshapes_geometry.main.cppwg.cpp | Updates module includes to consolidated Point wrapper header. |
| examples/shapes/wrapper/composites/Square.cppwg.cpp | Moves typedef into per-class register block layout (generator consistency). |
| examples/cells/dynamic/wrappers/all/Scene.cppwg.hpp | Consolidates multiple Scene instantiation declarations into one header. |
| examples/cells/dynamic/wrappers/all/Scene.cppwg.cpp | Consolidates multiple Scene instantiation definitions into one cpp. |
| examples/cells/dynamic/wrappers/all/Scene_3.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/cells/dynamic/wrappers/all/Scene_3.cppwg.cpp | Removes per-instantiation cpp in favor of consolidated cpp. |
| examples/cells/dynamic/wrappers/all/PottsMesh.cppwg.hpp | Adds consolidated PottsMesh wrapper header declaring both instantiations. |
| examples/cells/dynamic/wrappers/all/PottsMesh.cppwg.cpp | Consolidates PottsMesh instantiation definitions into one cpp. |
| examples/cells/dynamic/wrappers/all/PottsMesh_3.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/cells/dynamic/wrappers/all/PottsMesh_3.cppwg.cpp | Removes per-instantiation cpp in favor of consolidated cpp. |
| examples/cells/dynamic/wrappers/all/PottsMesh_2.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/cells/dynamic/wrappers/all/PetscUtils.cppwg.cpp | Moves typedef into per-class register block layout (generator consistency). |
| examples/cells/dynamic/wrappers/all/Node.cppwg.hpp | Consolidates multiple Node instantiation declarations into one header. |
| examples/cells/dynamic/wrappers/all/Node.cppwg.cpp | Consolidates multiple Node instantiation definitions into one cpp. |
| examples/cells/dynamic/wrappers/all/Node_3.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/cells/dynamic/wrappers/all/Node_3.cppwg.cpp | Removes per-instantiation cpp in favor of consolidated cpp. |
| examples/cells/dynamic/wrappers/all/MeshFactory.cppwg.hpp | Adds consolidated MeshFactory wrapper header declaring both instantiations. |
| examples/cells/dynamic/wrappers/all/MeshFactory.cppwg.cpp | Consolidates MeshFactory instantiation definitions into one cpp. |
| examples/cells/dynamic/wrappers/all/MeshFactory_PottsMesh_3.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/cells/dynamic/wrappers/all/MeshFactory_PottsMesh_3.cppwg.cpp | Removes per-instantiation cpp in favor of consolidated cpp. |
| examples/cells/dynamic/wrappers/all/MeshFactory_PottsMesh_2.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/cells/dynamic/wrappers/all/MacroMesh.cppwg.hpp | Adds consolidated MacroMesh wrapper header declaring both instantiations. |
| examples/cells/dynamic/wrappers/all/MacroMesh.cppwg.cpp | Consolidates MacroMesh instantiation definitions into one cpp. |
| examples/cells/dynamic/wrappers/all/MacroMesh_3_3.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/cells/dynamic/wrappers/all/MacroMesh_3_3.cppwg.cpp | Removes per-instantiation cpp in favor of consolidated cpp. |
| examples/cells/dynamic/wrappers/all/MacroMesh_2_2.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/cells/dynamic/wrappers/all/Facet.cppwg.hpp | Renames guard/header naming to consolidated per-class naming. |
| examples/cells/dynamic/wrappers/all/Facet.cppwg.cpp | Updates includes/typedef placement for consolidated per-class naming. |
| examples/cells/dynamic/wrappers/all/Corner.cppwg.hpp | Renames guard/header naming to consolidated per-class naming. |
| examples/cells/dynamic/wrappers/all/Corner.cppwg.cpp | Updates includes/typedef placement for consolidated per-class naming. |
| examples/cells/dynamic/wrappers/all/Cell.cppwg.cpp | Moves typedef into per-class register block layout (generator consistency). |
| examples/cells/dynamic/wrappers/all/AbstractMesh.cppwg.hpp | Adds consolidated AbstractMesh wrapper header declaring both instantiations. |
| examples/cells/dynamic/wrappers/all/AbstractMesh.cppwg.cpp | Consolidates AbstractMesh instantiation definitions into one cpp. |
| examples/cells/dynamic/wrappers/all/AbstractMesh_3_3.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/cells/dynamic/wrappers/all/AbstractMesh_3_3.cppwg.cpp | Removes per-instantiation cpp in favor of consolidated cpp. |
| examples/cells/dynamic/wrappers/all/AbstractMesh_2_2.cppwg.hpp | Removes per-instantiation header in favor of consolidated header. |
| examples/cells/dynamic/wrappers/all/_pycells_all.main.cppwg.cpp | Updates module includes to consolidated per-class wrapper headers. |
| cppwg/writers/module_writer.py | Switches module wrapper includes to per-class headers; adds collision detection. |
| cppwg/writers/class_writer.py | Implements shared cpp header + per-instantiation register blocks; emits single file pair per class. |
| cppwg/templates/pybind11_default.py | Splits templates into cpp preamble vs register blocks; updates hpp to declare all instantiations. |
| cppwg/info/class_info.py | Adds py_name_base() and uses it for consistent base naming across instantiations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…r file Add a README tip noting the combined-per-class output: Foo<2> and Foo<3> are wrapped in one Foo.cppwg.hpp/.cpp pair (with a register_Foo_2_class and register_Foo_3_class each) rather than a file pair per instantiation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The combined-per-class write() loop iterates class_info.decls and indexes class_info.py_names by the same position, but the precondition only compared decls against cpp_names. A short py_names would then fail as an IndexError mid-loop (or silently mislabel a wrapper). Check all three parallel lists are equal length up-front and raise a message naming the class and the lengths, so a desync fails deterministically and is easy to diagnose. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each class writes one wrapper file pair named after the class. If two classes in a module map to the same file stem, one overwrites the other's .hpp/.cpp and they collide on the register_..._class symbols, producing a broken output set that only surfaces later as a confusing compile/link error. There is no benign case, so raise a ValueError at generation time naming both classes and pointing at the fix, instead of logging a warning and continuing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Supports #30