Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 9 pull requests #129993

Closed
wants to merge 47 commits into from

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Noratrieb and others added 30 commits July 7, 2024 13:42
Because it's the target libdir.

`--print` uses the same terminology, and it's a simple way to make it
obviously different from `$sysroot/lib`.
It doesn't need to be in there, and the move simplifies lifetimes.
It's not necessary, and just complicates things.
By making it own the index maps, instead of holding references to them.
This requires moving the free function `find_candidate` into
`Candidate::reset_and_find`. It lets the `'alloc` lifetime be removed
everywhere that still has it.
…{in,out,err}` config helpers

Previously `Command::stdin` was actually just a stdin buffer helper, but
this is different from `std::process::Command::stdin`. This is
needlessly confusing, and blocks support to add `std{in,out,err}` config
that tests may want to use to e.g. redirect to `/dev/ptmx`.
GuillaumeGomez and others added 16 commits September 5, 2024 12:15
…-ozkan

Call the target libdir target libdir

Because it's the target libdir.

`--print` uses the same terminology, and it's a simple way to make it obviously different from `$sysroot/lib`.
fix: get llvm type of global val

using `LLVMTypeOf` on a global var always return ptr. so create a new function to access the value type of a global
`impl_trait_overcaptures`: Don't worry about uncaptured contravariant lifetimes if they outlive a captured lifetime

**NOTE:** Review only the first commit carefully. The second one is just moving stuff around, so you can turn whitespace off for that one.

This PR relaxes the `impl_trait_overcaptures` lint to not fire in cases like:

```rust
struct Ctxt<'tcx>(&'tcx ());

impl<'tcx> Ctxt<'tcx> {
    fn compute(&self) -> impl Sized + '_ { }
}
```

Specifically, the lint will not fire if **all** overcaptured regions (i.e. those which will be captured in edition 2024, which are not captured today) **satisfy**:
* The region is contravariant (or bivariant) in the function signature
* The region outlives some other region which is captured by the opaque

### The idea behind this

Why is this OK? My reasoning is that since the region is contravariant in the function signature, we know that it can be shortened arbitrarily at the call site. And specifically, we know it can be shortened to be equal to one of the regions that it outlives -- that's why we need to prove that it outlives some other region that *is* captured.

We could technically relax this further, but there would be (IMO somewhat easy) cases to make this a false negative in real code. For example, if the region is invariant, then we can hit issues like:

```rust
struct Ctxt<'tcx>(&'tcx mut &'tcx mut ());

impl<'tcx> Ctxt<'tcx> {
    fn compute(&self) -> impl Sized + use<'_, 'tcx> { }
    // We use `use<'_, 'tcx>` to show what happens in edition 2024
}

fn test<'a, 'b>(x: &'a Ctxt<'b>, y: &'a Ctxt<'a>) {
    let results = [x.compute(), y.compute()];
    //~^ ERROR lifetime may not live long enough
    // Since both opaques now capture `'tcx`, this enforces that `'a == 'b`.
}
```

### Is this actually totally fine?

There's one case where users might still hit issues, and it's if we turbofish lifetimes directly:

```rust
struct Ctxt<'tcx>(&'tcx ());

impl<'tcx> Ctxt<'tcx> {
    fn compute(&self) -> impl Sized + use<'_, 'tcx> { }
}

