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 8 pull requests #120304

Closed
wants to merge 28 commits into from
Closed

Rollup of 8 pull requests #120304

wants to merge 28 commits into from

Conversation

fmease
Copy link
Member

@fmease fmease commented Jan 24, 2024

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

compiler-errors and others added 28 commits December 25, 2023 20:31
Don't skip the inconsistent data layout check for custom LLVMs.

With rust-lang#118708, all targets will have a simple test that would trigger this
check if LLVM's data layouts do change - so data layouts would be
corrected during the LLVM upgrade. Therefore, with builtin targets, this
check won't trigger with our LLVM because each target will have been
confirmed to work. With non-builtin targets, this check is probably
useful to have because you can change the data layout in your target and
if its wrong then that could lead to bugs.

When using a custom LLVM, the same justification makes sense for
non-builtin targets as with our LLVM, the user can update their target to
match their LLVM and that's probably a good thing to do. However, with
a custom LLVM, the user cannot change the builtin target data layouts if
they don't match - though given that the compiler's data layout is used
for layout computation and a bunch of other things - you could get some
bugs because of the mismatch and probably want to know about that.

`CFG_LLVM_ROOT` was also always set during local development with
`download-ci-llvm` so this bug would never trigger locally.

Signed-off-by: David Wood <david@davidtw.co>
When an associated type `Self::Assoc` is part of a `where` clause,
we end up unable to evaluate the requirement and emit a E0275.

We now point at the associated type if specified in the `impl`. If
so, we also suggest using that type instead of `Self::Assoc`.
Otherwise, we explain that these are not allowed.

```
error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _`
  --> $DIR/impl-wf-cycle-1.rs:15:1
   |
LL | / impl<T: Grault> Grault for (T,)
LL | |
LL | | where
LL | |     Self::A: Baz,
LL | |     Self::B: Fiz,
   | |_________________^
