Upgrade to C++20 modules (#43)

Author: gf712

.github/workflows/premerge.yml

@@ -49,6 +49,11 @@ jobs:
       with:
         create-symlink: true
 
+    - name: Upgrade CMake
+      run: |
+        sudo apt-get purge cmake
+        sudo snap install cmake --classic
+
     - name: Configure CMake
       run: cmake --preset ${{env.CMAKE_PRESET}}
 

CLAUDE.md

@@ -9,8 +9,9 @@ This is an experimental Python 3.9-compatible interpreter implementation in C++.
 ## Build System
 
 ### Prerequisites
-- CMake 3.25+
-- C++23 compiler
+- CMake 3.30+ (`CMAKE_EXPERIMENTAL_CXX_IMPORT_STD`, used for `import std`)
+- A C++26 compiler supporting C++20 named modules and `import std`
+  (built and tested with GCC 16)
 - LLVM 23+ with MLIR (required for MLIR backend)
 - GMP (GNU Multiple Precision library)
 - ICU (International Components for Unicode)

CMakeLists.txt

@@ -1,15 +1,29 @@
-cmake_minimum_required(VERSION 3.25)
+cmake_minimum_required(VERSION 3.30)
 include(FetchContent)
 include(ExternalProject)
 include(CheckCXXSourceCompiles)
 
+set(CMAKE_EXPERIMENTAL_CXX_IMPORT_STD "f35a9ac6-8463-4d38-8eec-5d6008153e7d")
+
 project(python++)
 
 set(CMAKE_CXX_STANDARD 26)
+set(CMAKE_CXX_SCAN_FOR_MODULES OFF)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 include(cmake/CPM.cmake)
 
-CPMAddPackage("gh:gabime/[email protected]")
+# spdlog 1.11+ can format through std::format instead of its bundled fmt.
+# SPDLOG_USE_STD_FORMAT keeps fmt out of the build entirely, which matters for
+# modules: spdlog/fmt/fmt.h drags 231 libstdc++ headers, and anything reaching a
+# module's global module fragment lands in its BMI and then collides with the
+# same headers #included by consumers.
+CPMAddPackage(
+  NAME spdlog
+  GITHUB_REPOSITORY gabime/spdlog
+  VERSION 1.15.3
+  OPTIONS "SPDLOG_USE_STD_FORMAT ON" "SPDLOG_BUILD_PIC ON"
+)
 CPMAddPackage("gh:google/[email protected]")
 CPMAddPackage("gh:jarro2783/[email protected]")
 CPMAddPackage("gh:Tessil/[email protected]")

cmake/PythonCppFlags.cmake

@@ -1,4 +1,5 @@
-# Helper for giving every first-party target the same compiler flags.
+# Helper for giving every first-party target the same compiler flags and the
+# same C++ module configuration.
 #
 # The flags come from the external `project_options` package (added with CPM in
 # the top-level CMakeLists.txt), which exposes them as two INTERFACE targets:
@@ -16,6 +17,19 @@
 # not the usage requirements of libraries linked afterwards. Linking the flags
 # to `<name>` alone would therefore silently compile nothing with them, so this
 # always covers the `obj.<name>` twin as well.
+#
+# The same `obj.<name>` split applies to C++ module settings, and there it is
+# easier to miss: `CXX_SCAN_FOR_MODULES` set on `<name>` does not reach the
+# object library that actually compiles the sources, so those sources are built
+# by CMake's "unscanned" rule with no `-fmodule-mapper`. A TU that imports
+# `py.runtime` then fails with either "'import' does not name a type" or, worse,
+# a fallback lookup in `gcm.cache/`. Setting the properties on both twins is
+# what makes `import py.runtime;` work inside the MLIR layer.
+#
+# Note: do NOT add `-fmodules` here. CMake supplies `-fmodules-ts` together with
+# `-fmodule-mapper=` on its scanned compile rules; adding the flag by hand also
+# applies it to unscanned targets, which turns a clear diagnostic into a
+# confusing module-not-found error.
 
 include_guard(GLOBAL)
 
@@ -28,8 +42,26 @@ function(python_cpp_link_project_options)
       get_target_property(type ${name} TYPE)
       if(type STREQUAL "INTERFACE_LIBRARY")
         target_link_libraries(${name} INTERFACE project_options project_warnings)
-      else()
-        target_link_libraries(${name} PRIVATE project_options project_warnings)
+        continue()
+      endif()
+
+      target_link_libraries(${name} PRIVATE project_options project_warnings)
+
+      # Everything first-party either provides or consumes `py.runtime`, so
+      # scan it all. Scanning costs ~0.16s per TU (a preprocess-only pass) and
+      # removes a whole class of "this target cannot see the module" failures.
+      set_target_properties(${name} PROPERTIES CXX_SCAN_FOR_MODULES ON
+                                               CXX_MODULE_STD ON)
+
+      # Module imports resolve through link dependencies, so every consumer
+      # needs a path to python-runtime - the sole provider of `py.runtime`.
+      # It is linked directly rather than via python-cpp because python-cpp and
+      # python-mlir are mutually dependent: a module provider reached only
+      # through a link cycle cannot be ordered before its consumers, and they
+      # compile with an empty module map. python-runtime itself sits below that
+      # cycle, so linking it here is always acyclic.
+      if(TARGET python-runtime AND NOT ${target} STREQUAL "python-runtime")
+        target_link_libraries(${name} PRIVATE python-runtime)
       endif()
     endforeach()
   endforeach()

integration/program.cpp

@@ -1,19 +1,17 @@
-#include "executable/Program.hpp"
-#include "executable/bytecode/Bytecode.hpp"
-#include "interpreter/Interpreter.hpp"
-#include "parser/Parser.hpp"
-#include "runtime/PyDict.hpp"
-#include "runtime/PyFrame.hpp"
-#include "runtime/PyInteger.hpp"
-#include "runtime/PyList.hpp"
-#include "runtime/PyNumber.hpp"
-#include "runtime/PyObject.hpp"
-#include "runtime/PyString.hpp"
-#include "runtime/PyTuple.hpp"
-#include "runtime/types/builtin.hpp"
-#include "vm/VM.hpp"
+#include "core.hpp"
+#include "executable/common.hpp"
 
 #include "gtest/gtest.h"
+#include <gmpxx.h>
+#include <spdlog/spdlog.h>
+
+#include <cmath>
+
+import py.ast;
+import py.types;
+import py.lexer;
+import py.runtime;
+import std;
 
 using namespace py;
 

integration/tests/slots_uninitialised.py

@@ -0,0 +1,59 @@
+# An unset __slots__ entry must read as unset, however the slot storage was recycled.
+#
+# The storage lives in extra bytes past the object, and both the GC and the member
+# accessor test each entry against null. Slab memory is poisoned rather than zeroed,
+# so if the allocator does not clear those bytes an unset slot reads back as a
+# non-null garbage pointer: the accessor returns it instead of raising AttributeError,
+# and the GC dereferences it.
+
+
+class C:
+    __slots__ = ("a", "b", "c")
+
+
+def unset_raises(obj, name):
+    try:
+        getattr(obj, name)
+    except AttributeError:
+        return True
+    else:
+        return False
+
+
+c = C()
+c.a = 1
+assert c.a == 1
+assert unset_raises(c, "b"), "unset slot 'b' should raise AttributeError"
+assert unset_raises(c, "c"), "unset slot 'c' should raise AttributeError"
+
+# Churn so that later instances land on slots that were freed and poisoned.
+for i in range(2000):
+    x = C()
+    x.a = i
+    x.b = i
+    x.c = i
+
+for i in range(2000):
+    y = C()
+    y.a = i
+    assert y.a == i
+    assert unset_raises(y, "b"), "recycled slot 'b' should still read as unset"
+    assert unset_raises(y, "c"), "recycled slot 'c' should still read as unset"
+
+# Slots that are set must survive a collection with their values intact. The list is a
+# heap object reachable only through the slot, so the GC has to trace the slot correctly.
+kept = []
+for i in range(500):
+    z = C()
+    z.a = i
+    z.b = [i, i + 1]
+    kept.append(z)
+
+i = 0
+for z in kept:
+    assert z.a == i
+    assert z.b == [i, i + 1]
+    assert unset_raises(z, "c")
+    i += 1
+
+print("slots_uninitialised: ok")