From c01d383c533ef77cbd5126d293403b761417c63c Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 29 Jan 2021 14:28:42 +1000 Subject: [PATCH 1/9] start stubbing out sections --- src/SUMMARY.md | 34 +++++++++ src/about-this-guide.md | 2 + src/appendix/glossary.md | 5 ++ .../breaking-changes/behavior.md | 9 +++ .../breaking-changes/fundamental.md | 31 ++++++++ .../breaking-changes/new-trait-impls.md | 76 +++++++++++++++++++ .../breaking-changes/summary.md | 20 +++++ src/code-considerations/design/public-apis.md | 11 +++ src/code-considerations/design/summary.md | 9 +++ src/code-considerations/performance/inline.md | 24 ++++++ .../performance/summary.md | 11 +++ src/code-considerations/reviewer-checklist.md | 52 +++++++++++++ .../generic-impls-and-soundness.md | 31 ++++++++ .../safety-and-soundness/may-dangle.md | 44 +++++++++++ .../mem-and-exclusive-refs.md | 17 +++++ .../safety-and-soundness/summary.md | 14 ++++ .../using-unstable-lang/const-generics.md | 23 ++++++ .../using-unstable-lang/specialization.md | 63 +++++++++++++++ .../using-unstable-lang/summary.md | 11 +++ src/feature-lifecycle/deprecation.md | 5 ++ src/feature-lifecycle/landing-new-features.md | 5 ++ src/feature-lifecycle/stabilization.md | 18 +++++ src/feature-lifecycle/tracking-issues.md | 5 ++ src/getting-started.md | 23 ++++++ src/tools-and-bots/bors.md | 8 ++ src/tools-and-bots/crater.md | 5 ++ src/tools-and-bots/tidy.md | 5 ++ 27 files changed, 561 insertions(+) create mode 100644 src/appendix/glossary.md create mode 100644 src/code-considerations/breaking-changes/behavior.md create mode 100644 src/code-considerations/breaking-changes/fundamental.md create mode 100644 src/code-considerations/breaking-changes/new-trait-impls.md create mode 100644 src/code-considerations/breaking-changes/summary.md create mode 100644 src/code-considerations/design/public-apis.md create mode 100644 src/code-considerations/design/summary.md create mode 100644 src/code-considerations/performance/inline.md create mode 100644 src/code-considerations/performance/summary.md create mode 100644 src/code-considerations/reviewer-checklist.md create mode 100644 src/code-considerations/safety-and-soundness/generic-impls-and-soundness.md create mode 100644 src/code-considerations/safety-and-soundness/may-dangle.md create mode 100644 src/code-considerations/safety-and-soundness/mem-and-exclusive-refs.md create mode 100644 src/code-considerations/safety-and-soundness/summary.md create mode 100644 src/code-considerations/using-unstable-lang/const-generics.md create mode 100644 src/code-considerations/using-unstable-lang/specialization.md create mode 100644 src/code-considerations/using-unstable-lang/summary.md create mode 100644 src/feature-lifecycle/deprecation.md create mode 100644 src/feature-lifecycle/landing-new-features.md create mode 100644 src/feature-lifecycle/stabilization.md create mode 100644 src/feature-lifecycle/tracking-issues.md create mode 100644 src/getting-started.md create mode 100644 src/tools-and-bots/bors.md create mode 100644 src/tools-and-bots/crater.md create mode 100644 src/tools-and-bots/tidy.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index c055a42..30e1a9a 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -1,3 +1,37 @@ # Summary [About this guide](./about-this-guide.md) + +[Getting started](./getting-started.md) + +--- + +# The feature lifecycle + +- [Landing new features](./feature-lifecycle/landing-new-features.md) +- [Tracking issues](./feature-lifecycle/tracking-issues.md) +- [Stabilization](./feature-lifecycle/stabilization.md) +- [Deprecation](./feature-lifecycle/deprecation.md) + +# Code considerations + +- [Reviewer checklist](./code-considerations/reviewer-checklist.md) +- [Design](./code-considerations/design/summary.md) + - [Public APIs](./code-considerations/design/public-apis.md) +- [Breaking changes](./code-considerations/breaking-changes/summary.md) + - [Breaking behavior](./code-considerations/breaking-changes/behavior.md) + - [New trait impls](./code-considerations/breaking-changes/new-trait-impls.md) + - [`#[fundamental]` types](./code-considerations/breaking-changes/fundamental.md) +- [Safety and soundness](./code-considerations/safety-and-soundness/summary.md) + - [Generic impls and soundness](./code-considerations/safety-and-soundness/generic-impls-and-soundness.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) + +--- + +[Appendix A: Glossary](./appendix/glossary.md) diff --git a/src/about-this-guide.md b/src/about-this-guide.md index dc83830..f096a63 100644 --- a/src/about-this-guide.md +++ b/src/about-this-guide.md @@ -1,5 +1,7 @@ # About this guide +**Status:** Stub + This guide is for contributors and reviewers to Rust's standard library. ## Other places to find information diff --git a/src/appendix/glossary.md b/src/appendix/glossary.md new file mode 100644 index 0000000..a55eeba --- /dev/null +++ b/src/appendix/glossary.md @@ -0,0 +1,5 @@ +# Glossary + +Term | Meaning +-----------------------------------------------|-------- +FCP | Final Comment Period. An FCP is a formal process that's started by `@rfcbot` to get consensus from a Rust team. diff --git a/src/code-considerations/breaking-changes/behavior.md b/src/code-considerations/breaking-changes/behavior.md new file mode 100644 index 0000000..d06879a --- /dev/null +++ b/src/code-considerations/breaking-changes/behavior.md @@ -0,0 +1,9 @@ +# 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][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 new file mode 100644 index 0000000..fb6cd71 --- /dev/null +++ b/src/code-considerations/breaking-changes/fundamental.md @@ -0,0 +1,31 @@ +# `#[fundamental]` types + +**Status:** Stub + +Type annotated with the `#[fundamental]` attribute have different coherence rules. See [RFC 1023] for details. That includes: + +- `&T` +- `&mut T` +- `Box` +- `Pin` + +Typically, the scope of [breakage in new trait impls](./reviewing-code/breakage/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 +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]: ./crater.md diff --git a/src/code-considerations/breaking-changes/new-trait-impls.md b/src/code-considerations/breaking-changes/new-trait-impls.md new file mode 100644 index 0000000..2785295 --- /dev/null +++ b/src/code-considerations/breaking-changes/new-trait-impls.md @@ -0,0 +1,76 @@ +# 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. + +## 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 +// in `std` +impl From<&str> for Arc { .. } +``` + +```rust +// 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 +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] 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 +// in `std` +impl Add<&str> for String { .. } + +impl Deref for String { type Target = str; .. } +``` + +```rust +// in an external `lib` +let a = String::from("a"); +let b = String::from("b"); + +let c = a + &b; +``` + +then we add: + +```diff +impl Add<&str> for String { .. } ++ impl Add for String { .. } +``` + +then + +```rust +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] run should estimate the scope. + +[crater]: ./crater.md + +## 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 new file mode 100644 index 0000000..002066f --- /dev/null +++ b/src/code-considerations/breaking-changes/summary.md @@ -0,0 +1,20 @@ +# Breaking changes + +Breaking changes should be avoided when possible. [RFC 1105] 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] 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. + +[crater]: ./crater.md +[RFC 1105]: https://rust-lang.github.io/rfcs/1105-api-evolution.html + +## 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 new file mode 100644 index 0000000..9bcc137 --- /dev/null +++ b/src/code-considerations/design/public-apis.md @@ -0,0 +1,11 @@ +# Public API design + +**Status:** Stub + +Standard library APIs typically follow the [API Guidelines], which were originally spawned from the standard library itself. + +[API Guidelines]: https://rust-lang.github.io/api-guidelines/ + +## 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 new file mode 100644 index 0000000..5c75f20 --- /dev/null +++ b/src/code-considerations/design/summary.md @@ -0,0 +1,9 @@ +# 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/inline.md b/src/code-considerations/performance/inline.md new file mode 100644 index 0000000..e643c2f --- /dev/null +++ b/src/code-considerations/performance/inline.md @@ -0,0 +1,24 @@ +# When to `#[inline]` + +Inlining is a trade-off between potential execution speed, compile time and code size. There's some discussion about it in [this PR to the `hashbrown` crate][hashbrown/pull/119]. From the thread: + +> `#[inline]` is very different than simply just an inline hint. As I mentioned before, there's no equivalent in C++ for what `#[inline]` does. In debug mode rustc basically ignores `#[inline]`, pretending you didn't even write it. In release mode the compiler will, by default, codegen an `#[inline]` function into every single referencing codegen unit, and then it will also add `inlinehin`t. This means that if you have 16 CGUs and they all reference an item, every single one is getting the entire item's implementation inlined into it. + +You can add `#[inline]`: + +- To public, small, non-generic functions. + +You shouldn't need `#[inline]`: + +- On methods that have any generics in scope. +- On methods on traits that don't have a default implementation. + +`#[inline]` can always be introduced later, so if you're in doubt they can just be removed. + +## What about `#[inline(always)]`? + +You should just about never need `#[inline(always)]`. It may be beneficial for private helper methods that are used in a limited number of places or for trivial operators. A micro benchmark should justify the attribute. + +## For reviewers + +`#[inline]` can always be added later, so if there's any debate about whether it's appropriate feel free to defer it by removing the annotations for a start. diff --git a/src/code-considerations/performance/summary.md b/src/code-considerations/performance/summary.md new file mode 100644 index 0000000..aee2d7e --- /dev/null +++ b/src/code-considerations/performance/summary.md @@ -0,0 +1,11 @@ +# 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`] 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`. + +[`rust-timer`]: https://github.com/rust-lang-nursery/rustc-perf diff --git a/src/code-considerations/reviewer-checklist.md b/src/code-considerations/reviewer-checklist.md new file mode 100644 index 0000000..acfbc88 --- /dev/null +++ b/src/code-considerations/reviewer-checklist.md @@ -0,0 +1,52 @@ +# Reviewer checklist + +**Status:** Stub + +## Before you review + +Check the [getting started](./getting-started.md) guide for an introduction to developing in the standard library. + +If you'd like to reassign the PR, you can: + +``` +r? @user +``` + +- [ ] Are there any `#[stable]` attributes involved? + - [ ] Ping `@rust-lang/libs` for input. + +## As you review + +Look out for: + +- [ ] [Breaking changes](./code-considerations/breaking-changes/summary.md) +- [ ] [Safety and soundness](./code-considerations/safety-and-soundness/summary.md) +- [ ] [Quality](./code-considerations/quality/summary.md) +- [ ] [Use of unstable language features](./code-considerations/using-unstable-lang/summary.md) +- [ ] [Performance](./code-considerations/performance/summary.md) + +## Before you merge + +- [ ] Is the commit log tidy? Avoid merge commits, these can be squashed down. +- [ ] Can this change be rolled up? +- [ ] Is this a new unstable feature? + - [ ] Create a tracking issue. + - [ ] Update the `#[unstable]` attributes to point to it. + +## When you're ready + +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] for more details on when to rollup. + +To roll up: + +``` +@bors r+ rollup +``` + +or just: + +``` +@bors r+ +``` + +[rollup guidelines]: https://forge.rust-lang/org/compiler/reviews.md#rollups diff --git a/src/code-considerations/safety-and-soundness/generic-impls-and-soundness.md b/src/code-considerations/safety-and-soundness/generic-impls-and-soundness.md new file mode 100644 index 0000000..6cd5620 --- /dev/null +++ b/src/code-considerations/safety-and-soundness/generic-impls-and-soundness.md @@ -0,0 +1,31 @@ +# Relying on generic trait impls for soundness + +Be careful of generic types that interact with unsafe code. Unless the generic type is bounded by an unsafe trait that specifies its contract, we can't rely on the results of generic types being reliable or correct. + +A place where this commonly comes up is with the `RangeBounds` trait. You might assume that the start and end bounds given by a `RangeBounds` implementation will remain the same since it works through shared references. That's not necessarily the case though, an adversarial implementation may change the bounds between calls: + +```rust +struct EvilRange(Cell); + +impl RangeBounds for EvilRange { + fn start_bound(&self) -> Bound<&usize> { + Bound::Included(if self.0.get() { + &1 + } else { + self.0.set(true); + &0 + }) + } + fn end_bound(&self) -> Bound<&usize> { + Bound::Unbounded + } +} +``` + +This has [caused problems in the past](https://github.com/rust-lang/rust/issues/81138) for code making safety assumptions based on bounds without asserting they stay the same. + +Code using generic types to interact with unsafe should try convert them into known types first, then work with those instead of the generic. For our example with `RangeBounds`, this may mean converting into a concrete `Range`, or a tuple of `(Bound, Bound)`. + +## For reviewers + +Look out for generic functions that also contain unsafe blocks and consider how adversarial implementations of those generics could violate safety. diff --git a/src/code-considerations/safety-and-soundness/may-dangle.md b/src/code-considerations/safety-and-soundness/may-dangle.md new file mode 100644 index 0000000..524c85f --- /dev/null +++ b/src/code-considerations/safety-and-soundness/may-dangle.md @@ -0,0 +1,44 @@ +# Drop and `#[may_dangle]` + +A generic `Type` that manually implements `Drop` should consider whether a `#[may_dangle]` attribute is appropriate on `T`. The [Nomicon][dropck] has some details on what `#[may_dangle]` is all about. + +If a generic `Type` has a manual drop implementation that may also involve dropping `T` then dropck needs to know about it. If `Type`'s ownership of `T` is expressed through types that don't drop `T` themselves such as `ManuallyDrop`, `*mut T`, or `MaybeUninit` then `Type` also [needs a `PhantomData` field][RFC 0769 PhantomData] to tell dropck that `T` may be dropped. Types in the standard library that use the internal `Unique` pointer type don't need a `PhantomData` marker field. That's taken care of for them by `Unique`. + +As a real-world example of where this can go wrong, consider an `OptionCell` that looks something like this: + +```rust +struct OptionCell { + is_init: bool, + value: MaybeUninit, +} + +impl Drop for OptionCell { + fn drop(&mut self) { + if self.is_init { + // Safety: `value` is guaranteed to be fully initialized when `is_init` is true. + // Safety: The cell is being dropped, so it can't be accessed again. + unsafe { self.value.assume_init_drop() }; + } + } +} +``` + +Adding a `#[may_dangle]` attribute to this `OptionCell` that didn't have a `PhantomData` marker field opened up [a soundness hole](https://github.com/rust-lang/rust/issues/76367) for `T`'s that didn't strictly outlive the `OptionCell`, and so could be accessed after being dropped in their own `Drop` implementations. The correct application of `#[may_dangle]` also required a `PhantomData` field: + +```diff +struct OptionCell { + is_init: bool, + value: MaybeUninit, ++ _marker: PhantomData, +} + +- impl Drop for OptionCell { ++ unsafe impl<#[may_dangle] T> Drop for OptionCell { +``` + +[dropck]: https://doc.rust-lang.org/nomicon/dropck.html +[RFC 0769 PhantomData]: https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data + +## For reviewers + +If there's a manual `Drop` implementation, consider whether `#[may_dangle]` is appropriate. If it is, make sure there's a `PhantomData` too either through `Unique` or manually. 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 new file mode 100644 index 0000000..93cdbc8 --- /dev/null +++ b/src/code-considerations/safety-and-soundness/mem-and-exclusive-refs.md @@ -0,0 +1,17 @@ +# 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][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][Drop guarantee]. Having said that, there can still be exceptions for skipping destructors when deallocating if you can guarantee there's never pinning involved. + +[Drop guarantee]: https://doc.rust-lang.org/nightly/std/pin/index.html#drop-guarantee + +## 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 new file mode 100644 index 0000000..a4a4575 --- /dev/null +++ b/src/code-considerations/safety-and-soundness/summary.md @@ -0,0 +1,14 @@ +# 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 [tidy] 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] 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. + +[tidy]: ./tidy.md +[Unsafe Code Guidelines WG]: https://github.com/rust-lang/unsafe-code-guidelines diff --git a/src/code-considerations/using-unstable-lang/const-generics.md b/src/code-considerations/using-unstable-lang/const-generics.md new file mode 100644 index 0000000..ef1a1dd --- /dev/null +++ b/src/code-considerations/using-unstable-lang/const-generics.md @@ -0,0 +1,23 @@ +# Using const generics + +**Status:** Stub + +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/pull/79135). + +## For reviewers + +Look out for const operations on const generics in public APIs like: + +```rust +pub fn extend_array(arr: [T; N]) -> [T; N + 1] { + .. +} +``` + +or for const generics that aren't integers, bools, or chars: + +```rust +pub fn tag() { + .. +} +``` diff --git a/src/code-considerations/using-unstable-lang/specialization.md b/src/code-considerations/using-unstable-lang/specialization.md new file mode 100644 index 0000000..e39e88d --- /dev/null +++ b/src/code-considerations/using-unstable-lang/specialization.md @@ -0,0 +1,63 @@ +# Using specialization + +Specialization is currently unstable. You can track its progress [here][rust/issues/31844]. + +We try to avoid leaning on specialization too heavily, limiting its use to optimizing specific implementations. These specialized optimizations use a private trait to find the correct implementation, rather than specializing the public method itself. Any use of specialization that changes how methods are dispatched for external callers should be carefully considered. + +As an example of how to use specialization in the standard library, consider the case of creating an `Rc<[T]>` from a `&[T]`: + +```rust +impl From<&[T]> for Rc<[T]> { + #[inline] + fn from(v: &[T]) -> Rc<[T]> { + unsafe { Self::from_iter_exact(v.iter().cloned(), v.len()) } + } +} +``` + +It would be nice to have an optimized implementation for the case where `T: Copy`: + +```rust +impl From<&[T]> for Rc<[T]> { + #[inline] + fn from(v: &[T]) -> Rc<[T]> { + unsafe { Self::copy_from_slice(v) } + } +} +``` + +Unfortunately we couldn't have both of these impls normally, because they'd overlap. This is where private specialization can be used to choose the right implementation internally. In this case, we use a trait called `RcFromSlice` that switches the implementation: + +```rust +impl From<&[T]> for Rc<[T]> { + #[inline] + fn from(v: &[T]) -> Rc<[T]> { + >::from_slice(v) + } +} + +/// Specialization trait used for `From<&[T]>`. +trait RcFromSlice { + fn from_slice(slice: &[T]) -> Self; +} + +impl RcFromSlice for Rc<[T]> { + #[inline] + default fn from_slice(v: &[T]) -> Self { + unsafe { Self::from_iter_exact(v.iter().cloned(), v.len()) } + } +} + +impl RcFromSlice for Rc<[T]> { + #[inline] + fn from_slice(v: &[T]) -> Self { + unsafe { Self::copy_from_slice(v) } + } +} +``` + +Only specialization using the `min_specialization` feature should be used. The full `specialization` feature is known to be unsound. + +## 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/using-unstable-lang/summary.md b/src/code-considerations/using-unstable-lang/summary.md new file mode 100644 index 0000000..1000add --- /dev/null +++ b/src/code-considerations/using-unstable-lang/summary.md @@ -0,0 +1,11 @@ +# 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](./code-considerations/using-unstable-lang/const-generics.md) +- [Specialization](./code-considerations/using-unstable-lang/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/feature-lifecycle/deprecation.md b/src/feature-lifecycle/deprecation.md new file mode 100644 index 0000000..002a379 --- /dev/null +++ b/src/feature-lifecycle/deprecation.md @@ -0,0 +1,5 @@ +# Deprecating a feature + +**Status:** Stub + +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/landing-new-features.md b/src/feature-lifecycle/landing-new-features.md new file mode 100644 index 0000000..412c40b --- /dev/null +++ b/src/feature-lifecycle/landing-new-features.md @@ -0,0 +1,5 @@ +# New features + +**Status:** Stub + +New unstable features can be added and approved without going through a Libs FCP. There should be some buy-in from Libs that a feature is desirable and likely to be stabilized at some point before landing though. diff --git a/src/feature-lifecycle/stabilization.md b/src/feature-lifecycle/stabilization.md new file mode 100644 index 0000000..62daea1 --- /dev/null +++ b/src/feature-lifecycle/stabilization.md @@ -0,0 +1,18 @@ +# Stabilizing a feature + +**Status:** Stub + +## Before writing a PR to stabilize a feature + +Check to see if a [FCP] has completed on the [tracking issue]. If not, either ping `@rust-lang/libs` or leave a comment asking about the status of the feature. + +## When there's `const` involved + +Const functions can be stabilized in a PR that replaces `#[rustc_const_unstable]` attributes with `#[rustc_const_stable]` ones. The [Constant Evaluation WG] should be pinged for input on whether or not the `const`-ness is something we want to commit to. If it is an intrinsic being exposed that is const-stabilized then `@rust-lang/lang` should also be included in the FCP. + +Check whether the function internally depends on other unstable `const` functions through `#[allow_internal_unstable]` attributes and consider how the function could be implemented if its internally unstable calls were removed. See the _Stability attributes_ page for more details on `#[allow_internal_unstable]`. + +Where `unsafe` and `const` is involved, e.g., for operations which are "unconst", that the const safety argument for the usage also be documented. That is, a `const fn` has additional determinism (e.g. run-time/compile-time results must correspond and the function's output only depends on its inputs...) restrictions that must be preserved, and those should be argued when `unsafe` is used. + +[tracking issue]: ./tracking-issues.md +[FCP]: ./appendix/glossary.md#fcp diff --git a/src/feature-lifecycle/tracking-issues.md b/src/feature-lifecycle/tracking-issues.md new file mode 100644 index 0000000..a727f63 --- /dev/null +++ b/src/feature-lifecycle/tracking-issues.md @@ -0,0 +1,5 @@ +# 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. diff --git a/src/getting-started.md b/src/getting-started.md new file mode 100644 index 0000000..b03bc06 --- /dev/null +++ b/src/getting-started.md @@ -0,0 +1,23 @@ +# Getting started + +**Status:** Stub + +> Everything I wish I knew before somebody gave me `r+` + +This guide is an effort to capture some of the context needed to develop and maintain the Rust standard library. It’s 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. + +## If you’re ever unsure… + +Maintaining the standard library can feel like a daunting responsibility! Through [`highfive`], the automated reviewer assignment, you’ll find yourself dropped into a lot of new contexts. + +Ping the `@rust-lang/libs-impl` or `@rust-lang/libs` teams on GitHub anytime. We’re all here to help! + +## A tour of the standard library + +**Status:** Stub + +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/tools-and-bots/bors.md b/src/tools-and-bots/bors.md new file mode 100644 index 0000000..84783ec --- /dev/null +++ b/src/tools-and-bots/bors.md @@ -0,0 +1,8 @@ +# bors + +**Status:** Stub + +PRs to [`rust-lang/rust`] aren’t merged manually using GitHub’s UI or by pushing remote branches. Everything goes through [`bors`]. + +[`rust-lang/rust`]: https://github.com/rust-lang/rust +[`bors`]: https://github.com/rust-lang/homu diff --git a/src/tools-and-bots/crater.md b/src/tools-and-bots/crater.md new file mode 100644 index 0000000..fea442c --- /dev/null +++ b/src/tools-and-bots/crater.md @@ -0,0 +1,5 @@ +# Crater + +**Status:** Stub + +Crater is a tool that can test PRs against a public subset of the Rust ecosystem to estimate the scale of potential breakage. diff --git a/src/tools-and-bots/tidy.md b/src/tools-and-bots/tidy.md new file mode 100644 index 0000000..75bdad2 --- /dev/null +++ b/src/tools-and-bots/tidy.md @@ -0,0 +1,5 @@ +# `tidy` lints + +**Status:** Stub + +`tidy` is a tool that checks formatting and style lints to keep the standard library codebase consistent. From 3931fb4feadae9d01d5120f06861eb2fc29a739d Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 29 Jan 2021 15:09:51 +1000 Subject: [PATCH 2/9] tidy up sections --- src/SUMMARY.md | 8 ++++--- src/appendix/glossary.md | 5 ----- .../breaking-changes/behavior.md | 2 +- .../breaking-changes/fundamental.md | 4 ++-- .../breaking-changes/new-trait-impls.md | 4 +--- .../breaking-changes/summary.md | 5 +---- src/code-considerations/design/public-apis.md | 4 +--- src/code-considerations/performance/inline.md | 4 ++-- .../performance/summary.md | 4 +--- src/code-considerations/reviewer-checklist.md | 22 +++++++++---------- ...nd-soundness.md => generics-and-unsafe.md} | 2 +- .../safety-and-soundness/may-dangle.md | 7 ++---- .../mem-and-exclusive-refs.md | 6 ++--- .../safety-and-soundness/summary.md | 7 ++---- .../using-unstable-lang/const-generics.md | 4 +++- .../using-unstable-lang/specialization.md | 2 +- .../using-unstable-lang/summary.md | 4 ++-- src/feature-lifecycle/deprecation.md | 2 ++ src/feature-lifecycle/landing-new-features.md | 4 ++++ src/feature-lifecycle/stabilization.md | 9 ++++---- src/feature-lifecycle/tracking-issues.md | 2 ++ src/tools-and-bots/bors.md | 11 ++++++---- src/tools-and-bots/crater.md | 16 ++++++++++++-- src/tools-and-bots/tidy.md | 5 ----- src/tools-and-bots/timer.md | 9 ++++++++ 25 files changed, 79 insertions(+), 73 deletions(-) delete mode 100644 src/appendix/glossary.md rename src/code-considerations/safety-and-soundness/{generic-impls-and-soundness.md => generics-and-unsafe.md} (96%) delete mode 100644 src/tools-and-bots/tidy.md create mode 100644 src/tools-and-bots/timer.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 30e1a9a..7947f01 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -23,7 +23,7 @@ - [New trait impls](./code-considerations/breaking-changes/new-trait-impls.md) - [`#[fundamental]` types](./code-considerations/breaking-changes/fundamental.md) - [Safety and soundness](./code-considerations/safety-and-soundness/summary.md) - - [Generic impls and soundness](./code-considerations/safety-and-soundness/generic-impls-and-soundness.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) @@ -32,6 +32,8 @@ - [Performance](./code-considerations/performance/summary.md) - [When to `#[inline]`](./code-considerations/performance/inline.md) ---- +# Tools and bots -[Appendix A: Glossary](./appendix/glossary.md) +- [`@bors`](./tools-and-bots/bors.md) +- [`@rust-timer`](./tools-and-bots/timer.md) +- [`@craterbot`](./tools-and-bots/crater.md) diff --git a/src/appendix/glossary.md b/src/appendix/glossary.md deleted file mode 100644 index a55eeba..0000000 --- a/src/appendix/glossary.md +++ /dev/null @@ -1,5 +0,0 @@ -# Glossary - -Term | Meaning ------------------------------------------------|-------- -FCP | Final Comment Period. An FCP is a formal process that's started by `@rfcbot` to get consensus from a Rust team. diff --git a/src/code-considerations/breaking-changes/behavior.md b/src/code-considerations/breaking-changes/behavior.md index d06879a..09aac4d 100644 --- a/src/code-considerations/breaking-changes/behavior.md +++ b/src/code-considerations/breaking-changes/behavior.md @@ -1,6 +1,6 @@ # 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][rust/pull/46799] for an example. +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. diff --git a/src/code-considerations/breaking-changes/fundamental.md b/src/code-considerations/breaking-changes/fundamental.md index fb6cd71..f0cfe2d 100644 --- a/src/code-considerations/breaking-changes/fundamental.md +++ b/src/code-considerations/breaking-changes/fundamental.md @@ -2,7 +2,7 @@ **Status:** Stub -Type annotated with the `#[fundamental]` attribute have different coherence rules. See [RFC 1023] for details. That includes: +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` @@ -28,4 +28,4 @@ where 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]: ./crater.md +[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 index 2785295..c35b6eb 100644 --- a/src/code-considerations/breaking-changes/new-trait-impls.md +++ b/src/code-considerations/breaking-changes/new-trait-impls.md @@ -67,9 +67,7 @@ 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] run should estimate the scope. - -[crater]: ./crater.md +This kind of breakage can be ok, but a [crater](../../tools-and-bots/crater.md) run should estimate the scope. ## For reviewers diff --git a/src/code-considerations/breaking-changes/summary.md b/src/code-considerations/breaking-changes/summary.md index 002066f..34073b7 100644 --- a/src/code-considerations/breaking-changes/summary.md +++ b/src/code-considerations/breaking-changes/summary.md @@ -1,6 +1,6 @@ # Breaking changes -Breaking changes should be avoided when possible. [RFC 1105] 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] run. +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. @@ -12,9 +12,6 @@ If the impact isn't too high: - Looping in maintainers of broken crates and submitting PRs to fix them. -[crater]: ./crater.md -[RFC 1105]: https://rust-lang.github.io/rfcs/1105-api-evolution.html - ## 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 index 9bcc137..47964d4 100644 --- a/src/code-considerations/design/public-apis.md +++ b/src/code-considerations/design/public-apis.md @@ -2,9 +2,7 @@ **Status:** Stub -Standard library APIs typically follow the [API Guidelines], which were originally spawned from the standard library itself. - -[API Guidelines]: https://rust-lang.github.io/api-guidelines/ +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 diff --git a/src/code-considerations/performance/inline.md b/src/code-considerations/performance/inline.md index e643c2f..5192a9d 100644 --- a/src/code-considerations/performance/inline.md +++ b/src/code-considerations/performance/inline.md @@ -1,8 +1,8 @@ # When to `#[inline]` -Inlining is a trade-off between potential execution speed, compile time and code size. There's some discussion about it in [this PR to the `hashbrown` crate][hashbrown/pull/119]. From the thread: +Inlining is a trade-off between potential execution speed, compile time and code size. There's some discussion about it in [this PR to the `hashbrown` crate](https://github.com/rust-lang/hashbrown/pull/119). From the thread: -> `#[inline]` is very different than simply just an inline hint. As I mentioned before, there's no equivalent in C++ for what `#[inline]` does. In debug mode rustc basically ignores `#[inline]`, pretending you didn't even write it. In release mode the compiler will, by default, codegen an `#[inline]` function into every single referencing codegen unit, and then it will also add `inlinehin`t. This means that if you have 16 CGUs and they all reference an item, every single one is getting the entire item's implementation inlined into it. +> `#[inline]` is very different than simply just an inline hint. As I mentioned before, there's no equivalent in C++ for what `#[inline]` does. In debug mode rustc basically ignores `#[inline]`, pretending you didn't even write it. In release mode the compiler will, by default, codegen an `#[inline]` function into every single referencing codegen unit, and then it will also add `inlinehint`. This means that if you have 16 CGUs and they all reference an item, every single one is getting the entire item's implementation inlined into it. You can add `#[inline]`: diff --git a/src/code-considerations/performance/summary.md b/src/code-considerations/performance/summary.md index aee2d7e..21745a0 100644 --- a/src/code-considerations/performance/summary.md +++ b/src/code-considerations/performance/summary.md @@ -2,10 +2,8 @@ **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`] run. +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`. - -[`rust-timer`]: https://github.com/rust-lang-nursery/rustc-perf diff --git a/src/code-considerations/reviewer-checklist.md b/src/code-considerations/reviewer-checklist.md index acfbc88..6a279d3 100644 --- a/src/code-considerations/reviewer-checklist.md +++ b/src/code-considerations/reviewer-checklist.md @@ -2,9 +2,7 @@ **Status:** Stub -## Before you review - -Check the [getting started](./getting-started.md) guide for an introduction to developing in the standard library. +Check the [getting started](../getting-started.md) guide for an introduction to developing in the standard library. If you'd like to reassign the PR, you can: @@ -12,18 +10,20 @@ If you'd like to reassign the PR, you can: r? @user ``` +## Before you review + - [ ] Are there any `#[stable]` attributes involved? - [ ] Ping `@rust-lang/libs` for input. ## As you review -Look out for: +Look out for code considerations: -- [ ] [Breaking changes](./code-considerations/breaking-changes/summary.md) -- [ ] [Safety and soundness](./code-considerations/safety-and-soundness/summary.md) -- [ ] [Quality](./code-considerations/quality/summary.md) -- [ ] [Use of unstable language features](./code-considerations/using-unstable-lang/summary.md) -- [ ] [Performance](./code-considerations/performance/summary.md) +- [ ] [Design](./design/summary.md) +- [ ] [Breaking changes](./breaking-changes/summary.md) +- [ ] [Safety and soundness](./safety-and-soundness/summary.md) +- [ ] [Use of unstable language features](./using-unstable-lang/summary.md) +- [ ] [Performance](./performance/summary.md) ## Before you merge @@ -35,7 +35,7 @@ Look out for: ## When you're ready -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] for more details on when to rollup. +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.md#rollups) for more details on when to rollup. To roll up: @@ -48,5 +48,3 @@ or just: ``` @bors r+ ``` - -[rollup guidelines]: https://forge.rust-lang/org/compiler/reviews.md#rollups diff --git a/src/code-considerations/safety-and-soundness/generic-impls-and-soundness.md b/src/code-considerations/safety-and-soundness/generics-and-unsafe.md similarity index 96% rename from src/code-considerations/safety-and-soundness/generic-impls-and-soundness.md rename to src/code-considerations/safety-and-soundness/generics-and-unsafe.md index 6cd5620..4240cd0 100644 --- a/src/code-considerations/safety-and-soundness/generic-impls-and-soundness.md +++ b/src/code-considerations/safety-and-soundness/generics-and-unsafe.md @@ -1,4 +1,4 @@ -# Relying on generic trait impls for soundness +# Generics and unsafe Be careful of generic types that interact with unsafe code. Unless the generic type is bounded by an unsafe trait that specifies its contract, we can't rely on the results of generic types being reliable or correct. diff --git a/src/code-considerations/safety-and-soundness/may-dangle.md b/src/code-considerations/safety-and-soundness/may-dangle.md index 524c85f..dafdad0 100644 --- a/src/code-considerations/safety-and-soundness/may-dangle.md +++ b/src/code-considerations/safety-and-soundness/may-dangle.md @@ -1,8 +1,8 @@ # Drop and `#[may_dangle]` -A generic `Type` that manually implements `Drop` should consider whether a `#[may_dangle]` attribute is appropriate on `T`. The [Nomicon][dropck] has some details on what `#[may_dangle]` is all about. +A generic `Type` that manually implements `Drop` should consider whether a `#[may_dangle]` attribute is appropriate on `T`. The [Nomicon](https://doc.rust-lang.org/nomicon/dropck.html) has some details on what `#[may_dangle]` is all about. -If a generic `Type` has a manual drop implementation that may also involve dropping `T` then dropck needs to know about it. If `Type`'s ownership of `T` is expressed through types that don't drop `T` themselves such as `ManuallyDrop`, `*mut T`, or `MaybeUninit` then `Type` also [needs a `PhantomData` field][RFC 0769 PhantomData] to tell dropck that `T` may be dropped. Types in the standard library that use the internal `Unique` pointer type don't need a `PhantomData` marker field. That's taken care of for them by `Unique`. +If a generic `Type` has a manual drop implementation that may also involve dropping `T` then dropck needs to know about it. If `Type`'s ownership of `T` is expressed through types that don't drop `T` themselves such as `ManuallyDrop`, `*mut T`, or `MaybeUninit` then `Type` also [needs a `PhantomData` field](https://rust-lang.github.io/rfcs/0769-sound-generic-drop.html#phantom-data) to tell dropck that `T` may be dropped. Types in the standard library that use the internal `Unique` pointer type don't need a `PhantomData` marker field. That's taken care of for them by `Unique`. As a real-world example of where this can go wrong, consider an `OptionCell` that looks something like this: @@ -36,9 +36,6 @@ struct OptionCell { + unsafe impl<#[may_dangle] T> Drop for OptionCell { ``` -[dropck]: https://doc.rust-lang.org/nomicon/dropck.html -[RFC 0769 PhantomData]: https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data - ## For reviewers If there's a manual `Drop` implementation, consider whether `#[may_dangle]` is appropriate. If it is, make sure there's a `PhantomData` too either through `Unique` or manually. 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 index 93cdbc8..633e063 100644 --- a/src/code-considerations/safety-and-soundness/mem-and-exclusive-refs.md +++ b/src/code-considerations/safety-and-soundness/mem-and-exclusive-refs.md @@ -6,11 +6,9 @@ Any value behind a `&mut` reference can be replaced with a new one using `mem::r ## `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][Everyone Poops]. +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][Drop guarantee]. Having said that, there can still be exceptions for skipping destructors when deallocating if you can guarantee there's never pinning involved. - -[Drop guarantee]: https://doc.rust-lang.org/nightly/std/pin/index.html#drop-guarantee +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 diff --git a/src/code-considerations/safety-and-soundness/summary.md b/src/code-considerations/safety-and-soundness/summary.md index a4a4575..b42d71e 100644 --- a/src/code-considerations/safety-and-soundness/summary.md +++ b/src/code-considerations/safety-and-soundness/summary.md @@ -2,13 +2,10 @@ **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 [tidy] lint that checks this. The unsafe code also needs to actually be ok. +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] 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! +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. - -[tidy]: ./tidy.md -[Unsafe Code Guidelines WG]: https://github.com/rust-lang/unsafe-code-guidelines diff --git a/src/code-considerations/using-unstable-lang/const-generics.md b/src/code-considerations/using-unstable-lang/const-generics.md index ef1a1dd..48227c4 100644 --- a/src/code-considerations/using-unstable-lang/const-generics.md +++ b/src/code-considerations/using-unstable-lang/const-generics.md @@ -2,7 +2,9 @@ **Status:** Stub -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/pull/79135). +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 diff --git a/src/code-considerations/using-unstable-lang/specialization.md b/src/code-considerations/using-unstable-lang/specialization.md index e39e88d..b5421b2 100644 --- a/src/code-considerations/using-unstable-lang/specialization.md +++ b/src/code-considerations/using-unstable-lang/specialization.md @@ -1,6 +1,6 @@ # Using specialization -Specialization is currently unstable. You can track its progress [here][rust/issues/31844]. +Specialization is currently unstable. You can track its progress [here](https://github.com/rust-lang/rust/issues/31844). We try to avoid leaning on specialization too heavily, limiting its use to optimizing specific implementations. These specialized optimizations use a private trait to find the correct implementation, rather than specializing the public method itself. Any use of specialization that changes how methods are dispatched for external callers should be carefully considered. diff --git a/src/code-considerations/using-unstable-lang/summary.md b/src/code-considerations/using-unstable-lang/summary.md index 1000add..d97acf5 100644 --- a/src/code-considerations/using-unstable-lang/summary.md +++ b/src/code-considerations/using-unstable-lang/summary.md @@ -2,8 +2,8 @@ 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](./code-considerations/using-unstable-lang/const-generics.md) -- [Specialization](./code-considerations/using-unstable-lang/specialization.md) +- [Const generics](./const-generics.md) +- [Specialization](./specialization.md) - _Something missing?_ Please submit a PR to keep this list up-to-date! ## For reviewers diff --git a/src/feature-lifecycle/deprecation.md b/src/feature-lifecycle/deprecation.md index 002a379..a61342b 100644 --- a/src/feature-lifecycle/deprecation.md +++ b/src/feature-lifecycle/deprecation.md @@ -2,4 +2,6 @@ **Status:** Stub +Public APIs aren't deleted from the standard library. If something shouldn't be used anymore it gets deprecated by adding a `#[rustc_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/landing-new-features.md b/src/feature-lifecycle/landing-new-features.md index 412c40b..c366b24 100644 --- a/src/feature-lifecycle/landing-new-features.md +++ b/src/feature-lifecycle/landing-new-features.md @@ -3,3 +3,7 @@ **Status:** Stub New unstable features can be added and approved without going through a Libs FCP. There should be some buy-in from Libs that a feature is desirable and likely to be stabilized at some point before landing though. + +All public items in the standard library need a `#[stable]` or `#[unstable]` attribute on them. When a feature is first added, it gets a `#[unstable]` attribute. + +Before a new feature is merged, those `#[unstable]` attributes need to be linked to a [tracking issue](./tracking-issues.md). diff --git a/src/feature-lifecycle/stabilization.md b/src/feature-lifecycle/stabilization.md index 62daea1..7709bd0 100644 --- a/src/feature-lifecycle/stabilization.md +++ b/src/feature-lifecycle/stabilization.md @@ -2,17 +2,16 @@ **Status:** Stub +Feature stabilization involves replacing `#[unstable]` attributes for a feature with `#[stable]` ones. + ## Before writing a PR to stabilize a feature -Check to see if a [FCP] has completed on the [tracking issue]. If not, either ping `@rust-lang/libs` or leave a comment asking about the status of the feature. +Check to see if a FCP has completed on the [tracking issue](./tracking-issues.md). If not, either ping `@rust-lang/libs` or leave a comment asking about the status of the feature. ## When there's `const` involved -Const functions can be stabilized in a PR that replaces `#[rustc_const_unstable]` attributes with `#[rustc_const_stable]` ones. The [Constant Evaluation WG] should be pinged for input on whether or not the `const`-ness is something we want to commit to. If it is an intrinsic being exposed that is const-stabilized then `@rust-lang/lang` should also be included in the FCP. +Const functions can be stabilized in a PR that replaces `#[rustc_const_unstable]` attributes with `#[rustc_const_stable]` ones. The [Constant Evaluation WG](https://github.com/rust-lang/const-eval) should be pinged for input on whether or not the `const`-ness is something we want to commit to. If it is an intrinsic being exposed that is const-stabilized then `@rust-lang/lang` should also be included in the FCP. Check whether the function internally depends on other unstable `const` functions through `#[allow_internal_unstable]` attributes and consider how the function could be implemented if its internally unstable calls were removed. See the _Stability attributes_ page for more details on `#[allow_internal_unstable]`. Where `unsafe` and `const` is involved, e.g., for operations which are "unconst", that the const safety argument for the usage also be documented. That is, a `const fn` has additional determinism (e.g. run-time/compile-time results must correspond and the function's output only depends on its inputs...) restrictions that must be preserved, and those should be argued when `unsafe` is used. - -[tracking issue]: ./tracking-issues.md -[FCP]: ./appendix/glossary.md#fcp diff --git a/src/feature-lifecycle/tracking-issues.md b/src/feature-lifecycle/tracking-issues.md index a727f63..abbdb1b 100644 --- a/src/feature-lifecycle/tracking-issues.md +++ b/src/feature-lifecycle/tracking-issues.md @@ -3,3 +3,5 @@ **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. + +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. diff --git a/src/tools-and-bots/bors.md b/src/tools-and-bots/bors.md index 84783ec..5a72ef2 100644 --- a/src/tools-and-bots/bors.md +++ b/src/tools-and-bots/bors.md @@ -1,8 +1,11 @@ -# bors +# `@bors` **Status:** Stub -PRs to [`rust-lang/rust`] aren’t merged manually using GitHub’s UI or by pushing remote branches. Everything goes through [`bors`]. +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). -[`rust-lang/rust`]: https://github.com/rust-lang/rust -[`bors`]: https://github.com/rust-lang/homu +You can approve a PR with: + +``` +@bors r+ +``` diff --git a/src/tools-and-bots/crater.md b/src/tools-and-bots/crater.md index fea442c..5e36e3f 100644 --- a/src/tools-and-bots/crater.md +++ b/src/tools-and-bots/crater.md @@ -1,5 +1,17 @@ -# Crater +# `@craterbot` **Status:** Stub -Crater is a tool that can test PRs against a public subset of the Rust ecosystem to estimate the scale of potential breakage. +[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 with: + +``` +@bors try @craterbot check +``` + +to ensure crates compile, or: + +``` +@bors try @craterbot run mode=build-and-test +``` diff --git a/src/tools-and-bots/tidy.md b/src/tools-and-bots/tidy.md deleted file mode 100644 index 75bdad2..0000000 --- a/src/tools-and-bots/tidy.md +++ /dev/null @@ -1,5 +0,0 @@ -# `tidy` lints - -**Status:** Stub - -`tidy` is a tool that checks formatting and style lints to keep the standard library codebase consistent. diff --git a/src/tools-and-bots/timer.md b/src/tools-and-bots/timer.md new file mode 100644 index 0000000..fbb4317 --- /dev/null +++ b/src/tools-and-bots/timer.md @@ -0,0 +1,9 @@ +# `@rust-timer` + +**Status:** Stub + +You can kick off a performance test using `@rust-timer`: + +``` +@bors try @rust-timer queue +``` From 4efba526bb8ff17b066795352d5b0bd0b6afcc73 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 29 Jan 2021 15:42:23 +1000 Subject: [PATCH 3/9] update links in reviewer checklist --- .travis.yml | 1 - src/code-considerations/reviewer-checklist.md | 7 ++++--- src/getting-started.md | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1d67472..e08b033 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,6 @@ cache: - book/linkcheck/ before_install: - shopt -s globstar -- MAX_LINE_LENGTH=100 bash ci/check_line_lengths.sh src/**/*.md install: - source ~/.cargo/env || true - cargo install mdbook --version '^0.4.5' diff --git a/src/code-considerations/reviewer-checklist.md b/src/code-considerations/reviewer-checklist.md index 6a279d3..0c9eaf3 100644 --- a/src/code-considerations/reviewer-checklist.md +++ b/src/code-considerations/reviewer-checklist.md @@ -12,7 +12,8 @@ r? @user ## Before you review -- [ ] Are there any `#[stable]` attributes involved? +- [ ] Is this a [stabilization](../feature-lifecycle/stabilization.md) or [deprecation](../feature-lifecycle/deprecation.md)? + - [ ] Make sure there's a completed FCP somewhere for it. - [ ] Ping `@rust-lang/libs` for input. ## As you review @@ -29,8 +30,8 @@ Look out for code considerations: - [ ] Is the commit log tidy? Avoid merge commits, these can be squashed down. - [ ] Can this change be rolled up? -- [ ] Is this a new unstable feature? - - [ ] Create a tracking issue. +- [ ] Is this a [new unstable feature](../feature-lifecycle/landing-new-features.md)? + - [ ] Create a [tracking issue](../feature-lifecycle/tracking-issues.md). - [ ] Update the `#[unstable]` attributes to point to it. ## When you're ready diff --git a/src/getting-started.md b/src/getting-started.md index b03bc06..b3d38ed 100644 --- a/src/getting-started.md +++ b/src/getting-started.md @@ -4,7 +4,7 @@ > Everything I wish I knew before somebody gave me `r+` -This guide is an effort to capture some of the context needed to develop and maintain the Rust standard library. It’s 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. +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. ## If you’re ever unsure… From 363a4ce27ba456cee4167aab79b27f26858338ed Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 29 Jan 2021 15:54:52 +1000 Subject: [PATCH 4/9] working on features --- src/SUMMARY.md | 13 ++++++++++--- .../breaking-changes/new-trait-impls.md | 2 ++ src/code-considerations/reviewer-checklist.md | 2 +- ...ing-new-features.md => new-unstable-features.md} | 2 +- .../working-on-unstable-features.md | 7 +++++++ src/getting-started.md | 2 ++ 6 files changed, 23 insertions(+), 5 deletions(-) rename src/feature-lifecycle/{landing-new-features.md => new-unstable-features.md} (95%) create mode 100644 src/feature-lifecycle/working-on-unstable-features.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 7947f01..e643d91 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -8,13 +8,18 @@ # The feature lifecycle -- [Landing new features](./feature-lifecycle/landing-new-features.md) +This section describes the processes around how standard library features are developed and stabilized. + +- [Landing a new feature](./feature-lifecycle/new-unstable-features.md) - [Tracking issues](./feature-lifecycle/tracking-issues.md) -- [Stabilization](./feature-lifecycle/stabilization.md) -- [Deprecation](./feature-lifecycle/deprecation.md) +- [Working on an unstable feature](./feature-lifecycle/working-on-unstable-features.md) +- [Stabilizing a feature](./feature-lifecycle/stabilization.md) +- [Deprecating a feature](./feature-lifecycle/deprecation.md) # Code considerations +This section includes things to keep in mind when writing and reviewing standard library code. Most topics are relevant for both contributors and reviewers. + - [Reviewer checklist](./code-considerations/reviewer-checklist.md) - [Design](./code-considerations/design/summary.md) - [Public APIs](./code-considerations/design/public-apis.md) @@ -34,6 +39,8 @@ # Tools and bots +This section lists tools and bots that support the development process on the standard library. Most can be interacted with through GitHub mentions. + - [`@bors`](./tools-and-bots/bors.md) - [`@rust-timer`](./tools-and-bots/timer.md) - [`@craterbot`](./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 index c35b6eb..a95b76c 100644 --- a/src/code-considerations/breaking-changes/new-trait-impls.md +++ b/src/code-considerations/breaking-changes/new-trait-impls.md @@ -2,6 +2,8 @@ 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: diff --git a/src/code-considerations/reviewer-checklist.md b/src/code-considerations/reviewer-checklist.md index 0c9eaf3..8ebbdb2 100644 --- a/src/code-considerations/reviewer-checklist.md +++ b/src/code-considerations/reviewer-checklist.md @@ -30,7 +30,7 @@ Look out for code considerations: - [ ] Is the commit log tidy? Avoid merge commits, these can be squashed down. - [ ] Can this change be rolled up? -- [ ] Is this a [new unstable feature](../feature-lifecycle/landing-new-features.md)? +- [ ] Is this a [new unstable feature](../feature-lifecycle/new-unstable-features.md)? - [ ] Create a [tracking issue](../feature-lifecycle/tracking-issues.md). - [ ] Update the `#[unstable]` attributes to point to it. diff --git a/src/feature-lifecycle/landing-new-features.md b/src/feature-lifecycle/new-unstable-features.md similarity index 95% rename from src/feature-lifecycle/landing-new-features.md rename to src/feature-lifecycle/new-unstable-features.md index c366b24..eb51558 100644 --- a/src/feature-lifecycle/landing-new-features.md +++ b/src/feature-lifecycle/new-unstable-features.md @@ -1,4 +1,4 @@ -# New features +# Landing a new features **Status:** Stub diff --git a/src/feature-lifecycle/working-on-unstable-features.md b/src/feature-lifecycle/working-on-unstable-features.md new file mode 100644 index 0000000..f0ceb31 --- /dev/null +++ b/src/feature-lifecycle/working-on-unstable-features.md @@ -0,0 +1,7 @@ +# Working on an unstable feature + +**Status:** Stub + +The current state of an unstable feature should be outlined in its [tracking issue](./tracking-issues.md). + +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 index b3d38ed..497f3bf 100644 --- a/src/getting-started.md +++ b/src/getting-started.md @@ -4,6 +4,8 @@ > Everything I wish I knew before somebody gave me `r+` +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. ## If you’re ever unsure… From 2345c42406e3477f92e1d26f972d7896d790b1f2 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 29 Jan 2021 15:58:32 +1000 Subject: [PATCH 5/9] roll in-progress work into tracking issues page --- src/SUMMARY.md | 9 ++++----- src/feature-lifecycle/deprecation.md | 2 +- src/feature-lifecycle/new-unstable-features.md | 2 +- src/feature-lifecycle/stabilization.md | 2 +- src/feature-lifecycle/tracking-issues.md | 11 ++++++++++- src/feature-lifecycle/working-on-unstable-features.md | 7 ------- 6 files changed, 17 insertions(+), 16 deletions(-) delete mode 100644 src/feature-lifecycle/working-on-unstable-features.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index e643d91..3f658eb 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -10,11 +10,10 @@ This section describes the processes around how standard library features are developed and stabilized. -- [Landing a new feature](./feature-lifecycle/new-unstable-features.md) -- [Tracking issues](./feature-lifecycle/tracking-issues.md) -- [Working on an unstable feature](./feature-lifecycle/working-on-unstable-features.md) -- [Stabilizing a feature](./feature-lifecycle/stabilization.md) -- [Deprecating a feature](./feature-lifecycle/deprecation.md) +- [Landing new features](./feature-lifecycle/new-unstable-features.md) +- [Using tracking issues](./feature-lifecycle/tracking-issues.md) +- [Stabilizing features](./feature-lifecycle/stabilization.md) +- [Deprecating features](./feature-lifecycle/deprecation.md) # Code considerations diff --git a/src/feature-lifecycle/deprecation.md b/src/feature-lifecycle/deprecation.md index a61342b..68b90a4 100644 --- a/src/feature-lifecycle/deprecation.md +++ b/src/feature-lifecycle/deprecation.md @@ -1,4 +1,4 @@ -# Deprecating a feature +# Deprecating features **Status:** Stub diff --git a/src/feature-lifecycle/new-unstable-features.md b/src/feature-lifecycle/new-unstable-features.md index eb51558..9dafdd6 100644 --- a/src/feature-lifecycle/new-unstable-features.md +++ b/src/feature-lifecycle/new-unstable-features.md @@ -1,4 +1,4 @@ -# Landing a new features +# Landing new features **Status:** Stub diff --git a/src/feature-lifecycle/stabilization.md b/src/feature-lifecycle/stabilization.md index 7709bd0..8ac6795 100644 --- a/src/feature-lifecycle/stabilization.md +++ b/src/feature-lifecycle/stabilization.md @@ -1,4 +1,4 @@ -# Stabilizing a feature +# Stabilizing features **Status:** Stub diff --git a/src/feature-lifecycle/tracking-issues.md b/src/feature-lifecycle/tracking-issues.md index abbdb1b..082938d 100644 --- a/src/feature-lifecycle/tracking-issues.md +++ b/src/feature-lifecycle/tracking-issues.md @@ -1,7 +1,16 @@ -# Tracking issues +# 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/feature-lifecycle/working-on-unstable-features.md b/src/feature-lifecycle/working-on-unstable-features.md deleted file mode 100644 index f0ceb31..0000000 --- a/src/feature-lifecycle/working-on-unstable-features.md +++ /dev/null @@ -1,7 +0,0 @@ -# Working on an unstable feature - -**Status:** Stub - -The current state of an unstable feature should be outlined in its [tracking issue](./tracking-issues.md). - -If there's a change you'd like to make to an unstable feature, it can be discussed on the tracking issue first. From 2d2b1d483bdfb41e9037b11edde77f954af56236 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 29 Jan 2021 16:05:20 +1000 Subject: [PATCH 6/9] section comments never render --- src/SUMMARY.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 3f658eb..3a1bcff 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -8,8 +8,6 @@ # The feature lifecycle -This section describes the processes around how standard library features are developed and stabilized. - - [Landing new features](./feature-lifecycle/new-unstable-features.md) - [Using tracking issues](./feature-lifecycle/tracking-issues.md) - [Stabilizing features](./feature-lifecycle/stabilization.md) @@ -17,8 +15,6 @@ This section describes the processes around how standard library features are de # Code considerations -This section includes things to keep in mind when writing and reviewing standard library code. Most topics are relevant for both contributors and reviewers. - - [Reviewer checklist](./code-considerations/reviewer-checklist.md) - [Design](./code-considerations/design/summary.md) - [Public APIs](./code-considerations/design/public-apis.md) @@ -38,8 +34,6 @@ This section includes things to keep in mind when writing and reviewing standard # Tools and bots -This section lists tools and bots that support the development process on the standard library. Most can be interacted with through GitHub mentions. - - [`@bors`](./tools-and-bots/bors.md) - [`@rust-timer`](./tools-and-bots/timer.md) - [`@craterbot`](./tools-and-bots/crater.md) From ae27bb28874717ffb32b9d69534c127718fca783 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Fri, 29 Jan 2021 16:49:52 +1000 Subject: [PATCH 7/9] a few more updates --- src/SUMMARY.md | 4 ++-- src/code-considerations/design/summary.md | 2 +- src/feature-lifecycle/new-unstable-features.md | 2 ++ src/feature-lifecycle/stabilization.md | 14 ++++++++++++-- src/getting-started.md | 12 +++++++----- 5 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 3a1bcff..dd23e91 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -19,8 +19,8 @@ - [Design](./code-considerations/design/summary.md) - [Public APIs](./code-considerations/design/public-apis.md) - [Breaking changes](./code-considerations/breaking-changes/summary.md) - - [Breaking behavior](./code-considerations/breaking-changes/behavior.md) - - [New trait impls](./code-considerations/breaking-changes/new-trait-impls.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) - [Safety and soundness](./code-considerations/safety-and-soundness/summary.md) - [Generics and unsafe](./code-considerations/safety-and-soundness/generics-and-unsafe.md) diff --git a/src/code-considerations/design/summary.md b/src/code-considerations/design/summary.md index 5c75f20..c75e174 100644 --- a/src/code-considerations/design/summary.md +++ b/src/code-considerations/design/summary.md @@ -6,4 +6,4 @@ Most of the considerations in this guide are quality in some sense. This section ## 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? +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/feature-lifecycle/new-unstable-features.md b/src/feature-lifecycle/new-unstable-features.md index 9dafdd6..faa9076 100644 --- a/src/feature-lifecycle/new-unstable-features.md +++ b/src/feature-lifecycle/new-unstable-features.md @@ -4,6 +4,8 @@ New unstable features can be added and approved without going through a Libs FCP. There should be some buy-in from Libs that a feature is desirable and likely to be stabilized at some point before landing though. +If you're not sure, open an issue against `rust-lang/rust` first suggesting the feature before developing it. + All public items in the standard library need a `#[stable]` or `#[unstable]` attribute on them. When a feature is first added, it gets a `#[unstable]` attribute. Before a new feature is merged, those `#[unstable]` attributes need to be linked to a [tracking issue](./tracking-issues.md). diff --git a/src/feature-lifecycle/stabilization.md b/src/feature-lifecycle/stabilization.md index 8ac6795..f361efd 100644 --- a/src/feature-lifecycle/stabilization.md +++ b/src/feature-lifecycle/stabilization.md @@ -2,11 +2,21 @@ **Status:** Stub -Feature stabilization involves replacing `#[unstable]` attributes for a feature with `#[stable]` ones. +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 process, which occurs on the [tracking issue](./tracking-issues.md) for the feature. ## Before writing a PR to stabilize a feature -Check to see if a FCP has completed on the [tracking issue](./tracking-issues.md). If not, either ping `@rust-lang/libs` or leave a comment asking about the status of the feature. +Check to see if a FCP has completed first. If not, either ping `@rust-lang/libs` or leave a comment asking about the status of the feature. + +This will save you from opening a stabilization PR and having it need regular rebasing while the FCP process runs its course. + +## Writing a stabilization PR + +- Replace any `#[unstable]` attributes for the given feature with stable ones. The value of the `since` field is usually the current `nightly` version. +- Remove any `#![feature()]` attributes that were previously required. +- Submit a PR with a stabilization report. ## When there's `const` involved diff --git a/src/getting-started.md b/src/getting-started.md index 497f3bf..92ce024 100644 --- a/src/getting-started.md +++ b/src/getting-started.md @@ -2,22 +2,24 @@ **Status:** Stub -> Everything I wish I knew before somebody gave me `r+` - 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. -## If you’re ever unsure… +## Where to get help + +Maintaining the standard library can feel like a daunting responsibility! -Maintaining the standard library can feel like a daunting responsibility! Through [`highfive`], the automated reviewer assignment, you’ll find yourself dropped into a lot of new contexts. +Ping the `@rust-lang/libs-impl` or `@rust-lang/libs` teams on GitHub anytime. -Ping the `@rust-lang/libs-impl` or `@rust-lang/libs` teams on GitHub anytime. We’re all here to help! +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. From c938da2836509ea29ca87f3aa6e14d443101de06 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Sat, 30 Jan 2021 20:48:43 +1000 Subject: [PATCH 8/9] crater no wait --- src/tools-and-bots/crater.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/tools-and-bots/crater.md b/src/tools-and-bots/crater.md index 5e36e3f..a76562e 100644 --- a/src/tools-and-bots/crater.md +++ b/src/tools-and-bots/crater.md @@ -4,14 +4,20 @@ [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 with: +You can kick off a crater run by first calling: ``` -@bors try @craterbot check +@bors try +``` + +Once that finishes, you can then call: + +``` +@craterbot check ``` to ensure crates compile, or: ``` -@bors try @craterbot run mode=build-and-test +@craterbot run mode=build-and-test ``` From 4024cdf576bf99bac2aa9b1909bd7491593cafed Mon Sep 17 00:00:00 2001 From: KodrAus Date: Sat, 30 Jan 2021 21:13:09 +1000 Subject: [PATCH 9/9] update some more sections --- src/SUMMARY.md | 64 ++++++++++--------- src/code-considerations/reviewer-checklist.md | 51 --------------- .../safety-and-soundness/may-dangle.md | 2 +- .../safety-and-soundness/summary.md | 2 + src/code-considerations/summary.md | 11 ++++ src/feature-lifecycle/summary.md | 3 + src/reviewer-checklist.md | 51 +++++++++++++++ src/tools-and-bots/bors.md | 4 ++ src/tools-and-bots/summary.md | 3 + 9 files changed, 108 insertions(+), 83 deletions(-) delete mode 100644 src/code-considerations/reviewer-checklist.md create mode 100644 src/code-considerations/summary.md create mode 100644 src/feature-lifecycle/summary.md create mode 100644 src/reviewer-checklist.md create mode 100644 src/tools-and-bots/summary.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index dd23e91..4bc67bb 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -4,36 +4,38 @@ [Getting started](./getting-started.md) +[Reviewer checklist](./reviewer-checklist.md) + +--- + +- [The feature lifecycle](./feature-lifecycle/summary.md) + - [Landing new features](./feature-lifecycle/new-unstable-features.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) + - [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) + - [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) + --- -# The feature lifecycle - -- [Landing new features](./feature-lifecycle/new-unstable-features.md) -- [Using tracking issues](./feature-lifecycle/tracking-issues.md) -- [Stabilizing features](./feature-lifecycle/stabilization.md) -- [Deprecating features](./feature-lifecycle/deprecation.md) - -# Code considerations - -- [Reviewer checklist](./code-considerations/reviewer-checklist.md) -- [Design](./code-considerations/design/summary.md) - - [Public APIs](./code-considerations/design/public-apis.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) -- [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) - -# Tools and bots - -- [`@bors`](./tools-and-bots/bors.md) -- [`@rust-timer`](./tools-and-bots/timer.md) -- [`@craterbot`](./tools-and-bots/crater.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/code-considerations/reviewer-checklist.md b/src/code-considerations/reviewer-checklist.md deleted file mode 100644 index 8ebbdb2..0000000 --- a/src/code-considerations/reviewer-checklist.md +++ /dev/null @@ -1,51 +0,0 @@ -# Reviewer checklist - -**Status:** Stub - -Check the [getting started](../getting-started.md) guide for an introduction to developing in the standard library. - -If you'd like to reassign the PR, you can: - -``` -r? @user -``` - -## Before you review - -- [ ] Is this a [stabilization](../feature-lifecycle/stabilization.md) or [deprecation](../feature-lifecycle/deprecation.md)? - - [ ] Make sure there's a completed FCP somewhere for it. - - [ ] Ping `@rust-lang/libs` for input. - -## As you review - -Look out for code considerations: - -- [ ] [Design](./design/summary.md) -- [ ] [Breaking changes](./breaking-changes/summary.md) -- [ ] [Safety and soundness](./safety-and-soundness/summary.md) -- [ ] [Use of unstable language features](./using-unstable-lang/summary.md) -- [ ] [Performance](./performance/summary.md) - -## Before you merge - -- [ ] Is the commit log tidy? Avoid merge commits, these can be squashed down. -- [ ] Can this change be rolled up? -- [ ] Is this a [new unstable feature](../feature-lifecycle/new-unstable-features.md)? - - [ ] Create a [tracking issue](../feature-lifecycle/tracking-issues.md). - - [ ] Update the `#[unstable]` attributes to point to it. - -## When you're ready - -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.md#rollups) for more details on when to rollup. - -To roll up: - -``` -@bors r+ rollup -``` - -or just: - -``` -@bors r+ -``` diff --git a/src/code-considerations/safety-and-soundness/may-dangle.md b/src/code-considerations/safety-and-soundness/may-dangle.md index dafdad0..d5ff641 100644 --- a/src/code-considerations/safety-and-soundness/may-dangle.md +++ b/src/code-considerations/safety-and-soundness/may-dangle.md @@ -38,4 +38,4 @@ struct OptionCell { ## For reviewers -If there's a manual `Drop` implementation, consider whether `#[may_dangle]` is appropriate. If it is, make sure there's a `PhantomData` too either through `Unique` or manually. +If there's a manual `Drop` implementation, consider whether `#[may_dangle]` is appropriate. If it is, make sure there's a `PhantomData` too either through `Unique` or as a field directly. diff --git a/src/code-considerations/safety-and-soundness/summary.md b/src/code-considerations/safety-and-soundness/summary.md index b42d71e..3ef1520 100644 --- a/src/code-considerations/safety-and-soundness/summary.md +++ b/src/code-considerations/safety-and-soundness/summary.md @@ -9,3 +9,5 @@ The rules around what's sound and what's not can be subtle. See the [Unsafe Code ## 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 new file mode 100644 index 0000000..fb6618f --- /dev/null +++ b/src/code-considerations/summary.md @@ -0,0 +1,11 @@ +# 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/feature-lifecycle/summary.md b/src/feature-lifecycle/summary.md new file mode 100644 index 0000000..aadc7e6 --- /dev/null +++ b/src/feature-lifecycle/summary.md @@ -0,0 +1,3 @@ +# The feature lifecycle + +**Status:** Stub diff --git a/src/reviewer-checklist.md b/src/reviewer-checklist.md new file mode 100644 index 0000000..494c3be --- /dev/null +++ b/src/reviewer-checklist.md @@ -0,0 +1,51 @@ +# Reviewer checklist + +**Status:** Stub + +Check the [getting started](./getting-started.md) guide for an introduction to developing in the standard library. + +If you'd like to reassign the PR, you can: + +``` +r? @user +``` + +## Before you review + +- [ ] Is this a [stabilization](./feature-lifecycle/stabilization.md) or [deprecation](./feature-lifecycle/deprecation.md)? + - [ ] Make sure there's a completed FCP somewhere for it. + - [ ] Ping `@rust-lang/libs` for input. + +## As you review + +Look out for code considerations: + +- [ ] [Design](./code-considerations/design/summary.md) +- [ ] [Breaking changes](./code-considerations/breaking-changes/summary.md) +- [ ] [Safety and soundness](./code-considerations/safety-and-soundness/summary.md) +- [ ] [Use of unstable language features](./code-considerations/using-unstable-lang/summary.md) +- [ ] [Performance](./code-considerations/performance/summary.md) + +## Before you merge + +- [ ] Is the commit log tidy? Avoid merge commits, these can be squashed down. +- [ ] Can this change be rolled up? +- [ ] Is this a [new unstable feature](./feature-lifecycle/new-unstable-features.md)? + - [ ] Create a [tracking issue](./feature-lifecycle/tracking-issues.md). + - [ ] Update the `#[unstable]` attributes to point to it. + +## When you're ready + +Use [`@bors`](./tools-and-bots/bors.md) to merge the pull request. + +To roll up: + +``` +@bors r+ rollup +``` + +or just: + +``` +@bors r+ +``` diff --git a/src/tools-and-bots/bors.md b/src/tools-and-bots/bors.md index 5a72ef2..550e297 100644 --- a/src/tools-and-bots/bors.md +++ b/src/tools-and-bots/bors.md @@ -9,3 +9,7 @@ You can approve a PR with: ``` @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.md#rollups) for more details on when to rollup. diff --git a/src/tools-and-bots/summary.md b/src/tools-and-bots/summary.md new file mode 100644 index 0000000..6a76859 --- /dev/null +++ b/src/tools-and-bots/summary.md @@ -0,0 +1,3 @@ +# Tools and bots + +**Status:** Stub