diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 9a98611860..d082f6e253 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -50,6 +50,9 @@ - [About the compiler team](./compiler-team.md) - [Using Git](./git.md) - [Mastering @rustbot](./rustbot.md) +- [Running LLMs](./llm-guidance/index.md) + - [Writing code with LLMs](./llm-guidance/writing.md) + - [Reviewing code with LLMs](./llm-guidance/reviewing.md) - [Walkthrough: a typical contribution](./walkthrough.md) - [Implementing new language features](./implementing-new-features.md) - [Stability guarantees](./stability-guarantees.md) diff --git a/src/about-this-guide.md b/src/about-this-guide.md index 4dc453a204..cc98ad7b2b 100644 --- a/src/about-this-guide.md +++ b/src/about-this-guide.md @@ -72,6 +72,7 @@ You might also find the following sites useful: - [rustc API docs] -- rustdoc documentation for the compiler, devtools, and internal tools - [Forge] -- contains documentation about Rust infrastructure, team procedures, and more +- [`rust-lang/rust`]'s [LLM policy] - [compiler-team] -- the home-base for the Rust compiler team, with description of the team procedures, active working groups, and the team calendar. - [std-dev-guide] -- a similar guide for developing the standard library. @@ -93,7 +94,7 @@ You might also find the following sites useful: For example, searching for `* -> vec` should find all functions that return a `Vec`. _Hint:_ Find more tips and keyboard shortcuts by typing `?` on any Rustdoc page! - +[LLM policy]: https://forge.rust-lang.org/policies/llm-usage.html [rustc dev guide]: about-this-guide.md [gsearchdocs]: https://www.google.com/search?q=site:doc.rust-lang.org+your+query+here [stddocs]: https://doc.rust-lang.org/std @@ -113,3 +114,4 @@ You might also find the following sites useful: [std-dev-guide]: https://std-dev-guide.rust-lang.org/ [rust-analyzer book]: https://rust-analyzer.github.io/book/ [z]: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler +[`rust-lang/rust`]: https://github.com/rust-lang/rust/ diff --git a/src/contributing.md b/src/contributing.md index 7f4779515b..05685d9f2f 100644 --- a/src/contributing.md +++ b/src/contributing.md @@ -517,6 +517,12 @@ This is used for [RFCs], issues, and pull requests. [rfcbot]: https://github.com/anp/rfcbot-rs/ [RFCs]: https://github.com/rust-lang/rfcs +## LLM policy + +See [Forge][LLM policy]. + +[LLM policy]: https://forge.rust-lang.org/policies/llm-usage.html + ## Helpful links and information This section has moved to the ["About this guide"] chapter. diff --git a/src/conventions.md b/src/conventions.md index 0440fb0c5e..023db80481 100644 --- a/src/conventions.md +++ b/src/conventions.md @@ -141,6 +141,35 @@ if foo { If you want to leave a note in the codebase, use `// FIXME` instead. +### Follow the style of surrounding code + +Use existing helpers and avoid duplicating logic or validation. + +### [Avoid duplicated sources of truth](https://react.dev/learn/choosing-the-state-structure) + +Trying to keep data in sync between two different places is a code smell. + +### Use types to enforce invariants + +When practical, [make invalid states unrepresentable](https://kentcdodds.com/blog/make-impossible-states-impossible), [not just checked at construction time](https://lexi-lambda.github.io/blog/2020/11/01/names-are-not-type-safety/). + +### Write useful comments + +[Write comments that say *why*][mit-comment-style] you have done a thing, not *what* you have done. +It's ok to go into detail about non-obvious bugs. + +[mit-comment-style]: https://mitcommlab.mit.edu/broad/commkit/coding-and-comment-style/ + +### Preserve existing behavior + +Consider platform differences and error cases. +Look for relevant tests that exercise the edge cases. + +### Work in small steps + +Work in small, independently testable steps. +Run the relevant tests after every meaningful change, so you know where you first went wrong. + ## Using crates from crates.io @@ -159,11 +188,13 @@ you rename a method, then put that rename into its own commit, along with the renames of all the uses. **More commits is usually better.** If you are doing a large change, -it's almost always better to break it up into smaller steps that can be independently understood. +it's almost always better to break it up into smaller steps that can be [independently understood][atomic commits]. The one thing to be aware of is that if you introduce some code following one strategy, then change it dramatically (versus adding to it) in a later commit, that 'back-and-forth' can be confusing. +[atomic commits]: https://github.blog/developer-skills/github/write-better-commits-build-better-projects/#%e2%9a%9b%ef%b8%8f-resize-and-stabilize-the-commits + **Format liberally.** While only the final commit of a PR must be correctly formatted, it is both easier to review and less noisy to format each commit individually using `./x fmt`. diff --git a/src/getting-started.md b/src/getting-started.md index a5da912aa4..12167347d4 100644 --- a/src/getting-started.md +++ b/src/getting-started.md @@ -48,6 +48,8 @@ But avoid using LLM tools that generate long, complex words. In daily teamwork, **simple and clear words** are best for easy understanding. Even small typos or grammar mistakes can make you seem more human, and people connect better with humans. +See also [our LLM policy](https://forge.rust-lang.org/policies/llm-usage.html). + ### Experts Not all `t-compiler` members are experts on all parts of `rustc`; diff --git a/src/llm-guidance/index.md b/src/llm-guidance/index.md new file mode 100644 index 0000000000..3709e7ead8 --- /dev/null +++ b/src/llm-guidance/index.md @@ -0,0 +1,10 @@ +# Working with LLMs + +This section is a list of guidelines for working with LLMs, as well as a summary of the moderation policy. +This is not the policy itself; see [Forge][LLM policy]. +If the two conflict, Forge is canonical. + +If you are writing code using an LLM, see [Writing LLM-created code](./writing.md). +If you are reviewing LLM-created code, or reviewing using an LLM, see [Reviewing with LLMs](./reviewing.md). + +[LLM policy]: https://forge.rust-lang.org/policies/llm-usage.html diff --git a/src/llm-guidance/reviewing.md b/src/llm-guidance/reviewing.md new file mode 100644 index 0000000000..44ae4b9e6c --- /dev/null +++ b/src/llm-guidance/reviewing.md @@ -0,0 +1,107 @@ +# Reviewing with LLMs + +## Using an LLM to review code + +- If a more reliable tool, such as a linter or formatter, already exists for the language you're writing, we strongly suggest using that tool instead of or in addition to the LLM. +- Configure LLM review tools to reduce false positives and excessive focus on trivialities, as these are common, exhausting failure modes. +- Wherever possible, ask an LLM to *generate or configure a linter*, which you then tell it to run. + This both saves on token costs, and allows people who are not using an LLM to run the analysis. + For example, if you have a codebase-specific wrapper around command spawning, + rather than getting an LLM to look for places where you should use the wrapper, + [configure clippy to disallow `Command::new`][disallowed-methods]. +- LLMs sometimes prefer LLM-generated output, particularly output from the same + model. Treat LLM review as advisory, and do not rely on the model that + produced a change as its only reviewer. + +[disallowed-methods]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-methods + +## Reviewing LLM-created code + +First, add the new `ai-assisted` label to the PR. + +### Rules + +We expect everyone to follow the new policy, not just authors. +That means it is **your responsibility** to check whether an `ai-assisted` PR touches an area that's disallowed by the policy. +You may request that the author redo it without LLM-generated code, in which case this section doesn't apply. + +The following areas are currently banned: +- Code that affects soundness. If the author is not an org member who is experienced in the domain, you are required to close the PR. +- Diagnostics. All user-facing diagnostics must be human-written. +- Docs. All public doc-comments, and all `// SAFETY` comments, must be human-written. + +"Code that affects soundness" is both broader and narrower than it sounds. +It's broader because almost all of the compiler is relevant to soundness; +it's narrower because there's quite a lot of rust-lang/rust that isn't the compiler +(library, bootstrap, compiletest, rustdoc, CI, ...). + +If in doubt, we suggest this criteria: +Do not allow LLM-generated code for parts of the compiler where [wrong code does not look wrong][joel-wrong]. +Ultimately, this is up to your judgement as a reviewer. + +[joel-wrong]: https://www.joelonsoftware.com/2005/05/11/making-wrong-code-look-wrong/ + +You are still expected to respect your [r+ rights](../compiler-team.html#r-rights). +Please do not merge PRs unless you are confident in that part of that code, +even if the maintainer does not wish to review LLM PRs. + +### Guidelines + +Point people to [#llm-mentoring] liberally. +Deal with low-quality PRs by closing the PR and asking the author to follow the policy. +Deal with borderline PRs by asking the author to put in the work themselves rather than offloading it to you. +For example, ask them to reproduce the bug, explain the change in their own +words, identify relevant edge cases, or add or justify tests. + +If you find yourself suggesting the same fixes on multiple PRs, +consider adding them to the dev-guide. + +#### Missing disclosure + +If you see a PR that is "obviously" LLM-created without disclosing that use, you have the option—but not the responsibility—to close it unilaterally. + +We suggest using the following wording: + +```markdown +This PR appears to be LLM-generated without disclosing use of an LLM, so I am going to close this PR. +You are welcome to open additional PRs as long as they follow our [policy][forge-page]. +For more information, see [#llm-mentoring] on Zulip. + +[#llm-mentoring]: https://rust-lang.zulipchat.com/join/rlfvpemsaacs3pfi6kwqnqjb/ +[forge-page]: https://forge.rust-lang.org/policies/llm-usage.html +``` + +Examples of "obvious" LLM tells are: +- PR descriptions that are completely wrong/don't match the code. +- PR descriptions that state the exact tests that were run (e.g. `./x test --stage 1 tests/ui/.rs`) or useless tests such as `git diff --check`. +- Responses to reviewer questions that fall into one of the above categories. + +You do not have an obligation to detect LLM-created PRs; +you don't need to play detective. + +PR templates will have a "Did you use an LLM?" question so that this rarely comes up. +If the author deleted the question without answering it, you can close the PR, no questions asked. + +#### Missing solicited reviewer + +If the PR discloses use, but does not assign a reviewer following the [experiment guidelines], you can close it similarly: + +```markdown +You've opened an LLM-generated PR, but it's in the normal review queue, which breaks our [policy][experiment guidelines]. +I am going to close this PR. +Please do not re-open it until you find a project member who has volunteered to review it. +For more information, see [#llm-mentoring] on Zulip. + +[#llm-mentoring]: https://rust-lang.zulipchat.com/join/rlfvpemsaacs3pfi6kwqnqjb/ +[forge-page]: https://forge.rust-lang.org/policies/llm-usage.html +``` + +#### Missing tests, low-quality, or not self-reviewed + +If a PR is clearly not ready for review, you do not have to review it. +It's ok to simply skim the PR and tell the author "you need to add tests before I can review this". +If you notice on your skim that the PR is clearly the wrong approach, it's ok to close the PR and tell the author to talk with you in the [#llm-mentoring] channel before opening a new PR. + +[experiment guidelines]: https://forge.rust-lang.org/policies/llm-usage.html#experiment-llm-created-code-changes-intended-for-review +[#llm-mentoring]: https://rust-lang.zulipchat.com/join/rlfvpemsaacs3pfi6kwqnqjb/ +[forge-page]: https://forge.rust-lang.org/policies/llm-usage.html diff --git a/src/llm-guidance/writing.md b/src/llm-guidance/writing.md new file mode 100644 index 0000000000..6af83fd424 --- /dev/null +++ b/src/llm-guidance/writing.md @@ -0,0 +1,170 @@ +# Writing LLM-created code + +LLMs are a valuable tool, but one that is easy to misuse. +The main risks are **overwhelming volume** and **lack of understanding**. +When writing your PR, keep in mind that there is a person on the other end who needs to review and understand your change, +and that other people in the project will need to read your code for years to come. +Help us out by making your change small, targeted, and high-quality. + +Keep in mind this quote: + +> Programs must be written for people to read, and only incidentally for machines to execute. +> — Harold Abelson and Gerald Jay Sussman, [*Structure and Interpretation of Computer Programs*][sicp] + +[sicp]: https://web.mit.edu/6.001/6.037/sicp.pdf +[#llm-mentoring]: https://rust-lang.zulipchat.com/join/rlfvpemsaacs3pfi6kwqnqjb/ + +## Rules + +### Before you write code + +Before anything else, find a reviewer who volunteers to review your PR. +If you do not know where to find a reviewer, ask in [#llm-mentoring] on Zulip. +Your first message should say: + +- your relevant experience, so we can find an issue that's suitable for you +- which problem (or kind of problem) you want to work on +- (optional) ideas you have so far for a solution +- (optional) how you expect to test your solution + +Mentors are here to help. +Talking to them early helps you avoid wasted work. + +### While working + +Write your own doc-comments, `// SAFETY` comments, diagnostic wording, and soundness-critical code. +As before, you can use an LLM to review your work, but not to write it from scratch. +If you don't know what counts as soundness-critical, discuss it with your reviewer. + +### When opening a PR + +Disclose your use of LLMs, following the disclosure guidelines below. +Write the disclosure yourself. +You may use an LLM to privately review a disclosure you have written, but not to draft or rewrite it. + +**Write your own PR description and comments**. +We want to hear from you, not from your agent. +LLM-created PR descriptions are banned. +LLM-created Github comments are banned. + +## Guidelines + +### Before you write code + +Start with one PR at a time. +Keep your changes small enough that you and your reviewer can understand every part of them. +Go slow. + +Do not use an LLM for `E-easy` issues; those are meant for you to write the code yourself. +Ask first before working on an `E-mentor` issue; mentors may not want to work with LLM-generated code. + +Determine whether this is a *useful* and *well-scoped* change. +For example: + +- Search for related issues and PRs. +- Find relevant code, tests, git history, and Zulip discussion. +- If this is a cross-cutting change, consult the "cross-cutting" section of [the contributing docs](../contributing.md#pull-requests). +- [Make the smallest change that fixes the problem][small-cls]. + Do not combine it with unrelated refactors or cleanups. + +[small-cls]: https://google.github.io/eng-practices/review/developer/small-cls.html + +### While working + +When fixing a bug, verify that your test fails before and succeeds after your change. +Consult [adding new tests](../tests/adding.md) and [best practices](../tests/best-practices.md) for test procedures. +Tests are absolutely required; either existing tests or new tests you write. +Untested LLM PRs will not be merged. + +Mass renames or rewrites should *strongly* prefer using a proper syntax rewrite tool, such as [`ast-grep`]. +You may use an LLM for generating the instructions for that tool, but you should be very cautious about performing the rewrite directly with an LLM. + +Consider [performance] as you write. + +[performance]: ../contributing.md#performance + +Think before adding dependencies; +consult our [guidance for new dependencies][crates-io]. + +[crates-io]: ../crates-io.md + +Verify your understanding against the existing code, documentation, and tests. +You can get better advice from your LLM by telling *it* to read the relevant materials. +Do not rely on the LLM as a source of truth. + +### Write maintainable code + +Treat generated code as a *draft*, not a final product. +Follow our [correctness and maintainability conventions](../conventions.md#cc). + +Avoid unnecessary abstractions and compatibility layers. +Rustc does not have a stable API; you do not need to preserve backwards compatibility for internal compiler APIs. + +### Commit structure + +See ["How to structure your PR"](../contributing.md#er). +Commit messages must be authored by you, not your LLM. + +### Before opening a PR + +Review your own PR before opening it: +Does it make sense? Can you tell what the goal of the PR is? Does it achieve that goal? + +Remove outdated or prototyping code and debugging. + +Re-read the whole diff, *not* just your conversation with the agent. +Your reviewer is going to see your code, not your conversation. + +[Run tests](../tests/running.md) to verify your change works. +Do NOT report which UI tests you ran in the PR description; +that's noise, since CI will run them anyway. +If you did manual testing or benchmarking, do report that, +but note that all LLM PRs must have automated tests. + +[Review diagnostic snapshots](../tests/adding.md#step-4-review-the-output); +don't simply `--bless` them away. + +We recommend using a different model for adversarial local review before publishing your changes. +You're still responsible for reviewing your own changes yourself. + +### Understand your own change + +We want you to understand and be able to explain your own change and its edge cases. +Asking the LLM can be a starting point but it's not the same as explaining it yourself. + +Try explaining your change to yourself before opening the PR. +For example, ask yourself: + +- What is the original bug? When does it happen? How severe is it? What causes it? +- Why is this the right fix? Are there other fixes possible? What are their advantages or disadvantages? +- Are there any edge cases? Does your code handle them? +- Does the code have existing [invariants](https://brooker.co.za/blog/2023/07/28/ds-testing.html)? + Did you preserve those invariants? +- What behavior is *unchanged*? What test establishes that? +- Why does your test trigger the bug? +- What are you still not certain about? + +It's ok to be uncertain and to ask for help. +We would much rather help you because you're not sure than have you guess wrong and then have to reverse-engineer where you went wrong. + +[`ast-grep`]: https://astgrep.com/ + +### Disclosure guidelines + +Disclose the *extent* and *purpose* of your LLM use. +We don't care which model you used, but we do care whether you used the LLM to implement the idea or to come up with it. + +**Good** examples: + +> LLM disclosure: I wrote the three commits by hand after viewing profiling data. I used an LLM to review the commits before submitting. The LLM identified that `ImplString::is_negative` was no longer used, so I removed that field by hand. + +> Created with the help of an LLM, which: +> - traced the missing cache hits to the unconditional `return(pass)` by inspecting Fastly vs CloudFront headers, +> - reviewed the git history to understand why the snippet was added, and +> - made the VCL change. + +**Bad** examples: + +> 🤖 Generated with Claude Code + +> `Co-Authored-By: Claude Opus 4.6 (1M context) ` diff --git a/src/tests/best-practices.md b/src/tests/best-practices.md index b6daffa668..4e6f40c4b7 100644 --- a/src/tests/best-practices.md +++ b/src/tests/best-practices.md @@ -140,7 +140,7 @@ A good rule of thumb is non-trivial things exercised in the test deserves some explanation to help other contributors to understand. This may include remarks on: -- How an ICE can get triggered if it's quite elaborate. +- How an ICE or bug can get triggered if it's quite elaborate. - Related issues and tests (e.g. this test is like another test but is kept separate because...). - Platform-specific behaviors.