refactor: unify the CompilationUnitRef usage

[16bit-ykiko]

Summary by CodeRabbit

• Refactor
  • Improved internal compilation unit lifecycle management with enhanced move semantics and resource cleanup.
  • Restructured diagnostic collection and clang-tidy integration to use unified compilation context for better handling.
  • Enhanced destructor behavior and error handling patterns throughout the compilation pipeline.
  • Refined directive collection workflow to improve preprocessor integration consistency and accuracy.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors the compilation workflow by consolidating related functionality into CompilationUnitRef::Self. It introduces new lifecycle methods (create_invocation, run_clang, run_tidy), moves the stop control into Self, replaces free functions with member functions, and updates dependent code to access compiler state through a unified CompilationUnitRef interface.

Changes

Cohort / File(s)	Summary
API & Lifecycle Updates include/Compiler/CompilationUnit.h, src/Compiler/Implement.h	Added file_id(clang::FileEntryRef) method. Deleted copy assignment operator and added move assignment operator. Introduced new member methods on CompilationUnitRef::Self: destructor, create_diagnostic(), create_invocation(), run_clang(), run_tidy(), configure_tidy(). Moved stop as shared member. Removed free create_diagnostic() function.
Compiler Invocation & Lifecycle src/Compiler/Compilation.cpp	Replaced inline wrappers and Proxy classes with member-based implementations on CompilationUnitRef::Self. Added create_invocation() method for building CompilerInvocation with diagnostics engine. Introduced ProxyASTConsumer and ProxyAction classes. Moved stop into Self and updated cancellation checks. Refactored run_clang() as member function returning CompilationStatus with lifecycle hooks. Added run_tidy() and configure_tidy() for clang-tidy integration.
FileID Resolution src/Compiler/CompilationUnit.cpp	Added file_id(clang::FileEntryRef) overload delegating to self->SM().translateFile(). Updated existing string-based overload to route through the new overload.
Diagnostic Handling src/Compiler/Diagnostic.cpp	Converted free factory function create_diagnostic(CompilationUnitRef) to member function CompilationUnitRef::Self::create_diagnostic().
Directive Collection src/Compiler/Directive.cpp	Refactored DirectiveCollector constructor to accept CompilationUnitRef instead of separate Preprocessor and directives map. Updated all file lookups to use unit.file_id(), unit.file_content(), unit.file_offset(). Replaced direct Preprocessor/SourceManager access with unified unit-based access pattern throughout callback handlers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• refactor: incremental update for compilation database and introduce query toolchain #311: Modifies the same create_invocation() and run_clang() compilation invocation flow in src/Compiler/Compilation.cpp with overlapping changes to parameter handling.
• feat: run clang-tidy #200: Refactors clang-tidy integration and compilation/diagnostic plumbing including CompilationUnitRef::Self lifecycle, diagnostic collectors, and run_tidy/run_clang paths.
• refactor: diagnostic handling #337: Refactors CompilationUnit/CompilationUnitRef and diagnostics, moving create_diagnostic and lifecycle methods into CompilationUnitRef::Self with new file_id overloads.

Poem

🐰 The compilation pipeline now stands quite tall,
With Self at the center answering the call,
No more scattered helpers, one unified way,
Member functions bloom where free functions once lay! 🌱

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.13% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 accurately describes the main refactoring effort: unifying CompilationUnitRef usage across multiple files by consolidating member-based implementations, moving stop control into Self, and refactoring DirectiveCollector to use CompilationUnitRef instead of separate Preprocessor/SourceManager references.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d6733dd and b52f5f0.

📒 Files selected for processing (6)

• include/Compiler/CompilationUnit.h
• src/Compiler/Compilation.cpp
• src/Compiler/CompilationUnit.cpp
• src/Compiler/Diagnostic.cpp
• src/Compiler/Directive.cpp
• src/Compiler/Implement.h

🧰 Additional context used

🧬 Code graph analysis (5)

src/Compiler/CompilationUnit.cpp (1)

src/Compiler/Directive.cpp (2)

• file (105-118)
• file (105-107)

src/Compiler/Directive.cpp (3)

src/Feature/SemanticToken.cpp (7)

• location (93-116)
• location (93-93)
• fid (73-85)
• fid (88-91)
• fid (88-88)
• fid (119-202)
• fid (119-119)

