diff --git a/cppwg/generators.py b/cppwg/generators.py index ee6a230..a198591 100644 --- a/cppwg/generators.py +++ b/cppwg/generators.py @@ -7,7 +7,6 @@ import shutil import subprocess import uuid -from pathlib import Path import pygccxml @@ -227,8 +226,10 @@ def log_unknown_classes(self) -> None: source_locations = self.package_info._module_source_locations() def in_source_locations(file_path: str) -> bool: - parents = Path(file_path).parents - return any(location in parents for location in source_locations) + return any( + utils.path_is_within(file_path, location) + for location in source_locations + ) seen_class_names = set() for module_info in self.package_info.module_collection: diff --git a/cppwg/info/base_info.py b/cppwg/info/base_info.py index 4f2bbeb..4fe152c 100644 --- a/cppwg/info/base_info.py +++ b/cppwg/info/base_info.py @@ -254,15 +254,25 @@ def hierarchy_attribute(self, attribute_name: str) -> Any: Any The attribute value, or None if not found. """ + # Memoize by attribute name: this walks the class -> module -> package + # chain on every call, and is only read during generation, by which point + # the gathered config is fixed. Lazily created so any BaseInfo subclass + # works whether or not it ran BaseInfo.__init__. + cache = self.__dict__.setdefault("_hierarchy_attribute_cache", {}) + if attribute_name in cache: + return cache[attribute_name] + value = getattr(self, attribute_name, None) if value or isinstance(value, bool) or isinstance(value, Number): - return value - - if self.parent is None: + result = value + elif self.parent is None: # Reached the top of the hierarchy (i.e. PackageInfo) - return None + result = None + else: + result = self.parent.hierarchy_attribute(attribute_name) - return self.parent.hierarchy_attribute(attribute_name) + cache[attribute_name] = result + return result def hierarchy_attribute_gather(self, attribute_name: str) -> list[Any]: """ @@ -319,10 +329,18 @@ def hierarchy_attribute_gather_flat(self, attribute_name: str) -> list[Any]: list[Any] The flattened list of items. """ + # Memoize by attribute name (see hierarchy_attribute). The cached list is + # returned directly; callers only read/concatenate it, never mutate it. + cache = self.__dict__.setdefault("_hierarchy_gather_flat_cache", {}) + if attribute_name in cache: + return cache[attribute_name] + flat: list[Any] = [] for value in self.hierarchy_attribute_gather(attribute_name): if isinstance(value, (list, tuple, set)): flat.extend(value) else: flat.append(value) + + cache[attribute_name] = flat return flat diff --git a/cppwg/info/module_info.py b/cppwg/info/module_info.py index 768348f..2ed3e11 100644 --- a/cppwg/info/module_info.py +++ b/cppwg/info/module_info.py @@ -1,12 +1,11 @@ """Module information structure.""" -from pathlib import Path from typing import TYPE_CHECKING, Any from pygccxml import declarations from cppwg.info.base_info import BaseInfo -from cppwg.info.class_info import CppClassInfo +from cppwg.info.class_info import CppClassInfo, _unqualified_base_name from cppwg.info.enum_info import CppEnumInfo from cppwg.info.free_function_info import CppFreeFunctionInfo from cppwg.utils import utils @@ -174,11 +173,10 @@ def is_decl_in_source_path(self, decl: "declaration_t") -> bool: if not self.source_locations: return True - for location in self.source_locations: - if Path(location) in Path(decl.location.file_name).parents: - return True - - return False + return any( + utils.path_is_within(decl.location.file_name, location) + for location in self.source_locations + ) def sort_classes(self) -> None: """ @@ -209,21 +207,58 @@ def sort_classes(self) -> None: } # Inheritance is a hard ordering constraint: a base precedes its - # subclasses. + # subclasses. Precompute each class's unqualified name and the set of + # unqualified names of the bases it declares, so the check is a set + # membership test (matching CppClassInfo.extends) rather than a per-pair + # scan over base_decls with a name reduction on each element. + unqualified_name = {cls: _unqualified_base_name(cls.name) for cls in classes} + declared_base_names = { + cls: { + _unqualified_base_name(base_decl.name) + for base_decl in cls.base_decls + if base_decl is not None + } + for cls in classes + } for cls in classes: + base_names = declared_base_names[cls] + if not base_names: + continue for other in classes: - if other is not cls and cls.extends(other): + if other is not cls and unqualified_name[other] in base_names: predecessors[cls].add(other) # Signature dependencies order a class after a wrapped type it uses, # unless that contradicts an inheritance ordering already recorded. - # Argument type strings are gathered once per class to keep this cheap. - arg_types = {cls: cls.signature_arg_types() for cls in classes} + # Precompute, per class, its argument-type strings in canonical form and + # a compiled whole-token regex for its name, so the dependency test does + # not re-canonicalize and re-compile on every one of the ~C^2 pair + # comparisons. + canon_arg_types = { + cls: [ + utils.canonicalize_type_whitespace(arg_type) + for arg_type in cls.signature_arg_types() + ] + for cls in classes + } + name_regex = {cls: utils.compile_type_pattern(cls.name) for cls in classes} + requires_cache: dict[tuple[CppClassInfo, CppClassInfo], bool] = {} def requires(a: CppClassInfo, b: CppClassInfo) -> bool: - return any( - utils.type_string_matches(arg_type, b.name) for arg_type in arg_types[a] + # Whether any of a's public method/constructor argument types name b + # as a whole token. Equivalent to + # `any(utils.type_string_matches(t, b.name) for t in a's arg types)` + # but reusing the precomputed canonical arg types and compiled regex. + key = (a, b) + cached = requires_cache.get(key) + if cached is not None: + return cached + regex = name_regex[b] + result = regex is not None and any( + regex.search(arg_type) for arg_type in canon_arg_types[a] ) + requires_cache[key] = result + return result for cls in classes: for other in classes: diff --git a/cppwg/info/package_info.py b/cppwg/info/package_info.py index 7de61dd..8dd679b 100644 --- a/cppwg/info/package_info.py +++ b/cppwg/info/package_info.py @@ -5,7 +5,6 @@ import os import re from collections.abc import Iterator -from pathlib import Path from typing import TYPE_CHECKING, Any from pygccxml import declarations @@ -278,7 +277,7 @@ def collect_source_files( # Skip files in restricted paths if any( - Path(restricted_path) in Path(filepath).parents + utils.path_is_within(filepath, restricted_path) for restricted_path in restricted_paths ): continue @@ -358,7 +357,10 @@ def collect_source_cpp(self, restricted_paths: list[str]) -> None: cpp_files = [ filepath for filepath in cpp_files - if any(location in Path(filepath).parents for location in locations) + if any( + utils.path_is_within(filepath, location) + for location in locations + ) ] self.source_cpp_files = cpp_files @@ -760,7 +762,7 @@ def dependency(class_info: "CppClassInfo", decl) -> str | None: c for c in module_info.class_collection if c.cpp_names or c.excluded ] - def _module_source_locations(self) -> list[Path]: + def _module_source_locations(self) -> list[str]: """ Return the source-location paths that scope the wrapped source tree. @@ -773,15 +775,15 @@ def _module_source_locations(self) -> list[Path]: Returns ------- - list[pathlib.Path] + list[str] The directories that bound the project's own source files. """ - locations: list[Path] = [] + locations: list[str] = [] for module_info in self.module_collection: if module_info.source_locations: - locations.extend(Path(loc) for loc in module_info.source_locations) + locations.extend(module_info.source_locations) else: - locations.append(Path(self.source_root)) + locations.append(self.source_root) return locations def _build_type_header_map(self) -> dict[str, str]: @@ -807,8 +809,10 @@ def _build_type_header_map(self) -> dict[str, str]: source_locations = self._module_source_locations() def in_source_locations(file_path: str) -> bool: - parents = Path(file_path).parents - return any(location in parents for location in source_locations) + return any( + utils.path_is_within(file_path, location) + for location in source_locations + ) mapping: dict[str, str] = {} ambiguous: set[str] = set() diff --git a/cppwg/parsers/source_parser.py b/cppwg/parsers/source_parser.py index dd7d88a..14414e8 100644 --- a/cppwg/parsers/source_parser.py +++ b/cppwg/parsers/source_parser.py @@ -2,7 +2,6 @@ import logging import os -from pathlib import Path from pygccxml import declarations, parser from pygccxml.declarations import declaration_t @@ -117,7 +116,7 @@ def parse(self) -> namespace_t: source_decls: list[declaration_t] = [ decl for decl in filtered_decls - if Path(self.source_root) in Path(decl.location.file_name).parents + if utils.path_is_within(decl.location.file_name, self.source_root) or decl.location.file_name == self.wrapper_header_collection ] @@ -181,13 +180,13 @@ class name to the template argument lists found. global_ns: namespace_t = declarations.get_global_namespace(decls) + source_file_real = os.path.realpath(source_file) + for class_decl in global_ns.classes(allow_empty=True): # Keep only explicit instantiations defined in this file. if class_decl.location is None: continue - if os.path.realpath(class_decl.location.file_name) != os.path.realpath( - source_file - ): + if os.path.realpath(class_decl.location.file_name) != source_file_real: continue if not declarations.templates.is_instantiation(class_decl.name): diff --git a/cppwg/utils/utils.py b/cppwg/utils/utils.py index 75d329c..cb62577 100644 --- a/cppwg/utils/utils.py +++ b/cppwg/utils/utils.py @@ -244,18 +244,45 @@ def type_string_matches(type_string: str, pattern: str) -> bool: bool True if the pattern occurs in the type string as a whole token. """ + regex = compile_type_pattern(pattern) + if regex is None: + return False + + # Match on a whitespace-canonical form of the searched string so that + # differences in spacing around punctuation (which pygccxml and hand-written + # config may spell differently) do not defeat the match. + return regex.search(canonicalize_type_whitespace(type_string)) is not None + + +def compile_type_pattern(pattern: str) -> "re.Pattern | None": + """ + Compile a whole-token match regex for a C++ type pattern. + + Returns a compiled regex that matches ``pattern`` as a whole token in a + *whitespace-canonical* type string (see :func:`type_string_matches`), or + ``None`` if ``pattern`` is not a usable pattern (not a non-empty string, or + empty once canonicalized). Splitting the compile out lets a caller that + tests one pattern against many strings (e.g. class-dependency sorting) + canonicalize and compile the pattern once instead of on every comparison. + + Parameters + ---------- + pattern : str + The type pattern to look for. + + Returns + ------- + re.Pattern | None + The compiled whole-token regex, or None if the pattern is unusable. + """ # A non-string pattern (e.g. a yaml scalar like `arg_type_excludes: 5`) is # not a valid type pattern; treat it as non-matching rather than crashing. if not isinstance(pattern, str) or not pattern: - return False + return None - # Match on a whitespace-canonical form of both strings so that differences - # in spacing around punctuation (which pygccxml and hand-written config may - # spell differently) do not defeat the match. - type_string = canonicalize_type_whitespace(type_string) pattern = canonicalize_type_whitespace(pattern) if not pattern: - return False + return None # Enforce an identifier boundary only on an edge whose pattern character is # itself an identifier character. A pattern ending in e.g. > / * / & should @@ -264,8 +291,45 @@ def type_string_matches(type_string: str, pattern: str) -> bool: left = r"(? bool: + """ + Return whether ``path`` lies strictly beneath the directory ``ancestor``. + + Equivalent to ``Path(ancestor) in Path(path).parents`` but implemented with + normalized-string comparison rather than allocating ``Path`` objects and + scanning the ``parents`` sequence. This is a cheaper equivalent used in hot + paths and may run during millions of times per generation on large projects. + + Like ``Path.parents``, the test is *lexical* (no symlink resolution) and + *strict*: a path equal to ``ancestor`` is not "within" it. ``normcase`` is + applied so the comparison is case-insensitive on Windows, matching ``Path``. + + Parameters + ---------- + path : str + The candidate descendant path. + ancestor : str + The directory that ``path`` may live beneath. + + Returns + ------- + bool + True if ``path`` is strictly beneath ``ancestor``. + """ + ancestor = os.path.normcase(os.path.normpath(ancestor)) + path = os.path.normcase(os.path.normpath(path)) + if path == ancestor: + return False + # Compare against the ancestor plus a trailing separator, so a sibling whose + # name merely starts with the ancestor (e.g. "/src2" under "/src") is not + # matched. A root such as "/" (or a Windows drive root) already ends in a + # separator after normpath, so do not append a second one. + if not ancestor.endswith(os.sep): + ancestor += os.sep + return path.startswith(ancestor) def type_is_copy_assignable(decl_type: Any) -> bool: diff --git a/cppwg/writers/class_writer.py b/cppwg/writers/class_writer.py index 51ced1e..a00ed52 100644 --- a/cppwg/writers/class_writer.py +++ b/cppwg/writers/class_writer.py @@ -37,6 +37,75 @@ from cppwg.info.class_info import CppClassInfo +def virtual_method_signature(method_decl: "member_function_t") -> tuple: + """ + Return the identity used to match an override against a base virtual. + + The tuple is (name, const-ness, argument-type strings). The return type is + intentionally excluded so a covariant-return override still matches, and each + argument type has its whitespace canonicalized so spelling differences between + the derived and base declarations do not defeat the match. + """ + return ( + method_decl.name, + method_decl.has_const, + tuple( + canonicalize_type_whitespace(t.decl_string) + for t in method_decl.argument_types + ), + ) + + +def build_base_virtual_signature_index( + package_classes: set["class_t"], + package_class_infos: dict["class_t", "CppClassInfo"], +) -> dict["class_t", set]: + """ + Precompute, per wrapped class, the virtual signatures it actually binds. + + For every class wrapped in the package, collect the virtual_method_signature + of each public virtual member function it binds (i.e. not dropped by + CppMethodWrapperWriter.method_is_excluded). With this index, + _overrides_wrapped_base_virtual reduces to a set-membership test against a + base's entry, instead of re-querying and re-comparing the base's member + functions for every override of every derived class. Built once and shared + by all class writers. + + Parameters + ---------- + package_classes : set[pygccxml.declarations.class_t] + Declarations of every class wrapped anywhere in the package. + package_class_infos : dict[class_t, CppClassInfo] + Maps each such decl to its class_info, needed for method_is_excluded. + + Returns + ------- + dict[class_t, set[tuple]] + Maps each wrapped class decl to the set of virtual signatures it binds. + """ + index: dict["class_t", set] = {} + for base_decl in package_classes: + base_info = package_class_infos.get(base_decl) + signatures: set = set() + for base_method in base_decl.member_functions(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 an override redundant. + if base_method.access_type != "public": + continue + if base_method.virtuality not in ("virtual", "pure virtual"): + continue + # A base method excluded from wrapping (by name, return type or arg + # type) emits no binding, so it cannot make an override redundant. + if base_info is not None and CppMethodWrapperWriter.method_is_excluded( + base_info, base_decl, base_method + ): + continue + signatures.add(virtual_method_signature(base_method)) + index[base_decl] = signatures + return index + + class CppClassWrapperWriter(CppBaseWrapperWriter): """ Writer to generate wrapper code for C++ classes. @@ -71,6 +140,7 @@ def __init__( package_classes: set["class_t"] = None, overwrite: bool = False, package_class_infos: dict["class_t", "CppClassInfo"] = None, + base_virtual_signatures: dict["class_t", set] = None, ) -> None: logger = logging.getLogger() @@ -87,6 +157,12 @@ def __init__( self.package_class_infos = ( package_class_infos if package_class_infos is not None else {} ) + # Prebuilt per-package index of each base's bound virtual signatures, + # consulted by _overrides_wrapped_base_virtual. Normally supplied by the + # module writer so every class writer shares one build; if a caller omits + # it (e.g. a unit test), it is built lazily from this writer's package + # classes on first use (see the base_virtual_signatures property). + self._base_virtual_signatures = base_virtual_signatures self.overwrite = overwrite @@ -97,6 +173,43 @@ def __init__( # from the generated registration text in write(). Empty until then. self.typecaster_includes: list[str] = [] + # Memoization for the inherited-override test, which runs for every method + # (and every sibling overload) of every class this writer emits. The + # linked-base list is identical for all methods of a class_decl, and a + # method's signature is recomputed for each sibling scan, so both are + # cached rather than recomputed per call. See _overrides_wrapped_base_virtual. + self._linked_wrapped_bases_cache: dict["class_t", list["class_t"]] = {} + self._method_signature_cache: dict["member_function_t", tuple] = {} + + @property + def base_virtual_signatures(self) -> dict["class_t", set]: + """ + Per-base bound-virtual signature index, built lazily on first use. + + Only ``_overrides_wrapped_base_virtual`` reads this, and only when a class + enables ``exclude_inherited_overrides`` - so a package that never uses the + option does not build the index at all. The build scans every wrapped class's + member functions, so it is shared across all class writers of the package + by caching it on the package info (falling back to this writer when the + package info is not reachable, e.g. in unit tests). + """ + if self._base_virtual_signatures is not None: + return self._base_virtual_signatures + + module_info = getattr(self.class_info, "parent", None) + pkg_info = getattr(module_info, "package_info", None) + + index = getattr(pkg_info, "_base_virtual_signatures", None) + if index is None: + index = build_base_virtual_signature_index( + self.package_classes, self.package_class_infos + ) + if pkg_info is not None: + pkg_info._base_virtual_signatures = index + + self._base_virtual_signatures = index + return index + def prefix_block(self) -> str: """ Return the prefix text block for the top of a wrapper file. @@ -470,6 +583,10 @@ def _overrides_wrapped_base_virtual( test used by _is_inherited_override; it does not consider sibling overloads. + The wrapped-virtual match is a set lookup against + base_virtual_signatures (see build_base_virtual_signature_index), which + precomputes each base's bound virtual signatures once for the whole package. + Parameters ---------- class_decl : pygccxml.declarations.class_t @@ -485,18 +602,38 @@ def _overrides_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")) + signature = self._method_signature_cache.get(method_decl) + if signature is None: + signature = virtual_method_signature(method_decl) + self._method_signature_cache[method_decl] = signature - name = method_decl.name - arg_types = [ - canonicalize_type_whitespace(t.decl_string) - for t in method_decl.argument_types - ] + for base_decl in self._linked_wrapped_bases(class_decl): + if signature in self.base_virtual_signatures.get(base_decl, ()): + return True + return False + + def _linked_wrapped_bases(self, class_decl: "class_t") -> list["class_t"]: + """ + Return the wrapped bases whose bindings this class actually inherits. + + A base qualifies if it is wrapped in this package and its pybind base link + is emitted into the derived py::class_: a same-module base is always + linked, but a base wrapped in another module is linked only when + cross-module inheritance is enabled (`imports` set) - see bases_block. + Without that link the base's binding is not inherited, so an override of it + would become unreachable if skipped. + + The result is identical for every method of ``class_decl`` and is cached, + so recursive_bases is walked once per class rather than once per method. + """ + cached = self._linked_wrapped_bases_cache.get(class_decl) + if cached is not None: + return cached + + allow_external_bases = bool(self.class_info.hierarchy_attribute("imports")) + + bases: list["class_t"] = [] 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 @@ -508,36 +645,10 @@ def _overrides_wrapped_base_virtual( # base (in module_classes) is always linked. if base_decl not in self.module_classes and not allow_external_bases: continue + bases.append(base_decl) - 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 + self._linked_wrapped_bases_cache[class_decl] = bases + return bases def _is_inherited_override( self, class_decl: "class_t", method_decl: "member_function_t" diff --git a/tests/test_utils.py b/tests/test_utils.py index 40759a0..6806f92 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -19,6 +19,7 @@ is_scoped_enum_in_source_file, normalize_template_arg, parse_template_params, + path_is_within, read_source_file, split_template_args, str_to_num, @@ -47,6 +48,29 @@ def test_ensure_trailing_newline(code, expected): assert ensure_trailing_newline(code) == expected +@pytest.mark.parametrize( + "path, ancestor, expected", + [ + ("/src/a/foo.hpp", "/src", True), # nested descendant + ("/src/foo.hpp", "/src", True), # direct child + ("/src/foo.hpp", "/src/", True), # trailing separator on ancestor + ("/src", "/src", False), # equal path is not "within" (strict) + ("/src/", "/src", False), # equal after normpath + ("/src2/foo.hpp", "/src", False), # sibling with a shared name prefix + ("/other/foo.hpp", "/src", False), # unrelated tree + # A filesystem root as the ancestor must still match its descendants: + # normpath("/") == "/" already ends in a separator, so it must not become + # "//" (which would make every real descendant fail). + ("/src/foo.hpp", "/", True), + ("/", "/", False), # root equals itself + ("/a/../b/foo.hpp", "/b", True), # normalized before comparison + ], +) +def test_path_is_within(path, ancestor, expected): + """path_is_within is a strict, lexical descendant test (handles roots).""" + assert path_is_within(path, ancestor) is expected + + @pytest.mark.parametrize( "arg, expected", [