Skip to content

Commit 3807d91

Browse files
committed
fix(scanner, host modules, toolchain, config, bench): five open issues, and three of the reports were wrong about the cause
#606 THE SCANNER READ INSIDE COMMENTS, IN BOTH DIRECTIONS. Block-comment state never existed in the file's history -- `git log -S` returns no such commit -- so the bisect to "after 2026.9.7.1" dates when the defect became audible, not when it was introduced; the refusal added at 2026.9.9.1 is correct. The unreported form is worse: `export module y;` inside a block comment made a plain `.cpp` the recorded producer of `y.gcm`, and a real importer of `y` was then told `imports must be built before being imported`. And the OTHER direction, also unreported: a commented-out raw-string opener blanked every following line, so real imports were invisible to the scanner and visible to the compiler -- a missing dependency edge, which is a build-order race rather than a deterministic refusal. One pass over three states. Eight criteria in tests/e2e/639, two of them properties that were already correct and must not be lost; six of the eight go red against released 2026.9.10.2. #604 A TWO-TOKEN SWITCH LOST ITS SWITCH. The host-module flag collector de-duplicated per token and was written for GCC's one-token `-fmodules`. MSVC's reference is a pair whose first half legitimately repeats, so the pair lost `/reference` and cl read `<name>=<path>` as a source file (C1083). Appended verbatim now; the comment being replaced stated the filter's whole value ("harmless but noisy"). Plus `orphaned_reference` as a reader that names the cause before the command runs. #603 THE LEVEL IS A PROPERTY OF THE STL. Calling the existing probe from clang's path would have compared a clang version against an MSVC threshold. The toolset version is in the path of the `std.ixx` already selected; one function, both paths, and a test that the two forms agree for a well-formed installation. #564 `default_jobs` gained a reader and `default_backend` was removed. Two dead keys wanting opposite answers: one names a machine fact with no other home, the other promises a choice that does not exist. The test asserts the precedence, not the wiring. #599 A CHECK THAT HAD RUN IN ZERO CI JOBS. Three defects, not the two reported: the hub path was written for the current layout while naming a historical tree, the `uninit` branch printed a note that cannot turn a job red, and there is no bench workflow at all -- so the test's own justification for the note was false. All three fixed, and the note now fails under CI while staying a note locally.
1 parent 4d5077b commit 3807d91

25 files changed

Lines changed: 948 additions & 81 deletions

.agents/docs/2026-09-11-six-open-issues-analysis.md

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,3 +410,124 @@ leaving it in place:
410410
5. **#597** -- graduating a row that already exists, in the batch that added it.
411411

