Skip to content

Commit 13928b0

Browse files
committed
refactor(build): extract flag construction into shared helpers
Extract compute_cxxflags() and compute_cflags() from emit_ninja_string() into standalone helper functions in the anonymous namespace. These helpers encapsulate all flag computation logic (PIC, sysroot, binutils, musl opt, include dirs, user flags) and will be reused by compile_commands.json generation in a follow-up commit. The ninja output is byte-identical to before the refactoring.
1 parent 8889f3c commit 13928b0

2 files changed

Lines changed: 149 additions & 31 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ target/
33

44
# mcpp's per-workspace xlings sandbox + lockfile + diagnostic logs
55
/.xlings/
6+
.sisyphus
67
.claude
78
mcpp.lock
89
doctor.log

src/build/ninja_backend.cppm

Lines changed: 148 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,149 @@ bool is_c_source(const std::filesystem::path& src) {
126126
return src.extension() == ".c";
127127
}
128128

129+
// Compute the full C++ flags string (everything between compiler binary and -c).
130+
// Returns raw strings; escape_ninja_path() is applied by the caller where needed.
131+
std::string compute_cxxflags(const BuildPlan& plan) {
132+
// Detect whether any target needs PIC (shared library).
133+
bool need_pic = false;
134+
for (auto& lu : plan.linkUnits) {
135+
if (lu.kind == LinkUnit::SharedLibrary) { need_pic = true; break; }
136+
}
137+
const char* pic_flag = need_pic ? " -fPIC" : "";
138+
139+
// M5.0: -I from [build].include_dirs (resolved to absolute paths).
140+
std::string include_flags;
141+
for (auto& inc : plan.manifest.buildConfig.includeDirs) {
142+
auto abs = inc.is_absolute() ? inc : (plan.projectRoot / inc);
143+
include_flags += " -I" + escape_ninja_path(abs);
144+
}
145+
146+
// M5.5: --sysroot when probed.
147+
std::string sysroot_flag;
148+
if (!plan.toolchain.sysroot.empty()) {
149+
sysroot_flag = " --sysroot=" + escape_ninja_path(plan.toolchain.sysroot);
150+
}
151+
152+
// Locate binutils bin dir for -B flag (not needed for musl toolchain).
153+
bool isMuslTc = plan.toolchain.targetTriple.find("-musl") != std::string::npos;
154+
std::filesystem::path binutilsBin;
155+
if (!isMuslTc) {
156+
auto bp = plan.toolchain.binaryPath;
157+
std::filesystem::path xpkgsDir;
158+
for (auto p = bp.parent_path();
159+
p.has_parent_path() && p != p.root_path();
160+
p = p.parent_path()) {
161+
if (p.filename() == "xpkgs") { xpkgsDir = p; break; }
162+
}
163+
if (!xpkgsDir.empty()) {
164+
auto root = xpkgsDir / "xim-x-binutils";
165+
std::error_code ec;
166+
if (std::filesystem::exists(root, ec)) {
167+
for (auto& v : std::filesystem::directory_iterator(root, ec)) {
168+
auto candidate = v.path() / "bin";
169+
if (std::filesystem::exists(candidate / "ar", ec)) {
170+
binutilsBin = candidate;
171+
break;
172+
}
173+
}
174+
}
175+
}
176+
}
177+
178+
std::string b_flag;
179+
if (!binutilsBin.empty()) {
180+
b_flag = " -B" + escape_ninja_path(binutilsBin);
181+
}
182+
183+
// musl-gcc 15.1.0 ICEs at -O2; use -Og instead.
184+
const char* opt_flag = isMuslTc ? " -Og" : " -O2";
185+
186+
// User-supplied cxxflags.
187+
auto join_flags = [](const std::vector<std::string>& flags) {
188+
std::string out;
189+
for (auto& f : flags) { out += ' '; out += f; }
190+
return out;
191+
};
192+
std::string user_cxxflags = join_flags(plan.manifest.buildConfig.cxxflags);
193+
194+
return std::format("-std=c++23 -fmodules{}{}{}{}{}{}",
195+
opt_flag, pic_flag, sysroot_flag, b_flag, include_flags,
196+
user_cxxflags);
197+
}
198+
199+
// Compute the full C flags string (everything between compiler binary and -c).
200+
// Returns raw strings; escape_ninja_path() is applied by the caller where needed.
201+
std::string compute_cflags(const BuildPlan& plan) {
202+
// Detect whether any target needs PIC (shared library).
203+
bool need_pic = false;
204+
for (auto& lu : plan.linkUnits) {
205+
if (lu.kind == LinkUnit::SharedLibrary) { need_pic = true; break; }
206+
}
207+
const char* pic_flag = need_pic ? " -fPIC" : "";
208+
209+
// M5.0: -I from [build].include_dirs (resolved to absolute paths).
210+
std::string include_flags;
211+
for (auto& inc : plan.manifest.buildConfig.includeDirs) {
212+
auto abs = inc.is_absolute() ? inc : (plan.projectRoot / inc);
213+
include_flags += " -I" + escape_ninja_path(abs);
214+
}
215+
216+
// M5.5: --sysroot when probed.
217+
std::string sysroot_flag;
218+
if (!plan.toolchain.sysroot.empty()) {
219+
sysroot_flag = " --sysroot=" + escape_ninja_path(plan.toolchain.sysroot);
220+
}
221+
222+
// Locate binutils bin dir for -B flag (not needed for musl toolchain).
223+
bool isMuslTc = plan.toolchain.targetTriple.find("-musl") != std::string::npos;
224+
std::filesystem::path binutilsBin;
225+
if (!isMuslTc) {
226+
auto bp = plan.toolchain.binaryPath;
227+
std::filesystem::path xpkgsDir;
228+
for (auto p = bp.parent_path();
229+
p.has_parent_path() && p != p.root_path();
230+
p = p.parent_path()) {
231+
if (p.filename() == "xpkgs") { xpkgsDir = p; break; }
232+
}
233+
if (!xpkgsDir.empty()) {
234+
auto root = xpkgsDir / "xim-x-binutils";
235+
std::error_code ec;
236+
if (std::filesystem::exists(root, ec)) {
237+
for (auto& v : std::filesystem::directory_iterator(root, ec)) {
238+
auto candidate = v.path() / "bin";
239+
if (std::filesystem::exists(candidate / "ar", ec)) {
240+
binutilsBin = candidate;
241+
break;
242+
}
243+
}
244+
}
245+
}
246+
}
247+
248+
std::string b_flag;
249+
if (!binutilsBin.empty()) {
250+
b_flag = " -B" + escape_ninja_path(binutilsBin);
251+
}
252+
253+
// musl-gcc 15.1.0 ICEs at -O2; use -Og instead.
254+
const char* opt_flag = isMuslTc ? " -Og" : " -O2";
255+
256+
// User-supplied cflags.
257+
auto join_flags = [](const std::vector<std::string>& flags) {
258+
std::string out;
259+
for (auto& f : flags) { out += ' '; out += f; }
260+
return out;
261+
};
262+
std::string user_cflags = join_flags(plan.manifest.buildConfig.cflags);
263+
264+
std::string c_standard = plan.manifest.buildConfig.cStandard.empty()
265+
? std::string{"c11"} : plan.manifest.buildConfig.cStandard;
266+
267+
return std::format("-std={}{}{}{}{}{}{}",
268+
c_standard, opt_flag, pic_flag, sysroot_flag,
269+
b_flag, include_flags, user_cflags);
270+
}
271+
129272
} // namespace
130273

