fix: emit absolute -fprebuilt-module-path so clangd resolves modules (#77)

* fix: emit absolute `-fprebuilt-module-path` so clangd resolves modules

src/build/flags.cppm was emitting a bare `-fprebuilt-module-path=pcm.cache`
(or `=gcm.cache`). This works for `mcpp build` because ninja runs commands
with cwd = `<projectRoot>/target/<triple>/<fp>/` so the relative path
resolves correctly. But the same flag is captured verbatim into
`compile_commands.json`, whose `directory` field is `plan.projectRoot`.
Per CDB spec, clangd does `cd <directory>` before resolving the args, so
it looks for `<projectRoot>/pcm.cache` — which doesn't exist. Effect:
`import` resolution silently fails in clangd ("module 'X' not found"),
even when `mcpp build` succeeds.

The other `-fmodule-file=` flags in the same block were already absolute
(escape_path-wrapped); this one was an oversight.

Fix: format the flag with `escape_path(plan.outputDir / traits.bmiDir)`.
Single line. ninja still works (absolute paths are cwd-independent), and
clangd now finds the BMI cache.

Regression test: tests/e2e/47_cdb_prebuilt_module_path_abs.sh creates a
fresh project, runs `mcpp build`, parses `compile_commands.json`, asserts
every `-fprebuilt-module-path=X`:
  - is absolute (starts with `/` on POSIX or `<drive>:` on Windows)
  - ends in `/pcm.cache` or `/gcm.cache`
  - points to an existing directory
Pass-through for GCC-only flows that don't emit the flag at all.

* ci/test: un-escape ninja sequences in CDB args; harden e2e for Windows

First CI round revealed two layered issues exposed by the new e2e:

1. **The shipped CDB carries ninja-escape artefacts.** `flags.cppm`'s
   `escape_path` ninja-escapes paths (`$ ` for space, `$:` for colon,
   `$$` for literal `$`) so they survive embedding in ninja rule strings.
   But the same escaped strings flow verbatim through `f.cxx` into
   `compile_commands.json`, whose `arguments` array is exec'd by clangd
   without any ninja involved. On Windows that makes `C:` appear as
   `C$:` in CDB → clangd can't resolve the path. POSIX usually escaped
   nothing in practice, hiding the bug; Windows drive letters always
   trigger `$:`.

   Fix in `compile_commands.cppm::split_flags`: same single pass now
   splits tokens *and* unescapes the three ninja sequences. Splitting
   was also broken for paths with a literal space (would've broken
   `-isystem "/path with space/include"` mid-arg) — that's fixed
   simultaneously since `$ ` is no longer a token separator.

2. **e2e test 47 needed Windows-friendly checks.** The original `[^"]+`
   grep returned the raw JSON-escaped form (`\\Users\\...`) and the
   `^[A-Za-z]:` regex didn't match `C$:`. Rewrote using `jq -r` (which
   does JSON un-escape natively, preinstalled on all GHA runners) and:
   - explicit ninja-escape sniff (`$:`, `$ `, `$$`)
   - both POSIX and Windows-drive absolute-path checks
   - basename check normalised against `\` / `/`

Net: CDB now contains plain paths in both runtimes, and the test fails
loudly if either layer regresses.

* test(e2e/47): strip trailing CR from jq output on git-bash/Windows

The Windows CI run reported:
  FAIL: basename is not pcm.cache/gcm.cache: 'pcm.cache
  '
The closing quote on the next line is the giveaway — the value carries a
trailing `\r`. `jq.exe` on git-bash emits CRLF; `mapfile -t` strips LF
but leaves CR, so every downstream comparison saw `pcm.cache$'\r'` and
failed. Trim with `${v%$'\r'}` per element.

The source-code fix (un-escape ninja sequences) is working — the value
itself is now a clean `C:\Users\...\pcm.cache`, no `$:` artefact.

* test(e2e/47): drop bash-4 `mapfile` for macOS bash 3.2 compat

Apple ships GPLv2-era bash 3.2 on macOS, which has no `mapfile` /
`readarray`. The previous round failed on ci-macos with:
  line 31: mapfile: command not found
  FAIL: 47_cdb_prebuilt_module_path_abs.sh (exit 127)

Replace `mapfile -t vals < <(jq …)` with `jq … > tmp.txt` followed by a
`while … read … done < tmp.txt`. Using input redirection (not a `|`
pipeline) keeps the loop in the current shell so `fail=1` propagates;
the temp file lives under $TMP and is cleaned up by the script's trap.

Empty-list signal now uses `[[ ! -s "$TMP/vals.txt" ]]` instead of array
length — same semantics, no bashism.

Author: Sunrisepeak

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<std::string> split_flags(std::string_view s) {
     std::vector<std::string> 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;
 }
 

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
+        // `<projectRoot>/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,

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
+# `<projectRoot>/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/<triple>/<fp>/ — 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"