rust-lang / std-dev-guide Goto Github PK

On chapter 3, in New trait impls, there seems to be a typo regarding a type.
The Inference breaks when a second generic impl is introduced example uses an Arc as an example, however, in the very end of the example, we have

will no longer compile, because we've previously been relying on inference to figure out the T in Box<T>.

but Box isn't mentioned anywhere.

In rust-lang/rust#67290 we added drop guards to collections that will attempt to continue dropping in case dropping an item unwinds. That means collections will either not leak if an item unwinds during drop, or will abort.

In rust-lang/rust#75644 we've looked at preventing leaks if an unwind happens partway through initializing a MaybeUninit (usually for arrays).

We should come up with some concrete guidance on how we want to deal with leaks and unwinding

The "if it's a new trait it needs to have &mut blankets immediately" problem came up on IRLO recently: https://internals.rust-lang.org/t/implement-std-asrawfd-for-references/20132/5?u=scottmcm

We've been better about this for newer traits, but it reminded me that it would be good to have a list of "stuff we can't fix later" to double-check for every FCP, which would also be a good resource for people making ACPs to think about.

We should document which kind of changes are breaking and cannot be made to stable APIs.

E.g. changing a function from fn(i32) to fn(impl Into<i32>) can be breaking, but relaxing a <T: Ord> to <T: PartialOrd> might be fine (maybe?).

Thank you for this guide (and, of course, for core, alloc, std).

std-dev-guide.rust-lang.org/development/building-and-debugging.html#println-debugging-alloc-and-core suggests to use format!. But format is not available under core (at least not as std::format!, nor as core::format! - and I thought that format! generates String, which, of course, requires alloc, so format! can't be under core).

Would it work as alloc::format! with #[uses_alloc] or (whatever that is), or how?

If all that people need to debug are tests, or alloc/core functionality invoked from tests, how about mentioning something like the following (if it works):

/// inside (the body of) a non-test core or alloc function:
#[cfg(test)]
{
    use std::format;
    ...
}

Or, could we mention 3rd party #[no_std] (limited) heap-less formatting crates (there is at least one).

It happens sometimes that code gets submitted for inclusion in the standard library which was taken (almost) literally from e.g. itertools or another library. We should document what to do with these w.r.t. copyright and crediting the original author(s).

https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html has two links labeled "libs team reviewers" that lead to https://github.com/rust-lang/highfive/blob/master/highfive/configs/rust-lang/rust.json, which doesn't exist. It also says to "either ping @rust-lang/libs-api or leave a comment asking about the status of the feature", but it's not clear what "ping" means here; it seems to be distinct from "leav[ing] a comment", and even if it does mean leaving a comment with that tag in it, that doesn't appear to work (at least it doesn't get highlighted in the rendered comment like a user would be).

I got very confused about the exact steps I needed to do to test out some changes to std while drafting an idea to resolve an outstanding issue, just to check feasibility before I made too many claims. The issue comes in that the rustc-dev-guide is what's linked from from rust-lang/Rust, and the opening page mentions here, the std-dev-guide, which contains a lot of helpful procedural and code style/requirements info, and other neat internals info, but nothing about actually building or testing std(/core/alloc etc).

Unfortunately, you're then left confused, and going back to both the README of the main Rust repo, and the rustc-dev-guide to try and scan for any chapter heading mentioning "std" or "library" or similar, until you finally have to sit down and just read and realize it's in the guide called "building the compiler", and running tests is in "testing the compiler".

Honestly if I was newer to coding or otherwise didn't know how bootstrapping compilers and the distribution thereof worked, I probably would've assumed rustc-dev-guide was a lost cause altogether. It's an odd situation where the existence of this guide makes the presence of this info in rustc-dev-guide less obvious, because there's a specific place it "should" be. This is probably especially true for people newer to programming or open source than me who may be concerned that technical talk about building the compiler may go over their heads, but would be comfortable trying to play with std since that's mostly the type of code they're familiar with.

Note, this is mentioned here: #1 but I thought an explicit issue may be better than a comment on that:

