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 6 pull requests #91799

Merged
merged 34 commits into from
Dec 11, 2021
Merged

Rollup of 6 pull requests #91799

merged 34 commits into from
Dec 11, 2021

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

mbartlett21 and others added 30 commits November 10, 2021 08:41
This commit also updates `stdarch` git submodule.
This is a follow-up from rust-lang#91645, regarding [some remarks I made](https://rust-lang.zulipchat.com/#narrow/stream/187312-wg-async-foundations/topic/join!/near/264293660).

Mainly:
  - it hides the recursive munching through a private `macro`, to avoid leaking such details (a corollary is getting rid of the need to use `@` to disambiguate);
  - it uses a `match` binding, _outside_ the `async move` block, to better match the semantics from function-like syntax;
  - it pre-pins the future before calling into `poll_fn`, since `poll_fn`, alone, cannot guarantee that its capture does not move;
  - it uses `.ready()?` since it's such a neat pattern;
  - it renames `Took` to `Taken` for consistency with `Done`.
Co-authored-by: Ibraheem Ahmed <[email protected]>
…` point

```
error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
  --> $DIR/issue-72312.rs:10:24
   |
LL |     pub async fn start(&self) {
   |                        ^^^^^ this data with an anonymous lifetime `'_`...
...
LL |         require_static(async move {
   |         -------------- ...is required to live as long as `'static` here...
LL |             &self;
   |             ----- ...and is captured here
   |
note: `'static` lifetime requirement introduced by this trait bound
  --> $DIR/issue-72312.rs:2:22
   |
LL | fn require_static<T: 'static>(val: T) -> T {
   |                      ^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0759`.
```

Fix rust-lang#72312.
* take diagnostic logic out of happy-path
* sort/dedup once
* add more comments
More accurate filtering still needed.
Co-authored-by: Daniel Henry-Mantilla <[email protected]>
In Rust, nesting method calls with both require `&mut` access to `self`
produces a borrow-check error:

    error[E0499]: cannot borrow `*self` as mutable more than once at a time
     --> src/lib.rs:7:14
      |
    7 |     self.foo(self.bar());
      |     ---------^^^^^^^^^^-
      |     |    |   |
      |     |    |   second mutable borrow occurs here
      |     |    first borrow later used by call
      |     first mutable borrow occurs here

That's because Rust has a left-to-right evaluation order, and the method
receiver is passed first. Thus, the argument to the method cannot then
mutate `self`.

There's an easy solution to this error: just extract a local variable
for the inner argument:

    let tmp = self.bar();
    self.foo(tmp);

However, the error doesn't give any suggestion of how to solve the
problem. As a result, new users may assume that it's impossible to
express their code correctly and get stuck.

This commit adds a (non-structured) suggestion to extract a local
variable for the inner argument to solve the error. The suggestion uses
heuristics that eliminate most false positives, though there are a few
false negatives (cases where the suggestion should be emitted but is
not). Those other cases can be implemented in a future change.
Suggest using a temporary variable to fix borrowck errors

Fixes rust-lang#77834.

In Rust, nesting method calls with both require `&mut` access to `self`
produces a borrow-check error:

    error[E0499]: cannot borrow `*self` as mutable more than once at a time
     --> src/lib.rs:7:14
      |
    7 |     self.foo(self.bar());
      |     ---------^^^^^^^^^^-
      |     |    |   |
      |     |    |   second mutable borrow occurs here
      |     |    first borrow later used by call
      |     first mutable borrow occurs here

That's because Rust has a left-to-right evaluation order, and the method
receiver is passed first. Thus, the argument to the method cannot then
mutate `self`.

There's an easy solution to this error: just extract a local variable
for the inner argument:

    let tmp = self.bar();
    self.foo(tmp);

However, the error doesn't give any suggestion of how to solve the
problem. As a result, new users may assume that it's impossible to
express their code correctly and get stuck.

This commit adds a (non-structured) suggestion to extract a local
variable for the inner argument to solve the error. The suggestion uses
heuristics that eliminate most false positives, though there are a few
false negatives (cases where the suggestion should be emitted but is
not). Those other cases can be implemented in a future change.
Point at capture points for non-`'static` reference crossing a `yield` point

```
error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
  --> $DIR/issue-72312.rs:10:24
   |
LL |     pub async fn start(&self) {
   |                        ^^^^^ this data with an anonymous lifetime `'_`...
...
LL |         require_static(async move {
   |         -------------- ...is required to live as long as `'static` here...
LL |             &self;
   |             ----- ...and is captured here
   |
note: `'static` lifetime requirement introduced by this trait bound
  --> $DIR/issue-72312.rs:2:22
   |
LL | fn require_static<T: 'static>(val: T) -> T {
   |                      ^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0759`.
```

Fix rust-lang#72312.
Make `Borrow` and `BorrowMut` impls `const`

Tracking issue: rust-lang#91522
Const `Option::cloned`

This constifies the two `Option::cloned` functions, bounded on `~const Clone`.
…anieu

Add spin_loop hint for RISC-V architecture

This commit uses the PAUSE instruction (rust-lang/stdarch#1262) to implement RISC-V spin loop, and updates `stdarch` submodule to use the merged PAUSE instruction.
…triplett

Minor improvements to `future::join!`'s implementation

This is a follow-up from rust-lang#91645, regarding [some remarks I made](https://rust-lang.zulipchat.com/#narrow/stream/187312-wg-async-foundations/topic/join!/near/264293660).

Mainly:
  - it hides the recursive munching through a private `macro`, to avoid leaking such details (a corollary is getting rid of the need to use ``@`` to disambiguate);
  - it uses a `match` binding, _outside_ the `async move` block, to better match the semantics from function-like syntax;
  - it pre-pins the future before calling into `poll_fn`, since `poll_fn`, alone, cannot guarantee that its capture does not move (to clarify: I believe the previous code was sound, thanks to the outer layer of `async`. But I find it clearer / more robust to refactorings this way 🙂).
  - it uses `@ibraheemdev's` very neat `.ready()?`;
  - it renames `Took` to `Taken` for consistency with `Done` (tiny nit 😄).

~~TODO~~Done:

  - [x] Add unit tests to enforce the function-like `:value` semantics are respected.

r? `@nrc`
@rustbot rustbot added the rollup A PR which is a rollup label Dec 11, 2021
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=6

@bors
Copy link
Contributor

bors commented Dec 11, 2021

📌 Commit ed81098 has been approved by matthiaskrgr

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 11, 2021
@bors
Copy link
Contributor

bors commented Dec 11, 2021

⌛ Testing commit ed81098 with merge 928783d...

@bors
Copy link
Contributor

bors commented Dec 11, 2021

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 928783d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 11, 2021
@bors bors merged commit 928783d into rust-lang:master Dec 11, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 11, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (928783d): comparison url.

Summary: This change led to very large relevant mixed results 🤷 in compiler performance.

  • Very large improvement in instruction counts (up to -5.9% on incr-unchanged builds of inflate)
  • Small regression in instruction counts (up to 0.8% on incr-unchanged builds of issue-88862)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

@rustbot rustbot added the perf-regression Performance regression. label Dec 11, 2021
@matthiaskrgr matthiaskrgr deleted the rollup-b38xx6i branch December 12, 2021 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.