412412
The first four are one release. #597 belongs to the platform batch.
413+
414+
## 8. Self-review, and the one plan a measurement changed
415+
416+
Written after §1-§7 and before any implementation. Four of the six plans
417+
survive unchanged. One is wrong, one has an unstated cost, and the review
418+
found the defect the plan for #606 would have half-fixed.
419+
420+
### 8.1 #606: the defect is bidirectional, and the other direction is worse
421+
422+
§1 proposed "a `bool in_block` carried across iterations, with the stripping
423+
done before `strip_line_comment`", and gave as a criterion that
424+
`/* */ import x;` on one line "still records the import". Both are wrong.
425+
426+
Measured on 2026.9.10.2, by whether mcpp emits its own
427+
`imported but not provided` warning (which only the scanner can produce, so it
428+
separates "the scanner saw it" from "the compiler saw it"):
429+
430+
| source | scanner | correct |
431+
|---|---|---|
432+
| `import x;` | sees it | sees it |
433+
| `const char* s = "a /* b";` then `import x;` | sees it | sees it |
434+
| `/* */ import x;` | **misses it** | sees it |
435+
| `// R"(` then `import x;` | **misses it** | sees it |
436+
| `/*`, `R"(`, `*/` then `import x;` | **misses it** | sees it |
437+
| `/*`, `export module y;`, `*/` | records a phantom producer | ignores it |
438+
| `/*`, `module (exe)`, `*/` | refuses the build | ignores it |
439+
440+
So `/* */ import x;` is not a behaviour to preserve -- it is a fourth wrong
441+
answer. And two of the wrong answers run in the **opposite** direction to the
442+
reported one: a `//`-commented or block-commented raw-string opener puts
443+
`strip_raw_strings` into raw mode, which blanks every following line until a
444+
`)"` that never comes, and real declarations after it are invisible to the
445+
scanner while remaining visible to the compiler.
446+
447+
A missed `import` is worse in kind than a refused build. It is a **missing
448+
dependency edge**: the compile is not ordered after the BMI it needs, so the
449+
failure is a build-order race that appears under parallelism as
450+
`failed to read compiled module` and disappears on a retry. #606's reported
451+
form is at least deterministic.
452+
453+
The two directions have one cause. The scanner has three lexical states --
454+
code, block comment, raw string -- which are mutually exclusive and decided by
455+
whichever opener comes first. It implements one and a half: raw strings fully,
456+
line comments as an unconditional `find("//")`, block comments not at all, and
457+
the three passes run in a fixed order that cannot express "whichever came
458+
first". Fixing block comments alone, in either order relative to the existing
459+
passes, produces one of the two wrong directions:
460+
461+
* strip comments first, and `R"( /* )"` opens a comment inside a string;
462+
* strip raw strings first, and `// R"(` opens a string inside a comment --
463+
which is the defect measured above.
464+
465+
### 8.2 The revised plan for #606
466+
467+
One pass over the line with the three states, replacing `strip_raw_strings` and
468+
`strip_line_comment` at the call site. It blanks non-code and preserves
469+
offsets, so the reported column stays correct. State carried across lines is
470+
what it already is (`in_raw`, `raw_close`) plus `in_block`.
471+
472+
Not a lexer: character and string literals need no tokenising, because the only
473+
question asked of the result is whether the trimmed line *starts with* a
474+
keyword, and an ordinary `"..."` cannot begin a line with one. The one thing
475+
the pass must respect about them is `"a /* b"` -- a `/*` inside an ordinary
476+
string must not open a comment -- which is one state, not a literal parser.
477+
478+
Criteria, one per row of the table above, with the last two being the pair that
479+
separates a fix from a mute:
480+
481+
* the phantom-producer case generates a graph with no `gcm.cache/y.gcm` output;
482+
* the four-line file from the report builds;
483+
* `/* */ import x;` records the import -- a *new* property, and the one that a
484+
cheap "skip any line starting with `/*`" would fail;
485+
* `"a /* b"` then `import x;` still records the import -- currently correct by
486+
luck, and the property that stops the fix from treating every `/*` as an
487+
opener.
488+
489+
### 8.3 #603: one function, and the two answers must be measured to agree
490+
491+
§3 left open whether the MSVC path keeps the cl banner. It must not: two
492+
readers of one question is what this codebase treats as the defect, and the
493+
`std.ixx` path is the better input on both paths, because it describes the STL
494+
that will actually be compiled rather than the one a fresh search finds first.
495+
The unit test therefore asserts that for a well-formed VC layout the path
496+
answer equals what the banner answer would have been -- otherwise the change
497+
is a silent behaviour change on the one path that was verified.
498+
499+
### 8.4 #599: the cost of running the check is not stated
500+
501+
§4 proposes `submodules: true` on the checkout of whichever shard runs `233`.
502+
The bench workloads are pinned full source trees of mcpp and xlings, so this is
503+
not free, and the plan does not say what it costs. Measure before choosing;
504+
if it is large, the cheaper shape is a job that checks out **only**
505+
`bench/projects` and runs `233` alone, since the check needs trees and no
506+
toolchain at all.
507+
508+
### 8.5 #604 and #564 stand, with one narrowing each
509+
510+
#604: append verbatim, and de-duplicate nothing. The alternatives considered --
511+
de-duplicate by logical module name, or by contiguous subsequence -- are both
512+
correct and both add a rule to keep an argv tidy. The rule being removed was
513+
wrong; replacing it with a better rule for the same cosmetic purpose is the
514+
kind of trade this codebase records as a mistake. The comment says the repeat
515+
is harmless; the fix should rely on that sentence rather than work around it.
516+
517+
#564: the e2e must assert the *precedence*, not the plumbing. A fixture that
518+
only sets `default_jobs` and reads `-j3` would pass if the global value were
519+
wired in above `MCPP_JOBS` instead of below it. Two invocations of one
520+
fixture, with and without `MCPP_JOBS`, is the smallest thing that distinguishes
521+
them.
522+
523+
### 8.6 #597 stands, and route A now makes a second row measurable
524+
525+
Unchanged. Noted here because the platform record's Android rows were resolved
526+
by the same kind of measurement in the same session: `qemu-aarch64 -L <root
527+
extracted from the system image>` executes the **default, dynamic**
528+
configuration for `aarch64-linux-android`, with `libc++_shared.so` supplied
529+
from the NDK's own directory outside the `-L` prefix. The emulator route is
530+
refuted for every build the vendor manifest currently serves, measured across
531+
all four Linux host entries rather than the pinned one. So both remaining
532+
platform rows are executable on an x86_64 Linux runner with no device and no
533+
virtualization, which is what a CI lane needs.

