diff --git a/README.md b/README.md index 0109cc9..fcbf4ce 100644 --- a/README.md +++ b/README.md @@ -346,5 +346,16 @@ r = Rectangle(4, 5) See the [pybind11 docs on partitioning code over multiple extension modules](https://pybind11.readthedocs.io/en/stable/advanced/misc.html#partitioning-code-over-multiple-extension-modules). +- To shrink wrappers, set `exclude_inherited_overrides: True` (package level). + cppwg then does not emit a binding for a method that merely overrides a virtual + method already wrapped on a wrapped base class: pybind11 inheritance plus C++ virtual + dispatch already expose it through the base binding, so the derived `.def` is + pure duplication. The virtual **trampoline** is still generated, so Python + subclasses can still override the method. A method is skipped only when a + wrapped, non-class-excluded base declares a virtual of the same name, argument + types and const-ness (the return type is not compared, so a covariant-return + override still matches); if the base lists the method under its + `excluded_methods` the override is kept, since the base does not wrap it. Off by + default. - See the [pybind11 documentation](https://pybind11.readthedocs.io/) for help on pybind11 wrapper code. diff --git a/cppwg/info/package_info.py b/cppwg/info/package_info.py index 2b828e1..88a559a 100644 --- a/cppwg/info/package_info.py +++ b/cppwg/info/package_info.py @@ -109,6 +109,12 @@ class PackageInfo(BaseInfo): exception translator is generated automatically for each. exclude_default_args : bool Exclude default arguments from method wrappers. + exclude_inherited_overrides : bool + Skip emitting a binding for a method that overrides a virtual method already + wrapped on a wrapped base class (pybind11 inheritance + virtual dispatch + already expose it, so the derived binding is redundant). The virtual + trampoline is still generated, so Python subclasses can override the + method. Off by default. name : str The name of the package source_cpp_patterns : list[str] @@ -151,6 +157,7 @@ def __init__(self, name: str, package_config: dict[str, Any] | None = None) -> N self.common_include_file: bool = False self.exceptions: list[str | dict[str, str]] = [] self.exclude_default_args: bool = False + self.exclude_inherited_overrides: bool = False self.source_cpp_patterns: list[str] = ["*.cpp"] self.source_hpp_patterns: list[str] = ["*.hpp"] self.typecasters: list[dict[str, Any]] = [] @@ -176,6 +183,9 @@ def __init__(self, name: str, package_config: dict[str, Any] | None = None) -> N self.exclude_default_args = package_config.get( "exclude_default_args", self.exclude_default_args ) + self.exclude_inherited_overrides = package_config.get( + "exclude_inherited_overrides", self.exclude_inherited_overrides + ) self.source_cpp_patterns = package_config.get( "source_cpp_patterns", self.source_cpp_patterns ) diff --git a/cppwg/parsers/package_info_parser.py b/cppwg/parsers/package_info_parser.py index 94f03c9..cde784c 100644 --- a/cppwg/parsers/package_info_parser.py +++ b/cppwg/parsers/package_info_parser.py @@ -86,6 +86,7 @@ def parse(self) -> PackageInfo: "common_include_file": True, "exceptions": [], "exclude_default_args": False, + "exclude_inherited_overrides": False, "source_cpp_patterns": ["*.cpp"], "source_hpp_patterns": ["*.hpp"], "typecasters": [], @@ -103,6 +104,9 @@ def parse(self) -> PackageInfo: package_config["exclude_default_args"] = utils.convert_to_bool( package_config["exclude_default_args"] ) + package_config["exclude_inherited_overrides"] = utils.convert_to_bool( + package_config["exclude_inherited_overrides"] + ) # Convert custom generator path to a full path self.convert_custom_generator(package_config) diff --git a/cppwg/writers/class_writer.py b/cppwg/writers/class_writer.py index f5202ed..91f521b 100644 --- a/cppwg/writers/class_writer.py +++ b/cppwg/writers/class_writer.py @@ -14,6 +14,7 @@ ) from cppwg.utils.utils import ( call_generator_hook, + canonicalize_type_whitespace, ensure_trailing_newline, type_string_matches, write_file_if_changed, @@ -46,6 +47,9 @@ class CppClassWrapperWriter(CppBaseWrapperWriter): package_classes : set[pygccxml.declarations.class_t] Decls for every class wrapped anywhere in the package (all modules). Used to detect base classes wrapped in another module of this package. + package_class_infos : dict[pygccxml.declarations.class_t, CppClassInfo] + Maps each wrapped decl to its class_info. Used by the inherited-override + skip to consult a base class's excluded_methods. overwrite : bool Force rewrite of the class wrapper files, even if unchanged hpp_string : str @@ -61,6 +65,7 @@ def __init__( module_classes: dict["class_t", str], package_classes: set["class_t"] = None, overwrite: bool = False, + package_class_infos: dict["class_t", "CppClassInfo"] = None, ) -> None: logger = logging.getLogger() @@ -74,6 +79,9 @@ def __init__( self.module_classes = module_classes self.package_classes = package_classes if package_classes is not None else set() + self.package_class_infos = ( + package_class_infos if package_class_infos is not None else {} + ) self.overwrite = overwrite @@ -430,6 +438,162 @@ def build_cpp_header( return_typedefs=return_typedefs, ) + def _overrides_wrapped_base_virtual( + self, class_decl: "class_t", method_decl: "member_function_t" + ) -> bool: + """ + Return True if a method overrides a virtual wrapped on a wrapped base. + + The method qualifies if it is virtual or pure virtual and some base class, + wrapped in this package and not class-excluded, declares its own member + function of the same name, argument types and const-ness that is itself + virtual (the return type is intentionally not compared, so a + covariant-return override still matches), and that base does not itself + exclude the method from wrapping - by name, return type or arg type, the + same rules as CppMethodWrapperWriter.method_is_excluded (otherwise the base + emits no binding, so this override is the only one). The base must also be + reachable from + the derived py::class_ via an emitted pybind11 base link: a same-module + base is always linked, but a base wrapped in another module of the package + is only linked when cross-module inheritance is enabled (`imports` set) - + see bases_block. Without that link the base's binding is not inherited, so + the override would become unreachable if skipped. This is the per-method + test used by _is_inherited_override; it does not consider sibling + overloads. + + Parameters + ---------- + class_decl : pygccxml.declarations.class_t + The class declaration owning the method. + method_decl : pygccxml.declarations.member_function_t + The candidate member function. + + Returns + ------- + bool + True if the method overrides a wrapped base virtual. + """ + if method_decl.virtuality not in ("virtual", "pure virtual"): + return False + + # Cross-module inheritance is only linked into the derived py::class_ when + # the module opts in via `imports` (see bases_block). Without it, a base + # wrapped in another module contributes no inherited binding, so an + # override of it here is the sole binding and must not be skipped. + allow_external_bases = bool(self.class_info.hierarchy_attribute("imports")) + + name = method_decl.name + arg_types = [ + canonicalize_type_whitespace(t.decl_string) + for t in method_decl.argument_types + ] + + for hierarchy_info in class_decl.recursive_bases: + base_decl = hierarchy_info.related_class + # Skip bases pygccxml could not resolve, and bases not wrapped in this + # package (an unwrapped base provides no binding to inherit). + if base_decl is None or base_decl not in self.package_classes: + continue + # A base wrapped in another module only provides an inherited binding + # when its pybind base link is emitted (imports enabled). A same-module + # base (in module_classes) is always linked. + if base_decl not in self.module_classes and not allow_external_bases: + continue + + for base_method in base_decl.member_functions(name, allow_empty=True): + # Only public methods are bound (build_class_register filters on + # public access), so a protected/private base virtual is not + # wrapped on the base and cannot make this override redundant. + if base_method.access_type != "public": + continue + if base_method.virtuality not in ("virtual", "pure virtual"): + continue + if base_method.has_const != method_decl.has_const: + continue + base_arg_types = [ + canonicalize_type_whitespace(t.decl_string) + for t in base_method.argument_types + ] + if base_arg_types != arg_types: + continue + # The base declares a matching virtual. Keep the override only if + # the base does not actually wrap it: a base method excluded from + # wrapping (by name, return type or arg type - the same rules as + # CppMethodWrapperWriter) emits no binding, so this override is the + # sole binding and must not be skipped. + base_info = self.package_class_infos.get(base_decl) + if base_info is not None and CppMethodWrapperWriter.method_is_excluded( + base_info, base_decl, base_method + ): + continue + return True + + return False + + def _is_inherited_override( + self, class_decl: "class_t", method_decl: "member_function_t" + ) -> bool: + """ + Return True if a method's binding is a redundant inherited override. + + With ``exclude_inherited_overrides`` set, a method that overrides a virtual + method already wrapped on a wrapped base class need not be bound again: pybind11 + inheritance exposes the base binding and virtual dispatch routes the call + to this override. Only the ``.def`` is skipped - the virtual trampoline + (see virtual_overrides) is unaffected, so Python subclasses can still + override the method. + + The method must itself override a wrapped base virtual + (_overrides_wrapped_base_virtual) AND *every other public overload of the + same name that will actually be wrapped* must too. This overload guard + matters because pybind11 resolves overloads by name: a derived binding of + a name shadows the inherited base binding for that name. So if the class + still binds some other overload of this name, that surviving binding would + hide the base's binding of this one - making it uncallable from Python. + Skipping is safe only when the class binds none of that name and thus + inherits the base's full overload set. A sibling overload that is itself + excluded from wrapping (excluded_methods / return_type_excludes / + arg_type_excludes, i.e. CppMethodWrapperWriter.method_is_excluded) emits + no binding, so it cannot shadow and does not block skipping. + + Parameters + ---------- + class_decl : pygccxml.declarations.class_t + The class declaration owning the method. + method_decl : pygccxml.declarations.member_function_t + The candidate member function. + + Returns + ------- + bool + True if the binding should be skipped as a redundant override. + """ + if not self.class_info.hierarchy_attribute("exclude_inherited_overrides"): + return False + + if not self._overrides_wrapped_base_virtual(class_decl, method_decl): + return False + + # Overload-shadowing guard: skip only if no other public overload of the + # same name survives as a binding (i.e. every same-name overload is also a + # skippable override). Otherwise the surviving overload shadows the base. + query = access_type_matcher_t("public") + for sibling in class_decl.member_functions( + method_decl.name, function=query, allow_empty=True + ): + if sibling is method_decl: + continue + # A sibling that will not be wrapped (excluded by name/type) emits no + # binding, so it cannot shadow the base and does not force keeping. + if CppMethodWrapperWriter.method_is_excluded( + self.class_info, class_decl, sibling + ): + continue + if not self._overrides_wrapped_base_virtual(class_decl, sibling): + return False + + return True + def build_class_register(self, template_idx: int) -> tuple[str, str]: """ Build the registration block for one template instantiation. @@ -482,7 +646,10 @@ def build_class_register(self, template_idx: int) -> tuple[str, str]: for constructor in class_decl.constructors(function=query, allow_empty=True) ) - # Add public member functions + # Add public member functions. A method that redundantly overrides a + # virtual already wrapped on a wrapped base class is skipped when + # exclude_inherited_overrides is set (its trampoline, built separately in + # virtual_overrides, is kept). methods = "".join( CppMethodWrapperWriter( self.class_info, @@ -493,6 +660,7 @@ def build_class_register(self, template_idx: int) -> tuple[str, str]: for member_function in class_decl.member_functions( function=query, allow_empty=True ) + if not self._is_inherited_override(class_decl, member_function) ) block = self.wrapper_templates["class_cpp_register"].substitute( diff --git a/cppwg/writers/method_writer.py b/cppwg/writers/method_writer.py index 8532df1..7311913 100644 --- a/cppwg/writers/method_writer.py +++ b/cppwg/writers/method_writer.py @@ -73,33 +73,66 @@ def exclude(self) -> bool: bool True if the method should be excluded, False otherwise """ + return self.method_is_excluded( + self.class_info, self.class_decl, self.method_decl + ) + + @staticmethod + 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. + + The per-instance exclude() delegates here so the same decision can be + reused without a writer. The inherited-override overload-shadowing guard + (see class_writer._is_inherited_override) needs it: a sibling overload + that is excluded emits no binding, so it cannot shadow an inherited base + overload set and must not block skipping a redundant override. + + 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 self.class_info.excluded_methods: - if self.method_decl.name in self.class_info.excluded_methods: + if class_info.excluded_methods: + if method_decl.name in class_info.excluded_methods: return True # Exclude private methods - if self.method_decl.access_type == "private": + if method_decl.access_type == "private": return True # Exclude sub class (e.g. iterator) methods such as: # class Foo { # public: # class FooIterator { - if self.method_decl.parent != self.class_decl: + 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 = self.class_info.hierarchy_attribute_gather_flat( + calldef_excludes = class_info.hierarchy_attribute_gather_flat( "calldef_excludes" ) return_type_excludes = ( - self.class_info.hierarchy_attribute_gather_flat("return_type_excludes") + class_info.hierarchy_attribute_gather_flat("return_type_excludes") + calldef_excludes ) - return_type = self.method_decl.return_type.decl_string + return_type = method_decl.return_type.decl_string if any( utils.type_string_matches(return_type, pattern) for pattern in return_type_excludes @@ -109,10 +142,10 @@ def exclude(self) -> bool: # Exclude by argument type. arg_type_excludes targets argument types on # methods and constructors; the deprecated calldef_excludes applies too. arg_type_excludes = ( - self.class_info.hierarchy_attribute_gather_flat("arg_type_excludes") + class_info.hierarchy_attribute_gather_flat("arg_type_excludes") + calldef_excludes ) - for argument_type in self.method_decl.argument_types: + for argument_type in method_decl.argument_types: arg_type = argument_type.decl_string if any( utils.type_string_matches(arg_type, pattern) diff --git a/cppwg/writers/module_writer.py b/cppwg/writers/module_writer.py index 93d4569..fbf53b2 100644 --- a/cppwg/writers/module_writer.py +++ b/cppwg/writers/module_writer.py @@ -18,6 +18,7 @@ from pygccxml.declarations.class_declaration import class_t + from cppwg.info.class_info import CppClassInfo from cppwg.info.module_info import ModuleInfo @@ -73,12 +74,19 @@ def __init__( # all of its modules). Used to detect base classes that are wrapped in a # different module of the same package, which are therefore known to be # registered and safe to reference as external bases. + # + # package_class_infos maps each such decl back to its class_info, so the + # inherited-override skip can consult a base class's excluded_methods (a + # base method excluded there is not wrapped, so its override must be kept). self.package_classes: set["class_t"] = set() + self.package_class_infos: dict["class_t", "CppClassInfo"] = {} for module_info in self.module_info.package_info.module_collection: for class_info in module_info.class_collection: if class_info.excluded: continue self.package_classes.update(class_info.decls) + for decl in class_info.decls: + self.package_class_infos[decl] = class_info def generate_exception_translator(self) -> str: """ @@ -307,6 +315,7 @@ def write_class_wrappers(self) -> None: self.classes, self.package_classes, self.overwrite, + self.package_class_infos, ) # Write the class wrappers into /path/to/wrapper_root/modulename/ diff --git a/tests/test_class_writer.py b/tests/test_class_writer.py index 7a74767..be512c1 100644 --- a/tests/test_class_writer.py +++ b/tests/test_class_writer.py @@ -58,6 +58,8 @@ def __init__( self.custom_generator_instance = generator self._name_base = name_base or name self._attrs = attrs + # Consulted by CppMethodWrapperWriter.method_is_excluded. + self.excluded_methods = [] def py_name_base(self): return self._name_base @@ -707,3 +709,336 @@ def test_includes_block_emits_angle_bracket_typecaster(): '#include "wrapper_header_collection.cppwg.hpp"\n' "#include \n" ) + + +# --------------------------------------------------------------------------- +# exclude_inherited_overrides: _is_inherited_override predicate +# --------------------------------------------------------------------------- + + +class _FakeArgType: + """Stand-in for a pygccxml argument type.""" + + def __init__(self, decl_string): + self.decl_string = decl_string + + +class _FakeMethodDecl: + """Stand-in for a pygccxml member_function_t.""" + + def __init__( + self, + name, + virtuality="virtual", + arg_types=(), + has_const=False, + access_type="public", + return_type="void", + ): + self.name = name + self.virtuality = virtuality + self.argument_types = [_FakeArgType(t) for t in arg_types] + self.has_const = has_const + self.access_type = access_type + # Needed by CppMethodWrapperWriter.method_is_excluded (the overload- + # shadowing guard). parent is set by the owning _FakeDerivedDecl. + self.return_type = _FakeArgType(return_type) + self.parent = None + + +class _FakeBaseDecl: + """Stand-in for a base class_t answering member_functions(name).""" + + def __init__(self, methods=()): + self._methods = list(methods) + # Own the methods, so method_is_excluded's parent check (used when the + # base's own exclusion is consulted) treats them as this base's members. + for method in self._methods: + method.parent = self + + def member_functions(self, name=None, allow_empty=False): + return [m for m in self._methods if name is None or m.name == name] + + +class _FakeHierarchyInfo: + """Stand-in for a pygccxml hierarchy_info_t.""" + + def __init__(self, related_class): + self.related_class = related_class + + +class _FakeDerivedDecl: + """Stand-in for the derived class_t exposing recursive_bases and own methods.""" + + def __init__(self, bases=(), methods=()): + self.recursive_bases = [_FakeHierarchyInfo(b) for b in bases] + self._methods = list(methods) + # Own the methods, so method_is_excluded's parent check treats them as + # this class's members (not sub-class/iterator methods). + for method in self._methods: + method.parent = self + + def member_functions(self, name=None, function=None, allow_empty=False): + result = [m for m in self._methods if name is None or m.name == name] + # Honour the `function` predicate as pygccxml does. Production passes + # access_type_matcher_t("public"); that matcher needs a real class_t + # parent to call, so instead read its target access_type and filter on + # each method's own access_type. Non-public overloads are then excluded + # here, exactly as pygccxml would, rather than leaking into the caller's + # overload-shadowing scan. + if function is not None: + target = getattr(function, "access_type", None) + if target is not None: + result = [m for m in result if m.access_type == target] + return result + + +class _FakeBaseInfo: + """Minimal class_info carrying the base's own wrapping-exclusion config.""" + + def __init__(self, excluded_methods=(), excludes=None): + self.excluded_methods = list(excluded_methods) + # return_type_excludes / arg_type_excludes / calldef_excludes, keyed by + # name, as CppMethodWrapperWriter.method_is_excluded gathers them. + self._excludes = excludes or {} + + def hierarchy_attribute_gather_flat(self, name): + return list(self._excludes.get(name, [])) + + +def _override_writer(enabled, base_decl, base_info=None, attrs=None, same_module=True): + """A class writer with the option set and a base wired into the package. + + By default the base is in the same module as the derived class (the common + case, where the pybind base link is always emitted). Pass same_module=False + to model a base wrapped in another module of the package; then whether the + override is skippable depends on cross-module inheritance being enabled via + an `imports` entry (see attrs). + """ + class_attrs = {"exclude_inherited_overrides": enabled} + if attrs: + class_attrs.update(attrs) + class_info = _FakeClassInfo( + "Derived", + object(), + class_attrs, + "Derived.hpp", + ) + module_classes = {base_decl: "Base"} if same_module else {} + writer = CppClassWrapperWriter( + class_info, template_collection, module_classes=module_classes + ) + writer.package_classes = {base_decl} + if base_info is not None: + writer.package_class_infos = {base_decl: base_info} + return writer + + +def test_inherited_override_skipped_when_base_wraps_matching_virtual(): + """A virtual override of a wrapped base virtual is flagged for skipping.""" + base = _FakeBaseDecl([_FakeMethodDecl("GetNumNodes")]) + writer = _override_writer(True, base) + class_decl = _FakeDerivedDecl(bases=[base]) + method = _FakeMethodDecl("GetNumNodes") + assert writer._is_inherited_override(class_decl, method) is True + + +def test_inherited_override_kept_when_option_off(): + """With the option off the method is never skipped.""" + base = _FakeBaseDecl([_FakeMethodDecl("GetNumNodes")]) + writer = _override_writer(False, base) + class_decl = _FakeDerivedDecl(bases=[base]) + method = _FakeMethodDecl("GetNumNodes") + assert writer._is_inherited_override(class_decl, method) is False + + +def test_inherited_override_kept_when_base_excludes_method(): + """If the base excludes the method it is unwrapped there, so keep the override.""" + base = _FakeBaseDecl([_FakeMethodDecl("GetNumNodes")]) + base_info = _FakeBaseInfo(excluded_methods=["GetNumNodes"]) + writer = _override_writer(True, base, base_info) + class_decl = _FakeDerivedDecl(bases=[base]) + method = _FakeMethodDecl("GetNumNodes") + assert writer._is_inherited_override(class_decl, method) is False + + +def test_inherited_override_kept_when_base_virtual_excluded_by_return_type(): + """A base virtual excluded by return type isn't wrapped, so keep the override. + + The base declares a matching virtual, but the base's own wrapper drops it via + return_type_excludes (CppMethodWrapperWriter.method_is_excluded), so the base + emits no binding. Skipping the derived override would remove the method's only + Python-visible binding, so it must be kept. + """ + base = _FakeBaseDecl([_FakeMethodDecl("GetPtr", return_type="RawPtr *")]) + base_info = _FakeBaseInfo(excludes={"return_type_excludes": ["RawPtr"]}) + writer = _override_writer(True, base, base_info) + class_decl = _FakeDerivedDecl(bases=[base]) + # Covariant override (return type not compared), same name/args/const-ness. + method = _FakeMethodDecl("GetPtr", return_type="DerivedPtr *") + assert writer._is_inherited_override(class_decl, method) is False + + +def test_inherited_override_kept_for_non_virtual_method(): + """A non-virtual same-name method is not an override and is not skipped.""" + base = _FakeBaseDecl([_FakeMethodDecl("GetNumNodes")]) + writer = _override_writer(True, base) + class_decl = _FakeDerivedDecl(bases=[base]) + method = _FakeMethodDecl("GetNumNodes", virtuality="not virtual") + assert writer._is_inherited_override(class_decl, method) is False + + +def test_inherited_override_kept_when_base_not_wrapped(): + """An unwrapped base provides no binding to inherit, so keep the override.""" + base = _FakeBaseDecl([_FakeMethodDecl("GetNumNodes")]) + writer = _override_writer(True, base) + writer.package_classes = set() # base not wrapped anywhere + class_decl = _FakeDerivedDecl(bases=[base]) + method = _FakeMethodDecl("GetNumNodes") + assert writer._is_inherited_override(class_decl, method) is False + + +def test_inherited_override_distinguishes_overloads_by_args(): + """A same-name base virtual with different args is a different overload.""" + base = _FakeBaseDecl([_FakeMethodDecl("GetNode", arg_types=["unsigned int"])]) + writer = _override_writer(True, base) + class_decl = _FakeDerivedDecl(bases=[base]) + # Derived declares an overload with an extra argument. + method = _FakeMethodDecl("GetNode", arg_types=["unsigned int", "double"]) + assert writer._is_inherited_override(class_decl, method) is False + + +def test_inherited_override_matches_regardless_of_return_type(): + """Return type is not compared, so a covariant-return override still matches.""" + base = _FakeBaseDecl( + [_FakeMethodDecl("GetMesh", arg_types=[], has_const=True)] + ) + writer = _override_writer(True, base) + class_decl = _FakeDerivedDecl(bases=[base]) + method = _FakeMethodDecl("GetMesh", arg_types=[], has_const=True) + assert writer._is_inherited_override(class_decl, method) is True + + +def test_inherited_override_kept_when_sibling_overload_survives(): + """A redundant override is kept if another same-name overload would bind. + + pybind11 resolves overloads by name, so a derived binding of a name shadows + the inherited base binding. If the class still binds a sibling overload, the + override must be kept - otherwise that override would become unreachable. + """ + base = _FakeBaseDecl( + [_FakeMethodDecl("GetLineTensionParameter", arg_types=["int", "int"])] + ) + writer = _override_writer(True, base) + override = _FakeMethodDecl("GetLineTensionParameter", arg_types=["int", "int"]) + # A 0-arg non-virtual overload that is NOT a redundant override, so it would + # still be bound and shadow the base. + sibling = _FakeMethodDecl( + "GetLineTensionParameter", virtuality="not virtual", arg_types=[] + ) + class_decl = _FakeDerivedDecl(bases=[base], methods=[override, sibling]) + assert writer._is_inherited_override(class_decl, override) is False + + +def test_inherited_override_skipped_when_sibling_overload_is_excluded(): + """An excluded same-name sibling emits no binding, so the override is skipped. + + The sibling overload is excluded from wrapping (here via arg_type_excludes), + so it produces no `.def` and cannot shadow the inherited base overloads. + Without filtering it out, the guard would treat it as a surviving binding and + wrongly keep the redundant override - which would itself shadow the base. + """ + base = _FakeBaseDecl([_FakeMethodDecl("foo", arg_types=["int"])]) + writer = _override_writer(True, base, attrs={"arg_type_excludes": ["BadType"]}) + override = _FakeMethodDecl("foo", arg_types=["int"]) + # A non-override overload that will be excluded from wrapping by its arg type. + excluded_sibling = _FakeMethodDecl( + "foo", virtuality="not virtual", arg_types=["BadType"] + ) + class_decl = _FakeDerivedDecl(bases=[base], methods=[override, excluded_sibling]) + assert writer._is_inherited_override(class_decl, override) is True + + +def test_inherited_override_skipped_when_all_overloads_are_overrides(): + """If every same-name overload is a redundant override, all are skipped.""" + base = _FakeBaseDecl( + [ + _FakeMethodDecl("foo", arg_types=[]), + _FakeMethodDecl("foo", arg_types=["int"]), + ] + ) + writer = _override_writer(True, base) + foo0 = _FakeMethodDecl("foo", arg_types=[]) + foo1 = _FakeMethodDecl("foo", arg_types=["int"]) + class_decl = _FakeDerivedDecl(bases=[base], methods=[foo0, foo1]) + assert writer._is_inherited_override(class_decl, foo0) is True + assert writer._is_inherited_override(class_decl, foo1) is True + + +def test_inherited_override_skipped_despite_non_public_sibling_overload(): + """A non-public same-name overload does not block skipping the override. + + Only public methods are bound, so a protected/private overload of the same + name cannot shadow the inherited base binding. The overload-shadowing guard + scans public overloads only (access_type_matcher_t("public")), so such a + sibling is filtered out and the redundant public override is still skipped. + This case only passes when the fake honours that predicate. + """ + base = _FakeBaseDecl([_FakeMethodDecl("foo", arg_types=[])]) + writer = _override_writer(True, base) + override = _FakeMethodDecl("foo", arg_types=[]) + # A protected overload that is NOT a redundant override; if the public filter + # were ignored it would be seen as a surviving binding and keep the override. + protected_sibling = _FakeMethodDecl( + "foo", virtuality="not virtual", arg_types=["int"], access_type="protected" + ) + class_decl = _FakeDerivedDecl(bases=[base], methods=[override, protected_sibling]) + assert writer._is_inherited_override(class_decl, override) is True + + +def test_inherited_override_kept_when_base_in_other_module_without_imports(): + """A cross-module base without `imports` provides no inherited binding. + + bases_block only links a base wrapped in another module into the derived + py::class_ when cross-module inheritance is enabled (`imports` set). Without + that link the base binding is not inherited, so the override is the sole + binding and must be kept. + """ + base = _FakeBaseDecl([_FakeMethodDecl("GetNumNodes")]) + # Base wrapped elsewhere in the package (package_classes) but NOT in this + # module, and imports is unset. + writer = _override_writer(True, base, same_module=False) + class_decl = _FakeDerivedDecl(bases=[base]) + method = _FakeMethodDecl("GetNumNodes") + assert writer._is_inherited_override(class_decl, method) is False + + +def test_inherited_override_skipped_when_base_in_other_module_with_imports(): + """A cross-module base with `imports` set is linked, so the override is skipped. + + With cross-module inheritance enabled the base link is emitted and its + binding is inherited, making the derived override redundant. + """ + base = _FakeBaseDecl([_FakeMethodDecl("GetNumNodes")]) + writer = _override_writer( + True, base, attrs={"imports": ["othermod"]}, same_module=False + ) + class_decl = _FakeDerivedDecl(bases=[base]) + method = _FakeMethodDecl("GetNumNodes") + assert writer._is_inherited_override(class_decl, method) is True + + +def test_inherited_override_kept_when_base_virtual_not_public(): + """A protected/private base virtual is not wrapped, so keep the override. + + Only public methods get a binding, so a same-signature virtual that is + protected on the base provides no inherited binding - dropping the override + would make it unreachable. + """ + base = _FakeBaseDecl( + [_FakeMethodDecl("GetValue", access_type="protected")] + ) + writer = _override_writer(True, base) + class_decl = _FakeDerivedDecl(bases=[base]) + method = _FakeMethodDecl("GetValue") # public override + assert writer._is_inherited_override(class_decl, method) is False diff --git a/tests/test_package_info_parser.py b/tests/test_package_info_parser.py index 2308b93..1f39358 100644 --- a/tests/test_package_info_parser.py +++ b/tests/test_package_info_parser.py @@ -254,3 +254,36 @@ def test_no_deprecation_warning_for_current_options(tmp_path, caplog): PackageInfoParser(config_path, str(tmp_path)).parse() assert not any("deprecated" in message.lower() for message in caplog.messages) + + +def test_parses_exclude_inherited_overrides(tmp_path): + """A package-level `exclude_inherited_overrides: True` is parsed as a bool.""" + config_path = _write_config( + tmp_path, + """ + name: testpkg + exclude_inherited_overrides: True + modules: + - name: mymod + """, + ) + + package_info = PackageInfoParser(config_path, str(tmp_path)).parse() + + assert package_info.exclude_inherited_overrides is True + + +def test_exclude_inherited_overrides_defaults_to_false(tmp_path): + """`exclude_inherited_overrides` defaults to False when not set.""" + config_path = _write_config( + tmp_path, + """ + name: testpkg + modules: + - name: mymod + """, + ) + + package_info = PackageInfoParser(config_path, str(tmp_path)).parse() + + assert package_info.exclude_inherited_overrides is False