LL |   {
LL |       type A = ();
   |       ------ associated type `<(T,) as Grault>::A` is specified here
   |
note: required for `(T,)` to implement `Grault`
  --> $DIR/impl-wf-cycle-1.rs:15:17
   |
LL | impl<T: Grault> Grault for (T,)
   |                 ^^^^^^     ^^^^
...
LL |     Self::A: Baz,
   |              --- unsatisfied trait bound introduced here
   = note: 1 redundant requirement hidden
   = note: required for `(T,)` to implement `Grault`
help: associated type for the current `impl` cannot be restricted in `where` clauses, remove this bound
   |
LL -     Self::A: Baz,
LL +     ,
   |
```
```
error[E0275]: overflow evaluating the requirement `<T as B>::Type == <T as B>::Type`
  --> $DIR/impl-wf-cycle-3.rs:7:1
   |
LL | / impl<T> B for T
LL | | where
LL | |     T: A<Self::Type>,
   | |_____________________^
LL |   {
LL |       type Type = bool;
   |       --------- associated type `<T as B>::Type` is specified here
   |
note: required for `T` to implement `B`
  --> $DIR/impl-wf-cycle-3.rs:7:9
   |
LL | impl<T> B for T
   |         ^     ^
LL | where
LL |     T: A<Self::Type>,
   |        ------------- unsatisfied trait bound introduced here
help: replace the associated type with the type specified in this `impl`
   |
LL |     T: A<bool>,
   |          ~~~~
```
```
error[E0275]: overflow evaluating the requirement `<T as Filter>::ToMatch == <T as Filter>::ToMatch`
  --> $DIR/impl-wf-cycle-4.rs:5:1
   |
LL | / impl<T> Filter for T
LL | | where
LL | |     T: Fn(Self::ToMatch),
   | |_________________________^
   |
note: required for `T` to implement `Filter`
  --> $DIR/impl-wf-cycle-4.rs:5:9
   |
LL | impl<T> Filter for T
   |         ^^^^^^     ^
LL | where
LL |     T: Fn(Self::ToMatch),
   |        ----------------- unsatisfied trait bound introduced here
note: associated types for the current `impl` cannot be restricted in `where` clauses
  --> $DIR/impl-wf-cycle-4.rs:7:11
   |
LL |     T: Fn(Self::ToMatch),
   |           ^^^^^^^^^^^^^
```

Fix rust-lang#116925
These were added with good intentions, but a recent change in LLVM 18
emits a warning while examining .rmeta sections in .rlib files. Since
this flag is a nice-to-have and users can update their LLVM linker
independently of rustc's LLVM version, we can just omit the flag.
Rename `link_(dylib,framework)`, remove `link_rust_dylib`.
Rename `link(_whole)(staticlib,rlib)` to something more suitable.
Consolidating this code into flatter functions reduces the amount of
pointer-chasing required to read and modify it.
…=oli-obk

Add `AsyncFn` family of traits

I'm proposing to add a new family of `async`hronous `Fn`-like traits to the standard library for experimentation purposes.

## Why do we need new traits?

On the user side, it is useful to be able to express `AsyncFn` trait bounds natively via the parenthesized sugar syntax, i.e. `x: impl AsyncFn(&str) -> String` when experimenting with async-closure code.

This also does not preclude `AsyncFn` becoming something else like a trait alias if a more fundamental desugaring (which can take many[^1] different[^2] forms) comes around. I think we should be able to play around with `AsyncFn` well before that, though.

I'm also not proposing stabilization of these trait names any time soon (we may even want to instead express them via new syntax, like `async Fn() -> ..`), but I also don't think we need to introduce an obtuse bikeshedding name, since `AsyncFn` just makes sense.

## The lending problem: why not add a more fundamental primitive of `LendingFn`/`LendingFnMut`?

Firstly, for `async` closures to be as flexible as possible, they must be allowed to return futures which borrow from the async closure's captures. This can be done by introducing `LendingFn`/`LendingFnMut` traits, or (equivalently) by adding a new generic associated type to `FnMut` which allows the return type to capture lifetimes from the `&mut self` argument of the trait. This was proposed in one of [Niko's blog posts](https://smallcultfollowing.com/babysteps/blog/2023/05/09/giving-lending-and-async-closures/).

Upon further experimentation, for the purposes of closure type- and borrow-checking, I've come to the conclusion that it's significantly harder to teach the compiler how to handle *general* lending closures which may borrow from their captures. This is, because unlike `Fn`/`FnMut`, the `LendingFn`/`LendingFnMut` traits don't form a simple "inheritance" hierarchy whose top trait is `FnOnce`.

```mermaid
flowchart LR
    Fn
    FnMut
    FnOnce
    LendingFn
    LendingFnMut

    Fn -- isa --> FnMut
    FnMut -- isa --> FnOnce

    LendingFn -- isa --> LendingFnMut

    Fn -- isa --> LendingFn
    FnMut -- isa --> LendingFnMut
```

For example:

```
fn main() {
  let s = String::from("hello, world");
  let f = move || &s;
  let x = f(); // This borrows `f` for some lifetime `'1` and returns `&'1 String`.
```

That trait hierarchy means that in general for "lending" closures, like `f` above, there's not really a meaningful return type for `<typeof(f) as FnOnce>::Output` -- it can't return `&'static str`, for example.

### Special-casing this problem:

By splitting out these traits manually, and making sure that each trait has its own associated future type, we side-step the issue of having to answer the questions of a general `LendingFn`/`LendingFnMut` implementation, since the compiler knows how to generate built-in implementations for first-class constructs like async closures, including the required future types for the (by-move) `AsyncFnOnce` and (by-ref) `AsyncFnMut`/`AsyncFn` trait implementations.

[^1]: For example, with trait transformers, we may eventually be able to write: `trait AsyncFn = async Fn;`
[^2]: For example, via the introduction of a more fundamental "`LendingFn`" trait, plus a [special desugaring with augmented trait aliases](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Lending.20closures.20and.20Fn*.28.29.20-.3E.20impl.20Trait/near/408471480).
Provide more context on recursive `impl` evaluation overflow

When an associated type `Self::Assoc` is part of a `where` clause, we end up unable to evaluate the requirement and emit a E0275.

We now point at the associated type if specified in the `impl`. If so, we also suggest using that type instead of `Self::Assoc`. Otherwise, we explain that these are not allowed.

```
error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _`
  --> $DIR/impl-wf-cycle-1.rs:15:1
   |
LL | / impl<T: Grault> Grault for (T,)
LL | |
LL | | where
LL | |     Self::A: Baz,
LL | |     Self::B: Fiz,
   | |_________________^
LL |   {
LL |       type A = ();
   |       ------ associated type `<(T,) as Grault>::A` is specified here
   |
note: required for `(T,)` to implement `Grault`
  --> $DIR/impl-wf-cycle-1.rs:15:17
   |
LL | impl<T: Grault> Grault for (T,)
   |                 ^^^^^^     ^^^^
...
LL |     Self::A: Baz,
   |              --- unsatisfied trait bound introduced here
   = note: 1 redundant requirement hidden
   = note: required for `(T,)` to implement `Grault`
help: associated type for the current `impl` cannot be restricted in `where` clauses, remove this bound
   |
LL -     Self::A: Baz,
   |
```
```
error[E0275]: overflow evaluating the requirement `<T as B>::Type == <T as B>::Type`
  --> $DIR/impl-wf-cycle-3.rs:7:1
   |
LL | / impl<T> B for T
LL | | where
LL | |     T: A<Self::Type>,
   | |_____________________^
LL |   {
LL |       type Type = bool;
   |       --------- associated type `<T as B>::Type` is specified here
   |
note: required for `T` to implement `B`
  --> $DIR/impl-wf-cycle-3.rs:7:9
   |
LL | impl<T> B for T
   |         ^     ^
LL | where
LL |     T: A<Self::Type>,
   |        ------------- unsatisfied trait bound introduced here
help: replace the associated type with the type specified in this `impl`
   |
LL |     T: A<bool>,
   |          ~~~~
```
```
error[E0275]: overflow evaluating the requirement `<T as Filter>::ToMatch == <T as Filter>::ToMatch`
  --> $DIR/impl-wf-cycle-4.rs:5:1
   |
LL | / impl<T> Filter for T
LL | | where
LL | |     T: Fn(Self::ToMatch),
   | |_________________________^
   |
note: required for `T` to implement `Filter`
  --> $DIR/impl-wf-cycle-4.rs:5:9
   |
LL | impl<T> Filter for T
   |         ^^^^^^     ^
LL | where
LL |     T: Fn(Self::ToMatch),
   |        ----------------- unsatisfied trait bound introduced here
note: associated types for the current `impl` cannot be restricted in `where` clauses
  --> $DIR/impl-wf-cycle-4.rs:7:11
   |
LL |     T: Fn(Self::ToMatch),
   |           ^^^^^^^^^^^^^
```

Fix rust-lang#116925
…r=wesleywiser

llvm: change data layout bug to an error and make it trigger more

Fixes rust-lang#33446.

Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets.

With rust-lang#118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs.

When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it?

`CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally.

In rust-lang#33446, two points are raised:

- In the issue itself, changing this from a `bug!` to a proper error is what is suggested, by using `isCompatibleDataLayout` from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call.
- ````@Mark-Simulacrum```` suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.
linker: Refactor library linking methods in `trait Linker`

Linkers are not aware of Rust libraries, they look like regular static or dynamic libraries to them, so Rust-specific methods in `trait Linker` do not make much sense.
They can be either removed or renamed to something more suitable.

Commits after the second one are cleanups.
Bump some deps with syn 1.0 dependencies

cc rust-lang#109302

`cargo update`ing `unic-langid` and `object` removes two dependencies on Syn 1.0.
…chaelwoerister

Assert that a single scope is passed to `for_scope`

Addresses rust-lang#118518 (comment)

r? `@michaelwoerister`
…sm, r=oli-obk

Remove --fatal-warnings on wasm targets

These were added with good intentions, but a recent change in LLVM 18 emits a warning while examining .rmeta sections in .rlib files. Since this flag is a nice-to-have and users can update their LLVM linker independently of rustc's LLVM version, we can just omit the flag.

See [this comment on wasm targets' uses of `--fatal-warnings`](llvm/llvm-project#78658 (comment)).
coverage: Dismantle `Instrumentor` and flatten span refinement

This is a combination of two refactorings that are unrelated, but would otherwise have a merge conflict.

No functional changes, other than a small tweak to debug logging as part of rearranging some functions.

Ignoring whitespace is highly recommended, since most of the modified lines have just been reindented.

---

The first change is to dismantle `Instrumentor` into ordinary functions.

This is one of those cases where encapsulating several values into a struct ultimately hurts more than it helps. With everything stored as local variables in one main function, and passed explicitly into helper functions, it's easier to see what is used where, and make changes as necessary.

---

The second change is to flatten the functions for extracting/refining coverage spans.

Consolidating this code into flatter functions reduces the amount of pointer-chasing required to read and modify it.
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 24, 2024
@rustbot rustbot added 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. rollup A PR which is a rollup labels Jan 24, 2024
@fmease
Copy link
Member Author

fmease commented Jan 24, 2024

@bors r+ rollup=never p=8

@bors
Copy link
Contributor

bors commented Jan 24, 2024

📌 Commit bc241fd has been approved by fmease

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 Jan 24, 2024
@bors
Copy link
Contributor

bors commented Jan 24, 2024

⌛ Testing commit bc241fd with merge 62730f2...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 24, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#119305 (Add `AsyncFn` family of traits)
 - rust-lang#119389 (Provide more context on recursive `impl` evaluation overflow)
 - rust-lang#120062 (llvm: change data layout bug to an error and make it trigger more)
 - rust-lang#120099 (linker: Refactor library linking methods in `trait Linker`)
 - rust-lang#120201 (Bump some deps with syn 1.0 dependencies)
 - rust-lang#120230 (Assert that a single scope is passed to `for_scope`)
 - rust-lang#120278 (Remove --fatal-warnings on wasm targets)
 - rust-lang#120292 (coverage: Dismantle `Instrumentor` and flatten span refinement)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Jan 25, 2024

💥 Test timed out

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

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@fmease fmease closed this Jan 25, 2024
@fmease fmease deleted the rollup-h8gaxdp branch January 25, 2024 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc rollup A PR which is a rollup 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.