131274
std::string emit_ninja_string(const BuildPlan& plan) {
@@ -158,13 +301,6 @@ std::string emit_ninja_string(const BuildPlan& plan) {
158301
const char* full_static = (plan.manifest.buildConfig.linkage == "static")
159302
? " -static" : "";
160303

161-
// M5.0: -I from [build].include_dirs (resolved to absolute paths).
162-
std::string include_flags;
163-
for (auto& inc : plan.manifest.buildConfig.includeDirs) {
164-
auto abs = inc.is_absolute() ? inc : (plan.projectRoot / inc);
165-
include_flags += " -I" + escape_ninja_path(abs);
166-
}
167-
168304
// M5.5: --sysroot when probed (needed since we bypass any xlings wrapper).
169305
std::string sysroot_flag;
170306
if (!plan.toolchain.sysroot.empty()) {
@@ -214,12 +350,6 @@ std::string emit_ninja_string(const BuildPlan& plan) {
214350
b_flag = " -B" + escape_ninja_path(binutilsBin);
215351
}
216352

217-
// musl-gcc 15.1.0 ICEs in tree-ssa-ccp on libstdc++'s std::format
218-
// (`__write_padded` template) at -O2. Drop to -Og for the musl path
219-
// until either musl-gcc 16.1 lands or upstream fixes the pass.
220-
// TODO(musl-gcc-upstream): remove once musl-gcc@16+ ships.
221-
const char* opt_flag = isMuslTc ? " -Og" : " -O2";
222-
223353
// M5.x: any C sources in the plan? If so we emit a `cc` variable and a
224354
// separate `c_object` rule so .c files are compiled by the C frontend
225355
// (gcc / clang / cc) rather than g++. .c files compiled with g++ get
@@ -231,29 +361,16 @@ std::string emit_ninja_string(const BuildPlan& plan) {
231361
if (is_c_source(cu.source)) { need_c_rule = true; break; }
232362
}
233363

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;
364+
// Use helper functions to compute cxxflags and cflags.
365+
std::string cxxflags = compute_cxxflags(plan);
366+
std::string cflags = compute_cflags(plan);
244367

245368
append(std::format("cxx = {}\n", escape_ninja_path(plan.toolchain.binaryPath)));
246-
append(std::format("cxxflags = -std=c++23 -fmodules{}{}{}{}{}{}\n",
247-
opt_flag, pic_flag, sysroot_flag, b_flag, include_flags,
248-
user_cxxflags));
369+
append(std::format("cxxflags = {}\n", cxxflags));
249370
if (need_c_rule) {
250371
auto cc_path = derive_c_compiler(plan.toolchain.binaryPath);
251372
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));
373+
append(std::format("cflags = {}\n", cflags));
257374
}
258375
append(std::format("ldflags ={}{}{}{}\n",
259376
full_static, static_stdlib, sysroot_flag, b_flag));

0 commit comments

Comments
 (0)