src/Compiler/CompilationUnit.cpp (2)

• kind (7-9)
• kind (7-7)

src/Compiler/Compilation.cpp (2)

• instance (216-221)
• instance (216-217)

src/Compiler/Implement.h (3)

src/Compiler/Compilation.cpp (11)

• Self (16-25)
• create_invocation (27-144)
• create_invocation (28-30)
• configure_tidy (146-148)
• configure_tidy (146-146)
• run_tidy (150-163)
• run_tidy (150-150)
• run_clang (232-321)
• run_clang (232-236)
• run_clang (323-345)
• run_clang (323-326)

src/Compiler/Diagnostic.cpp (2)

• create_diagnostic (252-254)
• create_diagnostic (252-252)

src/Compiler/Directive.cpp (2)

• collect_directives (262-264)
• collect_directives (262-262)

include/Compiler/CompilationUnit.h (1)

src/Compiler/CompilationUnit.cpp (7)

• file_id (15-17)
• file_id (15-15)
• file_id (19-26)
• file_id (19-19)
• file_id (75-77)
• file_id (75-75)
• CompilationUnit (330-332)

src/Compiler/Compilation.cpp (4)

src/Compiler/Implement.h (1)

• Self (55-101)

src/Compiler/Tidy.cpp (2)

• configure (317-389)
• configure (317-318)

src/Compiler/CompilationUnit.cpp (2)

• top_level_decls (221-223)
• top_level_decls (221-221)

src/AST/Utility.cpp (2)

• is_implicit_template_instantiation (85-87)
• is_implicit_template_instantiation (85-85)