* [ ]  How to build/test/document the standard library (`x.py build/test/doc --stage 0 library/std` respectively

I can try drafting a simple PR to either copy over and simplify this info as appropriate, or at least say "for building and testing follow the instructions at (link to building the compiler) and (link to testing the compiler)" when I have time, but for now I thought I'd at least raise the issue.

(Edit: I do see now in the rustc book there's a listing in the Getting Started that lists std, core, alloc etc, but it's easy to miss and doesn't change much for this book. x.py also explicitly asks what you want to do but you're unlikely to get there yourself IMO.)

Look at how external crates are used in std, requirements for bringing in external crates (do they need to be part of rust-lang?) and who decides what crates to use internally (that sound like a Libs Impl decision).

Our initial CI script in copied from the rustc dev guide, but we don't currently have a gh-pages branch, so will probably need to do some more work to get it readable.

We should document what we expect from // Safety: .. comments.

See also rust-lang/rust#66219

This is a tracking issue for content we'd like to collect up in the standard library dev guide.

Please feel free to edit this issue or leave a comment with other things!

• Anything from https://github.com/rust-lang/rustc-dev-guide?
• https://forge.rust-lang.org/libs/maintaining-std.html

Should WG-rustc-dev-guide have permissions to maintain the std-dev-guide as well? It seems that only T-libs has permissions currently.

A lot of iterator adapters have specialized implementations for when the underlying iterator implements e.g. TrustedRandomAccess.Traits like that are marked with #[rustc_specialization_marker] or #[rustc_unsafe_specialization_marker]. We should document when to use this, and what it means for stability guarantees.

cc @matthewjasper

Note that an ACP is not strictly required: you can just go ahead and submit a pull request with an implementation of your proposed API, with the risk of wasted effort if the library team ends up rejecting this feature. However do note that this risk is always present even if an ACP is accepted, as the library team can end up rejecting a feature in the later parts of the stabilization process.

std-dev-guide/src/development/feature-lifecycle.md

Line 26 in 23d4ff6

Note that an ACP is not strictly required: you can just go ahead and submit a pull request with an implementation of your proposed API, with the risk of wasted effort if the library team ends up rejecting this feature. However do note that this risk is always present even if an ACP is accepted, as the library team can end up rejecting a feature in the later parts of the stabilization process.

When contributing to std, a lot of contributors find this line and believe they don't have to file an ACP. They may submit a PR, wait days for a review, and finally get a response that the PR can't be reviewed until an ACP has been accepted.

Filing an ACP seems de facto required and would cut out a lot of wasted lead time on contributors' behalf if they submitted the ACP along with the PR instead of risking its optionality. Being told you need to write a report days or weeks after you filed the PR, while it's no longer fresh in your mind, is a demotivating and frustrating experience for contributors.

Observing new feature addition PRs since the ACP process was added, it seems overwhelmingly that these PRs require an ACP. I would like to see this clause clarified to encourage contributors to submit an ACP anyway to make the process smoother and less surprising for them.

As a sidenote, I needed to file an ACP for extending a pre-existing feature to another type, so perhaps that should be called out too. I don't know the policy, but for example: "Expanding the scope of an already existing feature may/will require an ACP." This way contributors won't be surprised and will be prepared to do that if the necessary.

This is a tracking issue for making the std-dev-guide available via GitHub Pages on std-dev-guide.rust-lang.org (consistent with rustc-dev-guide.rust-lang.org).

We now have a gh-pages branch with a index.html at the root. There's also a CNAME in our main branch, but this isn't being deployed to the gh-pages branch yet. I'm guessing we'll need to do that.

cc @Mark-Simulacrum

In rust-lang/rust#81272 it was pointed out that refactoring to avoid the tidy file length check can be performance sensitive so we want to rollup=never on them.

Those PRs are also prone to merge conflicts so may need to be handled in chunks, which could also make checking diffs easier to make sure nothing actually changed in the move.

Do we want to signal sections of the guide that have had more thorough ratification than others? For the API Guidelines for instance we're now gating new changes to the content itself on FCPs, but that probably would've stopped us from ever getting it bootstrapped if we did that for a start.

Our current guidance on using #[inline] should mention profiling as our guiding principle even in places where we currently think the attribute "shouldn't" be needed (and maybe we want to revisit those "shouldn't"s). I think it can be inferred from the docs that making changes to #[inline] can be a separate PR to other changes but we should make that more explicit.

See also rust-lang/rust#109546. cc @saethlin

We're using Add:add(..) explicitly in a few places that should probably just use #[rustc_inherit_overflow_checks] instead.

We should document how the attribute works and when to use it.

(cc @eddyb)