Repository navigation
fix: build on stable toolchain and expose buffer font zoom - #3
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Caution PR Summary Skipped - Monthly Quota ExceededPR summary skipped as you have reached the free tier limit of 50 PR summaries per month. Please upgrade to a paid plan for MatterAI. Current Plan: Free Tier Upgrade your plan on the console here: https://app.matterai.so/ai-code-reviews?tab=Billing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR replaces ChangesPlatform cfg selection
Profiler cold-path helper
Theme settings re-export
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the unstable cfg_select! macro with standard #[cfg] blocks in telemetry.rs and extension_builder.rs, introduces a stable cold_path helper in profiler.rs to replace std::hint::cold_path, and cleans up imports in theme_settings.rs. The review feedback highlights two issues: first, os_version in telemetry.rs needs a fallback block to prevent compilation failures on unsupported platforms; second, the cold_path helper in profiler.rs should use #[inline(never)] instead of #[inline] to ensure the #[cold] optimization hint is not discarded by the compiler.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| #[cfg(all(not(feature = "test-support"), target_os = "windows"))] | ||
| { | ||
| let mut info = unsafe { std::mem::zeroed() }; | ||
| let status = unsafe { windows::Wdk::System::SystemServices::RtlGetVersion(&mut info) }; | ||
| if status.is_ok() { | ||
| semver::Version::new( | ||
| info.dwMajorVersion as _, | ||
| info.dwMinorVersion as _, | ||
| info.dwBuildNumber as _, | ||
| ) | ||
| .to_string() | ||
| } else { | ||
| "unknown".to_string() | ||
| } | ||
| } |
There was a problem hiding this comment.
If the client crate is compiled on an unsupported platform (such as Android, iOS, or WebAssembly) without the test-support feature, none of the #[cfg] blocks will match. This will result in a compilation error because the function body of os_version will be empty and fail to return a String.
Adding a fallback block ensures the function compiles successfully on all platforms.
#[cfg(all(not(feature = "test-support"), target_os = "windows"))]
{
let mut info = unsafe { std::mem::zeroed() };
let status = unsafe { windows::Wdk::System::SystemServices::RtlGetVersion(&mut info) };
if status.is_ok() {
semver::Version::new(
info.dwMajorVersion as _,
info.dwMinorVersion as _,
info.dwBuildNumber as _,
)
.to_string()
} else {
"unknown".to_string()
}
}
#[cfg(not(any(
feature = "test-support",
target_os = "macos",
target_os = "linux",
target_os = "freebsd",
target_os = "windows",
)))]
{
"unknown".to_string()
}| #[cold] | ||
| #[inline] | ||
| pub(crate) fn cold_path() {} |
There was a problem hiding this comment.
Using #[inline] on a #[cold] function can cause the compiler to inline the empty function body {} directly into the caller. When this happens, the function call is eliminated, and the #[cold] attribute has no effect on the caller's branch prediction.
To ensure the compiler outlines the cold path and treats the branch as unlikely, the function should be marked with #[inline(never)] instead.
| #[cold] | |
| #[inline] | |
| pub(crate) fn cold_path() {} | |
| #[cold] | |
| #[inline(never)] | |
| pub(crate) fn cold_path() {} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/gpui/src/profiler.rs (1)
23-29: 🚀 Performance & Scalability | 🔵 Trivial
#[inline]risks eliminating the call site required for#[cold]branch hints.Combining
#[inline]with#[cold]is contradictory for this utility. The#[cold]attribute relies on the function call remaining to signal to LLVM that the path is unlikely.#[inline]encourages the optimizer to merge the function body into the caller, which, especially for an empty function, will likely strip the call entirely and nullify the branch-weight hint. Remove#[inline]to ensure the call site persists.♻️ Proposed change
#[cold] -#[inline] pub(crate) fn cold_path() {}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gpui/src/profiler.rs` around lines 23 - 29, The `cold_path` utility in `profiler.rs` combines `#[cold]` with `#[inline]`, which can let the optimizer inline away the call site and defeat the branch hint. Remove the `#[inline]` attribute from `cold_path` so the call remains visible to LLVM while keeping the function as the stable stand-in for `std::hint::cold_path`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/gpui/src/profiler.rs`:
- Around line 23-29: The `cold_path` utility in `profiler.rs` combines `#[cold]`
with `#[inline]`, which can let the optimizer inline away the call site and
defeat the branch hint. Remove the `#[inline]` attribute from `cold_path` so the
call remains visible to LLVM while keeping the function as the stable stand-in
for `std::hint::cold_path`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc490c83-d876-482a-abc5-200dade0cd54
📒 Files selected for processing (5)
crates/client/src/telemetry.rscrates/extension/src/extension_builder.rscrates/gpui/src/profiler.rscrates/gpui/src/profiler/actions.rscrates/theme_settings/src/theme_settings.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Caution PR Summary Skipped - Monthly Quota ExceededPR summary skipped as you have reached the free tier limit of 50 PR summaries per month. Please upgrade to a paid plan for MatterAI. Current Plan: Free Tier Upgrade your plan on the console here: https://app.matterai.so/ai-code-reviews?tab=Billing |
Summary by cubic
Build now works on stable Rust by removing nightly-only macros/hints, and buffer font zoom is now controllable from the UI via
theme_settings.New Features
adjust_buffer_font_sizeintheme_settingsso the UI can zoom buffer fonts.Refactors
cfg_select!with#[cfg]in telemetry OS detection and WASI SDK asset selection; added a stablecold_path()ingpuiand updated profiler and action timing code to use it instead ofstd::hint::cold_path.Written for commit cdfe93e. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Refactor / Performance
API Changes
adjust_buffer_font_sizein theme settings, making it available as part of the module’s public surface.