Scan for modules and imports with clang-scan-deps instead of regexes #45

New issue

Open

opened 2026-10-31 22:34:59 +00:00 by jorijnvdgraaf · 0 comments

jorijnvdgraaf commented

2026-10-31 22:34:59 +00:00

Owner

Summary

Configuration::GetInterfacesAndImplementations finds a project's module declarations and imports by running regexes over each source's text, after deleting comments with two more regexes. Those regexes skip neither string literals nor comments that span lines, ignore the preprocessor, and expect exactly one space after import. So a file can gain a module or an import it does not have, or lose one it does. The fix proposed here is clang's own dependency scanner, clang-scan-deps -format=p1689, which preprocesses each file with its real compile command.

Line numbers refer to 909e027.

Details

implementations/Crafter.Build-Clang.cpp:98-99 and :195-196 delete comments with //[^\n]* and /\*.*?\*/. In std::regex, . does not match a newline, so a block comment that spans lines stays in the text.
:110 and :123 find export module X; and export module X:P;, and :201 finds an implementation unit's module X;. :153, :164, :179, :208, :223 and :238 find import X; and import :P;. Every pattern has a single literal space after module or import, and the import patterns allow no space before the ;.

The same patterns, run on a few inputs:

Source text	What the scanner sees
`"export module std;"` inside a string literal	a module declaration, `module std;`
`import Foo;` on its own line inside a `/* … */` that spans lines	an import of `Foo`
`import Foo;` between `#if 0` and `#endif`	an import of `Foo`
`import Foo;` (two spaces) or `import Foo ;`	nothing

A declaration read where there is none becomes a spurious dependency, or a hard error when no such module is in the build. A real import that is missed is a missing edge: on a clean build, the unit can compile before the module it imports has a BMI.

Reproduction

Found in PR #44. tests/MingwSysroot/main.cpp writes a stand-in std module with Write(..., "export module std;\n"). The pattern at :201 matched module std; inside the literal and took the test for an implementation unit of a module std, so crafter-build test MingwSysroot stopped before compiling anything:

Module std not found not found, referenced in .../tests/MingwSysroot/main.cpp

The test now writes that text with std::format("export module {};\n", "std"), with a comment saying why.

Suggested fix

Take each unit's provided and required modules from clang-scan-deps -format=p1689. It runs the preprocessor with the unit's compile command, so strings, comments, spacing, macros and #if come out the way clang sees them when it compiles the file, and it reports partitions as X:P. Both toolchains ship it: /usr/bin/clang-scan-deps on Arch, and /app/bin/clang-scan-deps in the Flatpak.

To settle:

When to scan. GetInterfacesAndImplementations runs inside CrafterBuildProject, often before the project has set its defines and flags, and the scan needs the final compile command. Build() already resolves imports late, in ResolvePendingImports(). The scan could move there, leaving GetInterfacesAndImplementations to record the files.
Cost. One clang-scan-deps per file adds a clang start-up per source to every build. It can also scan a whole compilation database in one process (-compilation-database=… -j N). Results can be kept per file, keyed on its bytes and its command, so unchanged files are not scanned again.
Afterwards: drop the std::format workaround in tests/MingwSysroot/main.cpp, fix the doubled "not found not found" at :235, and pin the behaviour with a test of the four inputs above.

A cheaper alternative was considered and set aside: blank out comments and string and character literals before the regexes, and match declarations only at the start of a line, where C++20 requires them. That fixes the first, second and fourth rows of the table, but not #if.

## Summary `Configuration::GetInterfacesAndImplementations` finds a project's module declarations and imports by running regexes over each source's text, after deleting comments with two more regexes. Those regexes skip neither string literals nor comments that span lines, ignore the preprocessor, and expect exactly one space after `import`. So a file can gain a module or an import it does not have, or lose one it does. The fix proposed here is clang's own dependency scanner, `clang-scan-deps -format=p1689`, which preprocesses each file with its real compile command. Line numbers refer to `909e027`. ## Details - `implementations/Crafter.Build-Clang.cpp:98-99` and `:195-196` delete comments with `//[^\n]*` and `/\*.*?\*/`. In `std::regex`, `.` does not match a newline, so a block comment that spans lines stays in the text. - `:110` and `:123` find `export module X;` and `export module X:P;`, and `:201` finds an implementation unit's `module X;`. `:153`, `:164`, `:179`, `:208`, `:223` and `:238` find `import X;` and `import :P;`. Every pattern has a single literal space after `module` or `import`, and the import patterns allow no space before the `;`. The same patterns, run on a few inputs: | Source text | What the scanner sees | |---|---| | `"export module std;"` inside a string literal | a module declaration, `module std;` | | `import Foo;` on its own line inside a `/* … */` that spans lines | an import of `Foo` | | `import Foo;` between `#if 0` and `#endif` | an import of `Foo` | | `import Foo;` (two spaces) or `import Foo ;` | nothing | A declaration read where there is none becomes a spurious dependency, or a hard error when no such module is in the build. A real import that is missed is a missing edge: on a clean build, the unit can compile before the module it imports has a BMI. ## Reproduction Found in PR #44. `tests/MingwSysroot/main.cpp` writes a stand-in std module with `Write(..., "export module std;\n")`. The pattern at `:201` matched `module std;` inside the literal and took the test for an implementation unit of a module `std`, so `crafter-build test MingwSysroot` stopped before compiling anything: ``` Module std not found not found, referenced in .../tests/MingwSysroot/main.cpp ``` The test now writes that text with `std::format("export module {};\n", "std")`, with a comment saying why. ## Suggested fix Take each unit's provided and required modules from `clang-scan-deps -format=p1689`. It runs the preprocessor with the unit's compile command, so strings, comments, spacing, macros and `#if` come out the way clang sees them when it compiles the file, and it reports partitions as `X:P`. Both toolchains ship it: `/usr/bin/clang-scan-deps` on Arch, and `/app/bin/clang-scan-deps` in the Flatpak. To settle: - **When to scan.** `GetInterfacesAndImplementations` runs inside `CrafterBuildProject`, often before the project has set its defines and flags, and the scan needs the final compile command. `Build()` already resolves imports late, in `ResolvePendingImports()`. The scan could move there, leaving `GetInterfacesAndImplementations` to record the files. - **Cost.** One `clang-scan-deps` per file adds a clang start-up per source to every build. It can also scan a whole compilation database in one process (`-compilation-database=… -j N`). Results can be kept per file, keyed on its bytes and its command, so unchanged files are not scanned again. - **Afterwards:** drop the `std::format` workaround in `tests/MingwSysroot/main.cpp`, fix the doubled "not found not found" at `:235`, and pin the behaviour with a test of the four inputs above. A cheaper alternative was considered and set aside: blank out comments and string and character literals before the regexes, and match declarations only at the start of a line, where C++20 requires them. That fixes the first, second and fourth rows of the table, but not `#if`.

jorijnvdgraaf referenced this issue

2026-10-31 22:35:08 +00:00

fix: give the mingw target a sysroot that holds mingw-w64 alone #44

jorijnvdgraaf referenced this issue from a commit

2026-10-31 22:35:49 +00:00

Merge branch 'claude/issue-43': mingw builds without the host's headers, std modules keyed on their flags

jorijnvdgraaf referenced this issue

2026-11-01 01:01:36 +00:00

Loading a project scans the same sources once per Configuration: a Crafter.Graphics consumer spends ~10 s before anything builds #47