diff --git a/src/build/compile_commands.cppm b/src/build/compile_commands.cppm index 29809aa..5f6b4c1 100644 --- a/src/build/compile_commands.cppm +++ b/src/build/compile_commands.cppm @@ -37,20 +37,47 @@ bool is_c_source(const std::filesystem::path& src) { return src.extension() == ".c"; } -// Split a flag string into individual tokens. +// Split a flag string into individual tokens AND un-escape ninja-style +// path escapes (`$ ` → space, `$:` → `:`, `$$` → `$`). +// +// `flags.cppm::escape_path` ninja-escapes path arguments so they survive +// embedding in ninja rule strings. Those escaped strings are then captured +// into `f.cxx` / `f.cc` which is what we receive here. CDB consumers like +// clangd exec the `arguments` array literally — no ninja involved — so +// escaped chars must be undone or paths like `C:\Users\...` come through +// as `C$:\Users\...` and break clangd's path resolution on Windows. (The +// same issue would silently affect any POSIX path containing a space or +// `$` — those just happen to be rare.) +// +// Splitting and un-escaping in one pass is correct: a literal space inside +// a path appears as `$ ` in the input, which we must NOT treat as a token +// separator. std::vector split_flags(std::string_view s) { std::vector out; - std::size_t i = 0; - while (i < s.size()) { - while (i < s.size() && s[i] == ' ') - ++i; - if (i >= s.size()) - break; - std::size_t start = i; - while (i < s.size() && s[i] != ' ') - ++i; - out.emplace_back(s.substr(start, i - start)); + std::string token; + auto flush = [&] { + if (!token.empty()) { + out.push_back(std::move(token)); + token.clear(); + } + }; + for (std::size_t i = 0; i < s.size(); ++i) { + char c = s[i]; + if (c == '$' && i + 1 < s.size()) { + char nc = s[i + 1]; + if (nc == ' ' || nc == ':' || nc == '$') { + token.push_back(nc); + ++i; + continue; + } + } + if (c == ' ') { + flush(); + } else { + token.push_back(c); + } } + flush(); return out; } diff --git a/src/build/flags.cppm b/src/build/flags.cppm index 0f70de0..6249170 100644 --- a/src/build/flags.cppm +++ b/src/build/flags.cppm @@ -212,7 +212,17 @@ CompileFlags compute_flags(const BuildPlan& plan) { auto traits = mcpp::toolchain::bmi_traits(plan.toolchain); std::string prebuilt_module_flag; if (traits.needsPrebuiltModulePath) { - prebuilt_module_flag = std::format(" -fprebuilt-module-path={}", traits.bmiDir); + // Absolute path: a bare `pcm.cache` / `gcm.cache` works at ninja + // time because ninja runs commands with cwd = outputDir, but the + // same flag ends up verbatim in `compile_commands.json` whose + // `directory` field is the project root. clangd does `cd directory` + // before resolving the flag, so a bare relative path points at + // `/pcm.cache` (which doesn't exist) and `import` + // resolution fails with `module 'X' not found`. The other + // `-fmodule-file=` flags in this block are already escape_path'd + // (absolute) for the same reason — this one was a leftover. + prebuilt_module_flag = std::format(" -fprebuilt-module-path={}", + escape_path(plan.outputDir / traits.bmiDir)); } f.cxx = std::format("-std=c++23{}{}{}{}{}{}{}{}{}{}", module_flag, std_module_flag, std_compat_module_flag, prebuilt_module_flag, diff --git a/tests/e2e/47_cdb_prebuilt_module_path_abs.sh b/tests/e2e/47_cdb_prebuilt_module_path_abs.sh new file mode 100755 index 0000000..6283322 --- /dev/null +++ b/tests/e2e/47_cdb_prebuilt_module_path_abs.sh @@ -0,0 +1,88 @@ +#!/usr/bin/env bash +# requires: +# 47_cdb_prebuilt_module_path_abs.sh — `-fprebuilt-module-path` in +# compile_commands.json must be an ABSOLUTE path, NOT a bare `pcm.cache`, +# AND must not carry ninja-escape artefacts like `C$:` on Windows. +# Reason: CDB `directory` is the project root and clangd does `cd +# directory` before running the args, so a bare relative path points at +# `/pcm.cache` (missing) and a `C$:` prefix is treated as a +# literal string, not a Windows drive letter. Both modes silently break +# clangd's module resolution while `mcpp build` itself keeps working +# (ninja runs from outputDir AND unescapes its own escape sequences). +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT + +cd "$TMP" +"$MCPP" new app > /dev/null +cd app +"$MCPP" build > /dev/null + +cdb=compile_commands.json +[[ -f "$cdb" ]] || { echo "FAIL: no $cdb generated"; exit 1; } + +command -v jq >/dev/null 2>&1 || { + echo "SKIP: jq not on PATH (preinstalled on GitHub-hosted runners)" + exit 0 +} + +# jq returns each value JSON-unescaped (\\ → \, etc.). Stage to a temp +# file then read with a redirected while loop — bash 3.2 on macOS lacks +# `mapfile`/`readarray`, and a `| while` pipeline puts the loop in a +# subshell so `fail=1` would not propagate. Input redirection runs the +# loop in the current shell, preserving the flag. +jq -r ' + .[] | .arguments[]? + | select(type == "string" and startswith("-fprebuilt-module-path=")) + | sub("^-fprebuilt-module-path="; "") +' "$cdb" > "$TMP/vals.txt" + +if [[ ! -s "$TMP/vals.txt" ]]; then + # GCC's libstdc++ flow uses -fmodules / gcm.cache without the explicit + # -fprebuilt-module-path flag (see bmi_traits.needsPrebuiltModulePath). + # Nothing to assert in that mode. + echo "OK (no prebuilt-module-path flag — GCC toolchain)" + exit 0 +fi + +fail=0 +while IFS= read -r v; do + # `jq` on git-bash/Windows emits CRLF; strip the trailing CR so basename + # / regex comparisons don't trip over an invisible `\r`. + v="${v%$'\r'}" + [[ -z "$v" ]] && continue + echo " checking: $v" + + # Must NOT carry ninja-escape artefacts. The key signal is `$:` (drive + # letter) or `$ ` / `$$` (path with space / dollar). If any of these + # survives into CDB the JSON-args runtime treats them as literal text + # → clangd fails to find the BMI. + if [[ "$v" == *'$:'* || "$v" == *'$ '* || "$v" == *'$$'* ]]; then + echo "FAIL: value retains ninja escape sequence ('\$:' / '\$ ' / '\$\$') — must be plain path in CDB" + fail=1 + fi + + # Absolute: POSIX (starts with '/') or Windows drive (e.g. 'C:'). + if [[ "$v" =~ ^/ || "$v" =~ ^[A-Za-z]: ]]; then + : + else + echo "FAIL: value is relative: '$v'" + echo " CDB 'directory' is the project root, but the BMI cache" + echo " lives under target/// — clangd resolves to" + echo " the wrong location and module imports fail." + fail=1 + fi + + # Basename must be pcm.cache or gcm.cache (cross-platform: normalise + # backslashes first so Windows paths like 'C:\foo\pcm.cache' work). + normalised="${v//\\//}" + case "${normalised##*/}" in + pcm.cache|gcm.cache) ;; + *) echo "FAIL: basename is not pcm.cache/gcm.cache: '${normalised##*/}'" + fail=1 ;; + esac +done < "$TMP/vals.txt" + +[[ $fail -eq 0 ]] || exit 1 +echo "OK"