Skip to content

Commit b4f7590

Browse files
authored
0.2.6:生成的头必须能被单独 include,以及给「每条规则都编译过」补分母 (#10)
* fix(spirv): 生成的头必须能被单独 include 沙箱里一个把生成头放在第一行的程序: tri_vert.h:3:7: error: 'uint32_t' does not name a type glslc 那条路早就是自包含的 —— 它发的是初始化列表,所以规则本来就要在外面写声明,顺手 写了 `#pragma once` 与 `#include <cstdint>`。glslang 那条路发的是**完整的 C 声明**, 于是规则什么都不写,而那个文件点名 `uint32_t` 却什么都不 include。 **两个编译器产出「等价的头」正是这条规则可以在它们之间选择的前提**,而「等价」必须包括 这一条。此前每个能用的消费者都在它前面放了一个 Vulkan 头,所以一个不完整的头读起来一直 像是好的 —— 直到有人第一个 include 它。 改成两条路同一个形状:编译器写 `<base>.inc`,规则写 `<base>.h`。 判据是 spirv-consumer 的 `src/main.cpp`:生成的头是**第一个** include,`<cstdint>` 不再放在它前面 —— 放在前面就是替这个头满足了编译器,而那正好盖住要测的性质。实测:把 `#include <cstdint>` 从生成器里拿掉,这条当场变红。 * ci: 给「每条规则都为本宿主编译过」这条判据补它自己的分母 这个夹具断言的是「**每一条**规则都编译过」,而「每一条」是它自己carry的一张清单 —— 于是给这个包加第七个成员时,一条名字说它已经被覆盖的步骤会继续绿。 所以在构建之前先把那张清单与包自己的 `[features]` 比一次,两者必须相等。 实测两条腿:相等时通过;从夹具里删掉 `rules-hip` 当场变红。 * 0.2.6
1 parent e75e4f5 commit b4f7590

4 files changed

Lines changed: 93 additions & 22 deletions

File tree

.github/workflows/ci.yml

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,24 @@ jobs:
201201
# every rule returns before looking for a payload and nothing is
202202
# downloaded; what it asserts is that all six modules compile here.
203203
- name: every rule module compiles for this host
204-
working-directory: tests/all-rules-compile
205204
run: |
206205
set -e
206+
# THE FIXTURE'S OWN DENOMINATOR. It asserts "every rule compiles
207+
# here", and "every" is a list it carries -- so a seventh member
208+
# added to this package would be covered by a step whose name says it
209+
# already is. The list is therefore compared against the package's
210+
# own `[features]` before the build, and the two must be equal.
211+
feats=$(sed -n '/^\[features\]/,/^# /p' mcpp.toml \
212+
| grep -oE '^[a-z-]+ +=' | sed 's/ *=//' | grep -v '^default$' | sort)
213+
used=$(sed -n '/features = \[/,/\], host-module/p' tests/all-rules-compile/mcpp.toml \
214+
| grep -oE '"[a-z-]+"' | tr -d '"' | sort)
215+
[ -n "$feats" ] || { echo "FAIL: read no features out of mcpp.toml"; exit 1; }
216+
[ "$feats" = "$used" ] || {
217+
echo "FAIL: the fixture does not name every published feature"
218+
echo " package: $(echo $feats)"
219+
echo " fixture: $(echo $used)"
220+
exit 1; }
221+
cd tests/all-rules-compile
207222
"$MCPP" build
208223
"$MCPP" run | tee run.log
209224
grep -q '^all-rules-compile ok' run.log
@@ -470,9 +485,24 @@ jobs:
470485
# `no member named 'popen' in the global namespace`, reported against a
471486
# consumer that had nothing to do with it.
472487
- name: every rule module compiles for this host
473-
working-directory: tests/all-rules-compile
474488
run: |
475489
set -e
490+
# THE FIXTURE'S OWN DENOMINATOR. It asserts "every rule compiles
491+
# here", and "every" is a list it carries -- so a seventh member
492+
# added to this package would be covered by a step whose name says it
493+
# already is. The list is therefore compared against the package's
494+
# own `[features]` before the build, and the two must be equal.
495+
feats=$(sed -n '/^\[features\]/,/^# /p' mcpp.toml \
496+
| grep -oE '^[a-z-]+ +=' | sed 's/ *=//' | grep -v '^default$' | sort)
497+
used=$(sed -n '/features = \[/,/\], host-module/p' tests/all-rules-compile/mcpp.toml \
498+
| grep -oE '"[a-z-]+"' | tr -d '"' | sort)
499+
[ -n "$feats" ] || { echo "FAIL: read no features out of mcpp.toml"; exit 1; }
500+
[ "$feats" = "$used" ] || {
501+
echo "FAIL: the fixture does not name every published feature"
502+
echo " package: $(echo $feats)"
503+
echo " fixture: $(echo $used)"
504+
exit 1; }
505+
cd tests/all-rules-compile
476506
"$MCPP" build
477507
"$MCPP" run | tee run.log
478508
grep -q '^all-rules-compile ok' run.log

mcpp.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
[package]
22
name = "plugins"
33
namespace = "mcpp"
4-
version = "0.2.5"
4+
version = "0.2.6"
55
description = "Official mcpp build plugins: rule packages under mcpp.rules.*, build-time utilities under mcpp.tools.*, each member selected by a feature"
66
license = "Apache-2.0"
77
authors = ["mcpp-community"]

rules/spirv.cppm

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@
3636
// lines are not interchangeable. glslang's `-x --vn <name>` emits a COMPLETE C
3737
// declaration -- `const uint32_t <name>[] = { ... };` -- while glslc's
3838
// `-mfmt=c` emits a BARE INITIALISER LIST, `{ ... }`, which is not a
39-
// translation unit on its own. A rule that takes both therefore has to write
40-
// the declaration around glslc's output itself, which `wrap_glslc_output`
41-
// below does.
39+
// translation unit on its own. So on both routes the compiler's output is a
40+
// `<base>.inc` and the PUBLIC header `<base>.h` is written by this rule --
41+
// `write_header` below. It has to be that way round for a second reason: a
42+
// generated header must be includable on its own, and glslang's complete
43+
// declaration names `uint32_t` while including nothing.
4244
//
4345
// An earlier revision of this file supported glslang alone, and said why:
4446
// nothing in this ecosystem published glslc, "and a route with no payload
@@ -447,20 +449,47 @@ inline std::vector<std::string> device_shaders() {
447449
// plan time, before any action runs. Its content does not depend on the
448450
// shader's text, which is why nothing has to re-derive it when the shader
449451
// changes: ninja rebuilds the `.inc`, the `#include` picks it up.
450-
inline bool wrap_glslc_output(const std::string& header, const std::string& inc,
451-
const std::string& sym) {
452+
// THE PUBLIC HEADER IS WRITTEN BY THE RULE ON BOTH ROUTES, AND IT HAS TO BE
453+
// INCLUDABLE ON ITS OWN.
454+
//
455+
// glslc emits an initialiser list, so a declaration had to be written around it
456+
// and that header was self-contained by construction. glslang's `-x --vn` emits
457+
// a complete C declaration, so the rule wrote nothing -- and that file names
458+
// `uint32_t` while including nothing:
459+
//
460+
// tri_vert.h:3:7: error: 'uint32_t' does not name a type
461+
//
462+
// Measured in a sandbox, on a program whose first include was the generated
463+
// header. Every consumer that had worked put a Vulkan header ahead of it, which
464+
// is why an incomplete header read as a working one for as long as nobody
465+
// included it first. Two compilers producing an EQUIVALENT header is the whole
466+
// premise of this rule choosing between them, and "equivalent" has to include
467+
// this.
468+
//
469+
// So both routes now produce the same two files: `<base>.inc` from the
470+
// compiler, and `<base>.h` from here.
471+
inline bool write_header(const std::string& header, const std::string& inc,
472+
const std::string& sym, flavour kind) {
452473
std::ofstream out{header, std::ios::trunc};
453474
if (!out) {
454475
std::cerr << std::format("mcpp.rules.spirv: cannot write {}", header) << '\n';
455476
return false;
456477
}
457-
out << "// Generated by mcpp.rules.spirv. glslc emits an initialiser list;\n"
458-
"// this declaration is what makes it a translation unit.\n"
478+
const auto incName = std::filesystem::path(inc).filename().string();
479+
out << "// Generated by mcpp.rules.spirv.\n"
459480
"#pragma once\n"
460-
"#include <cstdint>\n"
461-
"static const uint32_t " << sym << "[] =\n"
462-
"#include \"" << std::filesystem::path(inc).filename().string() << "\"\n"
463-
";\n";
481+
"#include <cstdint>\n";
482+
if (kind == flavour::glslc) {
483+
out << "// glslc emits an initialiser list; this declaration is what makes it\n"
484+
"// a translation unit.\n"
485+
"static const uint32_t " << sym << "[] =\n"
486+
"#include \"" << incName << "\"\n"
487+
";\n";
488+
} else {
489+
out << "// glslang emits a complete `const uint32_t " << sym << "[]`; what this\n"
490+
"// adds is the type it names and a guard.\n"
491+
"#include \"" << incName << "\"\n";
492+
}
464493
return out.good();
465494
}
466495

@@ -573,12 +602,14 @@ inline bool compile(std::span<const std::string> shaders, options opt = {}) {
573602
// named strings for the life of the statement that submits.
574603
const std::string id = "spirv:" + src;
575604
const std::string desc = std::string(cc.name()) + " " + src;
576-
// For glslc the action's output is the initialiser list; for glslang it
577-
// is the header itself.
605+
// ONE SHAPE FOR BOTH ROUTES: the compiler writes `<base>.inc` and this
606+
// rule writes `<base>.h` around it. glslang used to write the header
607+
// itself, which made the two routes' headers differ in whether they
608+
// could be included first -- see `write_header`.
578609
const std::string inc = base + ".inc";
579-
const std::string output = cc.kind == flavour::glslc ? inc : header;
610+
const std::string output = inc;
580611

581-
if (cc.kind == flavour::glslc && !wrap_glslc_output(header, inc, sym)) return false;
612+
if (!write_header(header, inc, sym, cc.kind)) return false;
582613

583614
mcpp::action a;
584615
a.id = id.c_str();

tests/spirv-consumer/src/main.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,19 @@
1-
// The generated header declares `const uint32_t scale_comp_spv[]`; the name
2-
// is the shader's stem, its stage, and `_spv`, as mcpp.rules.spirv documents.
3-
#include <cstdint>
4-
#include <cstdio>
1+
// THE GENERATED HEADER IS THE FIRST INCLUDE, AND THAT ORDER IS THE ASSERTION.
2+
//
3+
// It declares `const uint32_t scale_comp_spv[]` -- the name is the shader's
4+
// stem, its stage, and `_spv`, as mcpp.rules.spirv documents -- and a header
5+
// that names a type has to bring it. Putting `<cstdint>` above this line would
6+
// satisfy the compiler on behalf of the header and hide whether the header can
7+
// stand on its own; every consumer that had worked did exactly that, with a
8+
// Vulkan header in front, and a sandbox found the state that leaves:
9+
//
10+
// tri_vert.h:3:7: error: 'uint32_t' does not name a type
11+
//
12+
// in a build the project did not write.
513
#include "scale_comp.h"
614

15+
#include <cstdio>
16+
717
int main() {
818
const std::uint32_t magic = scale_comp_spv[0];
919
std::printf("magic=%08x words=%zu\n", magic, sizeof scale_comp_spv / sizeof scale_comp_spv[0]);

0 commit comments

Comments
 (0)