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 #60211

Merged
merged 30 commits into from
Apr 23, 2019
Merged

Rollup of 6 pull requests #60211

merged 30 commits into from
Apr 23, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Apr 23, 2019

Successful merges:

Failed merges:

r? @ghost

KodrAus and others added 30 commits April 10, 2019 20:26
This tests that async functions drop parameters in the same order as
regular functions.
This will be used to keep track of the origin of a local in the AST. In
particular, it will be used by `async fn` lowering for the locals in
`let <pat>: <ty> = __arg0;` statements.
This commit adds an `AsyncArgument` struct to the AST that contains the
generated argument and statement that will be used in HIR lowering, name
resolution and def collection.
This commit takes advantage of `AsyncArgument` type that was added in a
previous commit to replace the arguments of the `async fn` in the HIR
and add statements to move the bindings from the new arguments to the
pattern from the old argument.

For example, the async function `foo` below:

    async fn foo((x, _y): (T, V)) {
        async move {
        }
    }

becomes:

    async fn foo(__arg0: (T, V)) {
        async move {
            let (x, _y) = __arg0;
        }
    }
This commit extends the previous commit to apply to trait methods as
well as free functions.
This avoids issues with `impl_trait_in_bindings` as the type from the
argument is normally used as the let binding, but `impl Trait` is
unstable in binding position.
This commit introduces an `ArgSource` enum that is lowered into the HIR
so that diagnostics can correctly refer to the argument pattern's
original name rather than the generated pattern.
This commit changes the order that arguments and bodies of async
functions are lowered so that when the body attempts to `lower_def` of a
upvar then the id has already been assigned by lowering the argument
first.
This commit displays the original pattern in generated documentation for
async functions rather than the synthesized pattern.
This commit introduces a `assert_drop_order_after_poll` helper function
to the test case for this case to reduce repetitive noise and documents
what each function aims to test.
[wg-async-await] Drop `async fn` arguments in async block

Fixes rust-lang#54716.

This PR modifies the HIR lowering (and some other places to make this work) so that unused arguments to a async function are always dropped inside the async move block and not at the end of the function body.

```
async fn foo(<pattern>: <type>) {
  async move {
  }
} // <-- dropped as you "exit" the fn

// ...becomes...
fn foo(__arg0: <ty>) {
  async move {
    let <pattern>: <ty> = __arg0;
  } // <-- dropped as you "exit" the async block
}
```

However, the exact ordering of drops is not the same as a regular function, [as visible in this playground example](https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=be39af1a58e5d430be1eb3c722cb1ec3) - I believe this to be an unrelated issue. There is a [Zulip topic](https://rust-lang.zulipchat.com/#narrow/stream/187312-t-compiler.2Fwg-async-await/topic/.2354716.20drop.20order) for this.

r? @cramertj
cc @nikomatsakis
Warn on unused results for operation methods on nums

From a suggestion by @llogiq

Adds a `#[must_use]` attribute to operation methods on integers that take self by value as the first operand and another value as the second. It makes it clear that these methods return the result of the operation instead of mutating `self`, which is the source of a rather embarrassing bug I had in a codebase of mine recently...

As an example:

```rust
struct Int {
   value: i64,
}

impl Int {
    fn add(&mut self, other: i64) {
        self.value.wrapping_add(other);
    }
}
```

Will produce a warning like:

```
warning: unused return value of `core::num::<impl i64>::wrapping_add` that must be used
 --> src/main.rs:7:7
  |
7 |       self.value.wrapping_add(other);
  |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: #[warn(unused_must_use)] on by default
  = note: this returns the result of the operation, without modifying the original
```

If this is something we're on board with, we could do something similar for `f32` and `f64` too. There are probably other methods on integers that make sense.
…dreavus

Update fonts used by rustdoc

Our version of Source Serif Pro is pretty old and is causing issues on Linux, see https://bugzilla.mozilla.org/show_bug.cgi?id=1545317 . I took this opportunity to update all of the fonts we use.

r? @steveklabnik @QuietMisdreavus
…ine, r=kennytm

Warn when ignore-tidy-linelength is present, but no lines are too long

It's easy for a `// ignore-tidy-linelength` to be added when there is a genuine need to ignore a file's line length, but then after refactoring the need is gone, but the tidy directive is not removed. This means that in the future, further editing may accidentally add unnecessarily long lines. This change forces `// ignore-tidy-linelength` to be used exactly when necessary, to make sure such changes are intentional.
Add f16c target_feature

This is requires for Intel 16-bit half-precision float intrinsics: https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=fp16&expand=1769 - see companion stdsimd PR: rust-lang/stdarch#737.

LLVM, Wikipedia CPUID page, and the Intel Dev Manual all call this CPUID feature "F16C", but the Intel intrinsics guide calls this "FP16C" - this is probably a bug in the intrinsics guide which we are tracking here: rust-lang/stdarch#738

r? @alexcrichton
@Centril
Copy link
Contributor Author

Centril commented Apr 23, 2019

@bors r+ p=6

@bors
Copy link
Contributor

bors commented Apr 23, 2019

📌 Commit 0353fa5 has been approved by Centril

@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 Apr 23, 2019
@bors
Copy link
Contributor

bors commented Apr 23, 2019

⌛ Testing commit 0353fa5 with merge e938c2b...

bors added a commit that referenced this pull request Apr 23, 2019
Rollup of 6 pull requests

Successful merges:

 - #59823 ([wg-async-await] Drop `async fn` arguments in async block )
 - #59839 (Warn on unused results for operation methods on nums)
 - #60146 (Update fonts used by rustdoc)
 - #60169 (Warn when ignore-tidy-linelength is present, but no lines are too long)
 - #60177 (Promote rust comments to rustdoc)
 - #60191 (Add f16c target_feature)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Apr 23, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: Centril
Pushing e938c2b to master...

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. 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.

8 participants