feature: scan deps when server starts

[16bit-ykiko]

Summary by CodeRabbit

• Chores

  • Optimized build configuration by disabling C++20 module scanning for improved compilation performance.
  • Enhanced debug builds with additional dependency scanning tooling capabilities.

• Tests

  • Added unit test coverage for dependency scanning functionality.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Build configuration changes add clangDependencyScanning library to debug linking across CMake and xmake, disable C++20 module scanning via CMakeLists.txt, introduce a cached option to disable C++20 modules for cpptrace, and scaffold a dependency scanning unit test with TODO marker.

Changes

Cohort / File(s)	Summary
Build configuration CMakeLists.txt, cmake/package.cmake, xmake.lua	Added clangDependencyScanning to debug-mode LLVM component linking; disabled C++20 module scanning in CMakeLists.txt; introduced CPPTRACE_DISABLE_CXX_20_MODULES cached boolean option set to ON
Test scaffolding tests/unit/Compiler/CompilerTests.cpp	Added new unit test case ScanDeps that instantiates DependencyScanningService and DependencyScanningTool, calls getDependencyFile with empty inputs, and marks success path with TODO

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Potential attention point: Review compatibility with PR #304, which removes clangDependencyScanning from the same debug link lists—verify this PR's intent does not conflict with that change.
• Test stub contains TODO marker on success path; clarify expected test behavior before merge.

Possibly related PRs

• build: update llvm checksum and refactor ci #304: Removes clangDependencyScanning and clangIndex from debug-mode LLVM linking, directly conflicting with this PR's addition of clangDependencyScanning to the same link configurations.

Poem

🐰 Module scanning OFF, dependencies ON,
A rabbit hops through CMake at dawn,
Clang tools linked, tests waiting with care,
Build configurations dancing in air! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding dependency scanning functionality when the server starts, which is directly reflected in the changeset.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch scan-deps

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

cmake/package.cmake (1)

133-141: cpptrace C++20 modules explicitly disabled via cache option

Setting CPPTRACE_DISABLE_CXX_20_MODULES to ON before FetchContent_MakeAvailable(cpptrace) is a sensible way to avoid cpptrace’s C++20 module support interfering with your own build/module/dependency‑scanning setup. It should be a no‑op on older cpptrace versions, but verify that v1.0.4 understands this cache option so you’re not relying on undefined behavior; if it does, you might also want to give the cache entry a short description instead of an empty string for easier configuration inspection.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cec13ec and ee915bc.

📒 Files selected for processing (4)

• CMakeLists.txt (1 hunks)
• cmake/package.cmake (2 hunks)
• tests/unit/Compiler/CompilerTests.cpp (2 hunks)
• xmake.lua (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)

• GitHub Check: build (windows-2025, RelWithDebInfo, clang, clang++)
• GitHub Check: build (ubuntu-24.04, Debug, clang-20, clang++-20)
• GitHub Check: build (macos-15, Debug, clang, clang++)
• GitHub Check: build (macos-15, releasedbg)
• GitHub Check: build (windows-2025, releasedbg)
• GitHub Check: build (ubuntu-24.04, releasedbg)
• GitHub Check: build (macos-15, debug)
• GitHub Check: build (ubuntu-24.04, debug)

🔇 Additional comments (4)

CMakeLists.txt (1)

6-6: Explicitly disabling CMake C++ module scanning looks reasonable

Setting CMAKE_CXX_SCAN_FOR_MODULES to OFF before creating targets is consistent with relying on Clang’s dependency scanning instead of CMake’s built‑in module scanning; it will apply globally to all subsequent C++ targets, which seems to match the direction of this PR.

xmake.lua (1)

65-83: Debug LLVM link set now includes clangDependencyScanning

Adding clangDependencyScanning to the debug‑only links for the llvm package keeps xmake’s configuration aligned with the CMake llvm-libs wiring and is appropriate for enabling dependency scanning in debug builds. Just make sure all prebuilt LLVM bundles used by clice-llvm actually ship this library on every supported platform/mode, or debug builds will fail at link time.

cmake/package.cmake (1)

19-36: llvm-libs debug config correctly wires clangDependencyScanning

Extending the explicit debug‑mode target_link_libraries(llvm-libs ...) set with clangDependencyScanning is consistent with the xmake configuration and required for the new dependency scanning usage in tests/server. This looks correct; just confirm that the LLVM distribution you pin (21.1.4) ships this library on every supported platform so debug builds don’t hit missing‑symbol issues.

tests/unit/Compiler/CompilerTests.cpp (1)

8-9: Dependency scanning test is currently a no‑op placeholder

The new ScanDeps test successfully wires DependencyScanningService/DependencyScanningTool and exercises getDependencyFile, which is useful to validate headers and linking. However, it doesn't currently assert anything (the TODO inside if (!err) is empty), so it won't catch regressions beyond compilation/link success.

Consider:

• Adding a direct include for the service header (e.g. DependencyScanningService.h) instead of relying on transitive includes, and
• Either adding at least a minimal assertion around the Expected<> result or clearly marking this test as TODO/disabled until you define what behavior you want to guarantee.

This keeps the test suite meaningful once dependency scanning behavior stabilizes.