cleanup: Ensure we limit the system headers included in .h files.

Most system headers contain functions (e.g. `memcpy` in `string.h`)
which aren't needed in our own header files. For the most part, our own
headers should only include types needed to declare our own types and
functions. We now enforce this so we think twice about which headers we
really need in the .h files.

.circleci/config.yml

@@ -89,6 +89,7 @@ jobs:
       - run: *apt_install
       - run: apt-get install -y --no-install-recommends cppcheck g++ llvm-dev
       - checkout
+      - run: other/analysis/check_includes
       - run: other/analysis/check_logger_levels
       - run: other/analysis/run-check-recursion
       - run: other/analysis/run-clang

.github/workflows/ci.yml

@@ -17,10 +17,8 @@ jobs:
         run: pip install mypy
       - name: Run mypy
         run: |
-          mypy --strict \
-            $(echo $(find . -name "*.py" -and -not -name "conanfile.py") \
-                   $(grep -lR '^#!.*python3' .) \
-              | tr ' ' '\n' | sort -u | tr '\n' ' ')
+          (find . -name "*.py" -and -not -name "conanfile.py"; grep -lR '^#!.*python') \
+              | xargs -n1 -P8 mypy --strict
 
   build-msan:
     runs-on: ubuntu-latest

.gitignore

@@ -2,6 +2,7 @@
 .DS_Store
 .DS_Store?
 ._*
+.mypy_cache
 .Spotlight-V100
 .Trash*
 Icon?

other/analysis/check_includes

@@ -0,0 +1,57 @@
+#!/usr/bin/env python3
+import subprocess
+import sys
+from typing import Tuple
+
+ALLOWLIST: Tuple[str, ...] = (
+    # system headers
+    "pthread.h",
+    "stdarg.h",
+    "stdbool.h",
+    "stddef.h",
+    "stdint.h",
+    # toxav stuff, maybe not worth abstracting away
+    "opus.h",
+    "vpx/vp8cx.h",
+    "vpx/vp8dx.h",
+    "vpx/vpx_decoder.h",
+    "vpx/vpx_encoder.h",
+    "vpx/vpx_image.h",
+)
+
+out = (subprocess.run(
+    [
+        "grep", "-R", "^#include <.*>", "other", "toxav", "toxcore",
+        "toxencryptsave"
+    ],
+    check=True,
+    capture_output=True,
+).stdout.decode("utf-8").rstrip())
+
+errors = 0
+for line in out.split("\n"):
+    # other/fun can do what it wants.
+    if "/fun/" in line:
+        continue
+    filename, include = line.split(":", 1)
+    # We only check headers.
+    if not filename.endswith(".h"):
+        continue
+    # ccompat.h can include some things we don't really want elsewhere.
+    allowlist = ALLOWLIST
+    if filename == "toxcore/ccompat.h":
+        allowlist += ("alloca.h", "malloc.h", "stdlib.h")
+    header = include[include.index("<") + 1:include.index(">")]
+    if header not in allowlist:
+        source = filename[:-2] + ".c"
+        print(
+            f"{filename}: includes system header <{header}>, which is not allowed in .h files"
+        )
+        print(
+            " " * len(filename) +
+            f"  consider including it in {source} and exporting an abstraction, instead"
+        )
+        errors += 1
+
+if errors:
+    sys.exit(1)

other/analysis/check_recursion

@@ -27,7 +27,7 @@ def load_callgraph() -> Dict[str, List[str]]:
     Returns graph as dict[str, list[str]] containing nodes with their outgoing
     edges.
     """
-    graph = collections.defaultdict(set)
+    graph: Dict[str, Set[str]] = collections.defaultdict(set)
     cur = None
     for line in fileinput.input():
         found = re.search("Call graph node for function: '(.*)'", line)

toxav/audio.c

@@ -4,6 +4,7 @@
  */
 #include "audio.h"
 
+#include <assert.h>
 #include <stdlib.h>
 #include <string.h>
 

toxav/msi.h

@@ -5,8 +5,8 @@
 #ifndef C_TOXCORE_TOXAV_MSI_H
 #define C_TOXCORE_TOXAV_MSI_H
 
-#include <inttypes.h>
 #include <pthread.h>
+#include <stdint.h>
 
 #include "audio.h"
 #include "video.h"

toxcore/ccompat.h

@@ -8,7 +8,6 @@
 #ifndef C_TOXCORE_TOXCORE_CCOMPAT_H
 #define C_TOXCORE_TOXCORE_CCOMPAT_H
 
-#include <assert.h>
 #include <stdbool.h>
 
 bool unused_for_tokstyle(void);

toxcore/onion_client.c

@@ -9,6 +9,7 @@
  */
 #include "onion_client.h"
 
+#include <assert.h>
 #include <stdlib.h>
 #include <string.h>