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 3 pull requests #118003

Merged
merged 14 commits into from
Nov 17, 2023
Merged

Rollup of 3 pull requests #118003

merged 14 commits into from
Nov 17, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

RalfJung and others added 14 commits November 4, 2023 11:22
In cases like Kani, we will invoke the rustc_internal run command
directly for now. It would be handly to be able to have a callback
that can return a value.

We also need extra methods to convert stable constructs into internal
ones, so we can break down the transition into finer grain commits.
We were not iterating over all local variables due to a typo.
Add more APIs to retrieve information about types, and add more instance
resolution options.

Make `Instance::body()` return an Option<Body>, since not every instance
might have an available body. For example, foreign instances, virtual
instances, dependencies.
We were previously skipping it for non-generic functions, but this was
leaving some constants unevaluated.
Fixed the `has_body()` function operator. Before that, this function was
returning false for all shims.

Change resolve_drop_in_place() to also return an instance for empty
shims, since they may still be required for vtable construction.
When writing a pattern to collect multiple entries of a slice in a
single binding, it is easy to misremember or typo the appropriate syntax
to do so, instead writing the experimental `X..` pattern syntax. When we
encounter a resolve error because `X` isn't available, we suggest
`X @ ..` as an alternative.

```
error[E0425]: cannot find value `rest` in this scope
  --> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:3:13
   |
LL |         [1, rest..] => println!("{rest:?}"),
   |             ^^^^ not found in this scope
   |
help: if you meant to collect the rest of the slice in `rest`, use the at operator
   |
LL |         [1, rest @ ..] => println!("{rest:?}"),
   |                  +
```

Fix rust-lang#88404.
…imulacrum

document ABI compatibility

I don't think we have any central place where we document our ABI compatibility rules, so let's create one. The `fn()` pointer type seems like a good place since ABI questions can only become relevant when invoking a function through a function pointer.

This will likely need T-lang FCP.
…er-errors

Misc changes to StableMIR required to Kani use case.

First, I wanted to say that I can split this review into multiple if it makes reviewing easier. I bundled them up, since I've been testing them together (See rust-lang/project-stable-mir#51 for the set of more thorough checks).

So far, this review includes 3 commits:

1. Add more APIs and fix `Instance::body`
    - Add more APIs to retrieve information about types.
    - Add a few more instance resolution options. For the drop shim, we return None if the drop body is empty. Not sure it will be enough.
    - Make `Instance::body()` return an Option<Body>, since not every instance might have an available body. For example, foreign instances, virtual instances, dependencies.
2. Fix a bug on MIRVisitor
    - We were not iterating over all local variables due to a typo.
3. Add more SMIR internal impl and callback return value
    - In cases like Kani, we will invoke the rustc_internal run command directly for now. It would be handly to be able to have a callback that can return a value.
    - We also need extra methods to convert stable constructs into internal ones, so we can break down the transition into finer grain commits.
    - For the internal implementation of Region, we're always returning `ReErased` for now.
On resolve error of `[rest..]`, suggest `[rest @ ..]`

When writing a pattern to collect multiple entries of a slice in a single binding, it is easy to misremember or typo the appropriate syntax to do so, instead writing the experimental `X..` pattern syntax. When we encounter a resolve error because `X` isn't available, we suggest `X @ ..` as an alternative.

```
error[E0425]: cannot find value `rest` in this scope
  --> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:3:13
   |
LL |         [1, rest..] => println!("{rest:?}"),
   |             ^^^^ not found in this scope
   |
help: if you meant to collect the rest of the slice in `rest`, use the at operator
   |
LL |         [1, rest @ ..] => println!("{rest:?}"),
   |                  +
```

Fix rust-lang#88404.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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 Nov 17, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=3

@bors
Copy link
Contributor

bors commented Nov 17, 2023

📌 Commit 488c2aa 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 Nov 17, 2023
@bors
Copy link
Contributor

bors commented Nov 17, 2023

⌛ Testing commit 488c2aa with merge ee5ef3a...

@bors
Copy link
Contributor

bors commented Nov 17, 2023

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 17, 2023
@bors bors merged commit ee5ef3a into rust-lang:master Nov 17, 2023
12 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Nov 17, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#115476 document ABI compatibility 5086b01e348ebaba30372796d158532a7f815773 (link)
#117688 Misc changes to StableMIR required to Kani use case. d64e352c4cc4d4af2ad0d3019f3df3e3909c40e7 (link)
#117998 On resolve error of [rest..], suggest [rest @ ..] 65f933be4071041d448af2e69d94a9ccc01639b5 (link)

previous master: 00bfd6b273

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 (ee5ef3a): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.0% [-2.0%, -2.0%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.3% [0.9%, 2.1%] 3
Regressions ❌
(secondary)
2.9% [1.4%, 3.9%] 3
Improvements ✅
(primary)
-1.0% [-1.1%, -0.8%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.4% [-1.1%, 2.1%] 5

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.6% [-0.7%, -0.5%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.6% [-0.7%, -0.5%] 2

Binary size

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

Bootstrap: 675.609s -> 675.386s (-0.03%)
Artifact size: 313.59 MiB -> 313.58 MiB (-0.00%)

@lqd
Copy link
Member

lqd commented Nov 17, 2023

(deep-vector is currently being noisy)

@matthiaskrgr matthiaskrgr deleted the rollup-80t3uky branch March 16, 2024 18:18
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.

8 participants