Add: codestyle rule on pto:: qualification in kernels - #1549
Conversation
Kernel sources carry two conventions. The collectives kernels qualify (pto::Stride<...>) and use no using-directive; the older scene-test kernels open with `using namespace pto;` and use bare names. Each is internally consistent, and nothing recorded which one new code should follow, so the split propagates by whichever file an author copied from. Rule 11 names qualification as the direction, because it is the only spelling that compiles whether or not a using-directive is in scope, and because a file-scope `using namespace` is what most style guides steer away from. New kernels qualify and do not add the directive; existing bare files are not defects and need no churn. The load-bearing part is the constraint that a file is qualified completely or not at all. Shape, Stride, Tile and GlobalTensor all live in namespace pto, so qualifying one and leaving its siblings bare yields adjacent lines that read worse than either consistent state. That is a real failure mode rather than a hypothetical: an earlier revision of PR hw-native-sys#1547 rewrote Stride across 24 files and left Shape alone, and the resulting mixture is why it was reverted. A repo-wide sweep is ruled out with the measurement that rules it out: the 142 files carrying the directive hold roughly 3,200 bare uses, of which Tile alone is 2080. Rewriting them would touch every kernel, collide with every in-flight branch, and risk regex damage for no functional gain. Unifying the tree in one go needs its own decision plus a lint rule to hold the line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 |
1 participant
Summary
Kernel sources carry two conventions, and nothing recorded which one new code should follow — so the split propagates by whichever file an author happened to copy from:
tests/st/worker/collectives, someexamples/workers/l3)pto::Stride<…>using namespace pto;Stride<…>Each is internally consistent, so neither is a defect on its own. Rule 11 names qualification as the direction — it is the only spelling that compiles whether or not a using-directive is in scope, and a file-scope
using namespaceis what most style guides steer away from.Deliberately incremental, in the same shape as rule 10: new kernels qualify and do not add the directive; existing bare files are not defects and need no churn.
The load-bearing part is the half-qualified ban
Shape,Stride,TileandGlobalTensorall live innamespace pto, so qualifying one and leaving its siblings bare produces:which reads worse than either consistent state. A half-qualified file is a defect where a fully bare one is not.
This is not hypothetical. An earlier revision of #1547 rewrote
Strideacross 24 files and leftShapealone; a reviewer caught the adjacent-line mixture and it was reverted. The rule exists so the next person does not repeat it.No repo-wide sweep, with the measurement that rules it out
As of 2026-07-28, the 142 files carrying
using namespace pto;contain roughly 3,200 bare uses:TileGlobalTensorShapeStrideRewriting them is purely stylistic, would touch every kernel in the repo, collide with every in-flight branch, and risk regex damage to member aliases and comments for no functional gain. Unifying in one go needs its own decision plus a lint rule (e.g.
google-build-using-namespace) to hold the line — otherwise it drifts back.Note also that stripping qualification is not a safe inverse: 12 files use
pto::and have no using-directive, directly or transitively — 9 of them intests/st/worker/collectives. Bare names would not compile there.Verification
markdownlint-cli2clean (v0.20.0, the version pre-commit pins)