diff --git a/cppwg/generators.py b/cppwg/generators.py index 80f94a0..ee6a230 100644 --- a/cppwg/generators.py +++ b/cppwg/generators.py @@ -223,18 +223,8 @@ def log_unknown_classes(self) -> None: # transitively-included dependencies (e.g. boost, PETSc, VTK). A project # may vendor such dependencies under the source root, so scoping to the # whole source root would report - and log - thousands of library-internal - # classes. A module that sets no source_locations wraps everything, so it - # contributes the source root; this must be decided per module, so one - # module restricting its locations does not narrow the scope for a module - # that wraps everything. - source_locations: list[Path] = [] - for module_info in self.package_info.module_collection: - if module_info.source_locations: - source_locations.extend( - Path(location) for location in module_info.source_locations - ) - else: - source_locations.append(Path(self.source_root)) + # classes. + source_locations = self.package_info._module_source_locations() def in_source_locations(file_path: str) -> bool: parents = Path(file_path).parents diff --git a/cppwg/info/module_info.py b/cppwg/info/module_info.py index 5428260..768348f 100644 --- a/cppwg/info/module_info.py +++ b/cppwg/info/module_info.py @@ -46,7 +46,12 @@ class wrappers (see CppClassWrapperWriter), so that a class in this module of an imported package via `external_bases`). Do not list this module itself, to avoid a circular import. source_locations : list[str] - A list of source locations for this module + Directories (relative to the source root) that scope this module's + source. They bound both which declarations are wrapped (see + is_decl_in_source_path) and which implementation (.cpp) files are scanned + for explicit template instantiations, so a same-named class from another + tree under the source root is not conflated with a wrapped class. Blank + means unrestricted (the whole source root). use_all_classes : bool Use all classes in the module use_all_free_functions : bool diff --git a/cppwg/info/package_info.py b/cppwg/info/package_info.py index 5854331..7de61dd 100644 --- a/cppwg/info/package_info.py +++ b/cppwg/info/package_info.py @@ -257,7 +257,21 @@ def collect_source_files( """ filepaths: list[str] = [] - for root, _, filenames in os.walk(self.source_root, followlinks=True): + for root, dirnames, filenames in os.walk(self.source_root, followlinks=True): + # A CMakeCache.txt marks a CMake build tree, which holds only + # generated or copied files - stale duplicate headers, cppwg's own + # .cppwg wrapper output, dependencies fetched under it - never original + # source. Skip it wholesale (prune the descent): a build tree can dwarf + # the source, so walking it wastes time, and collecting a stale or + # foreign copy of a file is never wanted. In particular a dependency + # vendored under the build tree must not have its classes mistaken for + # the project's own by unqualified base name. This assumes an + # out-of-source build (the source_root is not itself a build tree); + # in-source builds are not supported. + if "CMakeCache.txt" in filenames: + dirnames[:] = [] + continue + for pattern in patterns: for filename in fnmatch.filter(filenames, pattern): filepath = os.path.abspath(os.path.join(root, filename)) @@ -323,10 +337,31 @@ def collect_source_cpp(self, restricted_paths: list[str]) -> None: restricted_paths : list[str] A list of restricted paths to skip when collecting files. """ - self.source_cpp_files = self.collect_source_files( + cpp_files = self.collect_source_files( self.source_cpp_patterns, restricted_paths ) + # Scope the instantiation scan to the module source_locations - the same + # directories that bound which declarations are wrapped (see + # ModuleInfo.is_decl_in_source_path). The scan matches classes by + # unqualified base name, so without this a same-named class from another + # tree under the source root (a dependency vendored into the repo, an + # example project) would be conflated with a wrapped class and corrupt its + # discovered/pruned template instantiations. A module with no + # source_locations wraps everything and so contributes the source root, + # leaving the scan unrestricted for that module; with no modules at all + # there is nothing to scope by, so it is left unrestricted rather than + # dropping every file. The build-tree skip in collect_source_files applies + # either way, so a build tree is excluded even with no source_locations. + locations = self._module_source_locations() + if locations: + cpp_files = [ + filepath + for filepath in cpp_files + if any(location in Path(filepath).parents for location in locations) + ] + self.source_cpp_files = cpp_files + def update_from_source(self) -> None: """ Update with data from the source headers. @@ -732,7 +767,9 @@ def _module_source_locations(self) -> list[Path]: A module with no ``source_locations`` wraps everything, so it contributes the source root; this is decided per module so one module restricting its locations does not narrow the scope for a module that wraps everything. - Mirrors the scoping used by log_unknown_classes. + Shared by the instantiation scan (collect_source_cpp), the auto-include + type map (_build_type_header_map) and unknown-class logging + (CppWrapperGenerator.log_unknown_classes). Returns ------- diff --git a/doc/reference.md b/doc/reference.md index c760a76..06a5972 100644 --- a/doc/reference.md +++ b/doc/reference.md @@ -75,7 +75,7 @@ Each entry under `modules:`. | `free_functions` | list | `[]` | Free functions to wrap, or `CPPWG_ALL`. | | `imports` | list[str] | `[]` | Python modules to import at the start of this module, so their types are registered first. Required for cross-module inheritance. See [Cross-module inheritance](inheritance.md#imports). | | `name` | str | `cppwg_module` | Module name; the extension is `_{package}_{name}`. | -| `source_locations` | list[str] | `[]` | Directories (relative to the source root) whose classes this module wraps. | +| `source_locations` | list[str] | `[]` | Directories (relative to the source root) that scope this module: they bound both the classes wrapped and the `.cpp` files scanned for template instantiations. Blank means the whole source root. | All [common options](#common-options) may also be set here. diff --git a/examples/shapes/wrapper/package_info.yaml b/examples/shapes/wrapper/package_info.yaml index de476a9..07ba3d8 100644 --- a/examples/shapes/wrapper/package_info.yaml +++ b/examples/shapes/wrapper/package_info.yaml @@ -78,8 +78,9 @@ modules: # Module name - name: math_funcs - # List of source directories for this module relative to the source root. - # Restrict to headers from these directories. Blank means unrestricted. + # Directories (relative to the source root) that scope this module: both the + # classes wrapped and the .cpp files scanned for template instantiations. + # Blank means unrestricted (the whole source root). source_locations: # List of classes to wrap. Blank means none, CPPWG_ALL means discover all. diff --git a/tests/test_generators.py b/tests/test_generators.py index c8c96ed..8ddc88e 100644 --- a/tests/test_generators.py +++ b/tests/test_generators.py @@ -138,7 +138,11 @@ def test_log_unknown_classes_reports_text_scanned_classes(castxml_env, tmp_path, (tmp_path / "Widget.hpp").write_text("class Widget {};\n") module = SimpleNamespace(class_collection=[], source_locations=[]) gen.package_info = SimpleNamespace( - module_collection=[module], source_hpp_files=[str(tmp_path / "Widget.hpp")] + module_collection=[module], + source_hpp_files=[str(tmp_path / "Widget.hpp")], + # The module wraps everything (no source_locations), so scoping resolves + # to the whole source root - as PackageInfo._module_source_locations does. + _module_source_locations=lambda: [tmp_path], ) gen.source_ns = SimpleNamespace(classes=lambda allow_empty=True: []) diff --git a/tests/test_package_info.py b/tests/test_package_info.py index b28c9c3..a6b1db6 100644 --- a/tests/test_package_info.py +++ b/tests/test_package_info.py @@ -21,6 +21,98 @@ def test_collect_source_cpp_collects_implementation_files(tmp_path): assert basenames == {"Foo.cpp"} +def test_collect_source_files_skips_cmake_build_trees(tmp_path): + """A nested CMake build tree (marked by CMakeCache.txt) is not collected. + + Its files - stale duplicate headers, vendored dependencies fetched under it, + generated output - must not be scanned as source, so that e.g. a dependency's + same-named class cannot be conflated with the project's own. + """ + src = tmp_path / "src" + src.mkdir() + (src / "Foo.cpp").write_text("") + (src / "Foo.hpp").write_text("") + + # An out-of-source build tree nested under the source root, holding a foreign + # copy of Foo plus a vendored dependency, all under a CMakeCache.txt. + build = src / "build" + (build / "_deps" / "dep-src").mkdir(parents=True) + (build / "CMakeCache.txt").write_text("") + (build / "Foo.cpp").write_text("") # stale/foreign copy + (build / "_deps" / "dep-src" / "Bar.cpp").write_text("") + + package_info = PackageInfo("testpkg", {"source_root": str(src)}) + package_info.collect_source_cpp(restricted_paths=[]) + + # Only the real source Foo.cpp is collected; nothing from the build tree. + assert package_info.source_cpp_files == [str(src / "Foo.cpp")] + + +def test_collect_source_cpp_scopes_to_module_source_locations(tmp_path): + """The .cpp scan is restricted to the module source_locations. + + A same-named class in another tree under the source root is not scanned, so it + cannot be conflated with a wrapped class by unqualified base name. + """ + src = tmp_path / "src" + (src / "wanted").mkdir(parents=True) + (src / "other").mkdir(parents=True) + (src / "wanted" / "Foo.cpp").write_text("") + (src / "other" / "Foo.cpp").write_text("") # foreign, same basename + + package = PackageInfo("pkg", {"source_root": str(src)}) + module = ModuleInfo("mod") + module.source_locations = [str(src / "wanted")] + package.add_module(module) + package.collect_source_cpp(restricted_paths=[]) + + assert package.source_cpp_files == [str(src / "wanted" / "Foo.cpp")] + + +def test_collect_source_cpp_unions_multiple_module_source_locations(tmp_path): + """A file under any module's source_locations is kept (locations are unioned).""" + src = tmp_path / "src" + for name in ("a", "b", "c"): + (src / name).mkdir(parents=True) + (src / name / "Foo.cpp").write_text("") + + package = PackageInfo("pkg", {"source_root": str(src)}) + for module_name, location in (("m1", "a"), ("m2", "b")): + module = ModuleInfo(module_name) + module.source_locations = [str(src / location)] + package.add_module(module) + package.collect_source_cpp(restricted_paths=[]) + + # a and b are in scope (unioned across the two modules); c is not. + assert package.source_cpp_files == [ + str(src / "a" / "Foo.cpp"), + str(src / "b" / "Foo.cpp"), + ] + + +def test_collect_source_cpp_unrestricted_when_a_module_wraps_everything(tmp_path): + """A module with no source_locations wraps everything, so the scan stays + unrestricted even if another module restricts its own locations.""" + src = tmp_path / "src" + (src / "a").mkdir(parents=True) + (src / "b").mkdir(parents=True) + (src / "a" / "Foo.cpp").write_text("") + (src / "b" / "Foo.cpp").write_text("") + + package = PackageInfo("pkg", {"source_root": str(src)}) + restricted = ModuleInfo("restricted") + restricted.source_locations = [str(src / "a")] + package.add_module(restricted) + package.add_module(ModuleInfo("wraps_all")) # no source_locations + package.collect_source_cpp(restricted_paths=[]) + + # The unrestricted module contributes the source root, so both are kept. + assert package.source_cpp_files == [ + str(src / "a" / "Foo.cpp"), + str(src / "b" / "Foo.cpp"), + ] + + def test_collect_source_files_orders_same_basename_deterministically(tmp_path): """Files sharing a basename are ordered by full path, not os.walk order.""" src = tmp_path / "src"