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 5 pull requests #100213

Merged
merged 13 commits into from
Aug 7, 2022
Merged

Rollup of 5 pull requests #100213

merged 13 commits into from
Aug 7, 2022

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

klensy and others added 13 commits August 2, 2022 21:07
…ark-Simulacrum

deps: dedupe `annotate-snippets` crate versions

Dedupes `annotate-snippets` crate versions (https://github.com/rust-lang/annotate-snippets-rs/blob/0.9.1/CHANGELOG.md). Should work, but there is not a lot of tests.

Looks like switching to that crate a bit stalled.
Remove Windows function preloading

After `@Mark-Simulacrum` asked me to provide guidance for when optionally imported functions should be preloaded, I realised my justifications were now quite weak. I think the strongest argument that can be made is that it avoids some degree of nondeterminism when calling these functions (in as far as system API calls can be said to be deterministic). However, I don't think that's particularly convincing unless there's a real world use case where it matters. Further discussion with `@thomcc` has strengthened my feeling that preloading isn't really needed.

Note that `WaitOnAddress` needed some adjustment to work without preloading. I opted not to use a macro for this special case as it seemed silly to do so for just one thing (and I don't like macros tbh).
…pan, r=lcnr

Avoid pointing out `return` span if it has nothing to do with type error

This code:

```rust
fn f(_: String) {}

fn main() {
    let x = || {
        if true {
            return ();
        }
        f("");
    };
}
```

Emits this:
```
   Compiling playground v0.0.1 (/playground)
error[E0308]: mismatched types
 --> src/main.rs:8:11
  |
8 |         f("");
  |           ^^- help: try using a conversion method: `.to_string()`
  |           |
  |           expected struct `String`, found `&str`
  |
note: return type inferred to be `String` here
 --> src/main.rs:6:20
  |
6 |             return ();
  |                    ^^
```

Specifically, that note has nothing to do with the type error in question. This is because the change implemented in rust-lang#84244 tries to point out the `return` span on _any_ type coercion error within a closure that happens after a `return` statement, regardless of if the error has anything to do with it.

This is really easy to trigger -- just needs a closure (or an `async`) and an early return (or any other form, e.g. `?` operator suffices) -- and super distracting in production codebases. I'm letting rust-lang#84128 regress because that issue is much harder to fix correctly, and I can re-open that issue after this lands.

As a drive-by, I added a `resolve_vars_if_possible` to the coercion error logic, which leads to some error improvements. Unrelated to the issue above, though.
…o, r=workingjubilee

Optimize `pointer::as_aligned_to`

This PR replaces `addr % align` with `addr & align - 1`, which is correct due to `align` being a power of two.

Here is a proof that this makes things better: [[godbolt]](https://godbolt.org/z/Wbq3hx6YG).

This PR also removes `assume(align != 0)`, with the new impl it does not improve anything anymore ([[godbolt]](https://rust.godbolt.org/z/zcnrG4777), [[original concern]](rust-lang#95643 (comment))).
ascii -> ASCII in code comment

Easy one I spotted while reading source code.
@rustbot rustbot added 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 Aug 6, 2022
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Aug 6, 2022

📌 Commit ee0b755 has been approved by matthiaskrgr

It is now in the queue for this repository.

@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 Aug 6, 2022
@bors
Copy link
Contributor

bors commented Aug 6, 2022

⌛ Testing commit ee0b755 with merge cc71eef...

@bors
Copy link
Contributor

bors commented Aug 7, 2022

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 7, 2022
@bors bors merged commit cc71eef into rust-lang:master Aug 7, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 7, 2022
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Perf Build Sha
#100175 02aa32a857108851d0b75452c72e4e6a3a89df76
#100169 0192d40e95f942f956c43d1e5ee065ee492b0b6c
#100130 4fa29b2c271dfffc7ae764663a887433c30aae0d
#100127 d690a5d5b606852afda6c6ae66ed2fe8e1d8a142
#100071 5e4c2909806b51142f7f2df59e7cb1c5a82a38cc

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (cc71eef): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 😿 relevant regression found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
2.6% 2.6% 1
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) N/A N/A 0

Cycles

This benchmark run did not return any relevant results for this metric.

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

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change

  2. number of relevant changes

@matthiaskrgr matthiaskrgr deleted the rollup-mqe7t1n branch October 9, 2022 00:31
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. 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.

9 participants