From 283a56b6bde009ea4d6771df0c84f662c447ba2b Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sat, 24 Jun 2023 17:24:13 +0200 Subject: [PATCH 1/4] Move team pages into team/. --- src/SUMMARY.md | 8 ++++---- src/{ => team}/meetings.md | 0 src/{ => team}/membership.md | 0 src/{ => team}/reviewing.md | 0 src/{team.md => team/summary.md} | 0 5 files changed, 4 insertions(+), 4 deletions(-) rename src/{ => team}/meetings.md (100%) rename src/{ => team}/membership.md (100%) rename src/{ => team}/reviewing.md (100%) rename src/{team.md => team/summary.md} (100%) diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 955f7fa..c51f4fb 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -4,10 +4,10 @@ [Getting started](./getting-started.md) -- [The library team](./team.md) - - [Meetings](./meetings.md) - - [Membership](./membership.md) - - [Reviewing](./reviewing.md) +- [The library team](./team/summary.md) + - [Meetings](./team/meetings.md) + - [Membership](./team/membership.md) + - [Reviewing](./team/reviewing.md) --- diff --git a/src/meetings.md b/src/team/meetings.md similarity index 100% rename from src/meetings.md rename to src/team/meetings.md diff --git a/src/membership.md b/src/team/membership.md similarity index 100% rename from src/membership.md rename to src/team/membership.md diff --git a/src/reviewing.md b/src/team/reviewing.md similarity index 100% rename from src/reviewing.md rename to src/team/reviewing.md diff --git a/src/team.md b/src/team/summary.md similarity index 100% rename from src/team.md rename to src/team/summary.md From 0278893fdd2ee22915673a29efa596e888a6e34f Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sat, 24 Jun 2023 17:27:01 +0200 Subject: [PATCH 2/4] Remove tools and bots section. There's almost no information in there. We can add it back when we have anything useful to say. --- src/SUMMARY.md | 7 ------- src/tools-and-bots/bors.md | 15 --------------- src/tools-and-bots/crater.md | 23 ----------------------- src/tools-and-bots/summary.md | 3 --- src/tools-and-bots/timer.md | 9 --------- 5 files changed, 57 deletions(-) delete mode 100644 src/tools-and-bots/bors.md delete mode 100644 src/tools-and-bots/crater.md delete mode 100644 src/tools-and-bots/summary.md delete mode 100644 src/tools-and-bots/timer.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index c51f4fb..c64a529 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -51,10 +51,3 @@ - [safety comments policy](./documentation/safety-comments.md) - [how to write documentation](./documentation/how-to-write-documentation.md) - [reviewing doc changes](./documentation/reviewing-doc-changes.md) - ---- - -- [Tools and bots](./tools-and-bots/summary.md) - - [`@bors`](./tools-and-bots/bors.md) - - [`@rust-timer`](./tools-and-bots/timer.md) - - [`@craterbot`](./tools-and-bots/crater.md) diff --git a/src/tools-and-bots/bors.md b/src/tools-and-bots/bors.md deleted file mode 100644 index 3a213de..0000000 --- a/src/tools-and-bots/bors.md +++ /dev/null @@ -1,15 +0,0 @@ -# `@bors` - -**Status:** Stub - -PRs to the standard library aren’t merged manually using GitHub’s UI or by pushing remote branches. Everything goes through [`@bors`](https://github.com/rust-lang/homu). - -You can approve a PR with: - -```ignore -@bors r+ -``` - -## Rolling up - -For Libs PRs, rolling up is usually fine, in particular if it's only a new unstable addition or if it only touches docs. See the [rollup guidelines](https://forge.rust-lang.org/compiler/reviews.html#rollups) for more details on when to rollup. diff --git a/src/tools-and-bots/crater.md b/src/tools-and-bots/crater.md deleted file mode 100644 index 990c499..0000000 --- a/src/tools-and-bots/crater.md +++ /dev/null @@ -1,23 +0,0 @@ -# `@craterbot` - -**Status:** Stub - -[Crater](https://github.com/rust-lang/crater) is a tool that can test PRs against a public subset of the Rust ecosystem to estimate the scale of potential breakage. - -You can kick off a crater run by first calling: - -```ignore -@bors try -``` - -Once that finishes, you can then call: - -```ignore -@craterbot check -``` - -to ensure crates compile, or: - -```ignore -@craterbot run mode=build-and-test -``` diff --git a/src/tools-and-bots/summary.md b/src/tools-and-bots/summary.md deleted file mode 100644 index 6a76859..0000000 --- a/src/tools-and-bots/summary.md +++ /dev/null @@ -1,3 +0,0 @@ -# Tools and bots - -**Status:** Stub diff --git a/src/tools-and-bots/timer.md b/src/tools-and-bots/timer.md deleted file mode 100644 index 2f7c9b3..0000000 --- a/src/tools-and-bots/timer.md +++ /dev/null @@ -1,9 +0,0 @@ -# `@rust-timer` - -**Status:** Stub - -You can kick off a performance test using `@rust-timer`: - -```ignore -@bors try @rust-timer queue -``` From 9c201b6dc5559d7f1a5db743e9fb161d597e6d3b Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sat, 24 Jun 2023 18:36:52 +0200 Subject: [PATCH 3/4] Reorganise and clean up the whole guide. --- book.toml | 3 + src/SUMMARY.md | 75 ++++++--------- src/about-this-guide.md | 19 ---- src/about.md | 13 +++ .../doc-changes.md} | 11 +-- src/breaking-changes/new-trait-impls.md | 93 +++++++++++++++++++ .../breaking-changes/prelude.md | 9 +- src/breaking-changes/summary.md | 8 ++ .../breaking-changes/behavior.md | 9 -- .../breaking-changes/fundamental.md | 31 ------- .../breaking-changes/new-trait-impls.md | 76 --------------- .../breaking-changes/summary.md | 17 ---- src/code-considerations/design/public-apis.md | 9 -- src/code-considerations/design/summary.md | 9 -- .../performance/summary.md | 9 -- .../mem-and-exclusive-refs.md | 15 --- .../safety-and-soundness/summary.md | 13 --- src/code-considerations/summary.md | 11 --- .../using-unstable-lang/const-generics.md | 25 ----- .../using-unstable-lang/summary.md | 11 --- .../how-to-write-documentation.md | 0 .../stabilization.md | 2 +- src/documentation/summary.md | 4 - src/feature-lifecycle/api-change-proposals.md | 21 ----- src/feature-lifecycle/deprecation.md | 7 -- src/feature-lifecycle/summary.md | 3 - src/feature-lifecycle/tracking-issues.md | 16 ---- src/getting-started.md | 27 ------ .../doc-alias.md} | 0 .../performance => policy}/inline.md | 0 .../design => policy}/must-use.md | 0 .../safety-comments.md | 0 .../specialization.md | 4 - .../generics-and-unsafe.md | 0 .../may-dangle.md | 0 35 files changed, 151 insertions(+), 399 deletions(-) delete mode 100644 src/about-this-guide.md create mode 100644 src/about.md rename src/{documentation/reviewing-doc-changes.md => breaking-changes/doc-changes.md} (69%) create mode 100644 src/breaking-changes/new-trait-impls.md rename src/{code-considerations => }/breaking-changes/prelude.md (56%) create mode 100644 src/breaking-changes/summary.md delete mode 100644 src/code-considerations/breaking-changes/behavior.md delete mode 100644 src/code-considerations/breaking-changes/fundamental.md delete mode 100644 src/code-considerations/breaking-changes/new-trait-impls.md delete mode 100644 src/code-considerations/breaking-changes/summary.md delete mode 100644 src/code-considerations/design/public-apis.md delete mode 100644 src/code-considerations/design/summary.md delete mode 100644 src/code-considerations/performance/summary.md delete mode 100644 src/code-considerations/safety-and-soundness/mem-and-exclusive-refs.md delete mode 100644 src/code-considerations/safety-and-soundness/summary.md delete mode 100644 src/code-considerations/summary.md delete mode 100644 src/code-considerations/using-unstable-lang/const-generics.md delete mode 100644 src/code-considerations/using-unstable-lang/summary.md rename src/{documentation => development}/how-to-write-documentation.md (100%) rename src/{feature-lifecycle => development}/stabilization.md (98%) delete mode 100644 src/documentation/summary.md delete mode 100644 src/feature-lifecycle/api-change-proposals.md delete mode 100644 src/feature-lifecycle/deprecation.md delete mode 100644 src/feature-lifecycle/summary.md delete mode 100644 src/feature-lifecycle/tracking-issues.md delete mode 100644 src/getting-started.md rename src/{documentation/doc-alias-policy.md => policy/doc-alias.md} (100%) rename src/{code-considerations/performance => policy}/inline.md (100%) rename src/{code-considerations/design => policy}/must-use.md (100%) rename src/{documentation => policy}/safety-comments.md (100%) rename src/{code-considerations/using-unstable-lang => policy}/specialization.md (94%) rename src/{code-considerations/safety-and-soundness => tricky}/generics-and-unsafe.md (100%) rename src/{code-considerations/safety-and-soundness => tricky}/may-dangle.md (100%) diff --git a/book.toml b/book.toml index 4699954..32e2684 100644 --- a/book.toml +++ b/book.toml @@ -22,3 +22,6 @@ renderer = ["html"] git-repository-icon = "fa-github" git-repository-url = "https://github.com/rust-lang/std-dev-guide" edit-url-template = "https://github.com/rust-lang/std-dev-guide/edit/master/{path}" + +[output.html.redirect] +"/feature-lifecycle/stabilization.html" = "/development/stabilization.html" diff --git a/src/SUMMARY.md b/src/SUMMARY.md index c64a529..04b8a62 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -1,53 +1,30 @@ # Summary -[About this guide](./about-this-guide.md) - -[Getting started](./getting-started.md) +- [About this Guide](about.md) + +- [Contributing to the standard libraries]() + - [Building and debugging](./development/building-and-debugging.md) + - [Performance optimizations and benchmarking](./development/perf-benchmarking.md) + - [Writing documentation](./development/how-to-write-documentation.md) + - [Stabilizing a feature](./development/stabilization.md) + +- [Breaking changes](./breaking-changes/summary.md) + - [New trait implementations](./breaking-changes/new-trait-impls.md) + - [Prelude](./breaking-changes/prelude.md) + - [Doc changes](./breaking-changes/doc-changes.md) + +- [Policies]() + - [When to add `#[must_use]`](./policy/must-use.md) + - [Specialization](./policy/specialization.md) + - [When to `#[inline]`](./policy/inline.md) + - [Doc alias policy](./policy/doc-alias.md) + - [Safety comments policy](./policy/safety-comments.md) + +- [Tricky situations]() + - [Drop and `#[may_dangle]`](./tricky/may-dangle.md) + - [Generics and unsafe](./tricky/generics-and-unsafe.md) - [The library team](./team/summary.md) - - [Meetings](./team/meetings.md) - - [Membership](./team/membership.md) - - [Reviewing](./team/reviewing.md) - ---- - -- [Building and debugging libraries](./development/building-and-debugging.md) -- [Performance optimizations and benchmarking](./development/perf-benchmarking.md) - - ---- - -- [The feature lifecycle](./feature-lifecycle/summary.md) - - [API Change Proposals](./feature-lifecycle/api-change-proposals.md) - - [Using tracking issues](./feature-lifecycle/tracking-issues.md) - - [Stabilizing features](./feature-lifecycle/stabilization.md) - - [Deprecating features](./feature-lifecycle/deprecation.md) - ---- - -- [Code considerations](./code-considerations/summary.md) - - [Design](./code-considerations/design/summary.md) - - [Public APIs](./code-considerations/design/public-apis.md) - - [When to add `#[must_use]`](./code-considerations/design/must-use.md) - - [Breaking changes](./code-considerations/breaking-changes/summary.md) - - [Breakage from changing behavior](./code-considerations/breaking-changes/behavior.md) - - [Breakage from new trait impls](./code-considerations/breaking-changes/new-trait-impls.md) - - [`#[fundamental]` types](./code-considerations/breaking-changes/fundamental.md) - - [Breakage from changing the prelude](./code-considerations/breaking-changes/prelude.md) - - [Safety and soundness](./code-considerations/safety-and-soundness/summary.md) - - [Generics and unsafe](./code-considerations/safety-and-soundness/generics-and-unsafe.md) - - [Drop and `#[may_dangle]`](./code-considerations/safety-and-soundness/may-dangle.md) - - [`std::mem` and exclusive references](./code-considerations/safety-and-soundness/mem-and-exclusive-refs.md) - - [Using unstable language features](./code-considerations/using-unstable-lang/summary.md) - - [Const generics](./code-considerations/using-unstable-lang/const-generics.md) - - [Specialization](./code-considerations/using-unstable-lang/specialization.md) - - [Performance](./code-considerations/performance/summary.md) - - [When to `#[inline]`](./code-considerations/performance/inline.md) - ---- - -- [Documentation](./documentation/summary.md) - - [doc alias policy](./documentation/doc-alias-policy.md) - - [safety comments policy](./documentation/safety-comments.md) - - [how to write documentation](./documentation/how-to-write-documentation.md) - - [reviewing doc changes](./documentation/reviewing-doc-changes.md) + - [Meetings](./team/meetings.md) + - [Membership](./team/membership.md) + - [Reviewing](./team/reviewing.md) diff --git a/src/about-this-guide.md b/src/about-this-guide.md deleted file mode 100644 index f096a63..0000000 --- a/src/about-this-guide.md +++ /dev/null @@ -1,19 +0,0 @@ -# About this guide - -**Status:** Stub - -This guide is for contributors and reviewers to Rust's standard library. - -## Other places to find information - -You might also find the following sites useful: - -- [std API docs] -- rustdoc documentation for the standard library itself -- [Forge] -- contains documentation about rust infrastructure, team procedures, and more -- [libs-team] -- the home-base for the rust Library Team, with description - of the team procedures, active working groups, and the team calendar. - -[GitHub repository]: https://github.com/rust-lang/std-dev-guide/ -[std API docs]: https://doc.rust-lang.org/nightly/std/ -[Forge]: https://forge.rust-lang.org/ -[libs-team]: https://github.com/rust-lang/libs-team/ diff --git a/src/about.md b/src/about.md new file mode 100644 index 0000000..f39e210 --- /dev/null +++ b/src/about.md @@ -0,0 +1,13 @@ +# About this Guide + +Welcome to the std dev guide. + +This guide is maintained by the library team. + +The guide is not very complete yet. +Contributions to this guide are very welcome. + +Other useful documentation: + +- https://forge.rust-lang.org/ +- https://rustc-dev-guide.rust-lang.org/ diff --git a/src/documentation/reviewing-doc-changes.md b/src/breaking-changes/doc-changes.md similarity index 69% rename from src/documentation/reviewing-doc-changes.md rename to src/breaking-changes/doc-changes.md index 810dec3..79abbcc 100644 --- a/src/documentation/reviewing-doc-changes.md +++ b/src/breaking-changes/doc-changes.md @@ -1,13 +1,4 @@ -# Reviewing doc changes - -Most of the time, it is mostly reviewing that the documentation isn't wrong -in any way and that it follows the -[how to write documentation](./how-to-write-documentation.md) guideline. - -There is however something where we need to be extra careful: stability -guarantees. - -## Stability guarantees +# Breaking documentation changes First, short explanation about what a stability guarantee is: a statement in the document which explains what the item is doing in a precise case. For diff --git a/src/breaking-changes/new-trait-impls.md b/src/breaking-changes/new-trait-impls.md new file mode 100644 index 0000000..33d4cf8 --- /dev/null +++ b/src/breaking-changes/new-trait-impls.md @@ -0,0 +1,93 @@ +# Breakage from new trait impls + +A lot of PRs to the standard library are adding new impls for already stable traits, +which can break consumers in many weird and wonderful ways. +Below are some examples of breakage from new trait impls that +may not be obvious just from the change made to the standard library. + +## Inference breaks when a second generic impl is introduced + +Rust will use the fact that there's only a single impl for a generic trait during inference. +This breaks once a second impl makes the type of that generic ambiguous. +Say we have: + +```rust,ignore +// in `std` +impl From<&str> for Arc { .. } +``` + +```rust,ignore +// in an external `lib` +let b = Arc::from("a"); +``` + +then we add: + +```diff +impl From<&str> for Arc { .. } ++ impl From<&str> for Arc { .. } +``` + +then + +```rust,ignore +let b = Arc::from("a"); +``` + +will no longer compile, because we've previously been relying on inference to figure out the `T` in `Box`. + +This kind of breakage can be ok, but a [crater](https://github.com/rust-lang/crater/blob/master/docs/bot-usage.md) run should estimate the scope. + +## Deref coercion breaks when a new impl is introduced + +Rust will use deref coercion to find a valid trait impl if the arguments don't type check directly. +This only seems to occur if there's a single impl so introducing a new one may break consumers relying on deref coercion. +Say we have: + +```rust,ignore +// in `std` +impl Add<&str> for String { .. } + +impl Deref for String { type Target = str; .. } +``` + +```rust,ignore +// in an external `lib` +let a = String::from("a"); +let b = String::from("b"); + +let c = a + &b; +``` + +then we add: + +```diff,ignore + impl Add<&str> for String { .. } ++ impl Add for String { .. } +``` + +then + +```rust,ignore +let c = a + &b; +``` + +will no longer compile, because we won't attempt to use deref to coerce the `&String` into `&str`. + +This kind of breakage can be ok, but a [crater](https://github.com/rust-lang/crater/blob/master/docs/bot-usage.md) run should estimate the scope. + +## `#[fundamental]` types + +Type annotated with the `#[fundamental]` attribute have different coherence rules. +See [RFC 1023](https://rust-lang.github.io/rfcs/1023-rebalancing-coherence.html) for details. +That includes: + +- `&T` +- `&mut T` +- `Box` +- `Pin` + +Typically, the scope of breakage in new trait impls is limited to inference and deref-coercion. +New trait impls on `#[fundamental]` types may overlap with downstream impls and cause other kinds of breakage. + +[RFC 1023]: https://rust-lang.github.io/rfcs/1023-rebalancing-coherence.html diff --git a/src/code-considerations/breaking-changes/prelude.md b/src/breaking-changes/prelude.md similarity index 56% rename from src/code-considerations/breaking-changes/prelude.md rename to src/breaking-changes/prelude.md index 6c4a4d0..cf38517 100644 --- a/src/code-considerations/breaking-changes/prelude.md +++ b/src/breaking-changes/prelude.md @@ -1,15 +1,18 @@ # Breaking changes to the prelude -Making changes to the prelude can easily cause breakage because it impacts all Rust code. In most cases the impact is limited since prelude items have the lowest priority in name lookup (lower than glob imports), but there are two cases where this doesn't work. +Making changes to the prelude can easily cause breakage because it impacts all Rust code. +In most cases the impact is limited since prelude items have the lowest priority in name lookup (lower than glob imports), but there are two cases where this doesn't work. ## Traits -Adding a new trait to the prelude causes new methods to become available for existing types. This can cause name resolution errors in user code if a method with the same name is also available from a different trait. +Adding a new trait to the prelude causes new methods to become available for existing types. +This can cause name resolution errors in user code if a method with the same name is also available from a different trait. For this reason, [`TryFrom` and `TryInto`](https://github.com/rust-lang/rust/issues/33417) were only added to the prelude for the 2021 edition despite being stabilized in 2019. ## Macros -Unlike other item types, rustc's name resolution for macros does not support giving prelude macros a lower priority than other macros, even if the macro is unstable. As a general rule, avoid adding macros to the prelude except at edition boundaries. +Unlike other item types, rustc's name resolution for macros does not support giving prelude macros a lower priority than other macros, even if the macro is unstable. +As a general rule, avoid adding macros to the prelude except at edition boundaries. This issues was encoutered when trying to land the [`assert_matches!` macro](https://github.com/rust-lang/rust/issues/82913). diff --git a/src/breaking-changes/summary.md b/src/breaking-changes/summary.md new file mode 100644 index 0000000..9368382 --- /dev/null +++ b/src/breaking-changes/summary.md @@ -0,0 +1,8 @@ +# Breaking changes + +Breaking changes should be avoided when possible. +[RFC 1105](https://rust-lang.github.io/rfcs/1105-api-evolution.html) lays the foundations for what constitutes a breaking change. +Breakage may be deemed acceptable or not based on its actual impact, +which can be approximated with a [crater](https://github.com/rust-lang/crater/blob/master/docs/bot-usage.md) run. + +If the impact isn't too high, looping in maintainers of broken crates and submitting PRs to fix them can be a valid strategy. diff --git a/src/code-considerations/breaking-changes/behavior.md b/src/code-considerations/breaking-changes/behavior.md deleted file mode 100644 index 09aac4d..0000000 --- a/src/code-considerations/breaking-changes/behavior.md +++ /dev/null @@ -1,9 +0,0 @@ -# Breakage from changing behavior - -Breaking changes aren't just limited to compilation failures. Behavioral changes to stable functions generally can't be accepted. See [the `home_dir` issue](https://github.com/rust-lang/rust/pull/46799) for an example. - -An exception is when a behavior is specified in an RFC (such as IETF specifications for IP addresses). If a behavioral change fixes non-conformance then it can be considered a bug fix. In these cases, `@rust-lang/libs` should still be pinged for input. - -## For reviewers - -Look out for changes in existing implementations for stable functions, especially if assertions in test cases have been changed. diff --git a/src/code-considerations/breaking-changes/fundamental.md b/src/code-considerations/breaking-changes/fundamental.md deleted file mode 100644 index b3f3841..0000000 --- a/src/code-considerations/breaking-changes/fundamental.md +++ /dev/null @@ -1,31 +0,0 @@ -# `#[fundamental]` types - -**Status:** Stub - -Type annotated with the `#[fundamental]` attribute have different coherence rules. See [RFC 1023](https://rust-lang.github.io/rfcs/1023-rebalancing-coherence.html) for details. That includes: - -- `&T` -- `&mut T` -- `Box` -- `Pin` - -Typically, the scope of [breakage in new trait impls](./new-trait-impls.md) is limited to inference and deref-coercion. New trait impls on `#[fundamental]` types may overlap with downstream impls and cause other kinds of breakage. - -[RFC 1023]: https://rust-lang.github.io/rfcs/1023-rebalancing-coherence.html - -## For reviewers - -Look out for blanket trait implementations for fundamental types, like: - -```rust,ignore -impl<'a, T> PublicTrait for &'a T -where - T: SomeBound, -{ - -} -``` - -unless the blanket implementation is being stabilized along with `PublicTrait`. In cases where we really want to do this, a [crater] run can help estimate the scope of the breakage. - -[crater]: ../../tools-and-bots/crater.md diff --git a/src/code-considerations/breaking-changes/new-trait-impls.md b/src/code-considerations/breaking-changes/new-trait-impls.md deleted file mode 100644 index a42d41f..0000000 --- a/src/code-considerations/breaking-changes/new-trait-impls.md +++ /dev/null @@ -1,76 +0,0 @@ -# Breakage from new trait impls - -A lot of PRs to the standard library are adding new impls for already stable traits, which can break consumers in many weird and wonderful ways. The following sections gives some examples of breakage from new trait impls that may not be obvious just from the change made to the standard library. - -Also see [`#[fundamental]` types](./fundamental.md) for special considerations for types like `&T`, `&mut T`, `Box`, and other core smart pointers. - -## Inference breaks when a second generic impl is introduced - -Rust will use the fact that there's only a single impl for a generic trait during inference. This breaks once a second impl makes the type of that generic ambiguous. Say we have: - -```rust,ignore -// in `std` -impl From<&str> for Arc { .. } -``` - -```rust,ignore -// in an external `lib` -let b = Arc::from("a"); -``` - -then we add: - -```diff -impl From<&str> for Arc { .. } -+ impl From<&str> for Arc { .. } -``` - -then - -```rust,ignore -let b = Arc::from("a"); -``` - -will no longer compile, because we've previously been relying on inference to figure out the `T` in `Box`. - -This kind of breakage can be ok, but a [crater](../../tools-and-bots/crater.md) run should estimate the scope. - -## Deref coercion breaks when a new impl is introduced - -Rust will use deref coercion to find a valid trait impl if the arguments don't type check directly. This only seems to occur if there's a single impl so introducing a new one may break consumers relying on deref coercion. Say we have: - -```rust,ignore -// in `std` -impl Add<&str> for String { .. } - -impl Deref for String { type Target = str; .. } -``` - -```rust,ignore -// in an external `lib` -let a = String::from("a"); -let b = String::from("b"); - -let c = a + &b; -``` - -then we add: - -```diff,ignore -impl Add<&str> for String { .. } -+ impl Add for String { .. } -``` - -then - -```rust,ignore -let c = a + &b; -``` - -will no longer compile, because we won't attempt to use deref to coerce the `&String` into `&str`. - -This kind of breakage can be ok, but a [crater](../../tools-and-bots/crater.md) run should estimate the scope. - -## For reviewers - -Look out for new `#[stable]` trait implementations for existing stable traits. diff --git a/src/code-considerations/breaking-changes/summary.md b/src/code-considerations/breaking-changes/summary.md deleted file mode 100644 index 34073b7..0000000 --- a/src/code-considerations/breaking-changes/summary.md +++ /dev/null @@ -1,17 +0,0 @@ -# Breaking changes - -Breaking changes should be avoided when possible. [RFC 1105](https://rust-lang.github.io/rfcs/1105-api-evolution.html) lays the foundations for what constitutes a breaking change. Breakage may be deemed acceptable or not based on its actual impact, which can be approximated with a [crater](../../tools-and-bots/crater.md) run. - -There are strategies for mitigating breakage depending on the impact. - -For changes where the value is high and the impact is high too: - -- Using compiler lints to try phase out broken behavior. - -If the impact isn't too high: - -- Looping in maintainers of broken crates and submitting PRs to fix them. - -## For reviewers - -Look out for changes to documented behavior and new trait impls for existing stable traits. diff --git a/src/code-considerations/design/public-apis.md b/src/code-considerations/design/public-apis.md deleted file mode 100644 index 47964d4..0000000 --- a/src/code-considerations/design/public-apis.md +++ /dev/null @@ -1,9 +0,0 @@ -# Public API design - -**Status:** Stub - -Standard library APIs typically follow the [API Guidelines](https://rust-lang.github.io/api-guidelines/), which were originally spawned from the standard library itself. - -## For reviewers - -For new unstable features, look for any prior discussion of the proposed API to see what options and tradeoffs have already been considered. If in doubt, ping `@rust-lang/libs` for input. diff --git a/src/code-considerations/design/summary.md b/src/code-considerations/design/summary.md deleted file mode 100644 index c75e174..0000000 --- a/src/code-considerations/design/summary.md +++ /dev/null @@ -1,9 +0,0 @@ -# Design - -**Status:** Stub - -Most of the considerations in this guide are quality in some sense. This section has some general advice on maintaining code quality in the standard library. - -## For reviewers - -Think about how you would implement a feature and whether your approach would differ from what's being proposed. What trade-offs are being made? Is the weighting of those trade-offs the most appropriate? diff --git a/src/code-considerations/performance/summary.md b/src/code-considerations/performance/summary.md deleted file mode 100644 index 21745a0..0000000 --- a/src/code-considerations/performance/summary.md +++ /dev/null @@ -1,9 +0,0 @@ -# Performance - -**Status:** Stub - -Changes to hot code might impact performance in consumers, for better or for worse. Appropriate benchmarks should give an idea of how performance characteristics change. For changes that affect `rustc` itself, you can also do a [`rust-timer`](../../tools-and-bots/timer.md) run. - -## For reviewers - -If a PR is focused on performance then try get some idea of what the impact is. Also consider marking the PR as `rollup=never`. diff --git a/src/code-considerations/safety-and-soundness/mem-and-exclusive-refs.md b/src/code-considerations/safety-and-soundness/mem-and-exclusive-refs.md deleted file mode 100644 index 633e063..0000000 --- a/src/code-considerations/safety-and-soundness/mem-and-exclusive-refs.md +++ /dev/null @@ -1,15 +0,0 @@ -# Using `mem` to break assumptions - -## `mem::replace` and `mem::swap` - -Any value behind a `&mut` reference can be replaced with a new one using `mem::replace` or `mem::swap`, so code shouldn't assume any reachable mutable references can't have their internals changed by replacing. - -## `mem::forget` - -Rust doesn't guarantee destructors will run when a value is leaked (which can be done with `mem::forget`), so code should avoid relying on them for maintaining safety. Remember, [everyone poops](http://cglab.ca/~abeinges/blah/everyone-poops). - -It's ok not to run a destructor when a value is leaked because its storage isn't deallocated or repurposed. If the storage is initialized and is being deallocated or repurposed then destructors need to be run first, because [memory may be pinned](https://doc.rust-lang.org/nightly/std/pin/index.html#drop-guarantee). Having said that, there can still be exceptions for skipping destructors when deallocating if you can guarantee there's never pinning involved. - -## For reviewers - -If there's a `Drop` impl involved, look out for possible soundness issues that could come from that destructor never running. diff --git a/src/code-considerations/safety-and-soundness/summary.md b/src/code-considerations/safety-and-soundness/summary.md deleted file mode 100644 index 3ef1520..0000000 --- a/src/code-considerations/safety-and-soundness/summary.md +++ /dev/null @@ -1,13 +0,0 @@ -# Safety and soundness - -**Status:** Stub - -Unsafe code blocks in the standard library need a comment explaining why they're [ok](https://doc.rust-lang.org/nomicon). There's a lint that checks this. The unsafe code also needs to actually be ok. - -The rules around what's sound and what's not can be subtle. See the [Unsafe Code Guidelines WG](https://github.com/rust-lang/unsafe-code-guidelines) for current thinking, and consider pinging `@rust-lang/libs-impl`, `@rust-lang/lang`, and/or somebody from the WG if you're in _any_ doubt. We love debating the soundness of unsafe code, and the more eyes on it the better! - -## For reviewers - -Look out for any unsafe blocks. If they're optimizations consider whether they're actually necessary. If the unsafe code is necessary then always feel free to ping somebody to help review it. - -Look at the level of test coverage for the new unsafe code. Tests do catch bugs! diff --git a/src/code-considerations/summary.md b/src/code-considerations/summary.md deleted file mode 100644 index fb6618f..0000000 --- a/src/code-considerations/summary.md +++ /dev/null @@ -1,11 +0,0 @@ -# Code considerations - -Code considerations capture our experiences working on the standard library for all contributors. If you come across something new or unexpected then a code consideration is a great place to record it. Then other contributors and reviewers can find it by searching the guide. - -## How to write a code consideration - -Code considerations are a bit like guidelines. They should try make concrete recommendations that reviewers and contributors can refer to in discussions. A link to a real case where this was discussed or tripped us up is good to include. - -Code considerations should also try include a _For reviewers_ section. These can call out specific things to look out for in reviews that could suggest the consideration applies. They can also include advice on how to apply it. - -It's more important that we capture these experiences _somehow_ though, so don't be afraid to drop some sketchy notes in and debate the details later! diff --git a/src/code-considerations/using-unstable-lang/const-generics.md b/src/code-considerations/using-unstable-lang/const-generics.md deleted file mode 100644 index 15a0312..0000000 --- a/src/code-considerations/using-unstable-lang/const-generics.md +++ /dev/null @@ -1,25 +0,0 @@ -# Using const generics - -**Status:** Stub - -Complete const generics are currently unstable. You can track their progress [here](https://github.com/rust-lang/rust/issues/44580). - -Const generics are ok to use in public APIs, so long as they fit in the [`min_const_generics` subset](https://github.com/rust-lang/rust/issues/74878). - -## For reviewers - -Look out for const operations on const generics in public APIs like: - -```rust,ignore -pub fn extend_array(arr: [T; N]) -> [T; N + 1] { - .. -} -``` - -or for const generics that aren't integers, bools, or chars: - -```rust,ignore -pub fn tag() { - .. -} -``` diff --git a/src/code-considerations/using-unstable-lang/summary.md b/src/code-considerations/using-unstable-lang/summary.md deleted file mode 100644 index d97acf5..0000000 --- a/src/code-considerations/using-unstable-lang/summary.md +++ /dev/null @@ -1,11 +0,0 @@ -# Using unstable language features - -The standard library codebase is a great place to try unstable language features, but we have to be careful about exposing them publicly. The following is a list of unstable language features that are ok to use within the standard library itself along with any caveats: - -- [Const generics](./const-generics.md) -- [Specialization](./specialization.md) -- _Something missing?_ Please submit a PR to keep this list up-to-date! - -## For reviewers - -Look out for any use of unstable language features in PRs, especially if any new `#![feature]` attributes have been added. diff --git a/src/documentation/how-to-write-documentation.md b/src/development/how-to-write-documentation.md similarity index 100% rename from src/documentation/how-to-write-documentation.md rename to src/development/how-to-write-documentation.md diff --git a/src/feature-lifecycle/stabilization.md b/src/development/stabilization.md similarity index 98% rename from src/feature-lifecycle/stabilization.md rename to src/development/stabilization.md index ec8b964..5adf6c3 100644 --- a/src/feature-lifecycle/stabilization.md +++ b/src/development/stabilization.md @@ -5,7 +5,7 @@ Feature stabilization involves adding `#[stable]` attributes. They may be introduced alongside new trait impls or replace existing `#[unstable]` attributes. -Stabilization goes through the Libs FCP (Final Comment Period) process, which typically occurs on the [tracking issue](./tracking-issues.md) for the feature. +Stabilization goes through the Libs FCP (Final Comment Period) process, which typically occurs on the tracking issue for the feature. ## When is an FCP appropriate? diff --git a/src/documentation/summary.md b/src/documentation/summary.md deleted file mode 100644 index acd7b98..0000000 --- a/src/documentation/summary.md +++ /dev/null @@ -1,4 +0,0 @@ -- [doc alias policy](./doc-alias-policy.md) -- [safety comments policy](./safety-comments.md) -- [how to write documentation](./how-to-write-documentation.md) -- [reviewing doc changes](./reviewing-doc-changes.md) diff --git a/src/feature-lifecycle/api-change-proposals.md b/src/feature-lifecycle/api-change-proposals.md deleted file mode 100644 index f16680b..0000000 --- a/src/feature-lifecycle/api-change-proposals.md +++ /dev/null @@ -1,21 +0,0 @@ -# API Change Proposals (ACP) - -Changes to the standard library's unstable API go through the libs ACP process. - -The API Change Proposal process is intended to be a lightweight first step to -getting new APIs added to the standard library. The goal of this process is to -make sure proposed API changes have the best chance of success. The ACP process -accomplishes this by ensuring all changes have had a libs-api team member -second the proposal indicating that they are optimistic that the proposal will -pass its eventual FCP and by focusing the initial discussion on the problems -being solved and concrete motivating examples. - -You can create an ACP in the `rust-lang/libs-team` repo using [this issue template](https://github.com/rust-lang/libs-team/issues/new?assignees=&labels=api-change-proposal%2C+T-libs-api&template=api-change-proposal.md&title=%28My+API+Change+Proposal%29). - -Once a t-libs-api team member has reviewed the ACP and judged that it should -move forward they will second the ACP and initiate an ICP (inital comment -period). This initial comment period is intended to give other stake holders a -chance to participate in the initial discussion prior to starting the -implementation. Once this ICP has completed you should proceed with the -implementation of your proposal and then move on to the next step of the -feature lifecycle, creating a tracking issue. diff --git a/src/feature-lifecycle/deprecation.md b/src/feature-lifecycle/deprecation.md deleted file mode 100644 index e5523db..0000000 --- a/src/feature-lifecycle/deprecation.md +++ /dev/null @@ -1,7 +0,0 @@ -# Deprecating features - -**Status:** Stub - -Public APIs aren't deleted from the standard library. If something shouldn't be used anymore it gets deprecated by adding a `#[deprecated]` attribute. Deprecating need to go through a Libs FCP, just like stabilizations do. - -To try reduce noise in the docs from deprecated items, they should be moved to the bottom of the module or `impl` block so they're rendered at the bottom of the docs page. The docs should then be cut down to focus on why the item is deprecated rather than how you might use it. diff --git a/src/feature-lifecycle/summary.md b/src/feature-lifecycle/summary.md deleted file mode 100644 index aadc7e6..0000000 --- a/src/feature-lifecycle/summary.md +++ /dev/null @@ -1,3 +0,0 @@ -# The feature lifecycle - -**Status:** Stub diff --git a/src/feature-lifecycle/tracking-issues.md b/src/feature-lifecycle/tracking-issues.md deleted file mode 100644 index 082938d..0000000 --- a/src/feature-lifecycle/tracking-issues.md +++ /dev/null @@ -1,16 +0,0 @@ -# Using tracking issues - -**Status:** Stub - -Tracking issues are used to facilitate discussion and report on the status of standard library features. All public APIs need a dedicated tracking issue. Some larger internal units of work may also use them. - -## Creating a tracking issue - -There's a template that can be used to fill out the initial tracking issue. The Libs team also maintains [a Cargo tool](https://github.com/rust-lang/libs-team/tree/main/tools/unstable-api) that can be used to quickly dump the public API of an unstable feature. - -## Working on an unstable feature - -The current state of an unstable feature should be outlined in its tracking issue. - -If there's a change you'd like to make to an unstable feature, it can be discussed on the tracking issue first. - diff --git a/src/getting-started.md b/src/getting-started.md deleted file mode 100644 index 92ce024..0000000 --- a/src/getting-started.md +++ /dev/null @@ -1,27 +0,0 @@ -# Getting started - -**Status:** Stub - -Welcome to the standard library! - -This guide is an effort to capture some of the context needed to develop and maintain the Rust standard library. Its goal is to help members of the Libs team share the process and experience they bring to working on the standard library so other members can benefit. It’ll probably accumulate a lot of trivia that might also be interesting to members of the wider Rust community. - -## Where to get help - -Maintaining the standard library can feel like a daunting responsibility! - -Ping the `@rust-lang/libs-impl` or `@rust-lang/libs` teams on GitHub anytime. - -You can also reach out in the [`t-libs` stream on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/). - -## A tour of the standard library - -**Status:** Stub - -The standard library codebase lives in the [`rust-lang/rust`](https://github.com/rust-lang/rust) repository under the `/library` directory. - -The standard library is made up of three crates that exist in a loose hierarchy: - -- `core`: dependency free and makes minimal assumptions about the runtime environment. -- `alloc`: depends on `core`, assumes allocator support. `alloc` doesn't re-export `core`'s public API, so it's not strictly above it in the layering. -- `std`: depends on `core` and `alloc` and re-exports both of their public APIs. diff --git a/src/documentation/doc-alias-policy.md b/src/policy/doc-alias.md similarity index 100% rename from src/documentation/doc-alias-policy.md rename to src/policy/doc-alias.md diff --git a/src/code-considerations/performance/inline.md b/src/policy/inline.md similarity index 100% rename from src/code-considerations/performance/inline.md rename to src/policy/inline.md diff --git a/src/code-considerations/design/must-use.md b/src/policy/must-use.md similarity index 100% rename from src/code-considerations/design/must-use.md rename to src/policy/must-use.md diff --git a/src/documentation/safety-comments.md b/src/policy/safety-comments.md similarity index 100% rename from src/documentation/safety-comments.md rename to src/policy/safety-comments.md diff --git a/src/code-considerations/using-unstable-lang/specialization.md b/src/policy/specialization.md similarity index 94% rename from src/code-considerations/using-unstable-lang/specialization.md rename to src/policy/specialization.md index 820fe54..fd698b4 100644 --- a/src/code-considerations/using-unstable-lang/specialization.md +++ b/src/policy/specialization.md @@ -91,7 +91,3 @@ impl DoThing for T { ``` `rustc_unsafe_specialization_marker` exists to allow existing specializations that are based on marker traits exported from `std`, such as `Copy`, `FusedIterator` or `Eq`. - -## For reviewers - -Look out for any `default` annotations on public trait implementations. These will need to be refactored into a private dispatch trait. Also look out for uses of specialization that do more than pick a more optimized implementation. diff --git a/src/code-considerations/safety-and-soundness/generics-and-unsafe.md b/src/tricky/generics-and-unsafe.md similarity index 100% rename from src/code-considerations/safety-and-soundness/generics-and-unsafe.md rename to src/tricky/generics-and-unsafe.md diff --git a/src/code-considerations/safety-and-soundness/may-dangle.md b/src/tricky/may-dangle.md similarity index 100% rename from src/code-considerations/safety-and-soundness/may-dangle.md rename to src/tricky/may-dangle.md From ced20ea7437f06d858b038bdf0b1c1145ddfd5d5 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 26 Jun 2023 18:47:43 +0200 Subject: [PATCH 4/4] Add some info for reviewers. --- src/team/reviewing.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/team/reviewing.md b/src/team/reviewing.md index 567c6d9..507a0cf 100644 --- a/src/team/reviewing.md +++ b/src/team/reviewing.md @@ -17,6 +17,8 @@ But please keep in mind: Make sure to involve `@rust-lang/libs-api` on such changes. - Always be polite when reviewing: you are a representative of the Rust project, so it is expected that you will go above and beyond when it comes to the Code of Conduct. +See https://forge.rust-lang.org/compiler/reviews.html for more information on reviewing. + ## High-five rotation Some of the members of the team are part of the 'high-five rotation'; @@ -34,3 +36,12 @@ If you find yourself unable to do any reviews for an extended period of time, it might be a good idea to (temporarily) remove yourself from the list. To add or remove yourself from the list, send a PR to change the [triagebot configuration file](https://github.com/rust-lang/rust/blob/master/triagebot.toml). + +## Rolling up + +For library PRs, rolling up (`@bors r+ rollup`) is often fine, +in particular if it's only a new unstable addition or if it only touches docs. +PRs that impact performance should not be rolled up (`@bors rollup=never`), +PRs with subtle platform specific changes might also not be great candiates for rolling up. +See the [rollup guidelines](https://forge.rust-lang.org/compiler/reviews.html#rollups) for more +details on when to rollup.