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

Merged
merged 10 commits into from
May 9, 2024
Merged

Rollup of 5 pull requests #124934

merged 10 commits into from
May 9, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Xavier Denis and others added 10 commits May 8, 2024 19:22
The starting point for this was identical comments on two different
fields, in `ast::VariantData::Struct` and `hir::VariantData::Struct`:
```
    // FIXME: investigate making this a `Option<ErrorGuaranteed>`
    recovered: bool
```
I tried that, and then found that I needed to add an `ErrorGuaranteed`
to `Recovered::Yes`. Then I ended up using `Recovered` instead of
`Option<ErrorGuaranteed>` for these two places and elsewhere, which
required moving `ErrorGuaranteed` from `rustc_parse` to `rustc_ast`.

This makes things more consistent, because `Recovered` is used in more
places, and there are fewer uses of `bool` and
`Option<ErrorGuaranteed>`. And safer, because it's difficult/impossible
to set `recovered` to `Recovered::Yes` without having emitted an error.
Make a minimal amount of region APIs public

Tools like Creusot, Prusti or Gillian-Rust need to access information about the loans and regions that exist in MIR programs. While `rustc` provides information about loans, there is currently no public way to reason about the regions present in a MIR program. In particular, we to know which regions are actually equal to each other and which ones outlive each other. Currently, `rustc` provides access to `RegionInferenceContext` but the public api hides that last portion of the information.

This PR proposes to make a few apis public, allowing verifiers to reason about the lifetimes present in Rust programs:
- [eval_equal](https://doc.rust-lang.org/beta/nightly-rustc/rustc_borrowck/region_infer/struct.RegionInferenceContext.html#method.eval_equal)
- [eval_outlives](https://doc.rust-lang.org/beta/nightly-rustc/rustc_borrowck/region_infer/struct.RegionInferenceContext.html#method.eval_outlives)
- (Optional) [constraint_sccs](https://doc.rust-lang.org/beta/nightly-rustc/rustc_borrowck/region_infer/struct.RegionInferenceContext.html#method.constraint_sccs)

The first two functions would allow us to compare regions and from this we can construct the set of `RegionVid` which are actually equal to each other, and then recover the inclusions between those regions, while the second allows for more direct, but _low level_ access to that information.
…ranteed, r=compiler-errors

Add `ErrorGuaranteed` to `Recovered::Yes` and use it more.

The starting point for this was identical comments on two different fields, in `ast::VariantData::Struct` and `hir::VariantData::Struct`:
```
    // FIXME: investigate making this a `Option<ErrorGuaranteed>`
    recovered: bool
```
I tried that, and then found that I needed to add an `ErrorGuaranteed` to `Recovered::Yes`. Then I ended up using `Recovered` instead of `Option<ErrorGuaranteed>` for these two places and elsewhere, which required moving `ErrorGuaranteed` from `rustc_parse` to `rustc_ast`.

This makes things more consistent, because `Recovered` is used in more places, and there are fewer uses of `bool` and
`Option<ErrorGuaranteed>`. And safer, because it's difficult/impossible to set `recovered` to `Recovered::Yes` without having emitted an error.

r? `@oli-obk`
…piler-errors

interpret/miri: better errors on failing offset_from

Fixes rust-lang/miri#3104
… r=est31

Make `#![feature]` suggestion MaybeIncorrect

Fixes rust-lang/rust-clippy#12784

The `unstable_name_collisions` lint uses `disabled_nightly_features` to mention the feature name, but accepting the suggestion would result in an ambiguity error

There are other calls where accepting the feature gate would fix code when ran with `cargo fix --broken-code`, though it's not always desirable to add a feature gate even if the user is currently on nightly so MaybeIncorrect seems appropriate
@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. rollup A PR which is a rollup labels May 9, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented May 9, 2024

📌 Commit 779fe95 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 May 9, 2024
@bors
Copy link
Contributor

bors commented May 9, 2024

⌛ Testing commit 779fe95 with merge e6e262f...

@bors
Copy link
Contributor

bors commented May 9, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 9, 2024
@bors bors merged commit e6e262f into rust-lang:master May 9, 2024
7 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 9, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#124893 Make a minimal amount of region APIs public 5a8589d44b6b3ced8a57e45df23d4c90bb78878a (link)
#124919 Add ErrorGuaranteed to Recovered::Yes and use it more. 7a8f03b5b7972a84c7c45420017e3290a24e27f6 (link)
#124923 interpret/miri: better errors on failing offset_from 5c1a6486c20f406f85f296b4e8274d67dcb608bf (link)
#124924 chore: remove repetitive words 6d51904e4634b738120b375e2f1587b6acc34e1e (link)
#124926 Make #![feature] suggestion MaybeIncorrect a5a1c068f7cbb333b311bbc2c5ed40a7e052348c (link)

previous master: 238c1e798d

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

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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)
- - 0
Regressions ❌
(secondary)
3.5% [3.5%, 3.5%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

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

Binary size

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

Bootstrap: 675.773s -> 673.66s (-0.31%)
Artifact size: 315.82 MiB -> 315.97 MiB (0.05%)

@matthiaskrgr matthiaskrgr deleted the rollup-eqor0ot branch September 1, 2024 17:35
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants