Skip to content

Commit eb4e75b

Browse files
committed
feat: compile .c sources via a separate C rule
mcpp's ninja backend previously routed every non-.cppm source through one `cxx_object` rule that invokes g++ with `-std=c++23 -fmodules`. .c files went through cc1plus, which rejects mainstream C constructs (implicit `void*` conversion from malloc/calloc, the `restrict` keyword, ...) and silently warns on `-std=c++23`. Real-world C libraries — verified against mbedtls 3.6.1's 108 sources — cannot build under that path. Add a parallel C compile path: * `BuildConfig` gains `cflags`, `cxxflags`, `cStandard`. Both the `[build]` TOML section and the `mcpp = {}` xpkg segment parse them, so index entries can carry per-package flags. * The ninja backend probes the plan for `.c` sources; when present it derives a sibling C compiler from the resolved C++ binary (`g++ -> gcc`, `clang++ -> clang`, `c++ -> cc`, with prefix preserved for cross toolchains like `x86_64-linux-musl-gcc`) and emits `cc` / `cflags` / `c_object` alongside the existing `cxx_*` set. `cflags` carries `-std=$c_standard` (default `c11`) plus the shared opt / pic / sysroot / -B / include baseline and the user tail. * Compile-edge dispatch picks `cxx_module` for `.cppm`, `c_object` for `.c`, and `cxx_object` otherwise. dyndep phase-1 skips `.c` so they don't go through P1689 / `-fmodules`; the static-deps fallback skips BMI implicit inputs for the same reason. * Module scanner short-circuits `.c` files with an empty `SourceUnit` to avoid any chance of a benign C identifier (`import_foo`, `module_t`) being misparsed as a module statement. Coverage: two unit tests in `test_manifest.cpp` cover both parse paths, and a new `tests/e2e/26_c_language_support.sh` scaffolds a mixed `.c` + modular-`.cpp` project, asserts the generated `build.ninja` contains the expected `c_object` rule / `cc` variable / extension dispatch, exercises user-supplied `cflags`/`cxxflags` forwarding, and runs the resulting binary end-to-end.
1 parent 92f1335 commit eb4e75b

5 files changed

Lines changed: 282 additions & 17 deletions

File tree

src/build/ninja_backend.cppm

Lines changed: 106 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,39 @@ std::filesystem::path mcpp_exe_path() {
9393
return "mcpp"; // fall back to PATH lookup
9494
}
9595

96+
// Derive a sibling C compiler from a C++ compiler binary path. Used so .c
97+
// sources can be compiled by the actual C frontend (cc1), not g++ which
98+
// rejects implicit `void*` conversions and `restrict` etc.
99+
// .../bin/g++ → .../bin/gcc
100+
// .../bin/x86_64-linux-musl-g++ → .../bin/x86_64-linux-musl-gcc
101+
// .../bin/clang++ → .../bin/clang
102+
// .../bin/c++ → .../bin/cc
103+
// If no sibling exists, return "gcc" so PATH lookup is the final fallback
104+
// (this also keeps unit tests that don't touch a real toolchain happy).
105+
std::filesystem::path derive_c_compiler(const std::filesystem::path& cxx) {
106+
auto fname = cxx.filename().string();
107+
auto try_replace = [&](std::string_view from, std::string_view to)
108+
-> std::optional<std::filesystem::path>
109+
{
110+
auto pos = fname.rfind(from);
111+
if (pos == std::string::npos) return std::nullopt;
112+
std::string repl = fname;
113+
repl.replace(pos, from.size(), to);
114+
auto p = cxx.parent_path() / repl;
115+
std::error_code ec;
116+
if (std::filesystem::exists(p, ec)) return p;
117+
return std::nullopt;
118+
};
119+
if (auto p = try_replace("clang++", "clang")) return *p;
120+
if (auto p = try_replace("g++", "gcc")) return *p;
121+
if (auto p = try_replace("c++", "cc")) return *p;
122+
return "gcc";
123+
}
124+
125+
bool is_c_source(const std::filesystem::path& src) {
126+
return src.extension() == ".c";
127+
}
128+
96129
} // namespace
97130

