cut down comments - #50
Merged
Merged
Conversation
Apply the repo's own comment discipline to the three densest files in the
tree: delete banners, restatement, control-flow narration and test scaffolding
prose; compress surviving doc comments to one or two lines.
The load-bearing content is preserved, not lost:
- mm/src/tlb.rs — the 33-line banner replaying an old SMP hang is gone, but
the invariant it protected survives on PerCpuTlbState: ack goes true only
in the handler, false only in an initiator's push, both under queue's
lock, and wait_for_acks is a pure reader.
- The Amit et al. EuroSys '20 citation stays (a published technique, and the
only record of why the local invlpg trails the IPIs); asides naming
upstream implementation files do not.
- Every '# Safety' section is byte-for-byte unchanged.
Verified comments-only: a Rust-lexing comment stripper compares the code
token stream against HEAD, so any change to a non-comment byte fails. Its
self-test covers eight cases -- deleted line, altered literal, swapped memory
ordering, dropped visibility, mangled string, and a comma removed inside a
string literal all reject; comment removal and rustfmt's trailing-comma
normalisation accept.
Co-Authored-By: Claude <noreply@anthropic.com>
Sweep of the kernel's comment surface against the discipline CLAUDE.md already states -- default to no comment, express intent through naming and types, treat the urge to comment as a smell. Removes banners and section dividers, comments that restate the line beneath them, control-flow narration, design history and war stories, and test scaffolding prose. Doc comments are compressed to the facts a reader cannot recover from the code rather than deleted. Preserved deliberately: every '# Safety' / '# Panics' / '# Errors' section byte-for-byte, hardware and errata notes, locking and IRQ-ordering requirements, and specification citations -- the Linux ABI and RFC references are interoperability and licensing evidence, so they stay while asides naming an upstream implementation file do not. Also drops call-site copies of contracts the callee already documents: the SafeStack frame-lifetime rationale duplicated at two scheduler sites lives in run_switch, and AbortOnUnwind's own doc already names the scheduler context-switch spans as an audited holder. Every file is verified comments-only, not merely reviewed: a Rust-lexing stripper compares the code token stream against HEAD, so touching any non-comment byte fails. String and char literals are placeholder-protected so their contents compare exactly, while rustfmt's trailing-comma normalisation is tolerated. Its self-test covers eight cases -- deleted line, altered literal, swapped memory ordering, dropped visibility, mangled string, and a comma removed inside a string literal all reject; comment removal and reformatting accept. just build passes; check_stack_sizes, check_kernel_softfloat, check_registry_sections and check_kernel_pml4_writer all unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
Second batch of the comment cleanup, plus the non-Rust surfaces. Assembly is deliberately near-untouched: CLAUDE.md exempts register-contract notes, and in these four files that is most of the comment volume -- the syscall entry state (RCX/R11/RAX, user RSP intact, SWAPGS not yet performed), which MSR installs each entry point, and why offsets are const offset_of! operands rather than hand-mirrored numbers. Only banners and step narration go. scripts/*.sh is left alone on purpose. CLAUDE.md cites those headers as the documentation of each check, and the --self-test paths embed deliberate Rust comment fixtures that gate_expect_silent asserts the gate stays quiet on; deleting a "comment" inside one of those heredocs would break a gate self-test, which the build treats as a failure. Verification tools are now type-aware -- feeding a .go file to a Rust parser reported a spurious failure, so parsecheck/codeguard/hygiene skip non-.rs and Go is checked with gofmt, go vet and go build instead. just build passes. gofmt clean, go vet and go build pass, just --list parses. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the sweep. Also fixes a false alarm in the verification pipeline. Removing a comment that sat between `use` or `mod` items unblocks rustfmt's reorder_imports and reorder_modules, both default-on, so the formatter then sorts items it previously could not touch. The comments-only checker saw that as a code change. It is not: the token multiset is identical and item order carries no meaning in Rust. The checker now reports three verdicts -- OK (identical token stream), REORDER (same multiset, rustfmt sort, accepted), CORRUPT (hard failure) -- and proves the reorder case with a real lexer rather than whitespace splitting, which had been gluing `TestResult};` into a single token and manufacturing differences. Verification order also corrected: prove comments-only against HEAD BEFORE running rustfmt, since formatting legitimately reorders tokens afterwards. just build passes; all four ELF gates unchanged. Self-test still rejects all six real violation classes. Co-Authored-By: Claude <noreply@anthropic.com>
Re-chunked the remaining work into 154 units (median ~500 comment lines each, down from ~1900) after the pilot critique traced quality variance to oversized chunks -- agents on very large files were silently ignoring the rubric's own limits. Assembly here is comment-only by construction: stripping every '#' comment from boot/limine_entry.s and slopos-ostd/src/irq/asm/handlers.s leaves an instruction and directive stream byte-identical to HEAD. The register contracts CLAUDE.md exempts are untouched; only banners and step narration went. Go host wrapper passes gofmt, go vet and go build. just build passes; all four ELF gates unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
Continues the tree-wide comment cleanup under the amended rubric. Every file verified comments-only against HEAD before formatting; just build and the four ELF gates pass. Co-Authored-By: Claude <noreply@anthropic.com>
The per-file `rustfmt --edition 2024` used during the sweep mis-sorted imports in the crates that declare `edition = "2021"` (hermetic, karch, ktesting, kernel-services, service-core), because rustfmt's import ordering changed in the 2024 style edition. `cargo fmt --all` reads each crate's own edition and is what CLAUDE.md mandates, so it is the authority here. Fifteen files re-sorted. Verified as pure reorderings: thirteen report REORDER (identical token multiset) and two are comments-only. `cargo fmt --all --check` is clean, just build passes, and all twenty framekernel gates pass -- including check_safe_contract_surface at 0 and tcb_ratio unchanged at 0.490 %. The commit script now runs `cargo fmt --all` so the mismatch cannot recur. Co-Authored-By: Claude <noreply@anthropic.com>
The one code change in this sweep, made deliberately and recorded here.
early_console.rs read:
static MOCK_BUFFER: [AtomicU8; MOCK_CAP] = {
// Inline constant initializer for an array of `AtomicU8`.
[const { AtomicU8::new(0) }; MOCK_CAP]
};
The comment restated the expression verbatim, so the sweep removed it. That
left a block whose only purpose had been to hold it. The code still compiled
because rustc's `unused_braces` skips multi-line blocks -- until `cargo fmt
--all` collapsed the block onto one line, at which point the lint fired and
`-D warnings` turned it into a build error.
Removing the braces is what rustc itself suggests and is behaviour-preserving:
a block wrapping a single expression evaluates to that expression.
Two similar single-expression blocks elsewhere (boot/src/tests/gdt_tests.rs,
drivers/src/tty/table.rs) were left alone -- they build clean and the second
scopes a lock guard, so its braces may be deliberate.
just build passes, userland builds, cargo fmt --all --check is clean.
Co-Authored-By: Claude <noreply@anthropic.com>
A five-lens audit re-read the whole 62k-line diff and checked, for every
high-risk deleted line, whether the fact survived anywhere in the tree. The
safety lens came back clean -- every '# Safety', '# Panics' and '# Errors'
section and every doc-test verified byte-identical. The other four found nine
facts that were genuinely gone. All are restored here, compressed rather than
reinstated verbatim.
Concurrency (the two that mattered most):
- commit_blocked_deschedule documented the lost-wakeup window before
unschedule_task but not the residual one after: a wake landing between the
re-check and the caller's schedule() enqueues via its own schedule_task, so
the task is dispatched later rather than stranded. That half is the reason
the function's contract is sound and it was nowhere in the tree.
- AbortOnUnwind's type doc insists every normal exit disarms. The scheduler
deliberately keeps a guard armed across a context switch, and the note
saying why (a descheduled frame cannot unwind until its task resumes) had
gone, leaving the site looking like a bug.
Licensing, per CLAUDE.md: mm/src/elf.rs lost the only naming of the ELF gABI /
System V ABI it reproduces ~20 constants from, and abi/src/quota.rs lost the
clause recording that ABI numbers and struct layouts are interface facts
carrying no copyright -- which is the evidence that the compatibility work is
sound, not merely a citation.
Also restored: the 60 Hz-frame derivation behind VBLANK_TIMEOUT_MS = 25, the
fixed-point rounding algebra that decodes the seven `+ 127` terms in
gfx/blend.rs, the Acquire/Release rationale on Task::fs_base (the cross-CPU
reader is the next prepare_switch_to), the collect-under-lock/dispatch-outside
rule in net/src/timer.rs, and a summary line on an ist_stacks doc that began on
a '# Panics' heading and so rendered headless in rustdoc.
The audit also caught the one place this pass invented prose instead of
shortening it -- a five-line rewrite in limine_entry.s that I had added. Its
content is correct and was verified against the code, but it broke the rule the
rest of the sweep was held to, so it is cut to two lines.
Verified comments-only; just build, all four ELF gates and cargo fmt --all
--check pass.
Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 1054 files, which is 954 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1054)
You can disable this status message by setting the 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.