fn test<'a, 'b>(x: &'a Ctxt<'b>, y: &'a Ctxt<'a>) {
    let results = [Ctxt::<'b>::compute(x), Ctxt::<'a>::compute(y)];
    //~^ ERROR lifetime may not live long enough
    // Since both opaques now capture `'tcx`, this enforces that `'a == 'b`.
    // Note that we don't shorten `'b` to `'a` since we turbofished it.
}
```

### Well... we should still warn?

I kinda don't care about this case, though I guess we could possibly downgrade the lint to something like `IMPL_TRAIT_OVERCAPTURES_STRICT` instead of suppressing it altogether. Thoughts? If we were to do this, then I'd probably also opt to include the invariant case in `IMPL_TRAIT_OVERCAPTURES_STRICT` and move it out of `IMPL_TRAIT_OVERCAPTURES`.
…d-items, r=t-rustdoc-frontend

[rustdoc] Sort impl associated items by kinds and then by appearance

Following [this zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/.22Freeze.22.20order.20of.20items.20in.20.28trait.29.20impls.3F), I implemented it.

This brings the following change: impl associated items will now be grouped by kind and will now be first sorted by kind and then by the order they are declared in the source code (like currently).

The kinds are sorted in the following order:
1. Constants
2. Types
3. Functions

The reason behind this order is that associated constants can be used in associated types (like length in arrays) and both associated types and associated constants can be used in associated functions. So if an associated item from the same impl is used, its definition will always be above where it's being used.

cc `@camelid`
r? `@notriddle`
Rename dump of coroutine by-move-body to be more consistent, fix ICE in dump_mir

First, we add a missing match for `DefKind::SyntheticCoroutineBody` in `dump_mir`. Fixes rust-lang#129703. The second commit (directly below) serves as a test.

Second, we reorder the `dump_mir` in `coroutine_by_move_body_def_id` to be *after* we adjust the body source, and change the disambiguator so it reads more like any other MIR body. This also serves as a test for the ICE, since we're dumping the MIR of a body with `DefKind::SyntheticCoroutineBody`.

Third, we change the parenting of the synthetic MIR body to have the *coroutine-closure* (i.e. async closure) as its parent, so we don't have long strings of `{closure#0}-{closure#0}-{closure#0}`.

try-job: test-various
… r=cjgillot

Simplify DestProp memory management

The DestProp MIR pass has some convoluted memory management. This PR simplifies it.

r? ``@davidtwco``
…, r=notriddle

Unify scraped examples with other code examples

Fixes rust-lang#129763.

This first PR both fixes rust-lang#129763 but also unifies buttons display for code examples:

![image](https://github.com/user-attachments/assets/c8475945-dcc3-4c25-8d7d-1659f85301c8)

You can test it [here](https://rustdoc.crud.net/imperio/unify-code-examples/doc/scrape_examples/fn.test.html) and [here](https://rustdoc.crud.net/imperio/unify-code-examples/doc/scrape_examples/fn.test_many.html).

I'm planning to send a follow-up to make the buttons generated in JS directly (or I can do it in this PR directly if you prefer).

cc ``@willcrichton``
r? ``@notriddle``
Elaborate on deriving vs implementing `Copy`

I was reading this documentation and this wasn't immediately clear to me.

In my mind, it seemed obvious that a type can only claim to be `Copy` if the bits it is storing can be `Copy`, and in the case of a generic struct that can only be the case if `T: Copy`. So the bound added by the derive seemed necessary at all times, and I thought what the documentation was trying to say is that the custom implementation allows you to add _additional bounds_.

Of course what it was actually trying to point out is that just because you have a generic parameter `T`, it doesn't necessarily mean you are storing the bits of `T`. And if you aren't, it may be the case that your own bits can be copied regardless of whether the bits of `T` can be safely copied.

Thought it may be worth elaborating to make that a bit more clear. Haven't tested/didn't try to figure out how to render this locally. Mainly not sure if the `PhantomData` back link is going to just work or need some extra stuff, but I figured someone else probably could just tell.
run_make_support: rename `Command::stdin` to `stdin_buf` and add `std{in,out,err}` config helpers

Previously `Command::stdin` was actually just a stdin buffer helper, but
this is different from `std::process::Command::stdin`. This is
needlessly confusing, and blocks support to add `std{in,out,err}` config
helpers that tests may want to use to e.g. redirect to `/dev/ptmx`.
@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Sep 5, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=9

@bors
Copy link
Contributor

bors commented Sep 5, 2024

📌 Commit 0be10be has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2024
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
#16 3.017 Building wheels for collected packages: reuse
#16 3.018   Building wheel for reuse (pyproject.toml): started
#16 3.265   Building wheel for reuse (pyproject.toml): finished with status 'done'
#16 3.266   Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-manylinux_2_35_x86_64.whl size=132715 sha256=dfa09868353292d98f811d3efdb0d54d07389e808efc71d68e3b93c514bf8bec
#16 3.266   Stored in directory: /tmp/pip-ephem-wheel-cache-u7tveeqw/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#16 3.268 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#16 3.650 Successfully installed attrs-23.2.0 binaryornot-0.4.4 boolean-py-4.0 chardet-5.2.0 jinja2-3.1.4 license-expression-30.3.0 markupsafe-2.1.5 python-debian-0.1.49 reuse-4.0.3 tomlkit-0.13.0
#16 3.650 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#16 DONE 3.7s
---
   Compiling home v0.5.9
error[E0599]: no method named `sysroot_libdir` found for reference `&core::builder::Builder<'_>` in the current scope
##[error]    --> src/core/build_steps/dist.rs:478:39
     |
478  |                 let src_dir = builder.sysroot_libdir(compiler, host).parent().unwrap().join("bin");
     |
help: there is a method `sysroot` with a similar name, but with different arguments
    --> src/core/builder.rs:1149:5
     |
---
   Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
error[E0599]: no method named `sysroot_libdir` found for reference `&core::builder::Builder<'_>` in the current scope
##[error]    --> src/core/build_steps/dist.rs:478:39
     |
478  |                 let src_dir = builder.sysroot_libdir(compiler, host).parent().unwrap().join("bin");
     |
help: there is a method `sysroot` with a similar name, but with different arguments
    --> src/core/builder.rs:1149:5
     |
---
   Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
error[E0599]: no method named `sysroot_libdir` found for reference `&core::builder::Builder<'_>` in the current scope
##[error]    --> src/core/build_steps/dist.rs:478:39
     |
478  |                 let src_dir = builder.sysroot_libdir(compiler, host).parent().unwrap().join("bin");
     |
help: there is a method `sysroot` with a similar name, but with different arguments
    --> src/core/builder.rs:1149:5
     |
---
   Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
error[E0599]: no method named `sysroot_libdir` found for reference `&core::builder::Builder<'_>` in the current scope
##[error]    --> src/core/build_steps/dist.rs:478:39
     |
478  |                 let src_dir = builder.sysroot_libdir(compiler, host).parent().unwrap().join("bin");
     |
help: there is a method `sysroot` with a similar name, but with different arguments
    --> src/core/builder.rs:1149:5
     |
---
   Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
error[E0599]: no method named `sysroot_libdir` found for reference `&core::builder::Builder<'_>` in the current scope
##[error]    --> src/core/build_steps/dist.rs:478:39
     |
478  |                 let src_dir = builder.sysroot_libdir(compiler, host).parent().unwrap().join("bin");
     |
help: there is a method `sysroot` with a similar name, but with different arguments
    --> src/core/builder.rs:1149:5
     |

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.