Clean up comments

Author: georgesittas

mypyc/build.py

@@ -619,38 +619,34 @@ def get_header_deps(cfiles: list[tuple[str, str]]) -> list[tuple[bool, str]]:
 def resolve_cfile_deps(
     cfile_dir: str, direct_includes: list[tuple[bool, str]], target_dir: str
 ) -> set[str]:
-    """Resolve a .c file's `#include` directives to on-disk paths, walking
-    transitively through resolved headers.
-
-    The C preprocessor resolves `#include "foo"` against the includer's
-    directory first, then via -I, while `#include <foo>` only uses -I. We
-    mirror that exactly: quoted includes are searched in (includer_dir,
-    target_dir) order, and angled includes are searched in target_dir only.
-    `target_dir` is the only -I path that holds files we generate; anything
-    we cannot resolve under it (or, for quoted form, the includer's dir) is
-    dropped — other headers like `<Python.h>` and `<CPy.h>` live elsewhere
-    and do not change between builds, so they are not real deps for
-    incremental purposes.
-
-    The walk is transitive: each resolved header is opened and scanned for
-    its own `#include` directives. Without this, cross-group export-table
-    headers reached via `__native_internal_<mod>.h` (which includes
-    `<other_group/__native_other.h>`) would be missed, and edits that shift
-    struct offsets in `other_group` would not trigger a recompile of the
-    consumer's .o file. Its baked-in offsets would then resolve to whatever
-    class/function now occupies that slot — silent runtime corruption.
-
-    Returns a set of resolved paths suitable for use as an Extension.depends
-    list.
+    """
+    Resolve a .c file's `#include`s to on-disk paths, walking transitively through resolved headers.
+
+    The C preprocessor resolves `#include "foo"` against the includer's directory first, then via
+    -I, while `#include <foo>` only uses -I. We mirror that exactly: quoted includes are searched
+    in (includer_dir, target_dir) order, and angled includes are searched in target_dir only.
+    `target_dir` is the only -I path that holds files we generate; anything we cannot resolve under
+    it (or, for quoted form, the includer's dir) is dropped. Other headers like `<Python.h>` and
+    `<CPy.h>` live elsewhere and do not change between builds, so they are not real dependencies
+    for incremental purposes.
+
+    The walk is transitive: each resolved header is opened and scanned for its own `#include`
+    directives. Without this, cross-group export-table headers reached via `__native_internal_<mod>.h`
+    (which includes `<other_group/__native_other.h>`) would be missed, and edits that shift struct
+    offsets in `other_group` would not trigger a recompile of the consumer's .o file. Its baked-in
+    offsets would then resolve to whatever class/function now occupies that slot => runtime corruption.
+
+    Returns a set of resolved paths suitable for use as an Extension.depends list.
     """
     resolved: set[str] = set()
-    # Worklist of (search_dir, is_angled, header_name). search_dir is the
-    # includer's directory — for the initial cfile it is the cfile's dir, for
-    # a transitively-included header it is that header's dir. It is only
-    # consulted for quoted-form includes.
+
+    # Worklist of (search_dir, is_angled, header_name). search_dir is the includer's directory; for the
+    # initial cfile it is the cfile's dir, for a transitively-included header it is that header's dir.
+    # It is only consulted for quoted-form includes.
     worklist: list[tuple[str, bool, str]] = [
         (cfile_dir, is_angled, dep) for is_angled, dep in direct_includes
     ]
+
     while worklist:
         search_dir, is_angled, dep = worklist.pop()
         # Quoted form: includer's dir first, then -I (target_dir).
@@ -663,13 +659,12 @@ def resolve_cfile_deps(
             if candidate in resolved:
                 break
             resolved.add(candidate)
-            # Recurse only into headers. Some lib-rt sources are pulled in
-            # as `#include "init.c"` etc.; those do not resolve under
-            # target_dir so they get filtered out before we would try to scan
+            # Recurse only into headers. Some lib-rt sources are pulled in as `#include "init.c"` etc.;
+            # those do not resolve under target_dir so they get filtered out before we would try to scan
             # them, but the .h guard is a cheap belt-and-braces.
             if candidate.endswith(".h"):
                 try:
-                    with open(candidate, encoding="utf-8", errors="replace") as f:
+                    with open(candidate, encoding="utf-8") as f:
                         header_contents = f.read()
                 except OSError:
                     header_contents = ""
@@ -731,23 +726,23 @@ def mypyc_build(
     # Write out the generated C and collect the files for each group.
     # Should this be here??
     #
-    # Header resolution is deferred to a second pass: a header in one group
-    # may include a header generated by another group, so resolving
-    # immediately would miss cross-group deps for groups processed first.
+    # Header resolution is deferred to a second pass: a header in one group may include a header
+    # generated by another group, so resolving here misses cross-group deps for groups processed first.
     pending: list[list[tuple[str, list[tuple[bool, str]]]]] = []
     for cfiles in group_cfiles:
         per_cfile_deps: list[tuple[str, list[tuple[bool, str]]]] = []
         for cfile, ctext in cfiles:
             cfile = os.path.join(compiler_options.target_dir, cfile)
+
             # Empty contents marks a file the previous run already wrote
             # (fully-cached group): skip the rewrite and just reuse it.
             if ctext and not options.mypyc_skip_c_generation:
                 write_file(cfile, ctext)
-            # For fully-cached groups ctext is empty; read the on-disk .c so
-            # the dep resolver can walk its transitive header chain and populate
-            # Extension.depends — otherwise cross-group export-table header
-            # changes (e.g. a new class shifting struct offsets) won't trigger
-            # a recompile of this cached consumer's .o.
+
+            # For fully-cached groups ctext is empty; read the on-disk .c so the dep resolver
+            # can walk its transitive header chain and populate Extension.depends. Otherwise,
+            # cross-group export-table header changes (e.g. a new class shifting struct offsets)
+            # won't trigger a recompile of this cached consumer's .o.
             if not ctext and os.path.exists(cfile):
                 try:
                     with open(cfile, encoding="utf-8") as _f:
@@ -757,9 +752,8 @@ def mypyc_build(
             per_cfile_deps.append((cfile, get_header_deps([(cfile, ctext)])))
         pending.append(per_cfile_deps)
 
-    # Second pass: assemble each group's .c filenames and resolve transitive
-    # deps now that every group's headers are on disk. See resolve_cfile_deps
-    # for the rules.
+    # Second pass: assemble each group's .c filenames and resolve transitive deps now that every group's
+    # headers are on disk. See resolve_cfile_deps for the rules.
     group_cfilenames: list[tuple[list[str], list[str]]] = []
     for per_cfile in pending:
         cfilenames = [cf for cf, _ in per_cfile if os.path.splitext(cf)[1] == ".c"]

mypyc/test/test_misc.py

@@ -26,23 +26,14 @@ def test_debug_op(self) -> None:
 
 
 class TestHeaderDeps(unittest.TestCase):
-    """Tests for the header-dependency tracking used to build
-    `Extension.depends`, which drives setuptools' `newer_group` decision
-    about whether to recompile a .o file on incremental builds.
-
-    The critical case is cross-group export-table headers: each module's
-    `__native_internal_<mod>.h` does `#include <other_group/__native_other.h>`,
-    and the consumer's compiled .o file bakes in byte offsets into that
-    header's `export_table_<group>` struct. If we miss this header in the
-    deps list, struct-layout changes in `other_group` won't trigger a
-    rebuild of the consumer, and its baked-in offsets will silently resolve
-    to whatever now occupies those slots.
+    """
+    Tests for the header-dependency tracking used to build `Extension.depends`, which drives
+    setuptools' `newer_group` decision about whether to recompile a .o file on incremental builds.
     """
 
     def test_get_header_deps_quoted_includes(self) -> None:
-        # Quoted includes — the historical form. Used by the .c file to
-        # reach its own __native_<mod>.h / __native_internal_<mod>.h. The
-        # `False` in each tuple marks the include as non-angled, which
+        # Quoted includes: the historical form. Used by the .c file to reach its own __native_<mod>.h /
+        # __native_internal_<mod>.h. The `False` in each tuple marks the include as non-angled, which
         # `resolve_cfile_deps` uses to search the includer's directory.
         cfile = '#include "__native_caller.h"\n#include "__native_internal_caller.h"\n'
         assert get_header_deps([("caller.c", cfile)]) == [
@@ -51,51 +42,46 @@ def test_get_header_deps_quoted_includes(self) -> None:
         ]
 
     def test_get_header_deps_angle_bracket_includes(self) -> None:
-        # Angle-bracket includes are also matched, and reported with
-        # is_angled=True so that the resolver skips the includer's dir
-        # for them (matching the C preprocessor). The cross-group export
-        # header is reached via `#include <other_group/__native_other.h>`
-        # in __native_internal_<mod>.h. Before this was matched the dep
-        # was missed entirely and the consumer's .o was never invalidated
-        # when the other group's struct layout shifted.
+        # Angle-bracket includes are also matched, and reported with is_angled=True so that the resolver
+        # skips the includer's dir for them (matching the C preprocessor). The cross-group export header
+        # is reached via `#include <other_group/__native_other.h>` in __native_internal_<mod>.h. Before
+        # this was matched the dep was missed entirely and the consumer's .o was never invalidated when
+        # the other group's struct layout shifted.
         cfile = "#include <Python.h>\n#include <lib/__native_functions.h>\n"
         assert get_header_deps([("caller.c", cfile)]) == [
             (True, "Python.h"),
             (True, "lib/__native_functions.h"),
         ]
 
     def test_get_header_deps_mixed_and_whitespace(self) -> None:
-        # The preprocessor tolerates whitespace and the leading-hash form.
-        # `get_header_deps` returns sorted tuples — non-angled (False) sorts
-        # before angled (True), then alphabetical within each kind.
+        # The preprocessor tolerates whitespace and the leading-hash form. `get_header_deps` returns sorted
+        # tuples — non-angled (False) sorts before angled (True), then alphabetical within each kind.
         cfile = '# include "a.h"\n#  include  <b.h>\n#include\t"c.h"\n'
         assert get_header_deps([("x.c", cfile)]) == [(False, "a.h"), (False, "c.h"), (True, "b.h")]
 
     def test_resolve_walks_transitively_through_headers(self) -> None:
-        # Reproduces the bug scenario: caller's .c only directly includes
-        # caller's own headers, but caller's __native_internal_caller.h
-        # includes the cross-group export header. The resolver must follow
-        # that chain so setuptools sees the cross-group header as a dep.
+        # Reproduces the bug scenario: caller's .c only directly includes caller's own headers, but
+        # caller's __native_internal_caller.h includes the cross-group export header. The resolver
+        # must follow that chain so setuptools sees the cross-group header as a dep.
         with tempfile.TemporaryDirectory() as tmp:
             build_dir = tmp
             os.makedirs(os.path.join(build_dir, "lib"))
             os.makedirs(os.path.join(build_dir, "other_group"))
 
-            # caller.c's directly-included headers — both live alongside
+            # caller.c's directly-included headers, both live alongside
             # caller.c under build/ (resolved via target_dir).
             internal_h = os.path.join(build_dir, "__native_internal_caller.h")
             caller_h = os.path.join(build_dir, "__native_caller.h")
             cross_group_h = os.path.join(build_dir, "lib", "__native_functions.h")
             unrelated_h = os.path.join(build_dir, "other_group", "__native_other.h")
 
             with open(caller_h, "w") as f:
-                # lib-rt headers don't exist on disk under build/, so they
-                # get dropped during resolution and aren't recursed into.
+                # Headers outside build/ (CPython's <Python.h>, lib-rt's <CPy.h>) don't resolve under
+                # target_dir, so they get dropped during resolution and aren't recursed into.
                 f.write("#include <Python.h>\n#include <CPy.h>\n")
             with open(internal_h, "w") as f:
-                # The smoking gun: this header includes a header in another
-                # group via angle brackets. Pre-fix, this dep was invisible
-                # to setuptools.
+                # This header includes a header in another group via angle brackets. Pre-fix, this dep
+                # was invisible to setuptools.
                 f.write(
                     "#include <Python.h>\n"
                     '#include "__native_caller.h"\n'
@@ -104,15 +90,12 @@ def test_resolve_walks_transitively_through_headers(self) -> None:
             with open(cross_group_h, "w") as f:
                 f.write("struct export_table_lib___functions { int x; };\n")
             with open(unrelated_h, "w") as f:
-                # Sibling group not reached from caller's chain — must
-                # NOT appear in the resolved set.
+                # Sibling group not reached from caller's chain => must NOT appear in the resolved set.
                 f.write("struct unrelated { int x; };\n")
 
-            # caller.c is in build_dir, so its includer-dir is build_dir.
-            # Both directly-included headers are quoted (`False`); the
-            # cross-group header that __native_internal_caller.h reaches
-            # via `<lib/__native_functions.h>` is found by the recursive
-            # walk re-reading the on-disk header.
+            # caller.c is in build_dir, so its includer-dir is build_dir. Both directly-included headers
+            # are quoted (`False`); the cross-group header that __native_internal_caller.h reaches via
+            # `<lib/__native_functions.h>` is found by the recursive walk re-reading the on-disk header.
             deps = resolve_cfile_deps(
                 cfile_dir=build_dir,
                 direct_includes=[
@@ -128,11 +111,9 @@ def test_resolve_walks_transitively_through_headers(self) -> None:
             )
 
     def test_resolve_drops_unresolvable_includes(self) -> None:
-        # `<Python.h>`, `<CPy.h>`, etc. don't live under target_dir, so
-        # they're dropped from depends. They never change between builds,
-        # so this is the right behavior — and crucially it stops
-        # setuptools' `missing="newer"` from treating them as always-newer
-        # and force-rebuilding every translation unit.
+        # `<Python.h>`, `<CPy.h>`, etc. don't live under target_dir, so they're dropped from depends. They
+        # never change between builds, so this is the right behavior. Crucially, it stops setuptools'
+        # `missing="newer"` from treating them as always-newer and force-rebuilding every translation unit.
         with tempfile.TemporaryDirectory() as tmp:
             cfile_dir = tmp
             deps = resolve_cfile_deps(
@@ -143,11 +124,9 @@ def test_resolve_drops_unresolvable_includes(self) -> None:
             assert deps == set()
 
     def test_resolve_search_order_matches_preprocessor(self) -> None:
-        # When the same header name exists both next to the includer and
-        # under target_dir, the C preprocessor picks the includer-dir copy
-        # for `#include "shared.h"` and the target_dir copy for
-        # `#include <shared.h>`. The resolver must record the same path
-        # the compiler will actually consume, otherwise mtimes of the
+        # When the same header name exists both next to the includer and under target_dir, the C preprocessor
+        # picks the includer-dir copy for `#include "shared.h"` and the target_dir copy for `#include <shared.h>`.
+        # The resolver must record the same path the compiler will actually consume, otherwise mtimes of the
         # wrong file drive incremental rebuild decisions.
         with tempfile.TemporaryDirectory() as tmp:
             includer = os.path.join(tmp, "groupA")