⏰ 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: xmake / build (ubuntu-24.04, debug)
• GitHub Check: clice / publish-clice (windows-2025, clice.zip, clice-x64-windows-msvc.zip, clice-symbol.zip, clice-x64-w...
• GitHub Check: xmake / build (windows-2025, releasedbg)
• GitHub Check: cmake / build (windows-2025, RelWithDebInfo)
• GitHub Check: xmake / build (ubuntu-24.04, releasedbg)
• GitHub Check: cmake / build (ubuntu-24.04, Debug)
• GitHub Check: clice / publish-clice (macos-15, clice.tar.gz, clice-arm64-macos-darwin.tar.gz, clice-symbol.tar.gz, clic...
• GitHub Check: clice / publish-clice (ubuntu-24.04, clice.tar.gz, clice-x86_64-linux-gnu.tar.gz, clice-symbol.tar.gz, cl...

🔇 Additional comments (24)

src/Compiler/CompilationUnit.cpp (2)

15-17: LGTM!

The new overload correctly delegates to translateFile and aligns with the broader refactor to consolidate file ID resolution through CompilationUnitRef.

---

22-22: LGTM!

Routing through the new overload consolidates the translation logic and maintains consistency.

src/Compiler/Diagnostic.cpp (1)

252-254: LGTM!

The refactor from a free function to a member function aligns with the PR's goal of consolidating lifecycle methods into CompilationUnitRef::Self. The this pointer (of type Self*) correctly converts to CompilationUnitRef via the constructor defined in the header.

include/Compiler/CompilationUnit.h (3)

86-87: LGTM!

The new overload expands the public API to support direct FileEntryRef lookups, consistent with the implementation and the broader refactor.

---

234-234: LGTM!

Parameter name change improves clarity and consistency with the member variable name.

---

242-247: LGTM!

The move assignment operator correctly uses std::swap to transfer ownership. Combined with the move constructor (which nullifies other.self) and the destructor (which deletes self), this ensures proper cleanup without double-deletion.

src/Compiler/Directive.cpp (6)

15-15: LGTM!

The constructor refactor consolidates access through CompilationUnitRef, eliminating the need to pass separate Preprocessor and directives references.

---

22-24: LGTM!

The updated access pattern unit->directives[unit.file_id(location)] is consistent with the new unified interface and correctly obtains both the file ID and the corresponding directive container.

---

79-88: LGTM!

The InclusionDirective handler correctly uses the new unit-based access pattern to store include information.

---

111-116: LGTM!

The FileSkipped handler correctly uses the new unit.file_id(file) overload that accepts FileEntryRef, which matches the newly added public method in the header.

---

256-256: LGTM!

Replacing multiple reference members with a single CompilationUnitRef simplifies the class and aligns with the unified access pattern.

---

263-263: LGTM!

The updated construction correctly passes this (a Self*), which implicitly converts to CompilationUnitRef via the constructor defined in the header.

src/Compiler/Implement.h (2)

60-60: LGTM!

Changing stop to std::shared_ptr<std::atomic_bool> enables safe sharing of the cancellation signal across multiple contexts (e.g., between the compilation unit and external controllers). The combination of shared_ptr for lifetime management and atomic_bool for thread-safe access is appropriate.

---

103-126: LGTM!

The new public section consolidates lifecycle and compilation methods into Self, aligning with the PR's goal of unifying the CompilationUnitRef usage. The method signatures are well-structured:

• create_diagnostic() and create_invocation() are factory methods
• run_clang() uses the explicit object parameter (this Self& self) to clearly indicate it modifies the instance
• configure_tidy() and run_tidy() are now declarations, with implementations moved elsewhere

src/Compiler/Compilation.cpp (10)

16-25: LGTM! Clean destructor with proper lifecycle management.

The destructor correctly handles the cleanup sequence by detaching the preprocessor before calling EndSourceFile() while keeping it alive, which is necessary for Sema. The implementation properly consolidates cleanup logic that was previously scattered.

---

113-113: Good fix: Use numeric literal for integer option.

Changing TimeTraceGranularity from false to 0 is correct, as this field expects an unsigned integer type rather than a boolean.

---

168-209: LGTM! Proper filtering and cancellation logic.

The ProxyASTConsumer correctly:

• Filters declarations by file location to collect only relevant top-level declarations
• Excludes implicit template instantiations
• Checks the atomic stop flag to enable cancellation during AST parsing

The cancellation check after each declaration (line 200) provides responsive cancellation while maintaining parse granularity.

---

211-228: LGTM! Clean proxy action wrapper.

The ProxyAction properly wraps the frontend action and exposes EndSourceFile for controlled cleanup. The design allows the compilation unit to manage the action lifecycle through the proxy.

---

232-321: LGTM! Well-structured member-based compilation flow.

The refactored run_clang member function successfully consolidates the compilation workflow:

• Proper initialization sequence (diagnostic consumer → invocation → instance)
• Correct use of ProxyAction to enable cancellation and declaration filtering
• Clean integration of token collection and clang-tidy
• Appropriate status tracking and error handling

The use of explicit object parameters and member-based calls provides better encapsulation compared to the previous free-function approach.

---

323-345: LGTM! Proper integration of new member-based flow.

The top-level run_clang function correctly:

• Allocates Self on the heap for ownership by CompilationUnit
• Moves the stop flag from params into self for centralized cancellation control
• Delegates to the new member-based run_clang method
• Tracks build timing and status appropriately
• Executes the optional post-compilation callback only on successful completion

---

347-442: LGTM! Clean compilation entry points.

All compilation entry points (preprocess, compile, complete) properly utilize the new unified run_clang infrastructure with appropriate frontend actions and configuration callbacks. The code is clear and consistent.

---

237-237: create_diagnostic() member function is properly declared and defined.

Verification confirms that create_diagnostic() exists as a member function with correct declaration in Implement.h (line 106) and definition in Diagnostic.cpp (line 252). The usage at line 237 in Compilation.cpp is correct and matches the function signature.

---

28-30: No action needed. The project's CMakeLists.txt explicitly requires C++23 with CMAKE_CXX_STANDARD 23 and CMAKE_CXX_STANDARD_REQUIRED ON, which properly enforces compiler support for the explicit object parameter syntax used in the function signature.

---

150-163: Address the TODO about modifying ASTContext traversal scope.

The code modifies the ASTContext traversal scope (line 155) to exclude preamble declarations, but the TODO comment questions whether this is safe. Verify that modifying the traversal scope here doesn't affect other consumers or subsequent operations on the same ASTContext.

Note: The two EndSourceFile() calls are intentionally ordered and documented. The preprocessor's EndSourceFile() is called in run_tidy() (line 161), then the preprocessor is detached (line 22 of destructor) before action->EndSourceFile() is called in the destructor (line 23), preventing duplicate cleanup.

---

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.}