98131
std::string emit_ninja_string(const BuildPlan& plan) {
@@ -187,9 +220,41 @@ std::string emit_ninja_string(const BuildPlan& plan) {
187220
// TODO(musl-gcc-upstream): remove once musl-gcc@16+ ships.
188221
const char* opt_flag = isMuslTc ? " -Og" : " -O2";
189222

223+
// M5.x: any C sources in the plan? If so we emit a `cc` variable and a
224+
// separate `c_object` rule so .c files are compiled by the C frontend
225+
// (gcc / clang / cc) rather than g++. .c files compiled with g++ get
226+
// routed to cc1plus which rejects C-only constructs (implicit void*
227+
// conversion, `restrict` keyword, etc.) — fatal for libraries like
228+
// mbedtls / openssl.
229+
bool need_c_rule = false;
230+
for (auto& cu : plan.compileUnits) {
231+
if (is_c_source(cu.source)) { need_c_rule = true; break; }
232+
}
233+
234+
// User-supplied flag tails — appended verbatim to per-rule baselines.
235+
auto join_flags = [](const std::vector<std::string>& flags) {
236+
std::string out;
237+
for (auto& f : flags) { out += ' '; out += f; }
238+
return out;
239+
};
240+
std::string user_cxxflags = join_flags(plan.manifest.buildConfig.cxxflags);
241+
std::string user_cflags = join_flags(plan.manifest.buildConfig.cflags);
242+
std::string c_standard = plan.manifest.buildConfig.cStandard.empty()
243+
? std::string{"c11"} : plan.manifest.buildConfig.cStandard;
244+
190245
append(std::format("cxx = {}\n", escape_ninja_path(plan.toolchain.binaryPath)));
191-
append(std::format("cxxflags = -std=c++23 -fmodules{}{}{}{}{}\n",
192-
opt_flag, pic_flag, sysroot_flag, b_flag, include_flags));
246+
append(std::format("cxxflags = -std=c++23 -fmodules{}{}{}{}{}{}\n",
247+
opt_flag, pic_flag, sysroot_flag, b_flag, include_flags,
248+
user_cxxflags));
249+
if (need_c_rule) {
250+
auto cc_path = derive_c_compiler(plan.toolchain.binaryPath);
251+
append(std::format("cc = {}\n", escape_ninja_path(cc_path)));
252+
// C baseline: same opt/pic/sysroot/-B/include layout as cxxflags but
253+
// no -fmodules and -std= goes to a C dialect.
254+
append(std::format("cflags = -std={}{}{}{}{}{}{}\n",
255+
c_standard, opt_flag, pic_flag, sysroot_flag,
256+
b_flag, include_flags, user_cflags));
257+
}
193258
append(std::format("ldflags ={}{}{}{}\n",
194259
full_static, static_stdlib, sysroot_flag, b_flag));
195260
// `ar` for cxx_archive: prefer sandbox absolute path, fall back to PATH.
@@ -229,6 +294,14 @@ std::string emit_ninja_string(const BuildPlan& plan) {
229294
if (dyndep) append(" restat = 1\n");
230295
append("\n");
231296

297+
if (need_c_rule) {
298+
append("rule c_object\n");
299+
append(" command = $cc $cflags -c $in -o $out\n");
300+
append(" description = CC $out\n");
301+
if (dyndep) append(" restat = 1\n");
302+
append("\n");
303+
}
304+
232305
append("rule cxx_link\n");
233306
append(" command = $cxx $in -o $out $ldflags\n");
234307
append(" description = LINK $out\n\n");
@@ -275,12 +348,23 @@ std::string emit_ninja_string(const BuildPlan& plan) {
275348
return s;
276349
};
277350

351+
auto pick_rule = [](const std::filesystem::path& src) -> std::string {
352+
auto ext = src.extension();
353+
if (ext == ".cppm") return "cxx_module";
354+
if (ext == ".c") return "c_object";
355+
return "cxx_object";
356+
};
357+
278358
if (dyndep) {
279359
// ── Phase 1: scan edges (one .ddi per TU). ──────────────────────
280-
// .ddi is placed beside the object: obj/<src>.ddi
360+
// .ddi is placed beside the object: obj/<src>.ddi.
361+
// Skip .c files: they have no `import`s and don't need P1689 scan;
362+
// running them through cxx_scan would route them through g++ /
363+
// -fmodules which is exactly what C support is here to avoid.
281364
std::vector<std::string> ddi_paths;
282365
ddi_paths.reserve(plan.compileUnits.size());
283366
for (auto& cu : plan.compileUnits) {
367+
if (is_c_source(cu.source)) continue;
284368
auto ddi = (std::filesystem::path("obj")
285369
/ cu.source.filename()).string() + ".ddi";
286370
ddi_paths.push_back(ddi);
@@ -302,37 +386,42 @@ std::string emit_ninja_string(const BuildPlan& plan) {
302386
// them from the plan); the dyndep file adds implicit BMI INPUTS
303387
// (the requires) so ninja schedules in the right order.
304388
for (auto& cu : plan.compileUnits) {
305-
std::string rule = (cu.source.extension() == ".cppm")
306-
? "cxx_module" : "cxx_object";
389+
std::string rule = pick_rule(cu.source);
307390

308391
std::string out_line = "build " + escape_ninja_path(cu.object);
309392
if (cu.providesModule) {
310393
out_line += " | " + bmi_path(*cu.providesModule);
311394
}
312395
out_line += std::format(" : {} {}", rule,
313396
escape_ninja_path(cu.source));
314-
// build.ninja.dd is the dyndep file; ninja requires it as an
315-
// implicit input (so it's built before the compile runs).
316-
out_line += " | build.ninja.dd";
317-
out_line += "\n dyndep = build.ninja.dd\n";
397+
if (rule != "c_object") {
398+
// build.ninja.dd is the dyndep file; ninja requires it as an
399+
// implicit input (so it's built before the compile runs).
400+
out_line += " | build.ninja.dd";
401+
out_line += "\n dyndep = build.ninja.dd\n";
402+
} else {
403+
out_line += "\n";
404+
}
318405
append(std::move(out_line));
319406
}
320407
append("\n");
321408
} else {
322409
// ── Static-deps mode (M3.2 and earlier). ────────────────────────
323410
for (auto& cu : plan.compileUnits) {
411+
std::string rule = pick_rule(cu.source);
412+
324413
std::string implicit;
325-
for (auto& imp : cu.imports) {
326-
if (imp == "std" || imp == "std.compat") {
327-
implicit += " gcm.cache/std.gcm";
328-
continue;
414+
// .c files don't `import` modules; skip BMI implicit inputs.
415+
if (rule != "c_object") {
416+
for (auto& imp : cu.imports) {
417+
if (imp == "std" || imp == "std.compat") {
418+
implicit += " gcm.cache/std.gcm";
419+
continue;
420+
}
421+
implicit += " " + bmi_path(imp);
329422
}
330-
implicit += " " + bmi_path(imp);
331423
}
332424

333-
std::string rule = (cu.source.extension() == ".cppm")
334-
? "cxx_module" : "cxx_object";
335-
336425
std::string out_line = "build " + escape_ninja_path(cu.object);
337426
if (cu.providesModule) {
338427
out_line += " " + bmi_path(*cu.providesModule);

src/manifest.cppm

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ struct BuildConfig {
8080
// time from --static / --target / [target.<triple>].linkage. Wired
8181
// through to ninja backend as the `-static` link flag.
8282
std::string linkage;
83+
// M5.x C-language support. `cflags` / `cxxflags` are appended verbatim
84+
// to the per-rule baseline (see `ninja_backend` cflags / cxxflags).
85+
// `cStandard` controls -std= for the C compile rule (.c files).
86+
// Empty cStandard → backend default ("c11" today).
87+
std::vector<std::string> cflags;
88+
std::vector<std::string> cxxflags;
89+
std::string cStandard;
8390
};
8491

8592
// `[target.<triple>]` — per-target overrides.
@@ -371,6 +378,9 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
371378

372379
// [build] — backend tunables
373380
if (auto v = doc->get_bool("build.static_stdlib")) m.buildConfig.staticStdlib = *v;
381+
if (auto v = doc->get_string_array("build.cflags")) m.buildConfig.cflags = *v;
382+
if (auto v = doc->get_string_array("build.cxxflags")) m.buildConfig.cxxflags = *v;
383+
if (auto v = doc->get_string("build.c_standard")) m.buildConfig.cStandard = *v;
374384

375385
// [pack] — `mcpp pack` configuration. See docs/35-pack-design.md.
376386
if (auto v = doc->get_string("pack.default_mode")) {
@@ -1009,6 +1019,29 @@ synthesize_from_xpkg_lua(std::string_view luaContent,
10091019
}
10101020
cur.consume('}');
10111021
}
1022+
else if (key == "cflags" || key == "cxxflags") {
1023+
// `{ "-Dfoo", "-Wall", ... }` — appended to the per-rule baseline
1024+
// by ninja_backend. cflags goes to the C rule (.c files), cxxflags
1025+
// to C++ rule (.cpp/.cc/.cxx/.cppm).
1026+
if (!cur.consume('{')) {
1027+
return std::unexpected(ManifestError{
1028+
std::format("expected '{{' after `{} =`", key),
1029+
m.sourcePath, 0, 0});
1030+
}
1031+
cur.skip_ws_and_comments();
1032+
auto& target = (key == "cflags")
1033+
? m.buildConfig.cflags : m.buildConfig.cxxflags;
1034+
while (!cur.eof() && cur.peek() != '}') {
1035+
auto s = cur.read_string();
1036+
if (!s.empty()) target.push_back(std::move(s));
1037+
cur.skip_ws_and_comments();
1038+
}
1039+
cur.consume('}');
1040+
}
1041+
else if (key == "c_standard") {
1042+
auto v = cur.read_string();
1043+
if (!v.empty()) m.buildConfig.cStandard = v;
1044+
}
10121045
else {
10131046
// Unknown key — skip the value (string / bareword / table).
10141047
cur.skip_ws_and_comments();

src/modgraph/scanner.cppm

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,15 @@ std::expected<SourceUnit, ScanError> scan_file(const std::filesystem::path& file
190190
u.path = file;
191191
u.packageName = packageName;
192192

193+
// .c files are pure C: they cannot legally contain `module` / `import`
194+
// declarations, and we route them to the C-language compile rule (no
195+
// P1689 scan, no BMI lookups). Skip the line-by-line module scan to
196+
// avoid any chance of a benign C identifier (`import_foo`, `module_t`,
197+
// ...) being misparsed.
198+
if (file.extension() == ".c") {
199+
return u;
200+
}
201+
193202
int if_depth = 0; // #if/#ifdef nesting
194203
std::size_t lineno = 0;
195204
std::string line;

tests/e2e/26_c_language_support.sh

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
#!/usr/bin/env bash
2+
# C-language compile rule: .c files routed to `c_object` with cc / cflags,
3+
# distinct from the .cppm/.cpp `cxx_object` rule. Verifies that a mixed
4+
# C + modular-C++23 project links and runs, and that build.ninja contains
5+
# the expected `c_object` / `cc` / `cflags` plumbing.
6+
set -e
7+
8+
TMP=$(mktemp -d)
9+
trap "rm -rf $TMP" EXIT
10+
11+
cd "$TMP"
12+
"$MCPP" new cmix > /dev/null
13+
cd cmix
14+
15+
# A pure-C source that *requires* the C frontend: uses `restrict` and
16+
# implicit `void* → int*` from malloc(), both rejected by g++ in C++ mode.
17+
cat > src/cmix_core.c <<'EOF'
18+
#include <stdlib.h>
19+
#include <string.h>
20+
int *cmix_dup(const int *restrict src, size_t n) {
21+
int *out = malloc(n * sizeof(int));
22+
if (!out) return 0;
23+
memcpy(out, src, n * sizeof(int));
24+
return out;
25+
}
26+
int cmix_sum(const int *p, size_t n) {
27+
int s = 0;
28+
for (size_t i = 0; i < n; ++i) s += p[i];
29+
return s;
30+
}
31+
EOF
32+
33+
cat > src/main.cpp <<'EOF'
34+
import std;
35+
extern "C" int *cmix_dup(const int *src, std::size_t n);
36+
extern "C" int cmix_sum(const int *p, std::size_t n);
37+
int main() {
38+
int data[] = {1, 2, 3, 4, 5};
39+
int *copy = cmix_dup(data, 5);
40+
int s = cmix_sum(copy, 5);
41+
std::println("cmix sum = {}", s);
42+
std::free(copy);
43+
return s == 15 ? 0 : 1;
44+
}
45+
EOF
46+
47+
# Add user-supplied cflags/cxxflags so we also exercise the flag-forwarding
48+
# path through the manifest into the per-rule baselines.
49+
cat > mcpp.toml <<'EOF'
50+
[package]
51+
name = "cmix"
52+
version = "0.1.0"
53+
[build]
54+
cflags = ["-DCMIX_C_BUILD=1"]
55+
cxxflags = ["-DCMIX_CXX_BUILD=1"]
56+
c_standard = "c11"
57+
EOF
58+
59+
"$MCPP" build > build.log 2>&1 || { cat build.log; echo "build failed"; exit 1; }
60+
61+
ninja_file="$(find target -name build.ninja | head -1)"
62+
[[ -n "$ninja_file" ]] || { echo "no build.ninja generated"; exit 1; }
63+
64+
# c_object rule must be present alongside cxx_object.
65+
grep -q '^rule c_object' "$ninja_file" || { cat "$ninja_file"; echo "missing c_object rule"; exit 1; }
66+
# cc / cflags must be defined and pick a C compiler (gcc / cc / clang).
67+
grep -qE '^cc = .*(gcc|cc|clang)' "$ninja_file" || {
68+
echo "cc variable not pointing at a C compiler"; exit 1; }
69+
grep -qE '^cflags = -std=c11.*-DCMIX_C_BUILD=1' "$ninja_file" || {
70+
echo "cflags missing -std=c11 or user cflags tail"; exit 1; }
71+
grep -qE '^cxxflags = -std=c\+\+23.*-DCMIX_CXX_BUILD=1' "$ninja_file" || {
72+
echo "cxxflags missing -std=c++23 or user cxxflags tail"; exit 1; }
73+
# The .c source must be routed through c_object (not cxx_object).
74+
grep -qE 'build obj/cmix_core\.o : c_object .*cmix_core\.c' "$ninja_file" || {
75+
echo "cmix_core.c not routed to c_object rule"; exit 1; }
76+
grep -qE 'build obj/cmix_core\.o : cxx_object' "$ninja_file" && {
77+
echo "cmix_core.c was incorrectly routed to cxx_object"; exit 1; } || true
78+
79+
# Run the binary; it must print "cmix sum = 15" and exit 0.
80+
out="$("$MCPP" run 2>&1 | tail -1)"
81+
[[ "$out" == "cmix sum = 15" ]] || {
82+
echo "unexpected output: $out"; exit 1; }
83+
84+
echo "OK"

tests/unit/test_manifest.cpp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,56 @@ TEST(ListXpkgVersions, MissingXpmReturnsEmpty) {
144144
EXPECT_TRUE(mcpp::manifest::list_xpkg_versions(src, "linux").empty());
145145
}
146146

147+
TEST(Manifest, BuildCflagsCxxflagsAndCStandard) {
148+
constexpr auto src = R"(
149+
[package]
150+
name = "x"
151+
version = "0.1.0"
152+
[build]
153+
sources = ["src/**/*.{cppm,c}"]
154+
cflags = ["-Wall", "-DFOO=1"]
155+
cxxflags = ["-Wextra"]
156+
c_standard = "c11"
157+
[targets.x]
158+
kind = "lib"
159+
)";
160+
auto m = mcpp::manifest::parse_string(src);
161+
ASSERT_TRUE(m.has_value()) << m.error().format();
162+
ASSERT_EQ(m->buildConfig.cflags.size(), 2u);
163+
EXPECT_EQ(m->buildConfig.cflags[0], "-Wall");
164+
EXPECT_EQ(m->buildConfig.cflags[1], "-DFOO=1");
165+
ASSERT_EQ(m->buildConfig.cxxflags.size(), 1u);
166+
EXPECT_EQ(m->buildConfig.cxxflags[0], "-Wextra");
167+
EXPECT_EQ(m->buildConfig.cStandard, "c11");
168+
}
169+
170+
TEST(SynthesizeFromXpkgLua, CflagsCxxflagsAndCStandard) {
171+
constexpr auto src = R"(
172+
package = {
173+
spec = "1",
174+
name = "tinyc",
175+
xpm = { linux = { ["1.0.0"] = { url = "u", sha256 = "h" } } },
176+
mcpp = {
177+
sources = { "*/src/*.c" },
178+
cflags = { "-Wall", "-Dunused" },
179+
cxxflags = { "-Wextra" },
180+
c_standard = "c11",
181+
targets = { ["tinyc"] = { kind = "lib" } },
182+
},
183+
}
184+
)";
185+
auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyc", "1.0.0");
186+
ASSERT_TRUE(m.has_value()) << m.error().format();
187+
ASSERT_EQ(m->buildConfig.cflags.size(), 2u);
188+
EXPECT_EQ(m->buildConfig.cflags[0], "-Wall");
189+
EXPECT_EQ(m->buildConfig.cflags[1], "-Dunused");
190+
ASSERT_EQ(m->buildConfig.cxxflags.size(), 1u);
191+
EXPECT_EQ(m->buildConfig.cxxflags[0], "-Wextra");
192+
EXPECT_EQ(m->buildConfig.cStandard, "c11");
193+
ASSERT_EQ(m->modules.sources.size(), 1u);
194+
EXPECT_EQ(m->modules.sources[0], "*/src/*.c");
195+
}
196+
147197
TEST(ListXpkgVersions, IgnoresCommentedEntries) {
148198
constexpr auto src = R"(
149199
package = {

0 commit comments

Comments
 (0)