.github/actions/setup-macos-llvm/action.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ runs:
107107
108108
[build]
109109
default_jobs = 0
110-
default_backend = "ninja"
111110
EOF
112111
113112
cat "$HOME/.mcpp/config.toml"

.github/workflows/ci-linux-e2e.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,22 @@ jobs:
4242
# set only in the fresh-install workflows (cold bootstrap, no such asserts).
4343
# A specific test that needs verbose passes `--verbose` itself.
4444
steps:
45+
# `submodules: recursive` so tests/e2e/233_bench_matrix.sh can check that
46+
# each `hub`/`body` in bench/matrix.json exists in the tree it names.
47+
# Without the trees that check reads "submodule not initialised" and
48+
# reports nothing, which is how a hub path written for the CURRENT
49+
# layout stayed in matrix.json while the workload it named is a
50+
# HISTORICAL mcpp -- three cells reporting `skipped` on every bench run
51+
# and the job still green (#599).
52+
#
53+
# It is not free and it is not expensive: the three pinned workloads are
54+
# 725 + 701 + 806 tracked files, under 10 MB of source in total, and
55+
# nothing here builds them. Both shards carry it because run_all.sh
56+
# slices the file list round-robin, so which shard holds 233 moves when
57+
# a test is added.
4558
- uses: actions/checkout@v4
59+
with:
60+
submodules: recursive
4661

4762
# Same cache lineage as ci-linux.yml so this job lands on a warm
4863
# toolchain/sandbox instead of re-installing it.

CHANGELOG.md

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,112 @@
77

88
## [2026.9.11.2] - 2026-09-11
99

