From b242d2cdcd272d7a51a91c7d3039a8ed5df7e39c Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 08:42:11 +0100 Subject: [PATCH 01/12] #119 Consolidate member-exclusion predicates into cppwg.info.exclusions The "is this method/constructor/data member wrapped?" decision was stated twice for each kind - once in a writer (emits the binding) and once inline in PackageInfo._iter_wrapped_arg_return_types (yields the arg/return/member types for dependency pruning and auto-includes). The duplication was enforced only by a comment and tests, and the inline walk had silently drifted: it lacked the writers' parent / private / artificial-copy-constructor / private-pure-virtual exclusions. Add cppwg/info/exclusions.py with method_is_excluded, constructor_is_excluded and variable_exclusion_reason / variable_is_excluded as the single source. The three writers delegate to it (the member writer keeps its per-reason debug logs via the reason helper) and the info walk calls the same predicates, so the two can no longer diverge. It lives in the info layer (imports only utils + pygccxml) to respect the info-must-not-import-writers boundary. Behaviour-preserving: the shapes wrappers regenerate byte-identically and the example builds and passes. Reorder the artificial-copy-constructor check so the cheap is_artificial gate short-circuits the heavier is_copy_constructor call. Co-Authored-By: Claude Opus 4.8 --- cppwg/info/exclusions.py | 293 ++++++++++++++++++++++++ cppwg/info/package_info.py | 125 ++-------- cppwg/writers/constructor_writer.py | 85 +------ cppwg/writers/member_variable_writer.py | 90 ++------ cppwg/writers/method_writer.py | 50 +--- tests/test_constructor_writer.py | 38 ++- tests/test_package_info.py | 28 ++- 7 files changed, 387 insertions(+), 322 deletions(-) create mode 100644 cppwg/info/exclusions.py diff --git a/cppwg/info/exclusions.py b/cppwg/info/exclusions.py new file mode 100644 index 00000000..c685ecfa --- /dev/null +++ b/cppwg/info/exclusions.py @@ -0,0 +1,293 @@ +"""Shared "is this member wrapped?" predicates. + +The decision to wrap (or skip) a method, constructor or public data member is +needed in two places: the writers, which emit the binding, and +``PackageInfo._iter_wrapped_arg_return_types``, which yields the argument, +return and member *types* that end up in the generated wrapper (for dependency +pruning and auto-includes). Keeping the rule in one place here means the two +cannot drift out of lockstep. + +This lives in the info layer, not a writer: ``cppwg/info/`` must not import +``cppwg/writers/`` (the dependency only runs writers -> info). The writers import +these predicates; the info walk calls them directly. +""" + +from typing import TYPE_CHECKING + +from pygccxml import declarations +from pygccxml.declarations import type_traits_classes + +from cppwg.utils import utils + +if TYPE_CHECKING: + from pygccxml.declarations.calldef_members import constructor_t, member_function_t + from pygccxml.declarations.class_declaration import class_t + from pygccxml.declarations.variable import variable_t + + from cppwg.info.class_info import CppClassInfo + + +def method_is_excluded( + class_info: "CppClassInfo", + class_decl: "class_t", + method_decl: "member_function_t", +) -> bool: + """ + Return True if a method would be excluded from the wrapper code. + + Parameters + ---------- + class_info : CppClassInfo + The info for the class containing the method. + class_decl : pygccxml.declarations.class_t + The declaration of the class the method is being wrapped on. + method_decl : pygccxml.declarations.member_function_t + The candidate method. + + Returns + ------- + bool + True if the method should be excluded, False otherwise. + """ + # Skip methods marked for exclusion + if class_info.excluded_methods: + if method_decl.name in class_info.excluded_methods: + return True + + # Exclude private methods + if method_decl.access_type == "private": + return True + + # Exclude sub class (e.g. iterator) methods such as: + # class Foo { + # public: + # class FooIterator { + if method_decl.parent != class_decl: + return True + + # Exclude by return type. return_type_excludes targets return types; + # the deprecated calldef_excludes applies to both return and arg types. + calldef_excludes = class_info.hierarchy_attribute_gather_flat("calldef_excludes") + return_type_excludes = ( + class_info.hierarchy_attribute_gather_flat("return_type_excludes") + + calldef_excludes + ) + + return_type = method_decl.return_type.decl_string + if any( + utils.type_string_matches(return_type, pattern) + for pattern in return_type_excludes + ): + return True + + # Exclude by argument type. arg_type_excludes targets argument types on + # methods and constructors; the deprecated calldef_excludes applies too. + arg_type_excludes = ( + class_info.hierarchy_attribute_gather_flat("arg_type_excludes") + + calldef_excludes + ) + for argument_type in method_decl.argument_types: + arg_type = argument_type.decl_string + if any( + utils.type_string_matches(arg_type, pattern) + for pattern in arg_type_excludes + ): + return True + + return False + + +def constructor_is_excluded( + class_info: "CppClassInfo", + class_decl: "class_t", + ctor_decl: "constructor_t", +) -> bool: + """ + Return True if a constructor would be excluded from the wrapper code. + + Parameters + ---------- + class_info : CppClassInfo + The info for the class containing the constructor. + class_decl : pygccxml.declarations.class_t + The declaration of the class the constructor is being wrapped on. + ctor_decl : pygccxml.declarations.constructor_t + The candidate constructor. + + Returns + ------- + bool + True if the constructor should be excluded, False otherwise. + """ + # Exclude constructors for classes with private pure virtual methods + if any( + mf.virtuality == "pure virtual" and mf.access_type == "private" + for mf in class_decl.member_functions(allow_empty=True) + ): + return True + + # Exclude constructors for abstract classes inheriting from abstract bases. + # A base whose related_class is None could not be resolved by pygccxml; + # treat it as non-abstract (skip it) rather than dereferencing None. + if class_decl.is_abstract and len(class_decl.recursive_bases) > 0: + if any( + base.related_class is not None and base.related_class.is_abstract + for base in class_decl.recursive_bases + ): + return True + + # Exclude sub class (e.g. iterator) constructors such as: + # class Foo { + # public: + # class FooIterator { + if ctor_decl.parent != class_decl: + return True + + # Exclude compiler-added copy constructors e.g. Foo::Foo(Foo const & foo). + # Test is_artificial first: it is cheap and gates the heavier is_copy_constructor. + if ctor_decl.is_artificial and type_traits_classes.is_copy_constructor(ctor_decl): + return True + + # Argument type strings (canonical, as spelled by pygccxml) + arg_types = [x.decl_string for x in ctor_decl.argument_types] + + # Exclude constructors with "iterator" in args + for arg_type in arg_types: + if "iterator" in arg_type.lower(): + return True + + # Exclude by argument type. arg_type_excludes is the general arg-type exclude + # (methods and constructors); constructor_arg_type_excludes is a + # constructor-only refinement; the deprecated calldef_excludes applies too. + # All are matched the same (boundary-aware) way. + arg_type_excludes = ( + class_info.hierarchy_attribute_gather_flat("arg_type_excludes") + + class_info.hierarchy_attribute_gather_flat("constructor_arg_type_excludes") + + class_info.hierarchy_attribute_gather_flat("calldef_excludes") + ) + for arg_type in arg_types: + if any( + utils.type_string_matches(arg_type, pattern) + for pattern in arg_type_excludes + ): + return True + + # Exclude constructors matching a full signature in + # constructor_signature_excludes: same arity, and each argument type matches + # its positional pattern. + ctor_signature_excludes = class_info.hierarchy_attribute_gather_flat( + "constructor_signature_excludes" + ) + for exclude_types in ctor_signature_excludes: + # Each entry must be a sequence of per-argument patterns. Skip a mis-typed + # scalar (e.g. `constructor_signature_excludes: 5`, or a single string), + # which would otherwise crash on len() or be iterated character by + # character. + if not isinstance(exclude_types, (list, tuple)): + continue + + if len(exclude_types) != len(arg_types): + continue + + if all( + utils.type_string_matches(arg_type, exclude_type) + for arg_type, exclude_type in zip(arg_types, exclude_types) + ): + return True + + return False + + +def variable_exclusion_reason( + class_info: "CppClassInfo", + class_decl: "class_t", + variable_decl: "variable_t", +) -> "str | None": + """ + Return why a public data member is excluded, or None if it is wrapped. + + The reason is a short label (e.g. "bitfield", "static") that the member + writer uses for a debug log; callers that only need a yes/no answer use + variable_is_excluded. + + Parameters + ---------- + class_info : CppClassInfo + The info for the class owning the member. + class_decl : pygccxml.declarations.class_t + The declaration of the class the member is being wrapped on. + variable_decl : pygccxml.declarations.variable_t + The candidate data member. + + Returns + ------- + str | None + A short exclusion-reason label, or None if the member is wrapped. + """ + # Skip members marked for exclusion in the config. + excluded_variables = class_info.hierarchy_attribute_gather_flat( + "excluded_variables" + ) + if variable_decl.name in excluded_variables: + return "config-excluded" + + # Skip members belonging to a nested class. The variables() query is + # recursive, so it also returns fields of nested classes (e.g. an iterator); + # binding one as &Class::field would name a member the class does not have. + if variable_decl.parent is not class_decl: + return "nested-class" + + # A reference member (e.g. `T& field`) cannot be bound: you cannot form a + # pointer-to-member for a reference, so &Class::field is ill-formed. + if declarations.is_reference(variable_decl.decl_type): + return "reference" + + # A bitfield member has no address, so &Class::field is ill-formed and it + # cannot be bound with def_readwrite/def_readonly. + if variable_decl.bits is not None: + return "bitfield" + + # A C-style array member (e.g. `double coords[3]`) cannot be bound: a + # def_readwrite setter assigns to the member, but C arrays are not assignable, + # and pybind11 has no type caster for a raw array, so both def_readwrite and + # def_readonly fail to compile. is_const already sees through the array, so + # this also covers const arrays bound read-only. + if declarations.is_array(variable_decl.decl_type): + return "array" + + # Static data members need def_readwrite_static/def_readonly_static and, for + # in-class-initialised static const members, an out-of-line definition to take + # their address. Skip them for now (see issue #116 follow-up). + if ( + variable_decl.type_qualifiers is not None + and variable_decl.type_qualifiers.has_static + ): + return "static" + + # A mutable member is bound read-write, whose pybind11 setter assigns to the + # member (obj.*pm = value). If the type is not copy-assignable (e.g. + # std::unique_ptr, std::atomic, or a class with a deleted operator=) that + # assignment does not compile, so skip it. A const member is bound read-only + # (no setter), so it is unaffected. + if not declarations.is_const( + variable_decl.decl_type + ) and not utils.type_is_copy_assignable(variable_decl.decl_type): + return "non-copy-assignable" + + return None + + +def variable_is_excluded( + class_info: "CppClassInfo", + class_decl: "class_t", + variable_decl: "variable_t", +) -> bool: + """ + Return True if a public data member would be excluded from the wrapper code. + + Returns + ------- + bool + True if the member should be excluded, False otherwise. + """ + return variable_exclusion_reason(class_info, class_decl, variable_decl) is not None diff --git a/cppwg/info/package_info.py b/cppwg/info/package_info.py index 0e283184..bb315d24 100644 --- a/cppwg/info/package_info.py +++ b/cppwg/info/package_info.py @@ -12,6 +12,7 @@ from pygccxml.declarations import type_traits_classes from pygccxml.declarations.matchers import access_type_matcher_t +from cppwg.info import exclusions from cppwg.info.base_info import BaseInfo from cppwg.utils import utils from cppwg.utils.constants import CPPWG_EXT @@ -525,18 +526,16 @@ def _iter_wrapped_arg_return_types( class_info: "CppClassInfo", decl: Any ) -> "Iterator[declarations.type_t]": """ - Yield the arg/return types the writers will actually wrap for a class. + Yield the arg/return/member types the writers will actually wrap. - Walks the public member functions and constructors of ``decl`` and yields - the pygccxml type of every argument and return type that survives the same - config-driven exclusions the writers apply (``excluded_methods``, - ``return_type_excludes``, ``arg_type_excludes``, - ``constructor_arg_type_excludes``, ``constructor_signature_excludes``, - ``calldef_excludes``, iterator-argument and abstract-class-constructor - skips). A type reached only through an excluded method or constructor is - never yielded, so callers see exactly the types that end up in the - generated wrapper. Shared by dependency pruning and auto-include - resolution so the two cannot diverge from the writers. + Walks the public methods, constructors and data members of ``decl`` and + yields the pygccxml type of every argument, return and member type that + survives the shared exclusion predicates in ``cppwg.info.exclusions`` - + the same predicates the writers use to decide what to bind. A type + reached only through an excluded method, constructor or member is never + yielded, so callers see exactly the types that end up in the generated + wrapper. Shared by dependency pruning and auto-include resolution so the + two cannot diverge from the writers. Parameters ---------- @@ -562,103 +561,27 @@ def _iter_wrapped_arg_return_types( return query = access_type_matcher_t("public") - gather = class_info.hierarchy_attribute_gather_flat - calldef_excludes = gather("calldef_excludes") - return_type_excludes = gather("return_type_excludes") + calldef_excludes - arg_type_excludes = gather("arg_type_excludes") + calldef_excludes - ctor_arg_type_excludes = arg_type_excludes + gather( - "constructor_arg_type_excludes" - ) - ctor_signature_excludes = gather("constructor_signature_excludes") - excluded_methods = class_info.excluded_methods or [] - - def excluded(type_string: str, patterns: list[str]) -> bool: - return any( - utils.type_string_matches(type_string, pattern) for pattern in patterns - ) - - def signature_excluded(arg_strings: list[str]) -> bool: - # A constructor is excluded when a constructor_signature_excludes - # entry has the same arity and each argument matches its positional - # pattern (matching the constructor writer). - for exclude_types in ctor_signature_excludes: - if not isinstance(exclude_types, (list, tuple)): - continue - if len(exclude_types) != len(arg_strings): - continue - if all( - utils.type_string_matches(arg_string, exclude_type) - for arg_string, exclude_type in zip(arg_strings, exclude_types) - ): - return True - return False + # The exclusion predicates (shared with the writers via cppwg.info. + # exclusions) decide what is bound; anything they exclude introduces no + # wrapped type. Applying them here keeps this walk in lockstep with the + # generated bindings. for method in decl.member_functions(function=query, allow_empty=True): - if method.name in excluded_methods: - continue - return_type = method.return_type - if return_type is not None and excluded( - return_type.decl_string, return_type_excludes - ): - continue - if any( - excluded(arg.decl_string, arg_type_excludes) - for arg in method.argument_types - ): + if exclusions.method_is_excluded(class_info, decl, method): continue yield from method.argument_types - if return_type is not None: - yield return_type - - # Constructors are not wrapped for an abstract class that inherits from an - # abstract base (matching the constructor writer), so its constructor - # arguments cannot introduce a dependency. A base whose related_class is - # None could not be resolved by pygccxml; treat it as non-abstract (skip - # it) rather than dereferencing None. - ctors_wrapped = not ( - decl.is_abstract - and any( - base.related_class is not None and base.related_class.is_abstract - for base in decl.recursive_bases - ) - ) - if ctors_wrapped: - for ctor in decl.constructors(function=query, allow_empty=True): - arg_strings = [arg.decl_string for arg in ctor.argument_types] - if any("iterator" in s.lower() for s in arg_strings): - continue - if any(excluded(s, ctor_arg_type_excludes) for s in arg_strings): - continue - if signature_excluded(arg_strings): - continue - yield from ctor.argument_types + if method.return_type is not None: + yield method.return_type + + for ctor in decl.constructors(function=query, allow_empty=True): + if exclusions.constructor_is_excluded(class_info, decl, ctor): + continue + yield from ctor.argument_types # Public data members are bound with def_readwrite/def_readonly, so their - # types are wrapped too. Mirror the member writer's skips (nested-class - # members from the recursive query, excluded_variables, reference, - # bitfield, static, array, and non-copy-assignable mutable members) so a - # member type reached only through a skipped member does not trigger a - # dependency or auto-include. - excluded_variables = gather("excluded_variables") + # types are wrapped too. for variable in decl.variables(function=query, allow_empty=True): - if variable.parent is not decl: - continue - if variable.name in excluded_variables: - continue - if declarations.is_reference(variable.decl_type): - continue - if variable.bits is not None: - continue - if ( - variable.type_qualifiers is not None - and variable.type_qualifiers.has_static - ): - continue - if declarations.is_array(variable.decl_type): - continue - if not declarations.is_const( - variable.decl_type - ) and not utils.type_is_copy_assignable(variable.decl_type): + if exclusions.variable_is_excluded(class_info, decl, variable): continue yield variable.decl_type diff --git a/cppwg/writers/constructor_writer.py b/cppwg/writers/constructor_writer.py index 83c082b4..9d91dd52 100644 --- a/cppwg/writers/constructor_writer.py +++ b/cppwg/writers/constructor_writer.py @@ -3,8 +3,9 @@ import re from typing import TYPE_CHECKING -from pygccxml.declarations import type_traits, type_traits_classes +from pygccxml.declarations import type_traits +from cppwg.info import exclusions from cppwg.utils import utils from cppwg.writers.base_writer import CppBaseWrapperWriter @@ -73,87 +74,9 @@ def exclude(self) -> bool: bool True if the constructor should be excluded, False otherwise """ - # Exclude constructors for classes with private pure virtual methods - if any( - mf.virtuality == "pure virtual" and mf.access_type == "private" - for mf in self.class_decl.member_functions(allow_empty=True) - ): - return True - - # Exclude constructors for abstract classes inheriting from abstract bases. - # A base whose related_class is None could not be resolved by pygccxml; - # treat it as non-abstract (skip it) rather than dereferencing None. - if self.class_decl.is_abstract and len(self.class_decl.recursive_bases) > 0: - if any( - base.related_class is not None and base.related_class.is_abstract - for base in self.class_decl.recursive_bases - ): - return True - - # Exclude sub class (e.g. iterator) constructors such as: - # class Foo { - # public: - # class FooIterator { - if self.ctor_decl.parent != self.class_decl: - return True - - # Exclude compiler-added copy constructors e.g. Foo::Foo(Foo const & foo) - if ( - type_traits_classes.is_copy_constructor(self.ctor_decl) - and self.ctor_decl.is_artificial - ): - return True - - # Argument type strings (canonical, as spelled by pygccxml) - arg_types = [x.decl_string for x in self.ctor_decl.argument_types] - - # Exclude constructors with "iterator" in args - for arg_type in arg_types: - if "iterator" in arg_type.lower(): - return True - - # Exclude by argument type. arg_type_excludes is the general arg-type - # exclude (methods and constructors); constructor_arg_type_excludes is a - # constructor-only refinement; the deprecated calldef_excludes applies - # too. All are matched the same (boundary-aware) way. - arg_type_excludes = ( - self.class_info.hierarchy_attribute_gather_flat("arg_type_excludes") - + self.class_info.hierarchy_attribute_gather_flat( - "constructor_arg_type_excludes" - ) - + self.class_info.hierarchy_attribute_gather_flat("calldef_excludes") - ) - for arg_type in arg_types: - if any( - utils.type_string_matches(arg_type, pattern) - for pattern in arg_type_excludes - ): - return True - - # Exclude constructors matching a full signature in - # constructor_signature_excludes: same arity, and each argument type - # matches its positional pattern. - ctor_signature_excludes = self.class_info.hierarchy_attribute_gather_flat( - "constructor_signature_excludes" + return exclusions.constructor_is_excluded( + self.class_info, self.class_decl, self.ctor_decl ) - for exclude_types in ctor_signature_excludes: - # Each entry must be a sequence of per-argument patterns. Skip a - # mis-typed scalar (e.g. `constructor_signature_excludes: 5`, or a - # single string), which would otherwise crash on len() or be - # iterated character by character. - if not isinstance(exclude_types, (list, tuple)): - continue - - if len(exclude_types) != len(arg_types): - continue - - if all( - utils.type_string_matches(arg_type, exclude_type) - for arg_type, exclude_type in zip(arg_types, exclude_types) - ): - return True - - return False def generate_wrapper(self) -> str: """ diff --git a/cppwg/writers/member_variable_writer.py b/cppwg/writers/member_variable_writer.py index 3575bd05..477eb128 100644 --- a/cppwg/writers/member_variable_writer.py +++ b/cppwg/writers/member_variable_writer.py @@ -5,7 +5,7 @@ from pygccxml import declarations -from cppwg.utils import utils +from cppwg.info import exclusions from cppwg.writers.base_writer import CppBaseWrapperWriter if TYPE_CHECKING: @@ -64,83 +64,21 @@ def exclude(self) -> bool: bool True if the member should be excluded, False otherwise. """ - logger = logging.getLogger() - variable_decl = self.variable_decl - - # Skip members marked for exclusion in the config. - excluded_variables = self.class_info.hierarchy_attribute_gather_flat( - "excluded_variables" + reason = exclusions.variable_exclusion_reason( + self.class_info, self.class_decl, self.variable_decl ) - if variable_decl.name in excluded_variables: - return True - - # Skip members belonging to a nested class. The variables() query is - # recursive, so it also returns fields of nested classes (e.g. an - # iterator); binding one as &Class::field would name a member the class - # does not have. Mirrors the parent check in the method/constructor - # writers. - if variable_decl.parent is not self.class_decl: - logger.debug( - f"Skipping nested-class member {self.class_py_name}::" - f"{variable_decl.name}" - ) - return True - - # A reference member (e.g. `T& field`) cannot be bound: you cannot form a - # pointer-to-member for a reference, so &Class::field is ill-formed. - if declarations.is_reference(variable_decl.decl_type): - logger.debug( - f"Skipping reference member {self.class_py_name}::" - f"{variable_decl.name}" - ) - return True - - # A bitfield member has no address, so &Class::field is ill-formed and it - # cannot be bound with def_readwrite/def_readonly. - if variable_decl.bits is not None: - logger.debug( - f"Skipping bitfield member {self.class_py_name}::{variable_decl.name}" + if reason is None: + return False + + # Log the unbindable-member skips (nested-class, reference, bitfield, + # array, static, non-copy-assignable) at debug level. A config exclusion + # via excluded_variables is intentional, so it is not logged. + if reason != "config-excluded": + logging.getLogger().debug( + f"Skipping {reason} member " + f"{self.class_py_name}::{self.variable_decl.name}" ) - return True - - # A C-style array member (e.g. `double coords[3]`) cannot be bound: a - # def_readwrite setter assigns to the member, but C arrays are not - # assignable, and pybind11 has no type caster for a raw array, so both - # def_readwrite and def_readonly fail to compile. is_const already sees - # through the array, so this also covers const arrays bound read-only. - if declarations.is_array(variable_decl.decl_type): - logger.debug( - f"Skipping array member {self.class_py_name}::{variable_decl.name}" - ) - return True - - # Static data members need def_readwrite_static/def_readonly_static and, - # for in-class-initialised static const members, an out-of-line definition - # to take their address. Skip them for now (see issue #116 follow-up). - if ( - variable_decl.type_qualifiers is not None - and variable_decl.type_qualifiers.has_static - ): - logger.debug( - f"Skipping static member {self.class_py_name}::{variable_decl.name}" - ) - return True - - # A mutable member is bound read-write, whose pybind11 setter assigns to - # the member (obj.*pm = value). If the type is not copy-assignable (e.g. - # std::unique_ptr, std::atomic, or a class with a deleted operator=) that - # assignment does not compile, so skip it. A const member is bound - # read-only (no setter), so it is unaffected. - if not declarations.is_const( - variable_decl.decl_type - ) and not utils.type_is_copy_assignable(variable_decl.decl_type): - logger.debug( - f"Skipping non-copy-assignable member " - f"{self.class_py_name}::{variable_decl.name}" - ) - return True - - return False + return True def generate_wrapper(self) -> str: """ diff --git a/cppwg/writers/method_writer.py b/cppwg/writers/method_writer.py index 96a62505..f5420194 100644 --- a/cppwg/writers/method_writer.py +++ b/cppwg/writers/method_writer.py @@ -5,6 +5,7 @@ from pygccxml.declarations import type_traits +from cppwg.info import exclusions from cppwg.utils import utils from cppwg.writers.base_writer import CppBaseWrapperWriter @@ -106,54 +107,7 @@ def method_is_excluded( bool True if the method should be excluded, False otherwise. """ - # Skip methods marked for exclusion - if class_info.excluded_methods: - if method_decl.name in class_info.excluded_methods: - return True - - # Exclude private methods - if method_decl.access_type == "private": - return True - - # Exclude sub class (e.g. iterator) methods such as: - # class Foo { - # public: - # class FooIterator { - if method_decl.parent != class_decl: - return True - - # Exclude by return type. return_type_excludes targets return types; - # the deprecated calldef_excludes applies to both return and arg types. - calldef_excludes = class_info.hierarchy_attribute_gather_flat( - "calldef_excludes" - ) - return_type_excludes = ( - class_info.hierarchy_attribute_gather_flat("return_type_excludes") - + calldef_excludes - ) - - return_type = method_decl.return_type.decl_string - if any( - utils.type_string_matches(return_type, pattern) - for pattern in return_type_excludes - ): - return True - - # Exclude by argument type. arg_type_excludes targets argument types on - # methods and constructors; the deprecated calldef_excludes applies too. - arg_type_excludes = ( - class_info.hierarchy_attribute_gather_flat("arg_type_excludes") - + calldef_excludes - ) - for argument_type in method_decl.argument_types: - arg_type = argument_type.decl_string - if any( - utils.type_string_matches(arg_type, pattern) - for pattern in arg_type_excludes - ): - return True - - return False + return exclusions.method_is_excluded(class_info, class_decl, method_decl) def generate_wrapper(self) -> str: """ diff --git a/tests/test_constructor_writer.py b/tests/test_constructor_writer.py index 45527f74..e170c12b 100644 --- a/tests/test_constructor_writer.py +++ b/tests/test_constructor_writer.py @@ -1,7 +1,7 @@ """Unit tests for cppwg.writers.constructor_writer exclusion behaviour.""" +from cppwg.info import exclusions as exclusions_module from cppwg.info.base_info import BaseInfo -from cppwg.writers import constructor_writer as constructor_writer_module from cppwg.writers.constructor_writer import CppConstructorWrapperWriter @@ -51,7 +51,7 @@ def _writer(arg_types, signature_excludes, monkeypatch): # is_copy_constructor inspects a real pygccxml decl; stub it out so exclude() # reaches the signature-exclude loop with our lightweight fakes. monkeypatch.setattr( - constructor_writer_module.type_traits_classes, + exclusions_module.type_traits_classes, "is_copy_constructor", lambda decl: False, ) @@ -94,7 +94,9 @@ def __init__(self, virtuality="not virtual", access_type="public"): class _RichClassDecl: - def __init__(self, name="Foo", member_fns=(), is_abstract=False, recursive_bases=()): + def __init__( + self, name="Foo", member_fns=(), is_abstract=False, recursive_bases=() + ): self.name = name self._mfs = list(member_fns) self.is_abstract = is_abstract @@ -120,8 +122,9 @@ def __init__(self, arg_types=(), arguments=(), parent=None, is_artificial=False) def _no_copy_ctor(monkeypatch): + # The artificial-copy-constructor check moved to cppwg.info.exclusions. monkeypatch.setattr( - constructor_writer_module.type_traits_classes, + exclusions_module.type_traits_classes, "is_copy_constructor", lambda decl: False, ) @@ -135,7 +138,9 @@ def test_init_reads_template_metadata(): template_params=["DIM"], template_arg_lists=[["2"]], ) - writer = CppConstructorWrapperWriter(class_info, 0, _RichCtor(parent=class_decl), {}) + writer = CppConstructorWrapperWriter( + class_info, 0, _RichCtor(parent=class_decl), {} + ) assert writer.class_py_name == "Foo_2" assert writer.template_params == ["DIM"] assert writer.template_args == ["2"] @@ -149,7 +154,9 @@ def test_init_falls_back_to_decl_name_when_py_name_none(): template_params=None, template_arg_lists=None, ) - writer = CppConstructorWrapperWriter(class_info, 0, _RichCtor(parent=class_decl), {}) + writer = CppConstructorWrapperWriter( + class_info, 0, _RichCtor(parent=class_decl), {} + ) assert writer.class_py_name == "Foo" assert writer.template_args is None @@ -189,7 +196,7 @@ def test_exclude_subclass_constructor(monkeypatch): def test_exclude_artificial_copy_constructor(monkeypatch): monkeypatch.setattr( - constructor_writer_module.type_traits_classes, + exclusions_module.type_traits_classes, "is_copy_constructor", lambda decl: True, ) @@ -218,8 +225,13 @@ def test_exclude_by_arg_type(monkeypatch): assert _exclude_writer(monkeypatch, class_decl, ctor, class_info).exclude() is True -def _gen_writer(monkeypatch, ctor, template_params=None, template_args=None, - exclude_default_args=False): +def _gen_writer( + monkeypatch, + ctor, + template_params=None, + template_args=None, + exclude_default_args=False, +): _no_copy_ctor(monkeypatch) class_decl = _RichClassDecl(name="Foo") ctor.parent = class_decl @@ -259,13 +271,17 @@ def test_generate_wrapper_substitutes_template_param(monkeypatch): arg_types=["unsigned"], arguments=[_Arg("dim", default_value="DIM", decl_type="unsigned")], ) - writer = _gen_writer(monkeypatch, ctor, template_params=["DIM"], template_args=["2"]) + writer = _gen_writer( + monkeypatch, ctor, template_params=["DIM"], template_args=["2"] + ) assert writer.generate_wrapper() == 'py::init(), py::arg("dim") = 2' def test_generate_wrapper_empty_initializer_list(monkeypatch): monkeypatch.setattr( - type_traits, "remove_const", lambda decl_type: SimpleNamespace(decl_string="std::vector") + type_traits, + "remove_const", + lambda decl_type: SimpleNamespace(decl_string="std::vector"), ) ctor = _RichCtor( arg_types=["std::vector"], diff --git a/tests/test_package_info.py b/tests/test_package_info.py index ae15ae78..b28c9c3f 100644 --- a/tests/test_package_info.py +++ b/tests/test_package_info.py @@ -262,6 +262,13 @@ def __init__(self, argument_types=(), return_type=None, name="method"): self.name = name self.argument_types = [_FakeType(t) for t in argument_types] self.return_type = _FakeType(return_type) if return_type else None + # Attributes the shared exclusion predicates consult; defaults keep the + # calldef wrapped (public, own-class, not an artificial copy ctor). parent + # is set to the owning decl by _FakeDecl. + self.access_type = "public" + self.virtuality = None + self.is_artificial = False + self.parent = None class _FakeBase: @@ -290,6 +297,11 @@ def __init__( self.recursive_bases = list(recursive_bases) self.bases = [] self._variables = list(variables) + # A method/ctor's parent is its owning class unless the test marks it + # nested by supplying a different parent. + for calldef in self._methods + self._constructors: + if getattr(calldef, "parent", None) is None: + calldef.parent = self def member_functions(self, function=None, allow_empty=False): return self._methods @@ -992,6 +1004,12 @@ def __init__(self, arg_types, name=None, return_type=None): self.name = name self.return_type = _IterType(return_type) if return_type is not None else None self.argument_types = [_IterType(a) for a in arg_types] + # Attributes the shared exclusion predicates consult; defaults keep the + # calldef wrapped. parent is set to the owning decl by _IterDecl. + self.access_type = "public" + self.virtuality = None + self.is_artificial = False + self.parent = None class _IterVariable: @@ -1055,11 +1073,11 @@ def __init__( self.recursive_bases = list(recursive_bases) self._variables = list(variables) self._enumerations = list(enumerations) - # A direct member's parent is this decl; a variable that already carries a - # (nested) parent keeps it. - for variable in self._variables: - if variable.parent is None: - variable.parent = self + # A direct member's parent is this decl; a method/ctor/variable that + # already carries a (nested) parent keeps it. + for member in self._methods + self._ctors + self._variables: + if member.parent is None: + member.parent = self def member_functions(self, function=None, allow_empty=True): return self._methods From 7d631cf1cc19ddf2cf24157a7e3bfa18f762f8b5 Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 09:23:16 +0100 Subject: [PATCH 02/12] #119 Seed shared config options from a single BASE_INFO_OPTIONS schema The shared (BaseInfo-level) config options were restated in three places: BaseInfo's hand-written attribute defaults, BaseInfo's copy-from-config key list, and the parser's base_config seed dict. Adding an option to one and forgetting another silently ignored it - which is exactly how name_replacements became unreachable: it was a BaseInfo option with a default map, but the parser's seed omitted it, so a user's name_replacements: in the YAML was dropped. Introduce BASE_INFO_OPTIONS, one {option: default} schema, as the single source. BaseInfo seeds its defaults and copies overrides from it (deep-copying each default so info objects no longer alias a shared list/dict), and the parser builds base_config from it. name_replacements is now in the schema, so package- and class-level values are honoured (+2 regression tests). exclude_inherited_overrides stays special (its per-level False/None cascade must not get one shared default), noted via _EXTRA_CONFIG_KEYS. Behaviour-preserving: the shapes wrappers regenerate byte-identically and the example builds and passes. Co-Authored-By: Claude Opus 4.8 --- cppwg/info/base_info.py | 147 ++++++++++++--------------- cppwg/parsers/package_info_parser.py | 35 ++----- tests/test_package_info_parser.py | 50 +++++++++ 3 files changed, 126 insertions(+), 106 deletions(-) diff --git a/cppwg/info/base_info.py b/cppwg/info/base_info.py index 6f444b6c..a05bec4a 100644 --- a/cppwg/info/base_info.py +++ b/cppwg/info/base_info.py @@ -1,5 +1,6 @@ """Generic information structure.""" +import copy import importlib.util import logging import os @@ -12,6 +13,63 @@ from cppwg.templates.custom import Custom +# The configuration options shared by every info level (package, module, class, +# free function, ...), each mapped to its default value. This is the single +# source of truth for the shared options: BaseInfo seeds these as attribute +# defaults and copies any the config overrides, and the parser +# (cppwg.parsers.package_info_parser) builds its config dicts from the same +# schema. An option added here is therefore understood everywhere - defined in +# one place instead of being restated in BaseInfo and the parser (which is how +# options such as name_replacements previously became unreachable from the YAML). +# Mutable defaults are deep-copied per use so no two objects share a list/dict. +# See the class Attributes docstring for what each option means; tri-state +# options default to None, meaning "inherit from further up the info tree". +BASE_INFO_OPTIONS: dict[str, Any] = { + "arg_type_excludes": [], + "auto_includes": None, + "calldef_excludes": [], + "constructor_arg_type_excludes": [], + "constructor_signature_excludes": [], + "custom_generator": "", + "discover_arg_excludes": {}, + "discover_template_instantiations": None, + "excluded": False, + "excluded_methods": [], + "excluded_variables": [], + "export_values": None, + "name_replacements": { + "double": "Double", + "unsigned int": "Unsigned", + "Unsigned int": "Unsigned", + "unsigned": "Unsigned", + "std::vector": "Vector", + "std::pair": "Pair", + "std::map": "Map", + "std::string": "String", + "boost::shared_ptr": "SharedPtr", + "*": "Ptr", + "c_vector": "CVector", + "std::set": "Set", + }, + "pointer_call_policy": "", + "prefix_code": [], + "prefix_text": "", + "reference_call_policy": "", + "return_type_excludes": [], + "smart_ptr_type": "", + "source_includes": [], + "source_root": "", + "suffix_code": [], + "template_substitutions": [], +} + +# Options copied from the config but deliberately not in BASE_INFO_OPTIONS: +# exclude_inherited_overrides is a tri-state the parser seeds per level (package +# False, module/class None) to drive the package->module->class cascade, so it +# must not be given a single shared default here. +_EXTRA_CONFIG_KEYS: tuple[str, ...] = ("exclude_inherited_overrides",) + + class BaseInfo(ABC): """ A generic information structure for features. @@ -115,90 +173,19 @@ def __init__(self, name: str, info_config: dict[str, Any] | None = None) -> None """ self.name: str = name - # Paths - self.source_includes: list[str] = [] - self.source_root: str = "" - - # Exclusions - self.arg_type_excludes: list[str] = [] - self.calldef_excludes: list[str] = [] - self.constructor_arg_type_excludes: list[str] = [] - self.constructor_signature_excludes: list[list[str]] = [] - # Tri-state (None inherits): automatically add includes for the project - # types used in a class's wrapped signatures. Off unless set. - self.auto_includes: bool | None = None - # Tri-state: None means inherit from further up the info tree, so that a - # package/module-level setting propagates to classes (hierarchy_attribute - # stops at the first non-None value it finds ascending the tree). - self.discover_arg_excludes: dict[str, list] = {} - self.discover_template_instantiations: bool | None = None - self.excluded: bool = False - self.excluded_methods: list[str] = [] - self.excluded_variables: list[str] = [] - self.return_type_excludes: list[str] = [] - # Tri-state (None inherits): whether a wrapped enum exports its - # enumerators into the module scope (pybind11's .export_values()). Only - # meaningful for enums, but inheritable so a package/module setting - # applies to all enums below it. See CppEnumInfo.should_export_values. - self.export_values: bool | None = None - - # Pointers - self.pointer_call_policy: str = "" - self.reference_call_policy: str = "" - self.smart_ptr_type: str = "" - - # Substitutions - self.template_substitutions: list[dict[str, Any]] = [] - self.name_replacements: dict[str, str] = { - "double": "Double", - "unsigned int": "Unsigned", - "Unsigned int": "Unsigned", - "unsigned": "Unsigned", - "std::vector": "Vector", - "std::pair": "Pair", - "std::map": "Map", - "std::string": "String", - "boost::shared_ptr": "SharedPtr", - "*": "Ptr", - "c_vector": "CVector", - "std::set": "Set", - } - - # Custom Code - self.prefix_code: list[str] = [] - self.suffix_code: list[str] = [] - self.prefix_text: str = "" - self.custom_generator: str = "" + # Seed the shared options from the single schema, deep-copying each + # default so no two info objects share a mutable list/dict. See the + # Attributes docstring for what each option means. + for key, default in BASE_INFO_OPTIONS.items(): + setattr(self, key, copy.deepcopy(default)) self.custom_generator_instance: "Custom | None" = None if info_config: - for key in [ - "arg_type_excludes", - "auto_includes", - "calldef_excludes", - "constructor_arg_type_excludes", - "constructor_signature_excludes", - "custom_generator", - "discover_arg_excludes", - "discover_template_instantiations", - "exclude_inherited_overrides", - "excluded", - "excluded_methods", - "excluded_variables", - "export_values", - "name_replacements", - "pointer_call_policy", - "prefix_code", - "prefix_text", - "reference_call_policy", - "return_type_excludes", - "smart_ptr_type", - "source_includes", - "source_root", - "suffix_code", - "template_substitutions", - ]: + # Copy any option the config provides, over the schema defaults. + # exclude_inherited_overrides (in _EXTRA_CONFIG_KEYS) is copied when + # present but has no shared default - the parser seeds it per level. + for key in (*BASE_INFO_OPTIONS, *_EXTRA_CONFIG_KEYS): if key in info_config: setattr(self, key, info_config[key]) diff --git a/cppwg/parsers/package_info_parser.py b/cppwg/parsers/package_info_parser.py index f5fde2a7..46eca746 100644 --- a/cppwg/parsers/package_info_parser.py +++ b/cppwg/parsers/package_info_parser.py @@ -1,11 +1,13 @@ """Parser for input yaml.""" +import copy import logging import os from typing import Any import yaml +from cppwg.info.base_info import BASE_INFO_OPTIONS from cppwg.info.class_info import CppClassInfo from cppwg.info.enum_info import CppEnumInfo from cppwg.info.free_function_info import CppFreeFunctionInfo @@ -56,31 +58,14 @@ def parse(self) -> PackageInfo: # Warn about any deprecated options present anywhere in the raw config self.warn_deprecated_options(raw_package_info) - # Base config options that apply to package, modules, classes, etc. + # Base config options that apply to package, modules, classes, etc., + # seeded from the single BASE_INFO_OPTIONS schema (deep-copied so the + # per-key defaults are independent) so the parser and BaseInfo cannot + # drift. source_root is the one option whose default is parse-time. base_config: dict[str, Any] = { - "arg_type_excludes": [], - "auto_includes": None, - "calldef_excludes": [], - "constructor_arg_type_excludes": [], - "constructor_signature_excludes": [], - "custom_generator": "", - "discover_arg_excludes": {}, - "discover_template_instantiations": None, - "excluded": False, - "excluded_methods": [], - "excluded_variables": [], - "export_values": None, - "pointer_call_policy": "", - "prefix_code": [], - "prefix_text": "", - "reference_call_policy": "", - "return_type_excludes": [], - "smart_ptr_type": "", - "source_includes": [], - "source_root": self.source_root, - "suffix_code": [], - "template_substitutions": [], + key: copy.deepcopy(default) for key, default in BASE_INFO_OPTIONS.items() } + base_config["source_root"] = self.source_root # Get package config from the raw package info package_config: dict[str, Any] = { @@ -164,9 +149,7 @@ def parse(self) -> PackageInfo: module_config["variables"] ) - module_config["use_all_enums"] = utils.is_option_ALL( - module_config["enums"] - ) + module_config["use_all_enums"] = utils.is_option_ALL(module_config["enums"]) # Create the ModuleInfo object from the module config dict module_info = ModuleInfo(module_config["name"], module_config) diff --git a/tests/test_package_info_parser.py b/tests/test_package_info_parser.py index 58516236..10bba882 100644 --- a/tests/test_package_info_parser.py +++ b/tests/test_package_info_parser.py @@ -177,6 +177,56 @@ def test_class_auto_includes_defaults_to_none(tmp_path): assert cls.auto_includes is None +def test_parses_name_replacements(tmp_path): + """A name_replacements map set in the YAML reaches the info objects. + + Regression test for a DRY-drift bug: name_replacements is a BaseInfo option + with a default map, but the parser's base-config seed omitted it, so a user's + name_replacements: was silently dropped. Both are now seeded from the shared + BASE_INFO_OPTIONS schema, so package- and class-level values are honoured. + """ + config_path = _write_config( + tmp_path, + """ + name: testpkg + name_replacements: + MyType: Renamed + modules: + - name: mymod + classes: + - name: Foo + name_replacements: + Foo: Bar + """, + ) + + package_info = PackageInfoParser(config_path, str(tmp_path)).parse() + + assert package_info.name_replacements == {"MyType": "Renamed"} + cls = package_info.module_collection[0].class_collection[0] + assert cls.name_replacements == {"Foo": "Bar"} + + +def test_name_replacements_defaults_to_builtin_map(tmp_path): + """Unset, name_replacements keeps the built-in default map (e.g. c_vector).""" + config_path = _write_config( + tmp_path, + """ + name: testpkg + modules: + - name: mymod + classes: + - name: Foo + """, + ) + + package_info = PackageInfoParser(config_path, str(tmp_path)).parse() + + cls = package_info.module_collection[0].class_collection[0] + assert cls.name_replacements["c_vector"] == "CVector" + assert cls.name_replacements["double"] == "Double" + + def test_parses_module_external_bases(tmp_path): """A module-level `external_bases` list is parsed onto the module info.""" config_path = _write_config( From c24e18df46e8dc3cd5108ead19315c03e0e1b256 Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 09:29:41 +0100 Subject: [PATCH 03/12] #119 Share default-argument rendering across writers The ', py::arg("name") [= value]' fragment was rendered by three near-identical loops in the method, constructor and free-function writers (str_to_num normalisation, class-template-parameter substitution, and the constructor-only empty-initializer-list typing). The free-function copy had drifted: it gated the whole loop on exclude_default_args, so with the flag set it dropped the py::arg("name") keyword names too - unlike the method/constructor writers, which the docs match by omitting only the default values. Add CppBaseWrapperWriter.render_default_args as the single source (template substitution and empty-init-list typing behind parameters). The three writers call it; method/constructor output is byte-identical and free functions now keep their py::arg names when exclude_default_args is set. Drop the now-unused re/utils/type_traits imports from the method and constructor writers. Behaviour-preserving for the examples (they do not set exclude_default_args): the shapes wrappers regenerate byte-identically and the example builds/passes. Co-Authored-By: Claude Opus 4.8 --- cppwg/writers/base_writer.py | 83 +++++++++++++++++++++++++++ cppwg/writers/constructor_writer.py | 57 ++++-------------- cppwg/writers/free_function_writer.py | 18 ++---- cppwg/writers/method_writer.py | 42 +++----------- tests/test_free_function_writer.py | 28 ++++++--- 5 files changed, 126 insertions(+), 102 deletions(-) diff --git a/cppwg/writers/base_writer.py b/cppwg/writers/base_writer.py index c415ec1e..576ef8b4 100644 --- a/cppwg/writers/base_writer.py +++ b/cppwg/writers/base_writer.py @@ -1,8 +1,13 @@ """Base for wrapper code writers.""" +import re from collections import OrderedDict from typing import TYPE_CHECKING +from pygccxml.declarations import type_traits + +from cppwg.utils import utils + if TYPE_CHECKING: from string import Template @@ -55,3 +60,81 @@ def tidy_name(self, name: str) -> str: name = name.replace(key, value) return name + + def render_default_args( + self, + arguments, + exclude_default_args: bool, + template_params: "list[str] | None" = None, + template_args: "list[str] | None" = None, + class_name: "str | None" = None, + substitute_empty_init_list: bool = False, + ) -> str: + """ + Render the ``, py::arg("name")[ = value]`` fragment for a calldef. + + A ``py::arg("name")`` is always emitted for each argument (pybind11's + keyword name). The C++ default *value* is appended only when the argument + has one and ``exclude_default_args`` is False - so ``exclude_default_args`` + omits the default values, never the keyword names. Shared by the method, + constructor and free-function writers so the three cannot diverge; they + previously did - the free-function writer gated the whole loop, dropping + the keyword names too when ``exclude_default_args`` was set. + + Parameters + ---------- + arguments : iterable + The calldef arguments, each with ``name``, ``default_value`` and + ``decl_type``. + exclude_default_args : bool + When True, omit the ``= value`` while keeping the ``py::arg("name")``. + template_params, template_args : list[str] | None + Class template parameter names and their concrete arguments. When + given, a default value referencing a parameter (e.g. ``Foo::DIM`` or + ``DIM``) is substituted with its value - used by the method and + constructor writers on templated classes; free functions pass None. + class_name : str | None + The class name used to qualify a template parameter in a default + value (``Foo::DIM``). Required when template_params is given. + substitute_empty_init_list : bool + When True, a bare ``{}`` default is given its type, e.g. + ``std::vector {}`` (constructor writer only). + + Returns + ------- + str + The fragment, e.g. ``, py::arg("i") = 1, py::arg("b")``. + """ + fragment = "" + for arg in arguments: + fragment += f', py::arg("{arg.name}")' + + if arg.default_value is None or exclude_default_args: + continue + + # Try to convert "(-1)" to "-1" etc. + default_value = str(arg.default_value) + value = utils.str_to_num(default_value, integer="int" in str(arg.decl_type)) + if value is not None: + default_value = str(value) + + # Substitute class template parameters in the default value, e.g. + # Foo::DIM_A -> 2 and -> <2>. + if template_params: + for param, val in zip(template_params, template_args): + if param in default_value: + default_value = re.sub( + rf"\b{class_name}::{param}\b", str(val), default_value + ) + default_value = re.sub(rf"\b{param}\b", f"{val}", default_value) + + # An empty initializer list needs its type, e.g. + # `Foo(std::vector laminas = {})` generates + # py::arg("laminas") = std::vector {}. + if substitute_empty_init_list and default_value.replace(" ", "") == "{}": + decl_type = type_traits.remove_const(arg.decl_type) + default_value = decl_type.decl_string + " {}" + + fragment += f" = {default_value}" + + return fragment diff --git a/cppwg/writers/constructor_writer.py b/cppwg/writers/constructor_writer.py index 9d91dd52..bd751409 100644 --- a/cppwg/writers/constructor_writer.py +++ b/cppwg/writers/constructor_writer.py @@ -1,12 +1,8 @@ """Wrapper code writer for C++ class constructors.""" -import re from typing import TYPE_CHECKING -from pygccxml.declarations import type_traits - from cppwg.info import exclusions -from cppwg.utils import utils from cppwg.writers.base_writer import CppBaseWrapperWriter if TYPE_CHECKING: @@ -98,49 +94,16 @@ def generate_wrapper(self) -> str: arg_types = [t.decl_string for t in self.ctor_decl.argument_types] arg_signature = ", ".join(arg_types) - # Keyword args with default values e.g. py::arg("i") = 1 - keyword_args = "" - for arg in self.ctor_decl.arguments: - keyword_args += f', py::arg("{arg.name}")' - - if not ( - arg.default_value is None - or self.class_info.hierarchy_attribute("exclude_default_args") - ): - # Try to convert "(-1)" to "-1" etc. - default_value = str(arg.default_value) - value = utils.str_to_num( - default_value, integer="int" in str(arg.decl_type) - ) - if value is not None: - default_value = str(value) - - # Check for template params in default value - if self.template_params: - for param, val in zip(self.template_params, self.template_args): - if param in default_value: - # Replace e.g. Foo::DIM_A -> 2 - default_value = re.sub( - f"\\b{self.class_info.name}::{param}\\b", - str(val), - default_value, - ) - - # Replace e.g. -> <2> - default_value = re.sub( - f"\\b{param}\\b", f"{val}", default_value - ) - - # Add type if default value is an empty initializer list - # Example: - # `Foo(std::vector laminas = {})` is equivalent to - # `Foo(std::vector laminas = std::vector{})` - # which generates `py::arg("laminas") = std::vector{}` - if default_value.replace(" ", "") == "{}": - decl_type = type_traits.remove_const(arg.decl_type) - default_value = decl_type.decl_string + " {}" - - keyword_args += f" = {default_value}" + # Keyword args with default values e.g. py::arg("i") = 1. Empty + # initializer-list defaults ({}) are given their type (constructor-only). + keyword_args = self.render_default_args( + self.ctor_decl.arguments, + self.class_info.hierarchy_attribute("exclude_default_args"), + template_params=self.template_params, + template_args=self.template_args, + class_name=self.class_info.name, + substitute_empty_init_list=True, + ) ctor_dict = { "arg_signature": arg_signature, diff --git a/cppwg/writers/free_function_writer.py b/cppwg/writers/free_function_writer.py index 970a072f..081c8f64 100644 --- a/cppwg/writers/free_function_writer.py +++ b/cppwg/writers/free_function_writer.py @@ -47,19 +47,11 @@ def generate_wrapper(self) -> str: # Pybind11 arg string with or without default values. # e.g. without default values: ', py::arg("foo"), py::arg("bar")' # e.g. with default values: ', py::arg("foo") = 1, py::arg("bar") = 2' - default_args = "" - if not self.free_function_info.hierarchy_attribute("exclude_default_args"): - for arg in self.free_function_info.decls[0].arguments: - default_args += f', py::arg("{arg.name}")' - if arg.default_value is not None: - # Try to convert "(-1)" to "-1" etc. - default_value = str(arg.default_value) - value = utils.str_to_num( - default_value, integer="int" in str(arg.decl_type) - ) - if value is not None: - default_value = str(value) - default_args += f" = {default_value}" + # exclude_default_args omits the values but keeps the py::arg names. + default_args = self.render_default_args( + self.free_function_info.decls[0].arguments, + self.free_function_info.hierarchy_attribute("exclude_default_args"), + ) # Add the free function wrapper code to the wrapper string func_dict = { diff --git a/cppwg/writers/method_writer.py b/cppwg/writers/method_writer.py index f5420194..665682ab 100644 --- a/cppwg/writers/method_writer.py +++ b/cppwg/writers/method_writer.py @@ -1,12 +1,10 @@ """Wrapper code writer for C++ methods.""" -import re from typing import TYPE_CHECKING from pygccxml.declarations import type_traits from cppwg.info import exclusions -from cppwg.utils import utils from cppwg.writers.base_writer import CppBaseWrapperWriter if TYPE_CHECKING: @@ -148,39 +146,13 @@ def generate_wrapper(self) -> str: arg_signature = ", ".join(arg_types) # Keyword args with default values e.g. py::arg("i") = 1 - keyword_args = "" - for arg in self.method_decl.arguments: - keyword_args += f', py::arg("{arg.name}")' - - if not ( - arg.default_value is None - or self.class_info.hierarchy_attribute("exclude_default_args") - ): - # Try to convert "(-1)" to "-1" etc. - default_value = str(arg.default_value) - value = utils.str_to_num( - default_value, integer="int" in str(arg.decl_type) - ) - if value is not None: - default_value = str(value) - - # Check for template params in default value - if self.template_params: - for param, val in zip(self.template_params, self.template_args): - if param in default_value: - # Replace e.g. Foo::DIM_A -> 2 - default_value = re.sub( - f"\\b{self.class_info.name}::{param}\\b", - str(val), - default_value, - ) - - # Replace e.g. -> <2> - default_value = re.sub( - f"\\b{param}\\b", f"{val}", default_value - ) - - keyword_args += f" = {default_value}" + keyword_args = self.render_default_args( + self.method_decl.arguments, + self.class_info.hierarchy_attribute("exclude_default_args"), + template_params=self.template_params, + template_args=self.template_args, + class_name=self.class_info.name, + ) # Call policy, e.g. "py::return_value_policy::reference" call_policy = "" diff --git a/tests/test_free_function_writer.py b/tests/test_free_function_writer.py index fff189fb..de547aae 100644 --- a/tests/test_free_function_writer.py +++ b/tests/test_free_function_writer.py @@ -40,7 +40,9 @@ def test_free_function_arg_type_exclude_respects_boundaries(): """arg_type_excludes drops free functions by argument type, as a whole token.""" excludes = {"arg_type_excludes": ["Shape"]} - assert _writer(arg_types=["::Shape<2> const &"], excludes=excludes).exclude() is True + assert ( + _writer(arg_types=["::Shape<2> const &"], excludes=excludes).exclude() is True + ) assert ( _writer(arg_types=["::AbstractShape<2> const &"], excludes=excludes).exclude() is False @@ -105,7 +107,9 @@ def hierarchy_attribute(self, name): def test_generate_wrapper_builds_def_with_default_args(): """A def line is built with normalized py::arg default values.""" templates = { - "free_function": Template('.def("$function_name", &$function_name$default_args)') + "free_function": Template( + '.def("$function_name", &$function_name$default_args)' + ) } decl = _FullDecl("my_func", [_Arg("count", default_value="(-1)", decl_type="int")]) writer = CppFreeFunctionWrapperWriter(_FullInfo(decl), templates) @@ -124,13 +128,21 @@ def test_generate_wrapper_excluded_returns_empty(): assert CppFreeFunctionWrapperWriter(info, templates).generate_wrapper() == "" -def test_generate_wrapper_omits_defaults_when_option_set(): - """exclude_default_args suppresses the py::arg default clauses.""" +def test_generate_wrapper_omits_default_values_but_keeps_arg_names(): + """exclude_default_args omits the `= value`, but keeps the py::arg names. + + Regression test for a DRY-drift bug: the free-function writer gated the whole + loop on exclude_default_args, so it dropped the py::arg("name") keyword names + too - unlike the method/constructor writers, which only gate the value. All + three now share render_default_args, which gates the value only. + """ templates = {"free_function": Template("$function_name|$default_args")} - decl = _FullDecl("f", [_Arg("x", default_value="1")]) + decl = _FullDecl("f", [_Arg("x", default_value="1"), _Arg("y", default_value="2")]) info = _FullInfo(decl, exclude_default_args=True) - assert CppFreeFunctionWrapperWriter(info, templates).generate_wrapper() == "f|" + assert CppFreeFunctionWrapperWriter(info, templates).generate_wrapper() == ( + 'f|, py::arg("x"), py::arg("y")' + ) def test_generate_wrapper_arg_without_default_value(): @@ -147,6 +159,8 @@ def test_generate_wrapper_arg_without_default_value(): def test_generate_wrapper_keeps_non_numeric_default(): """A default value that is not a number is emitted verbatim.""" templates = {"free_function": Template("$default_args")} - decl = _FullDecl("f", [_Arg("mode", default_value='"auto"', decl_type="std::string")]) + decl = _FullDecl( + "f", [_Arg("mode", default_value='"auto"', decl_type="std::string")] + ) result = CppFreeFunctionWrapperWriter(_FullInfo(decl), templates).generate_wrapper() assert result == ', py::arg("mode") = "auto"' From 0692f4a8cc27e2fcd41a29a8248c32b4c38d4038 Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 09:42:14 +0100 Subject: [PATCH 04/12] #119 Share enum .value emission and the export-values rule The .value("NAME", Scope::NAME) lines were rendered by near-identical loops in the enum writer and the struct-enum class path, and the ".export_values() vs ;" decision lived in CppEnumInfo.should_export_values but was bypassed by the struct-enum template, which hardcoded .export_values(). So a scoped enum class nested in a wrapped struct emitted wrong C++ - exporting enumerators it should not. Add utils.render_enum_value_lines (the value lines) and utils.should_export_enum_values (the decision rule). The enum writer and CppEnumInfo.should_export_values use them, and build_struct_enum_register now renders the value lines and computes its terminator through the same rule; the struct_enum_register template takes a ${enum_terminator} instead of a hardcoded .export_values(). A scoped nested enum now closes with `;` (+1 regression test). Behaviour-preserving for the examples (their enums are unscoped): the shapes wrappers regenerate byte-identically and the example builds and passes. Co-Authored-By: Claude Opus 4.8 --- cppwg/info/enum_info.py | 7 ++-- cppwg/templates/pybind11_default.py | 2 +- cppwg/utils/utils.py | 55 +++++++++++++++++++++++++++++ cppwg/writers/class_writer.py | 29 +++++++++++---- cppwg/writers/enum_writer.py | 9 ++--- tests/test_class_writer.py | 41 ++++++++++++++++++++- 6 files changed, 124 insertions(+), 19 deletions(-) diff --git a/cppwg/info/enum_info.py b/cppwg/info/enum_info.py index 0ce5670a..c7ad5aea 100644 --- a/cppwg/info/enum_info.py +++ b/cppwg/info/enum_info.py @@ -39,10 +39,9 @@ def should_export_values(self) -> bool: the info tree (package/module); otherwise mirror the C++ enum kind - export for an unscoped enum, not for a scoped one. """ - override = self.hierarchy_attribute("export_values") - if override is not None: - return override - return not self.scoped + return utils.should_export_enum_values( + self.hierarchy_attribute("export_values"), self.scoped + ) def update_from_ns(self, source_ns: "namespace_t") -> None: """ diff --git a/cppwg/templates/pybind11_default.py b/cppwg/templates/pybind11_default.py index 2953d68c..0a4d4690 100644 --- a/cppwg/templates/pybind11_default.py +++ b/cppwg/templates/pybind11_default.py @@ -178,7 +178,7 @@ ' py::class_<${class_py_name}> myclass(m, "${class_py_name}");\n' ' py::enum_<${class_py_name}::${enum_name}>(myclass, "${enum_name}")\n' "${enum_values}" - " .export_values();\n" + "${enum_terminator}" "}\n" ) diff --git a/cppwg/utils/utils.py b/cppwg/utils/utils.py index 442c66ff..a6e5c9f6 100644 --- a/cppwg/utils/utils.py +++ b/cppwg/utils/utils.py @@ -392,6 +392,61 @@ def is_scoped_enum_in_source_file(source_file_path: str, enum_name: str) -> bool return re.search(pattern, source) is not None +def render_enum_value_lines(values: list, qualifier: str, indent: str = " ") -> str: + """ + Render one ``.value("NAME", ::NAME)`` line per enumerator. + + Shared by the enum writer (plain namespace-scope enums) and the struct-enum + class path so the two cannot diverge. + + Parameters + ---------- + values : list + The enumerators as (name, number) tuples in source order (pygccxml's + enum_t.values); only the name is used. + qualifier : str + The C++ scope the enumerator is named through, e.g. ``Color`` for a plain + enum or ``Foo::Value`` for an enum nested in a wrapped struct. + indent : str + Leading whitespace for each line (the templates differ in indentation). + + Returns + ------- + str + The concatenated ``.value(...)`` lines, each newline-terminated. + """ + return "".join( + f'{indent}.value("{value[0]}", {qualifier}::{value[0]})\n' for value in values + ) + + +def should_export_enum_values( + export_values_override: bool | None, scoped: bool +) -> bool: + """ + Decide whether pybind11's ``.export_values()`` is emitted for an enum. + + The ``export_values`` config override wins if set (not None); otherwise mirror + the C++ enum kind - export for an unscoped enum, not for a scoped one. Shared + by CppEnumInfo.should_export_values and the struct-enum class path. + + Parameters + ---------- + export_values_override : bool | None + The resolved ``export_values`` option (None means "not set"). + scoped : bool + Whether the enum is scoped (``enum class``/``enum struct``). + + Returns + ------- + bool + True if ``.export_values()`` should be emitted. + """ + if export_values_override is not None: + return export_values_override + return not scoped + + def split_template_args(arg_string: str) -> list[str]: """ Split a template argument string on its top-level commas. diff --git a/cppwg/writers/class_writer.py b/cppwg/writers/class_writer.py index 348b20d5..2d6d90b1 100644 --- a/cppwg/writers/class_writer.py +++ b/cppwg/writers/class_writer.py @@ -16,6 +16,9 @@ call_generator_hook, canonicalize_type_whitespace, ensure_trailing_newline, + is_scoped_enum_in_source_file, + render_enum_value_lines, + should_export_enum_values, type_string_matches, write_file_if_changed, ) @@ -736,15 +739,26 @@ def build_struct_enum_register(self, template_idx: int) -> str: # typedef'd to the C++ type, so the registration function name matches # the hpp declaration and the module's register_..._class call even when # class_py_name differs from the C++ decl name (templates, name overrides). - enum_values = "".join( - ' .value("{val}", {class_py_name}::{enum_name}::{val})\n'.format( - val=value[0], - class_py_name=class_py_name, - enum_name=enum_decl.name, - ) - for value in enum_decl.values + enum_values = render_enum_value_lines( + enum_decl.values, + f"{class_py_name}::{enum_decl.name}", + indent=" ", ) + # Emit .export_values() on the same terms as a plain enum (see + # CppEnumWrapperWriter / should_export_enum_values): only for an unscoped + # enum by default, unless the export_values option overrides it. A scoped + # `enum class` nested in the struct closes with a plain `;`. + scoped = is_scoped_enum_in_source_file( + enum_decl.location.file_name, enum_decl.name + ) + if should_export_enum_values( + self.class_info.hierarchy_attribute("export_values"), scoped + ): + enum_terminator = " .export_values();\n" + else: + enum_terminator = " ;\n" + return self.wrapper_templates["struct_enum_register"].substitute( generator_pre_code=call_generator_hook( generator, "get_class_cpp_pre_code", "", class_py_name @@ -753,6 +767,7 @@ def build_struct_enum_register(self, template_idx: int) -> str: class_cpp_name=class_cpp_name, enum_name=enum_decl.name, enum_values=enum_values, + enum_terminator=enum_terminator, ) def write(self, work_dir: str) -> None: diff --git a/cppwg/writers/enum_writer.py b/cppwg/writers/enum_writer.py index 0d913671..c721aa18 100644 --- a/cppwg/writers/enum_writer.py +++ b/cppwg/writers/enum_writer.py @@ -3,6 +3,7 @@ from typing import TYPE_CHECKING from cppwg.info.enum_info import CppEnumInfo +from cppwg.utils import utils from cppwg.writers.base_writer import CppBaseWrapperWriter if TYPE_CHECKING: @@ -53,12 +54,8 @@ def generate_wrapper(self) -> str: enum_cpp_name = enum_decl.name enum_py_name = self.enum_info.name_override or self.enum_info.name - # One .value("NAME", Enum::NAME) line per enumerator. enum_decl.values is - # a list of (name, number) tuples in source order; only the name is used. - enum_values = "".join( - f' .value("{value[0]}", {enum_cpp_name}::{value[0]})\n' - for value in enum_decl.values - ) + # One .value("NAME", Enum::NAME) line per enumerator. + enum_values = utils.render_enum_value_lines(enum_decl.values, enum_cpp_name) # .export_values() exports the enumerators into the enclosing (module) # scope. By default this mirrors the C++ enum kind (unscoped enums export, diff --git a/tests/test_class_writer.py b/tests/test_class_writer.py index ac6f7559..5dfd399c 100644 --- a/tests/test_class_writer.py +++ b/tests/test_class_writer.py @@ -17,9 +17,12 @@ def __init__(self, file_name): class _FakeEnum: """Stand-in for a pygccxml enumeration_t.""" - def __init__(self, name, values): + def __init__(self, name, values, file_name="/src/enum.hpp"): self.name = name self.values = values # list of (name, value) tuples + # build_struct_enum_register reads location.file_name to detect a scoped + # enum; the scoped check is stubbed in tests (see stub_unscoped_enum). + self.location = _FakeLocation(file_name) class _FakeStructDecl: @@ -1130,6 +1133,42 @@ def test_virtual_overrides_empty_without_virtual_methods(): from cppwg.writers import class_writer as class_writer_module # noqa: E402 +@pytest.fixture(autouse=True) +def _stub_unscoped_enum(monkeypatch): + """Default struct-enum tests to an unscoped enum. + + build_struct_enum_register reads the enum's source file to detect a scoped + enum, but the fake decls have no real file. Default to unscoped (so + .export_values() is emitted, matching the legacy struct-enum behaviour); the + scoped-enum test overrides this. + """ + monkeypatch.setattr( + class_writer_module, "is_scoped_enum_in_source_file", lambda *a, **k: False + ) + + +def test_struct_enum_scoped_omits_export_values(monkeypatch): + """A scoped nested enum (enum class) closes with `;`, not .export_values(). + + Regression test for a DRY-drift bug: the struct-enum template hardcoded + .export_values(), ignoring scopedness and the export_values option, while the + plain enum writer routed through should_export_values. A scoped enum nested in + a wrapped struct thus emitted wrong C++ (exporting enumerators it should not). + """ + monkeypatch.setattr( + class_writer_module, "is_scoped_enum_in_source_file", lambda *a, **k: True + ) + enum = _FakeEnum("Value", [("RED", 0), ("GREEN", 1)]) + decl = _FakeStructDecl("Color", "/src/Color.hpp", enum) + class_info = _FakeClassInfo("Color", decl, attrs={}, source_file="Color.hpp") + + block = _make_writer(class_info).build_struct_enum_register(0) + + assert '.value("RED", Color::Value::RED)' in block + assert ".export_values()" not in block + assert " ;\n}\n" in block + + def test_construction_rejects_mismatched_instantiation_lists(): """__init__ validates that decls, cpp_names and py_names are parallel.""" decl = _FakeStructDecl("Foo", "/src/Foo.hpp", _FakeEnum("V", [("A", 0)])) From 94247658ee537eec2159d75d10061ad6568501b4 Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 09:47:15 +0100 Subject: [PATCH 05/12] #119 Centralise the register__class symbol name The register__class affix was hand-typed in four places: the module main cpp's call (an f-string in module_writer) and the definition/declaration in three templates (class_cpp_register, struct_enum_register, class_hpp_register_declaration). Drift between the call and a definition is an undefined-symbol link error. Add utils.registration_function_name as the single source. The three templates take a ${register_function} placeholder fed from it, and module_writer builds the call with it, so the affix is defined once. Behaviour-preserving: the shapes wrappers regenerate byte-identically and the example builds and passes. Co-Authored-By: Claude Opus 4.8 --- cppwg/templates/pybind11_default.py | 6 +++--- cppwg/utils/utils.py | 22 ++++++++++++++++++++++ cppwg/writers/class_writer.py | 7 ++++++- cppwg/writers/module_writer.py | 3 ++- tests/test_utils.py | 8 ++++++++ 5 files changed, 41 insertions(+), 5 deletions(-) diff --git a/cppwg/templates/pybind11_default.py b/cppwg/templates/pybind11_default.py index 0a4d4690..d441a156 100644 --- a/cppwg/templates/pybind11_default.py +++ b/cppwg/templates/pybind11_default.py @@ -114,7 +114,7 @@ # A single register-function forward declaration, one per instantiation, joined # into ${register_declarations} above. class_hpp_register_declaration = Template( - "void register_${class_py_name}_class(pybind11::module &m);\n" + "void ${register_function}(pybind11::module &m);\n" ) # Preamble for a class wrapper cpp file, emitted once per class. The file-scope @@ -152,7 +152,7 @@ "${generator_pre_code}" "\n" "${override_class}" - "void register_${class_py_name}_class(py::module &m)\n" + "void ${register_function}(py::module &m)\n" "{\n" " py::class_<${class_py_name}${overrides_string}${ptr_support}${bases}>" '(m, "${class_py_name}")\n' @@ -174,7 +174,7 @@ struct_enum_register = Template( "${generator_pre_code}" "\n" - "void register_${class_py_name}_class(py::module &m){\n" + "void ${register_function}(py::module &m){\n" ' py::class_<${class_py_name}> myclass(m, "${class_py_name}");\n' ' py::enum_<${class_py_name}::${enum_name}>(myclass, "${enum_name}")\n' "${enum_values}" diff --git a/cppwg/utils/utils.py b/cppwg/utils/utils.py index a6e5c9f6..d723c4f5 100644 --- a/cppwg/utils/utils.py +++ b/cppwg/utils/utils.py @@ -392,6 +392,28 @@ def is_scoped_enum_in_source_file(source_file_path: str, enum_name: str) -> bool return re.search(pattern, source) is not None +def registration_function_name(class_py_name: str) -> str: + """ + Return the C++ name of a class's pybind11 registration function. + + e.g. ``Foo_2_2`` -> ``register_Foo_2_2_class``. Single source for this + ``register__class`` affix so the definition (in the class cpp/hpp), + the declaration in the header collection and the call in the module main cpp + cannot drift apart into an undefined-symbol link error. + + Parameters + ---------- + class_py_name : str + The Python/wrapper name of the class, e.g. ``Foo_2_2``. + + Returns + ------- + str + The registration function name, e.g. ``register_Foo_2_2_class``. + """ + return f"register_{class_py_name}_class" + + def render_enum_value_lines(values: list, qualifier: str, indent: str = " ") -> str: """ Render one ``.value("NAME", ::NAME)`` line per enumerator. diff --git a/cppwg/writers/class_writer.py b/cppwg/writers/class_writer.py index 2d6d90b1..0dd3f8b5 100644 --- a/cppwg/writers/class_writer.py +++ b/cppwg/writers/class_writer.py @@ -17,6 +17,7 @@ canonicalize_type_whitespace, ensure_trailing_newline, is_scoped_enum_in_source_file, + registration_function_name, render_enum_value_lines, should_export_enum_values, type_string_matches, @@ -401,7 +402,9 @@ def build_hpp(self, register_py_names: list[str]) -> str: """ decl_template = self.wrapper_templates["class_hpp_register_declaration"] register_declarations = "".join( - decl_template.substitute(class_py_name=class_py_name) + decl_template.substitute( + register_function=registration_function_name(class_py_name) + ) for class_py_name in register_py_names ) return self.wrapper_templates["class_hpp"].substitute( @@ -687,6 +690,7 @@ def build_class_register(self, template_idx: int) -> tuple[str, str]: generator_pre_code=call_generator_hook( generator, "get_class_cpp_pre_code", "", class_py_name ), + register_function=registration_function_name(class_py_name), class_py_name=class_py_name, class_cpp_name=class_cpp_name, override_class=override_class, @@ -763,6 +767,7 @@ def build_struct_enum_register(self, template_idx: int) -> str: generator_pre_code=call_generator_hook( generator, "get_class_cpp_pre_code", "", class_py_name ), + register_function=registration_function_name(class_py_name), class_py_name=class_py_name, class_cpp_name=class_cpp_name, enum_name=enum_decl.name, diff --git a/cppwg/writers/module_writer.py b/cppwg/writers/module_writer.py index fb1f93e4..6e3d7cf1 100644 --- a/cppwg/writers/module_writer.py +++ b/cppwg/writers/module_writer.py @@ -8,6 +8,7 @@ from cppwg.utils.utils import ( call_generator_hook, ensure_trailing_newline, + registration_function_name, write_file_if_changed, ) from cppwg.writers.class_writer import CppClassWrapperWriter @@ -225,7 +226,7 @@ def build_module_context(self) -> dict[str, str]: # Class registration calls, e.g. register_Foo_2_2_class(m); register_calls = "".join( - f" register_{py_name}_class(m);\n" + f" {registration_function_name(py_name)}(m);\n" for class_info in non_excluded_classes for py_name in class_info.py_names ) diff --git a/tests/test_utils.py b/tests/test_utils.py index 52b67169..57f7b1c7 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -604,6 +604,14 @@ def test_strip_outer_angle_brackets(signature, expected): assert strip_outer_angle_brackets(signature) == expected +def test_registration_function_name(): + """The register__class affix is applied to the wrapper name.""" + from cppwg.utils.utils import registration_function_name + + assert registration_function_name("Foo_2_2") == "register_Foo_2_2_class" + assert registration_function_name("ShapeMetrics") == "register_ShapeMetrics_class" + + def test_type_is_copy_assignable_for_non_class_types(): """Fundamental types and pointers are always copy-assignable.""" from pygccxml.declarations import cpptypes From b0d354bf08a3fdf918a9f227326103a64508aab1 Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 09:53:06 +0100 Subject: [PATCH 06/12] #119 Centralise class wrapper filename composition The {py_name_base}.cppwg.hpp / .cpp filename was composed in four places: the files written (class_writer.write_files), the module's #include of a class wrapper (module_writer), and the cpp's own #include (class_cpp_header template). Drift is a missing-header compile error. Add CppClassInfo.wrapper_header_filename / wrapper_source_filename as the single source. write_files and the module include use them, and the class_cpp_header template takes the filename via a ${class_hpp_filename} placeholder fed from the same method. write_files no longer needs its file_stem argument (always the class's own py_name_base). The module-main filename and the include guard are different patterns and left as they are. Behaviour-preserving: the shapes wrappers regenerate byte-identically and the example builds and passes. Co-Authored-By: Claude Opus 4.8 --- cppwg/info/class_info.py | 16 ++++++++++++++++ cppwg/templates/pybind11_default.py | 2 +- cppwg/writers/class_writer.py | 18 +++++++++--------- cppwg/writers/module_writer.py | 2 +- tests/test_class_writer.py | 6 ++++++ tests/test_module_writer.py | 11 +++++------ 6 files changed, 38 insertions(+), 17 deletions(-) diff --git a/cppwg/info/class_info.py b/cppwg/info/class_info.py index e3e17e6e..dfd2d180 100644 --- a/cppwg/info/class_info.py +++ b/cppwg/info/class_info.py @@ -9,6 +9,7 @@ from cppwg.info.cpp_entity_info import CppEntityInfo from cppwg.utils import utils +from cppwg.utils.constants import CPPWG_EXT if TYPE_CHECKING: from pygccxml.declarations import declaration_t @@ -542,6 +543,21 @@ class it is cleaned the same way as the instantiation names. base = base[0].capitalize() + base[1:] return base + def wrapper_header_filename(self) -> str: + """ + Return the class's wrapper header filename, e.g. ``Foo.cppwg.hpp``. + + All of the class's instantiations share this single header, named after + py_name_base(). Single source for the wrapper filename so the file that + is written, the module's ``#include`` of it and the cpp's own + ``#include`` cannot drift apart into a missing-header compile error. + """ + return f"{self.py_name_base()}.{CPPWG_EXT}.hpp" + + def wrapper_source_filename(self) -> str: + """Return the class's wrapper source filename, e.g. ``Foo.cppwg.cpp``.""" + return f"{self.py_name_base()}.{CPPWG_EXT}.cpp" + def update_py_names(self) -> None: """ Set the Python names for the class, accounting for template args. diff --git a/cppwg/templates/pybind11_default.py b/cppwg/templates/pybind11_default.py index d441a156..415ef3e9 100644 --- a/cppwg/templates/pybind11_default.py +++ b/cppwg/templates/pybind11_default.py @@ -132,7 +132,7 @@ "#include \n" "${includes}" "\n" - '#include "${class_hpp_name}.' + CPPWG_EXT + '.hpp"\n' + '#include "${class_hpp_filename}"\n' "\n" "namespace py = pybind11;\n" "${smart_ptr_handle};\n" diff --git a/cppwg/writers/class_writer.py b/cppwg/writers/class_writer.py index 0dd3f8b5..5628e65e 100644 --- a/cppwg/writers/class_writer.py +++ b/cppwg/writers/class_writer.py @@ -9,7 +9,6 @@ from cppwg.utils.constants import ( CPPWG_CLASS_OVERRIDE_SUFFIX, - CPPWG_EXT, CPPWG_HEADER_COLLECTION_FILENAME, ) from cppwg.utils.utils import ( @@ -440,7 +439,7 @@ def build_cpp_header( return self.wrapper_templates["class_cpp_header"].substitute( prefix_text=self.prefix_block(), includes=self.includes_block(), - class_hpp_name=self.class_info.py_name_base(), + class_hpp_filename=self.class_info.wrapper_header_filename(), smart_ptr_handle=self.smart_ptr_handle(), prefix_code=self.prefix_code(), class_typedefs=class_typedefs, @@ -887,7 +886,7 @@ def write(self, work_dir: str) -> None: class_typedefs_block, return_typedefs_block ) self.cpp_string += register_section - self.write_files(work_dir, self.class_info.py_name_base()) + self.write_files(work_dir) def _detect_typecasters(self, scan_text: str) -> list[str]: """ @@ -928,20 +927,21 @@ def _detect_typecasters(self, scan_text: str) -> list[str]: return headers - def write_files(self, work_dir: str, file_stem: str) -> None: + def write_files(self, work_dir: str) -> None: """ Write the hpp and cpp wrapper code to file. + The class's instantiations share one ``{py_name_base}.cppwg.hpp`` / + ``.cpp`` pair, named by the class info so it matches the module's include + and register call. + Parameters ---------- work_dir : str The directory to write the files to - file_stem : str - The wrapper file stem shared by all of the class's - instantiations, e.g. Foo (for Foo.cppwg.hpp / Foo.cppwg.cpp). """ - hpp_filepath = os.path.join(work_dir, f"{file_stem}.{CPPWG_EXT}.hpp") - cpp_filepath = os.path.join(work_dir, f"{file_stem}.{CPPWG_EXT}.cpp") + hpp_filepath = os.path.join(work_dir, self.class_info.wrapper_header_filename()) + cpp_filepath = os.path.join(work_dir, self.class_info.wrapper_source_filename()) write_file_if_changed(hpp_filepath, self.hpp_string, self.overwrite) write_file_if_changed(cpp_filepath, self.cpp_string, self.overwrite) diff --git a/cppwg/writers/module_writer.py b/cppwg/writers/module_writer.py index 6e3d7cf1..e379df82 100644 --- a/cppwg/writers/module_writer.py +++ b/cppwg/writers/module_writer.py @@ -191,7 +191,7 @@ def build_module_context(self) -> dict[str, str]: # instantiations share one wrapper hpp (named after the class), so this # is one include per class, e.g. #include "Foo.cppwg.hpp". class_includes = "".join( - f'#include "{class_info.py_name_base()}.{CPPWG_EXT}.hpp"\n' + f'#include "{class_info.wrapper_header_filename()}"\n' for class_info in non_excluded_classes ) diff --git a/tests/test_class_writer.py b/tests/test_class_writer.py index 5dfd399c..660cd269 100644 --- a/tests/test_class_writer.py +++ b/tests/test_class_writer.py @@ -69,6 +69,12 @@ def __init__( def py_name_base(self): return self._name_base + def wrapper_header_filename(self): + return f"{self._name_base}.cppwg.hpp" + + def wrapper_source_filename(self): + return f"{self._name_base}.cppwg.cpp" + def hierarchy_attribute(self, key): return self._attrs.get(key) diff --git a/tests/test_module_writer.py b/tests/test_module_writer.py index 8bd5e90c..b919d37f 100644 --- a/tests/test_module_writer.py +++ b/tests/test_module_writer.py @@ -50,6 +50,9 @@ def __init__(self, name, stem, excluded=False): def py_name_base(self): return self._stem + def wrapper_header_filename(self): + return f"{self._stem}.cppwg.hpp" + class _FakeClassWriter: def __init__(self, *args): @@ -61,12 +64,8 @@ def write(self, work_dir): def test_write_class_wrappers_rejects_duplicate_file_stem(tmp_path, monkeypatch): """Two classes mapping to the same wrapper file name fail fast.""" - monkeypatch.setattr( - module_writer_module, "CppClassWrapperWriter", _FakeClassWriter - ) - module = _module( - classes=[_ClassStub("Foo", "Widget"), _ClassStub("Bar", "Widget")] - ) + monkeypatch.setattr(module_writer_module, "CppClassWrapperWriter", _FakeClassWriter) + module = _module(classes=[_ClassStub("Foo", "Widget"), _ClassStub("Bar", "Widget")]) writer = CppModuleWrapperWriter(module, template_collection, str(tmp_path)) with pytest.raises(ValueError, match="used by both"): From ba54ad15e6e6c9fb3f5039ed08c8abae2a4b7c72 Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 09:57:40 +0100 Subject: [PATCH 07/12] #119 Centralise namespace-stripping into utils.unqualified_name The `name.split("::")[-1]` idiom - reduce a (possibly qualified) name to its unqualified form to match pygccxml's unqualified declaration names - was open-coded in four places (class_writer bases_block, source_parser, package_info, and utils itself) alongside class_info._unqualified_base_name, which additionally strips template arguments. Add utils.unqualified_name as the single source for the namespace strip. The four sites use it, and _unqualified_base_name composes it with a leading template-argument strip. Behaviour is unchanged (the four sites operate on names that carry no template arguments, or intentionally keep them). The `split("<")[0]` template-argument strip (generators, and the leading step of _unqualified_base_name) is a distinct operation and left as-is. Shapes wrappers regenerate byte-identically; example builds and passes. Co-Authored-By: Claude Opus 4.8 --- cppwg/info/class_info.py | 2 +- cppwg/info/package_info.py | 2 +- cppwg/parsers/source_parser.py | 2 +- cppwg/utils/utils.py | 25 ++++++++++++++++++++++++- cppwg/writers/class_writer.py | 3 ++- tests/test_utils.py | 9 +++++++++ 6 files changed, 38 insertions(+), 5 deletions(-) diff --git a/cppwg/info/class_info.py b/cppwg/info/class_info.py index dfd2d180..013a63da 100644 --- a/cppwg/info/class_info.py +++ b/cppwg/info/class_info.py @@ -26,7 +26,7 @@ def _unqualified_base_name(name: str) -> str: identity, which pygccxml does not preserve across template/typedef resolution. """ - return name.split("<", 1)[0].rsplit("::", 1)[-1].strip() + return utils.unqualified_name(name.split("<", 1)[0]).strip() class CppClassInfo(CppEntityInfo): diff --git a/cppwg/info/package_info.py b/cppwg/info/package_info.py index bb315d24..58543317 100644 --- a/cppwg/info/package_info.py +++ b/cppwg/info/package_info.py @@ -489,7 +489,7 @@ def discover_base_class_instantiations(self, source_ns: "namespace_t") -> None: ): continue base_name, args = declarations.templates.split(base_decl.name) - name = base_name.split("::")[-1] + name = utils.unqualified_name(base_name) if name not in targets or not args: continue args = [arg.strip() for arg in args] diff --git a/cppwg/parsers/source_parser.py b/cppwg/parsers/source_parser.py index 54de6187..dd7d88a1 100644 --- a/cppwg/parsers/source_parser.py +++ b/cppwg/parsers/source_parser.py @@ -199,7 +199,7 @@ class name to the template argument lists found. # pygccxml's class name is already unqualified (e.g. "Bar<2>" for # ::foo::Bar<2>), but strip any qualification defensively so the # two discovery paths stay consistent across pygccxml versions. - base = base.split("::")[-1] + base = utils.unqualified_name(base) # pygccxml renders an integer argument with its C++ literal suffix # (e.g. "2u" for an unsigned argument); normalize to the plain # form ("2") so names match the text-scan path and config. diff --git a/cppwg/utils/utils.py b/cppwg/utils/utils.py index d723c4f5..0bd72bee 100644 --- a/cppwg/utils/utils.py +++ b/cppwg/utils/utils.py @@ -145,6 +145,29 @@ def is_option_ALL(input_obj: Any) -> bool: return isinstance(input_obj, str) and input_obj.upper() == CPPWG_ALL_STRING +def unqualified_name(name: str) -> str: + """ + Strip any namespace qualification from a C++ name. + + Returns the final ``::``-separated segment, e.g. ``foo::bar::Baz`` -> ``Baz`` + and ``Baz`` -> ``Baz``. Template arguments are left intact + (``foo::Bar<2>`` -> ``Bar<2>``); to drop those first, split on ``<``. Single + source for the ``split("::")[-1]`` idiom used to match a (possibly qualified) + base or config name against pygccxml's unqualified declaration names. + + Parameters + ---------- + name : str + A C++ name, possibly namespace-qualified. + + Returns + ------- + str + The unqualified name. + """ + return name.rsplit("::", 1)[-1] + + # A single C++ identifier character, used to decide where identifier boundaries # apply when matching type patterns. _IDENTIFIER_CHAR = re.compile(r"[A-Za-z0-9_]") @@ -574,7 +597,7 @@ def find_template_instantiations_in_source( for match in _TEMPLATE_INSTANTIATION_RE.finditer(source): # e.g. "foo::Bar" -> "Bar" to match the unqualified class info name - name = match.group(1).split("::")[-1] + name = unqualified_name(match.group(1)) args = [ normalize_template_arg(arg) for arg in split_template_args(match.group(2)) diff --git a/cppwg/writers/class_writer.py b/cppwg/writers/class_writer.py index 5628e65e..fbea423e 100644 --- a/cppwg/writers/class_writer.py +++ b/cppwg/writers/class_writer.py @@ -20,6 +20,7 @@ render_enum_value_lines, should_export_enum_values, type_string_matches, + unqualified_name, write_file_if_changed, ) from cppwg.writers.base_writer import CppBaseWrapperWriter @@ -349,7 +350,7 @@ def bases_block(self, class_decl: "class_t") -> str: # Compare on unqualified names: the base's pygccxml name is unqualified, # so accept either a qualified or unqualified config entry (e.g. both # "foo::AbstractBar" and "AbstractBar" match a base named AbstractBar). - external_bases = {str(name).split("::")[-1] for name in external_bases} + external_bases = {unqualified_name(str(name)) for name in external_bases} for base in class_decl.bases: # type(base) -> hierarchy_info_t # Check that the base class is not private diff --git a/tests/test_utils.py b/tests/test_utils.py index 57f7b1c7..e8d65cbf 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -604,6 +604,15 @@ def test_strip_outer_angle_brackets(signature, expected): assert strip_outer_angle_brackets(signature) == expected +def test_unqualified_name(): + """Namespace qualification is stripped; template args are left intact.""" + from cppwg.utils.utils import unqualified_name + + assert unqualified_name("foo::bar::Baz") == "Baz" + assert unqualified_name("Baz") == "Baz" + assert unqualified_name("foo::Bar<2>") == "Bar<2>" # template args kept + + def test_registration_function_name(): """The register__class affix is applied to the wrapper name.""" from cppwg.utils.utils import registration_function_name From 3322b345eafa1f0086065efce2f1d9d9204c07a7 Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 10:01:56 +0100 Subject: [PATCH 08/12] #119 Share Python-name mangling via _mangle_py_token py_name_base (the class base name) and update_py_names (each template arg) mangled C++ tokens into Python-name-safe fragments with near-identical code that had diverged only on `<`/`,`: the base name removed them, while a template arg turned them into `_` so a nested template stays readable. Verified that divergence is deliberate, not a bug. Extract _mangle_py_token(text, separator): it applies name_replacements, turns `<`/`,` into the separator, removes `>`/spaces and capitalises. py_name_base passes separator="" and update_py_names passes "_", so the shared logic lives in one place and the intentional difference is a single explicit argument (+1 test pinning both). Byte-identical: shapes wrappers regenerate unchanged and the example builds and passes. Co-Authored-By: Claude Opus 4.8 --- cppwg/info/class_info.py | 60 ++++++++++++++++++++++++---------------- tests/test_class_info.py | 30 ++++++++++++++++---- 2 files changed, 61 insertions(+), 29 deletions(-) diff --git a/cppwg/info/class_info.py b/cppwg/info/class_info.py index 013a63da..a00dbbec 100644 --- a/cppwg/info/class_info.py +++ b/cppwg/info/class_info.py @@ -534,14 +534,40 @@ class it is cleaned the same way as the instantiation names. if not self.template_arg_lists: return base + # A class base name drops `<`/`,` (separator="") rather than turning them + # into `_`, since it is a single token, not a list of template args. + return self._mangle_py_token(base, separator="") + + def _mangle_py_token(self, text: str, separator: str = "") -> str: + """ + Mangle a C++ token into a Python-name-safe fragment. + + Applies the configured name_replacements, then reduces the C++ + punctuation: ``<`` and ``,`` become ``separator`` (``""`` to drop them in + a class base name, ``"_"`` to split nested template arguments), while + ``>`` and spaces are always removed. Finally the first character is + capitalised. Shared by py_name_base and update_py_names so the two + mangle names the same way apart from that deliberate separator choice. + + Parameters + ---------- + text : str + The C++ token to mangle, e.g. a class base name or a template arg. + separator : str + What ``<`` and ``,`` become ("" to remove, "_" to split). + + Returns + ------- + str + The mangled, Python-name-safe fragment. + """ for name, replacement in self.name_replacements.items(): - base = base.replace(name, replacement) - base = base.translate( - str.maketrans({"<": None, ">": None, ",": None, " ": None}) - ) - if len(base) > 1: - base = base[0].capitalize() + base[1:] - return base + text = text.replace(name, replacement) + text = text.replace("<", separator).replace(",", separator) + text = text.replace(">", "").replace(" ", "") + if len(text) > 1: + text = text[0].capitalize() + text[1:] + return text def wrapper_header_filename(self) -> str: """ @@ -575,29 +601,15 @@ class instantiation. For example, class "Foo" with template arguments self.py_names.append(class_name) return - # Table of special characters for removal - rm_chars = {"<": None, ">": None, ",": None, " ": None} - rm_table = str.maketrans(rm_chars) - # Create a string of template args separated by "_" e.g. 2_2 for template_arg_list in self.template_arg_lists: # Example template_arg_list : [2, 2] template_string = "" for idx, arg in enumerate(template_arg_list): - # Do standard name replacements - arg_str = str(arg) - for name, replacement in self.name_replacements.items(): - arg_str = arg_str.replace(name, replacement) - - # Remove special characters - arg_str = ( - arg_str.replace("<", "_").replace(",", "_").translate(rm_table) - ) - - # Capitalize the first letter - if len(arg_str) > 1: - arg_str = arg_str[0].capitalize() + arg_str[1:] + # A nested template arg keeps its structure via "_" separators, + # e.g. PottsMesh<2> -> PottsMesh_2 (separator="_"). + arg_str = self._mangle_py_token(str(arg), separator="_") # Add "_" between template arguments template_string += arg_str diff --git a/tests/test_class_info.py b/tests/test_class_info.py index 911c91d7..39740c15 100644 --- a/tests/test_class_info.py +++ b/tests/test_class_info.py @@ -115,6 +115,24 @@ def test_extract_templates_skips_non_dict_substitution(tmp_path): assert cls.template_params == [] +def test_mangle_py_token_separator_choice(): + """A base name drops `<`/`,`; a template arg splits them with `_`. + + py_name_base and update_py_names share _mangle_py_token but pass different + separators - "" for a class base name (a single token) and "_" for a template + argument (so a nested template stays readable). This pins that divergence. + """ + cls = CppClassInfo("Foo") + + # Base-name style (separator=""): `<`, `,`, `>` and spaces are all removed. + assert cls._mangle_py_token("Bar<2, 3>", separator="") == "Bar23" + + # Template-arg style (separator="_"): `<` and `,` become underscores, so a + # nested template argument keeps its structure. + assert cls._mangle_py_token("PottsMesh<2>", separator="_") == "PottsMesh_2" + assert cls._mangle_py_token("Foo<2, 3>", separator="_") == "Foo_2_3" + + def _discovery_class(name, params, excludes=None): """A class info with fixed template params and optional discover_arg_excludes.""" cls = CppClassInfo(name) @@ -200,7 +218,9 @@ def __init__(self, arg_strings): class _FakeClassDecl: - def __init__(self, name="Foo", methods=(), ctors=(), bases=(), file_name="/s/Foo.hpp"): + def __init__( + self, name="Foo", methods=(), ctors=(), bases=(), file_name="/s/Foo.hpp" + ): self.name = name self._methods = list(methods) self._ctors = list(ctors) @@ -276,7 +296,9 @@ def test_update_from_source_skips_excluded(): def test_update_from_ns_resolves_class_and_bases(): - foo_decl = _FakeClassDecl(name="Foo", bases=[SimpleNamespace(related_class="BaseObj")]) + foo_decl = _FakeClassDecl( + name="Foo", bases=[SimpleNamespace(related_class="BaseObj")] + ) ns = _FakeNs(classes={"Foo": foo_decl}) cls = CppClassInfo("Foo") cls.cpp_names = ["Foo"] @@ -289,9 +311,7 @@ def test_update_from_ns_resolves_class_and_bases(): def test_update_from_ns_resolves_via_typedef(): real_decl = _FakeClassDecl(name="Foo<2>") - typedef_decl = SimpleNamespace( - decl_type=SimpleNamespace(declaration=real_decl) - ) + typedef_decl = SimpleNamespace(decl_type=SimpleNamespace(declaration=real_decl)) ns = _FakeNs(typedefs={"Foo_2": typedef_decl}) cls = CppClassInfo("Foo") cls.cpp_names = ["Foo<2>"] From 38907a5ff25534afe89e9b66a505b24f82903c37 Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 10:39:27 +0100 Subject: [PATCH 09/12] #119 Deep-copy config values so info objects do not alias option defaults The parser shallow-copies one base_config into every package/module/class config, so the same list/dict object reaches many info objects. BaseInfo deep-copied the schema defaults but then assigned the config value by reference, re-sharing it - so e.g. every class's excluded_methods / name_replacements was the same object, and a later mutation of one would leak to its siblings and parent. Deep-copy each config value on assignment (+ regression test). Co-Authored-By: Claude Opus 4.8 --- cppwg/info/base_info.py | 11 ++++++++--- tests/test_package_info_parser.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/cppwg/info/base_info.py b/cppwg/info/base_info.py index a05bec4a..dcc26fa8 100644 --- a/cppwg/info/base_info.py +++ b/cppwg/info/base_info.py @@ -183,11 +183,16 @@ def __init__(self, name: str, info_config: dict[str, Any] | None = None) -> None if info_config: # Copy any option the config provides, over the schema defaults. - # exclude_inherited_overrides (in _EXTRA_CONFIG_KEYS) is copied when - # present but has no shared default - the parser seeds it per level. + # Deep-copy each value: the parser shallow-copies one base_config into + # every package/module/class config, so the same list/dict object + # reaches many info objects; without this copy they would alias it (a + # later mutation of one object's option would leak to its siblings and + # parent). exclude_inherited_overrides (in _EXTRA_CONFIG_KEYS) is + # copied when present but has no shared default - the parser seeds it + # per level. for key in (*BASE_INFO_OPTIONS, *_EXTRA_CONFIG_KEYS): if key in info_config: - setattr(self, key, info_config[key]) + setattr(self, key, copy.deepcopy(info_config[key])) self.load_custom_generator() diff --git a/tests/test_package_info_parser.py b/tests/test_package_info_parser.py index 10bba882..f093d013 100644 --- a/tests/test_package_info_parser.py +++ b/tests/test_package_info_parser.py @@ -177,6 +177,37 @@ def test_class_auto_includes_defaults_to_none(tmp_path): assert cls.auto_includes is None +def test_parsed_objects_do_not_alias_option_defaults(tmp_path): + """Each info object gets its own copy of a defaulted mutable option. + + The parser shallow-copies one base_config into every config, so without a + deep copy on assignment sibling and parent info objects would share the same + list/dict; mutating one would then leak to the others. + """ + config_path = _write_config( + tmp_path, + """ + name: testpkg + modules: + - name: mymod + classes: + - name: Foo + - name: Bar + """, + ) + + package_info = PackageInfoParser(config_path, str(tmp_path)).parse() + foo, bar = package_info.module_collection[0].class_collection + + assert foo.excluded_methods is not bar.excluded_methods + assert foo.name_replacements is not bar.name_replacements + assert foo.excluded_methods is not package_info.excluded_methods + + foo.excluded_methods.append("only_foo") + assert bar.excluded_methods == [] + assert package_info.excluded_methods == [] + + def test_parses_name_replacements(tmp_path): """A name_replacements map set in the YAML reaches the info objects. From f6fb134c369fafa4da52ba911392d5c60c6bff73 Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 10:39:28 +0100 Subject: [PATCH 10/12] #119 Fix unqualified_name and scope enum detection to the declaration Two correctness fixes to code added in this branch, both in utils and its callers (they share utils.py, so they land together): unqualified_name used rsplit("::", 1), which selects the last :: even inside a template argument: unqualified_name("foo::Bar>") returned "vector>" instead of "Bar>" (and could stop a templated external_bases entry matching). It now finds the last :: at template-nesting depth zero. is_scoped_enum_in_source_file scanned the whole file for `enum class `, so two same-named enums with different scopedness in one file (a scoped A::Value and an unscoped B::Value) confused each other. It now checks only the enum's own declaration line, from the decl's location.line, which both callers (the plain enum path and the struct-enum path) now pass. Both behaviour-preserving on the examples: shapes wrappers regenerate byte-identically (ShapeKind still exports, Handedness still does not) and the example builds and passes. +tests for both. Co-Authored-By: Claude Opus 4.8 --- cppwg/info/enum_info.py | 4 ++- cppwg/utils/utils.py | 63 ++++++++++++++++++++++++++--------- cppwg/writers/class_writer.py | 2 +- tests/test_class_writer.py | 3 +- tests/test_enum_info.py | 15 ++++++--- tests/test_utils.py | 27 +++++++++++---- 6 files changed, 84 insertions(+), 30 deletions(-) diff --git a/cppwg/info/enum_info.py b/cppwg/info/enum_info.py index c7ad5aea..4c272ed2 100644 --- a/cppwg/info/enum_info.py +++ b/cppwg/info/enum_info.py @@ -73,5 +73,7 @@ def update_from_ns(self, source_ns: "namespace_t") -> None: # pygccxml does not expose enum scopedness, so read it from the source # file the enum was declared in (via the resolved decl's location). self.scoped = utils.is_scoped_enum_in_source_file( - self.decls[0].location.file_name, self.decls[0].name + self.decls[0].location.file_name, + self.decls[0].name, + self.decls[0].location.line, ) diff --git a/cppwg/utils/utils.py b/cppwg/utils/utils.py index 0bd72bee..f59f6fa1 100644 --- a/cppwg/utils/utils.py +++ b/cppwg/utils/utils.py @@ -149,11 +149,13 @@ def unqualified_name(name: str) -> str: """ Strip any namespace qualification from a C++ name. - Returns the final ``::``-separated segment, e.g. ``foo::bar::Baz`` -> ``Baz`` - and ``Baz`` -> ``Baz``. Template arguments are left intact - (``foo::Bar<2>`` -> ``Bar<2>``); to drop those first, split on ``<``. Single - source for the ``split("::")[-1]`` idiom used to match a (possibly qualified) - base or config name against pygccxml's unqualified declaration names. + Returns the segment after the last top-level ``::``, e.g. + ``foo::bar::Baz`` -> ``Baz`` and ``Baz`` -> ``Baz``. Template arguments are + left intact, and a ``::`` *inside* template arguments does not count as a + separator, so ``foo::Bar>`` -> ``Bar>`` + (not ``vector>``). To drop the template arguments too, split on ``<`` + first. Single source for reducing a (possibly qualified) base or config name + to match pygccxml's unqualified declaration names. Parameters ---------- @@ -165,7 +167,23 @@ def unqualified_name(name: str) -> str: str The unqualified name. """ - return name.rsplit("::", 1)[-1] + # Find the last "::" at template-nesting depth zero (a "::" inside <...> is + # part of a template argument, not a namespace qualifier). + depth = 0 + cut = 0 + i = 0 + while i < len(name): + char = name[i] + if char == "<": + depth += 1 + elif char == ">": + depth -= 1 + elif depth == 0 and char == ":" and name[i + 1 : i + 2] == ":": + cut = i + 2 + i += 2 + continue + i += 1 + return name[cut:] # A single C++ identifier character, used to decide where identifier boundaries @@ -380,9 +398,11 @@ def find_classes_in_source_file( return classes -def is_scoped_enum_in_source_file(source_file_path: str, enum_name: str) -> bool: +def is_scoped_enum_in_source_file( + source_file_path: str, enum_name: str, line_number: int +) -> bool: """ - Return whether an enum is declared as a scoped enum in a C++ source file. + Return whether the enum declared at ``line_number`` is a scoped enum. A scoped enum is `enum class Name` or `enum struct Name`, whose enumerators live on the enum type; an unscoped `enum Name` also leaks its enumerators @@ -392,27 +412,40 @@ def is_scoped_enum_in_source_file(source_file_path: str, enum_name: str) -> bool config option can override it. pygccxml does not expose enum scopedness, so it is read from the source text. + The check is scoped to the enum's own declaration line (from its pygccxml + location) rather than the whole file, so two same-named enums with different + scopedness in one file (e.g. a scoped ``A::Value`` and an unscoped + ``B::Value``) do not confuse each other. + Parameters ---------- source_file_path : str The path to the source file declaring the enum. enum_name : str The enum name to check. + line_number : int + The 1-based line the enum is declared on (decl.location.line). Returns ------- bool True if the enum is declared scoped (`enum class`/`enum struct`). """ - source = read_source_file( - source_file_path, - strip_comments=True, - strip_preprocessor=True, - strip_whitespace=True, - ) + try: + with open(source_file_path) as source_file: + lines = source_file.readlines() + except OSError: + return False + + if not 1 <= line_number <= len(lines): + return False + + # The declaration line, with any trailing line comment removed so that a + # comment such as `enum Foo {}; // an enum class` cannot flip the result. + line = lines[line_number - 1].split("//", 1)[0] pattern = r"\benum\s+(?:class|struct)\s+" + re.escape(enum_name) + r"\b" - return re.search(pattern, source) is not None + return re.search(pattern, line) is not None def registration_function_name(class_py_name: str) -> str: diff --git a/cppwg/writers/class_writer.py b/cppwg/writers/class_writer.py index fbea423e..51ced1e3 100644 --- a/cppwg/writers/class_writer.py +++ b/cppwg/writers/class_writer.py @@ -754,7 +754,7 @@ def build_struct_enum_register(self, template_idx: int) -> str: # enum by default, unless the export_values option overrides it. A scoped # `enum class` nested in the struct closes with a plain `;`. scoped = is_scoped_enum_in_source_file( - enum_decl.location.file_name, enum_decl.name + enum_decl.location.file_name, enum_decl.name, enum_decl.location.line ) if should_export_enum_values( self.class_info.hierarchy_attribute("export_values"), scoped diff --git a/tests/test_class_writer.py b/tests/test_class_writer.py index 660cd269..79da4d28 100644 --- a/tests/test_class_writer.py +++ b/tests/test_class_writer.py @@ -10,8 +10,9 @@ class _FakeLocation: """Stand-in for a pygccxml declaration location.""" - def __init__(self, file_name): + def __init__(self, file_name, line=1): self.file_name = file_name + self.line = line class _FakeEnum: diff --git a/tests/test_enum_info.py b/tests/test_enum_info.py index fca359a0..22107370 100644 --- a/tests/test_enum_info.py +++ b/tests/test_enum_info.py @@ -6,16 +6,17 @@ class _FakeLocation: - def __init__(self, file_name): + def __init__(self, file_name, line=1): self.file_name = file_name + self.line = line class _FakeEnumDecl: """A minimal stand-in for a pygccxml enumeration_t.""" - def __init__(self, name, file_name): + def __init__(self, name, file_name, line=1): self.name = name - self.location = _FakeLocation(file_name) + self.location = _FakeLocation(file_name, line) class _FakeNamespace: @@ -47,12 +48,16 @@ def test_update_from_ns_records_declaration_and_detects_scope(tmp_path): header.write_text("enum class Scoped { A };\nenum Unscoped { B };\n") scoped = CppEnumInfo("Scoped") - scoped.update_from_ns(_FakeNamespace([_FakeEnumDecl("Scoped", str(header))])) + scoped.update_from_ns( + _FakeNamespace([_FakeEnumDecl("Scoped", str(header), line=1)]) + ) assert scoped.decls[0].name == "Scoped" assert scoped.scoped is True unscoped = CppEnumInfo("Unscoped") - unscoped.update_from_ns(_FakeNamespace([_FakeEnumDecl("Unscoped", str(header))])) + unscoped.update_from_ns( + _FakeNamespace([_FakeEnumDecl("Unscoped", str(header), line=2)]) + ) assert unscoped.scoped is False diff --git a/tests/test_utils.py b/tests/test_utils.py index e8d65cbf..73b68ffc 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -446,16 +446,27 @@ def test_find_classes_in_source_skips_scoped_enums(): def test_is_scoped_enum_in_source_file(tmp_path): - """`enum class`/`enum struct` are scoped; a plain `enum` is not.""" + """`enum class`/`enum struct` are scoped; a plain `enum` is not. + + The check is keyed on the enum's declaration line, so two same-named enums + with different scopedness in the same file do not confuse each other. + """ src = tmp_path / "Enums.hpp" src.write_text( - "enum Unscoped { A, B };\n" - "enum class Scoped : unsigned { C, D };\n" - "enum struct ScopedStruct { E };\n" + "enum Unscoped { A, B };\n" # line 1 + "enum class Scoped : unsigned { C, D };\n" # line 2 + "enum struct ScopedStruct { E };\n" # line 3 + "struct AA { enum class Value { X }; };\n" # line 4 (scoped Value) + "struct BB { enum Value { Y }; };\n" # line 5 (unscoped Value) ) - assert is_scoped_enum_in_source_file(str(src), "Scoped") is True - assert is_scoped_enum_in_source_file(str(src), "ScopedStruct") is True - assert is_scoped_enum_in_source_file(str(src), "Unscoped") is False + assert is_scoped_enum_in_source_file(str(src), "Unscoped", 1) is False + assert is_scoped_enum_in_source_file(str(src), "Scoped", 2) is True + assert is_scoped_enum_in_source_file(str(src), "ScopedStruct", 3) is True + # Same-named enums are told apart by their declaration line. + assert is_scoped_enum_in_source_file(str(src), "Value", 4) is True + assert is_scoped_enum_in_source_file(str(src), "Value", 5) is False + # An out-of-range line is treated as not-scoped rather than raising. + assert is_scoped_enum_in_source_file(str(src), "Value", 99) is False def test_find_classes_in_source_by_name_and_template(): @@ -611,6 +622,8 @@ def test_unqualified_name(): assert unqualified_name("foo::bar::Baz") == "Baz" assert unqualified_name("Baz") == "Baz" assert unqualified_name("foo::Bar<2>") == "Bar<2>" # template args kept + # A `::` inside template arguments is not a namespace separator. + assert unqualified_name("foo::Bar>") == "Bar>" def test_registration_function_name(): From 53e56a663a238c4b06e68d3cba8eee3847ccd9aa Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 11:22:37 +0100 Subject: [PATCH 11/12] #119 Detect enum scopedness across split declarations and comments The previous line-based check inspected only one physical line, so a scoped enum whose declaration is split across lines (enum class\nValue) or broken by a block comment (enum /* */ class Value) was misclassified as unscoped, wrongly emitting .export_values(). pygccxml also reports the line of the enum *name*, which can sit below the enum keyword, so scanning forward from it is not enough. Match the whole comment-stripped file (comments blanked line-preservingly so a match still maps to its source line) for every `enum [class|struct] ` declaration - \s+ spans the newlines/blanked comments of a split declaration - and classify the one whose name sits nearest the reported line. This keeps the same-named-enum disambiguation while tolerating split/comment-broken declarations. Regression tests cover split, block-comment, absent-enum and unreadable-file cases; shapes wrappers regenerate byte-identically. Co-Authored-By: Claude Opus 4.8 --- cppwg/utils/utils.py | 49 +++++++++++++++++++++++++++++++------------- tests/test_utils.py | 16 +++++++++++++-- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/cppwg/utils/utils.py b/cppwg/utils/utils.py index f59f6fa1..75d329c7 100644 --- a/cppwg/utils/utils.py +++ b/cppwg/utils/utils.py @@ -412,10 +412,14 @@ def is_scoped_enum_in_source_file( config option can override it. pygccxml does not expose enum scopedness, so it is read from the source text. - The check is scoped to the enum's own declaration line (from its pygccxml - location) rather than the whole file, so two same-named enums with different - scopedness in one file (e.g. a scoped ``A::Value`` and an unscoped - ``B::Value``) do not confuse each other. + The result is disambiguated by the enum's own declaration line (from its + pygccxml location), so two same-named enums with different scopedness in one + file (e.g. a scoped ``A::Value`` and an unscoped ``B::Value``) do not confuse + each other. The whole (comment-stripped) file is matched rather than a single + line, so a declaration split across lines (``enum class`` then ``Value`` on + the next line) or broken by a block comment (``enum /* */ class Value``) is + still classified correctly - pygccxml reports the line of the enum *name*, + which may sit below the ``enum`` keyword. Parameters ---------- @@ -424,7 +428,7 @@ def is_scoped_enum_in_source_file( enum_name : str The enum name to check. line_number : int - The 1-based line the enum is declared on (decl.location.line). + The 1-based line pygccxml reports for the enum (decl.location.line). Returns ------- @@ -433,19 +437,36 @@ def is_scoped_enum_in_source_file( """ try: with open(source_file_path) as source_file: - lines = source_file.readlines() + source = source_file.read() except OSError: return False - if not 1 <= line_number <= len(lines): - return False - - # The declaration line, with any trailing line comment removed so that a - # comment such as `enum Foo {}; // an enum class` cannot flip the result. - line = lines[line_number - 1].split("//", 1)[0] + # Strip comments while preserving line numbers: drop `//` comments (keeping + # their newline) and blank `/* */` comments to spaces but keep their newlines, + # so a match's position still maps to its original source line. + source = re.sub(r"//[^\n]*", "", source) + source = re.sub( + r"/\*.*?\*/", + lambda m: re.sub(r"[^\n]", " ", m.group(0)), + source, + flags=re.DOTALL, + ) - pattern = r"\benum\s+(?:class|struct)\s+" + re.escape(enum_name) + r"\b" - return re.search(pattern, line) is not None + # Find every `enum [class|struct] ` declaration (\s+ spans the newlines + # and blanked comments of a split declaration) and classify the one whose + # name sits closest to the reported declaration line. + pattern = re.compile( + r"\benum\s+(class\s+|struct\s+)?" + re.escape(enum_name) + r"\b" + ) + best_match = None + best_distance = None + for match in pattern.finditer(source): + name_line = source.count("\n", 0, match.end()) + 1 + distance = abs(name_line - line_number) + if best_distance is None or distance < best_distance: + best_match, best_distance = match, distance + + return best_match is not None and best_match.group(1) is not None def registration_function_name(class_py_name: str) -> str: diff --git a/tests/test_utils.py b/tests/test_utils.py index 73b68ffc..40759a09 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -458,6 +458,9 @@ def test_is_scoped_enum_in_source_file(tmp_path): "enum struct ScopedStruct { E };\n" # line 3 "struct AA { enum class Value { X }; };\n" # line 4 (scoped Value) "struct BB { enum Value { Y }; };\n" # line 5 (unscoped Value) + "enum class\n" # line 6 (keyword) - split declaration + "Split { Z };\n" # line 7 (name) - pygccxml reports this line + "enum /* an enum class */ Blocky { W };\n" # line 8 - block comment ) assert is_scoped_enum_in_source_file(str(src), "Unscoped", 1) is False assert is_scoped_enum_in_source_file(str(src), "Scoped", 2) is True @@ -465,8 +468,17 @@ def test_is_scoped_enum_in_source_file(tmp_path): # Same-named enums are told apart by their declaration line. assert is_scoped_enum_in_source_file(str(src), "Value", 4) is True assert is_scoped_enum_in_source_file(str(src), "Value", 5) is False - # An out-of-range line is treated as not-scoped rather than raising. - assert is_scoped_enum_in_source_file(str(src), "Value", 99) is False + # A declaration split across lines: pygccxml reports the name's line (7), + # while the `enum class` keyword is on line 6. + assert is_scoped_enum_in_source_file(str(src), "Split", 7) is True + # A block comment between tokens must not make a plain enum look scoped. + assert is_scoped_enum_in_source_file(str(src), "Blocky", 8) is False + # An out-of-range line still resolves the (only) same-named declaration. + assert is_scoped_enum_in_source_file(str(src), "Scoped", 99) is True + # An enum whose declaration is not found in the file is treated as unscoped. + assert is_scoped_enum_in_source_file(str(src), "Absent", 1) is False + # An unreadable/absent source file is treated as unscoped, not an error. + assert is_scoped_enum_in_source_file(str(tmp_path / "nope.hpp"), "Scoped", 1) is False def test_find_classes_in_source_by_name_and_template(): From 490352076fd34372e2bcfd6c1b5c13d453675c9d Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Tue, 11 Aug 2026 11:22:37 +0100 Subject: [PATCH 12/12] #119 Cover remaining branches to restore the 99% coverage target Add unit tests for #119 code paths that no coverage flag (unit, shapes, cells) exercised, which had pushed combined coverage below 99%: - exclusions.constructor_is_excluded: an abstract class with only non-abstract bases keeps its constructor, and a constructor_signature_excludes entry of the same arity but different types does not exclude. - base_writer.render_default_args: keyword/value rendering, class template parameter substitution (including a default that references no parameter), and the empty-initializer-list typing. These bring cppwg/info/exclusions.py and cppwg/writers/base_writer.py to 100%. Co-Authored-By: Claude Opus 4.8 --- tests/test_base_writer.py | 50 ++++++++++++++++++++++++++++++++ tests/test_constructor_writer.py | 20 +++++++++++++ 2 files changed, 70 insertions(+) diff --git a/tests/test_base_writer.py b/tests/test_base_writer.py index 93623777..e492590d 100644 --- a/tests/test_base_writer.py +++ b/tests/test_base_writer.py @@ -1,8 +1,14 @@ """Unit tests for cppwg.writers.base_writer.""" +from types import SimpleNamespace + from cppwg.writers.base_writer import CppBaseWrapperWriter +def _arg(name, default_value=None, decl_type="int"): + return SimpleNamespace(name=name, default_value=default_value, decl_type=decl_type) + + def test_tidy_name_replaces_cpp_syntax(): """A full C++ declaration is rewritten into a typedef-safe token.""" writer = CppBaseWrapperWriter({}) @@ -16,3 +22,47 @@ def test_tidy_name_handles_pointers_refs_and_negatives(): assert writer.tidy_name("Foo *") == "FooPtr" assert writer.tidy_name("Foo &") == "FooRef" assert writer.tidy_name("Foo<-1>") == "Foo_lt_neg1_gt_" + + +def test_render_default_args_keyword_and_value(): + """A default value is normalised and appended; exclude keeps only the name.""" + writer = CppBaseWrapperWriter({}) + args = [_arg("count", default_value="(-1)", decl_type="int")] + assert writer.render_default_args(args, exclude_default_args=False) == ( + ', py::arg("count") = -1' + ) + assert writer.render_default_args(args, exclude_default_args=True) == ( + ', py::arg("count")' + ) + # No default value -> a bare py::arg. + assert writer.render_default_args([_arg("x")], exclude_default_args=False) == ( + ', py::arg("x")' + ) + + +def test_render_default_args_substitutes_template_params(): + """A default referencing a class template param is substituted; others are not.""" + writer = CppBaseWrapperWriter({}) + args = [ + _arg("dim", default_value="DIM"), # references the param -> substituted + _arg("flag", default_value="true"), # no param -> left as-is (else branch) + ] + result = writer.render_default_args( + args, + exclude_default_args=False, + template_params=["DIM"], + template_args=["2"], + class_name="Foo", + ) + assert result == ', py::arg("dim") = 2, py::arg("flag") = true' + + +def test_render_default_args_types_empty_initializer_list(): + """An empty {} default is given its type only when requested (constructors).""" + from pygccxml import declarations + + writer = CppBaseWrapperWriter({}) + args = [_arg("v", default_value="{}", decl_type=declarations.int_t())] + assert writer.render_default_args( + args, exclude_default_args=False, substitute_empty_init_list=True + ) == ', py::arg("v") = int {}' diff --git a/tests/test_constructor_writer.py b/tests/test_constructor_writer.py index e170c12b..6a7315cf 100644 --- a/tests/test_constructor_writer.py +++ b/tests/test_constructor_writer.py @@ -69,6 +69,12 @@ def test_signature_exclude_matches_valid_signature(monkeypatch): assert writer.exclude() is True +def test_signature_exclude_same_arity_different_types_not_excluded(monkeypatch): + """A signature of the same arity but different types does not exclude.""" + writer = _writer(["int", "int"], [["double", "double"]], monkeypatch) + assert writer.exclude() is False + + def test_signature_exclude_skips_scalar_int(monkeypatch): """A mis-typed scalar (constructor_signature_excludes: 5) is skipped, not len()'d.""" writer = _writer(["int", "int", "int"], 5, monkeypatch) @@ -187,6 +193,20 @@ def test_exclude_abstract_with_abstract_base(monkeypatch): assert _exclude_writer(monkeypatch, class_decl, ctor).exclude() is True +def test_abstract_with_non_abstract_base_not_excluded(monkeypatch): + """An abstract class whose bases are all non-abstract keeps its constructor. + + Only an abstract class inheriting from an abstract base drops its + constructors; this exercises the fall-through when no base is abstract. + """ + concrete_base = _RichClassDecl(name="Base", is_abstract=False) + class_decl = _RichClassDecl( + is_abstract=True, recursive_bases=[SimpleNamespace(related_class=concrete_base)] + ) + ctor = _RichCtor(parent=class_decl) + assert _exclude_writer(monkeypatch, class_decl, ctor).exclude() is False + + def test_exclude_subclass_constructor(monkeypatch): class_decl = _RichClassDecl() other_parent = _RichClassDecl(name="Inner")