10+
### 扫描器读到了注释里面,而且是双向的
11+
12+
**报告的那半。** `/*` 单独占一行、下一行是 `module (`,那一行被当成模块声明匹配,
13+
四行普通 C++ 被拒绝(#606)。报告把它二分到「2026.9.7.1 之后的回归」。实测:
14+
`git log -S``src/modgraph/scanner.cppm` 上返回**零个**曾添加块注释状态的提交 ——
15+
它从来没有过。2026.9.9.1(#594)加的畸形名拒绝是**对的**,二分定位到的是缺陷**变响**
16+
的时间。
17+
18+
**没被报告、而更坏的那半。** 名字合法时那条拒绝不会触发,结果是:块注释里的
19+
`export module y;` 让一个普通 `.cpp` 被记录成 `gcm.cache/y.gcm`**生产者**,承诺一个
20+
编译器永不写出的 BMI。真正 `import y;` 的文件于是被告知 `imports must be built
21+
before being imported` —— 一个并不存在的顺序问题,而真正的提供者从未被查找。
22+
23+
**以及反方向。** `// R"(` 让 raw string 那一遍进入它出不来的状态,把之后每一行都
24+
抹空到一个永不出现的 `)"`;`import x;` 对扫描器不可见而对编译器可见。**那是一条
25+
缺失的依赖边** —— 并行下的构建顺序竞争,重试就好,比报告的那条更坏。`/* */ import
26+
x;` 同样被漏掉,是报告没有提到的第四个错答案。
27+
28+
根因是一段被当成已完成的论证,写在源码注释里:「普通 `"..."` 字符串故意原样保留:
29+
匹配器只在 trim 后**以关键字开头**的行上触发,而字符串体只有跨行(即 raw string)
30+
才做得到这件事。」前提成立,枚举少了一个 —— **块注释也能**
31+
32+
修法是**一遍走三个状态**:代码、块注释、raw string 互斥,由先出现的那个开启符决定,
33+
这是任何固定顺序的分遍都表达不了的。判据八条(`tests/e2e/639`),其中两条是
34+
「原本就对、不能弄坏」的:`/* */ import x;` 必须**看见**那个 import(否决「跳过任何
35+
`/*` 开头的行」这种哑修法),`"a /* b"` 必须****开启注释。拿已发布的
36+
2026.9.10.2 对照:八条里六条变红。
37+
38+
### 一个两 token 的开关丢掉了它的开关
39+
40+
`windows = "msvc@system"` 下,一个带 `host-module` 构建依赖的包在编译构建程序时失败:
41+
42+
```
43+
c1xx: fatal error C1083: Cannot open source file:
44+
'huxerui.rules.sources=...\huxerui.rules.sources.ifc'
45+
```
46+
47+
`cl.exe` 把模块引用读成了**源文件名**。成因是 host-module 的 flag 收集**按 token**
48+
去重,而它是为唯一一个「单 token 且幂等」的家族写的 —— GCC 的 `-fmodules`:
49+
50+
| 家族 | useFlags | token |
51+
|---|---|---|
52+
| GCC | `-fmodules` | 1,幂等 |
53+
| Clang | `-fmodule-file=<name>=<path>` | 1,唯一 |
54+
| MSVC | `/reference`, `<name>=<path>` | **2,第一个合法重复** |
55+
56+
内层 host module 追加时,`/reference` 已由内置 `mcpp` 模块放进列表 → 被跳过,只追加
57+
了对的后半。Clang 按构造免疫(一个词,永不等于已有元素),所以缺陷专属于那唯一一个
58+
在 Windows 上能在 c++20 达到 `import std;` 的工具链选择(#604)。
59+
60+
**逐字追加,不再去重。** 被替换的那句注释说明了这个过滤器的全部价值:「repeating it
61+
is harmless but noisy」—— 它买的是 argv 整洁,付的是坏命令行。按逻辑模块名去重、
62+
或按连续子序列去重,两者都正确,而两者都是为同一个装饰性目的**新增一条规则**。规则
63+
被删掉了。
64+
65+
另加 `mcpp::toolchain::orphaned_reference`:一个 `<name>=<path>` 前面没有开关时,
66+
在命令跑之前拒绝并说明,而不是从 cl 的 C1083 里去反推。这是纵深防御而不是修复本身。
67+
68+
### `import std;` 的档位问的是 STL,不是碰巧到达它的那个编译器
69+
70+
clang 在 Windows 上回落到 MSVC STL 的 `std.ixx` 时把 `importStdMinLevel` 硬编码成
71+
23,于是一个 c++20 工程在 Windows 上被拒绝,而同样的源码在 Linux 的 GCC 16.1、
72+
Linux 的 llvm 22.1.8 和 macOS 上都能构建(#603)。
73+
74+
那段注释把理由说对了 ——「`tc.version` 是 clang 的,所以它回答不了 cl banner 的问
75+
题」—— 却从中得出了错的结论。**照原样调用已有探针会更坏**:那会拿一个 clang 的版本号
76+
去和 MSVC 的 19.38 门槛比,clang 20.x 侥幸通过、clang 19.x 错误地答 23,两个答案都
77+
来自问错对象。
78+
79+
真正有约束力的版本在**刚刚选中的那个模块源文件的路径里** ——
80+
`<VS>/VC/Tools/MSVC/14.44.35207/modules/std.ixx` —— 而 toolset `14.<N>` 与 cl banner
81+
`19.<N>` 配对,所以现有的 `>= 38` 谓词原样迁移。新增
82+
`std_module_min_level_for_stl(path)`,**两条路径都调它**;单测断言对一个规整的安装,
83+
两种形态给出相同答案 —— 否则这次改动就是对唯一被验证过的那条路径的静默行为变更。
84+
85+
### `[build] default_jobs` 有了读者,`default_backend` 被删掉
86+
87+
两个键都被解析、都没有任何消费者(#564)。它们要的是**相反**的答案。
88+
89+
`default_jobs` 接上:它是三级优先级里唯一能承载**机器事实**的一级
90+
(`MCPP_JOBS` > `[build] jobs` > `default_jobs` > 0)。`--jobs` 每次调用都要重说,
91+
`[build] jobs` 是按包的而 `[workspace.build]` 正确地拒绝它,所以别无他处。它以
92+
**参数**而不是 config import 的形式到达 `resolve_jobs`,因为那个函数刻意只依赖
93+
manifest 和宿主。它**同时**约束 `mcpp test` 的并发,这一点被明写进文档 —— 一个键
94+
两种行为必须明说。单测断言的是**顺序**而不是接线:只设全局值再读回来的夹具,在参数
95+
被接到 `MCPP_JOBS` **之上**时同样会通过。
96+
97+
`default_backend` 删掉:`BackendKind` 有两个值而 `src/build/` 只有一个后端实现,
98+
这个键承诺了一个不存在的选择,而默认值 `"ninja"` 让它看起来是实现了的。四个 e2e
99+
夹具和一个 CI action 各自持有一份生成文件的副本,都同步了。
100+
101+
### bench 的 hub 检查从写下起在 CI 里跑过零次
102+
103+
`matrix.json` 给 mcpp 工作负载写的 hub 是 `modules/platform/src/platform.cppm`,而那个
104+
工作负载是一个**历史** mcpp,该文件在其中位于 `src/platform/platform.cppm`(#599)。
105+
三个 cell 的每一个扰动场景都会报 `skipped` 而 job 照常绿。
106+
107+
报告发现两个缺陷,实际有三个,而第三个吞掉了前两个:**`.github/workflows/` 里没有
108+
bench workflow**,也没有任何 job 检出 submodule。所以 233 自己那句理由 ——「bench
109+
workflow 会检出 submodule 并跑这个测试」—— 是假的。
110+
111+
三处都修:hub 路径按 pin 重读(并在 `matrix.json` 里记下「hub/body 属于 pin 而不属于
112+
本仓库」),`uninit` 分支在 `CI=true`**失败**而在本地打印提示(两类读者的答案相反),
113+
以及 `ci-linux-e2e.yml` 的两个 shard 都带上 `submodules: recursive` —— 三个 pin 合计
114+
2232 个 tracked 文件、不到 10 MB 源码,而且这里什么都不构建。
115+
10116
### 暂存对被分派的格式是**服务**,不是前置条件
11117

12118
`mcpp pack --format <name>` 在分派之前无条件先暂存一次,而暂存失败就让整条命令失

bench/matrix.json

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,23 @@
11
{
22
"schema": 2,
33
"_comment": [
4-
"THE benchmark matrix. Read by .github/workflows/bench.yml to plan its jobs and",
5-
"by tests/e2e/233_bench_matrix.sh to check this file against the harness's own",
6-
"vocabulary. bench/SPEC.md explains the axes; it deliberately does not repeat",
7-
"the cell list, because a matrix written down twice is a matrix that disagrees",
8-
"with itself.",
4+
"THE benchmark matrix. Read by tests/e2e/233_bench_matrix.sh, which checks it",
5+
"against the harness's own vocabulary and against the pinned trees. bench/SPEC.md",
6+
"explains the axes; it deliberately does not repeat the cell list, because a",
7+
"matrix written down twice is a matrix that disagrees with itself.",
8+
"",
9+
"THERE IS NO .github/workflows/bench.yml IN THIS REPOSITORY. This comment used",
10+
"to name one, and so did 233's own justification for printing a note instead of",
11+
"failing -- which is why the hub/body existence check ran in zero CI jobs from",
12+
"the day it was written, and why the three mcpp cells below carried a path from",
13+
"the CURRENT source layout while naming a HISTORICAL tree (#599). 233 now runs",
14+
"with the submodules checked out in ci-linux-e2e.yml and fails rather than notes",
15+
"when they are absent on a runner. Running the benchmark itself is still manual.",
16+
"",
17+
"A `hub` OR `body` PATH BELONGS TO THE PIN, NOT TO THIS REPOSITORY. Each",
18+
"workload is a historical commit, so a path that is correct in the current tree",
19+
"is wrong for it by construction and the next module move breaks it again. When",
20+
"a pin is bumped, re-read these two paths out of the new tree.",
921
"",
1022
"A cell is one CI job. Inside it the harness sweeps every engine x variant x",
1123
"scenario, so those axes are per-cell lists rather than more jobs: they share a",
@@ -133,7 +145,7 @@
133145
"engines": "mcpp,mcpp[schedule=on],cmake,xmake",
134146
"variants": "modules",
135147
"scenarios": "cold,noop,touch-hub,edit-body,edit-comment",
136-
"hub": "modules/platform/src/platform.cppm",
148+
"hub": "src/platform/platform.cppm",
137149
"body": "src/version_req.cppm",
138150
"note": "KNOWN GAP: cmake cannot GENERATE this project on the runner. It configures (the compiler probe passes) and then fails with `CMake Error: the \"CXX_MODULE_STD\" property ... requires that the \"__CMAKE::CXX23\" target exist, but it was not provided by the toolchain. Reason: Only `libstdc++` is supported`. Everything checkable from outside has been checked and every one of them AGREES with a developer box where the same arm configures, generates and builds: same cmake (the very same `xim-x-cmake/4.0.2` xlings payload, not merely the same version), same `xim-x-gcc/16.1.0`, `libstdc++.modules.json` present on the runner, both sources it names (`std.cc`, `std.compat.cc`) present on the runner, and the runner's exact flag shape (`-B<glibc>/lib -L<glibc>/lib`, the branch this machine does not normally take) reproduced locally via a fake MCPP_HOME with no subos — it works there. What is left is inside CMake's own detection. The arm's CMakeConfigureLog is now attached to the cell log on failure so the next look starts from CMake's own record rather than another hypothesis. BASELINE MOVED to the released mcpp for this cell, exactly as the xlings cells do: normalising against an engine that never produced a binary prints bare seconds under a heading that says `relative to`. xmake and both mcpp arms stay measured here. The published cmake numbers in bench/README were taken on a machine where this arm works.",
139151
"buildfiles": "mcpp",
@@ -147,7 +159,7 @@
147159
"engines": "mcpp,mcpp[schedule=on],cmake,xmake",
148160
"variants": "modules",
149161
"scenarios": "cold,noop,touch-hub,edit-body,edit-comment",
150-
"hub": "modules/platform/src/platform.cppm",
162+
"hub": "src/platform/platform.cppm",
151163
"body": "src/version_req.cppm",
152164
"buildfiles": "mcpp",
153165
"allow_failed": "xmake",
@@ -160,7 +172,7 @@
160172
"engines": "mcpp,mcpp[schedule=on],cmake,xmake",
161173
"variants": "modules",
162174
"scenarios": "cold,noop,touch-hub,edit-body,edit-comment",
163-
"hub": "modules/platform/src/platform.cppm",
175+
"hub": "src/platform/platform.cppm",
164176
"body": "src/version_req.cppm",
165177
"buildfiles": "mcpp",
166178
"allow_failed": "cmake,xmake",

0 commit comments

Comments
 (0)