Rollup of 5 pull requests
Successful merges:
- #78164 (Prefer regions with an `external_name` in `approx_universal_upper_bound`)
- #80003 (Fix overflow when converting ZST Vec to VecDeque)
- #80023 (Enhance error message when misspelled label to value in break expression)
- #80046 (Add more documentation to `Diagnostic` and `DiagnosticBuilder`)
- #80109 (Remove redundant and unreliable coverage test results)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Remove redundant and unreliable coverage test results
The `coverage-reports` tests still generate counters and JSON reports
for inspection, but these files are no longer used in Makefile diffs, to
reduce complexity and confusion from unreliable or unexpected test
results, especially when maintaining them (i.e., generating `--bless`ed
results).
The associated `expected_` files for counters and JSON reports have been
removed, leaving only the files actually used for testing: the `llvm-cov
show` reports.
r? `@tmandry`
Tyler - as we discussed offline...
FYI: `@wesleywiser` `@Swatinem`
Arpad, depending on the timing of this PR, it may not affect you, but I'm removing some of the files that produce slightly different results on Windows as they really aren't necessary to validate coverage results.
Prefer regions with an `external_name` in `approx_universal_upper_bound`
Fixes#75785
When displaying a MIR borrowcheck error, we may need to find an upper
bound for a region, which gives us a region to point to in the error
message. However, a region might outlive multiple distinct universal
regions, in which case the only upper bound is 'static
To try to display a meaningful error message, we compute an
'approximate' upper bound by picking one of the universal regions.
Currently, we pick the region with the lowest index - however, this
caused us to produce a suboptimal error message in issue #75785
This PR `approx_universal_upper_bound` to prefer regions with an
`external_name`. This causes us to prefer regions from function
arguments/upvars, which seems to lead to a nicer error message in some
cases.
Fixes#75785
When displaying a MIR borrowcheck error, we may need to find an upper
bound for a region, which gives us a region to point to in the error
message. However, a region might outlive multiple distinct universal
regions, in which case the only upper bound is 'static
To try to display a meaningful error message, we compute an
'approximate' upper bound by picking one of the universal regions.
Currently, we pick the region with the lowest index - however, this
caused us to produce a suboptimal error message in issue #75785
This PR `approx_universal_upper_bound` to prefer regions with an
`external_name`. This causes us to prefer regions from function
arguments/upvars, which seems to lead to a nicer error message in some
cases.
Move binder for dyn to each list item
This essentially changes `ty::Binder<&'tcx List<ExistentialTraitRef>>` to `&'tcx List<ty::Binder<ExistentialTraitRef>>`.
This is a first step in moving the `dyn Trait` representation closer to Chalk, which we've talked about in `@rust-lang/wg-traits.`
r? `@nikomatsakis`
Always run intrinsics lowering pass
Move intrinsics lowering pass from the optimization phase (where it
would not run if -Zmir-opt-level=0), to the drop lowering phase where it
runs unconditionally.
The implementation of those intrinsics in code generation and
interpreter is unnecessary. Remove it.
The `coverage-reports` tests still generate counters and JSON reports
for inspection, but these files are no longer used in Makefile diffs, to
reduce complexity and confusion from unreliable or unexpected test
results, especially when maintaining them (i.e., generating `--bless`ed
results).
The associated `expected_` files for counters and JSON reports have been
removed, leaving only the files actually used for testing: the `llvm-cov
show` reports.
Fixed conflict with drop elaboration and coverage
See
https://github.com/rust-lang/rust/issues/80045#issuecomment-745733339
Coverage statements are moved to the beginning of the BCB. This does
also affect what's counted before a panic, changing some results, but I
think these results may even be preferred? In any case, there are no
guarantees about what's counted when a panic occurs (by design).
r? `@tmandry`
FYI `@wesleywiser` `@ecstatic-morse`
Fix issue #78496
EarlyOtherwiseBranch finds MIR structures like:
```
bb0: {
...
_2 = discriminant(X)
...
switchInt(_2) -> [1_isize: bb1, otherwise: bb3]
}
bb1: {
...
_3 = discriminant(Y)
...
switchInt(_3) -> [1_isize: bb2, otherwise: bb3]
}
bb2: {...}
bb3: {...}
```
And transforms them into something like:
```
bb0: {
...
_2 = discriminant(X)
_3 = discriminant(Y)
_4 = Eq(_2, _3)
switchInt(_4) -> [true: bb4, otherwise: bb3]
}
bb2: {...} // unchanged
bb3: {...} // unchanged
bb4: {
switchInt(_2) -> [1_isize: bb2, otherwise: bb3]
}
```
But that is not always a safe thing to do -- sometimes the early `otherwise` branch is necessary so the later block could assume the value of `discriminant(X)`.
I am not totally sure what's the best way to detect that, but fixing #78496 should be easy -- we just check if `X` is a sub-expression of `Y`. A more precise test might be to check if `Y` contains a `Downcast(1)` of `X`, but I think this might be good enough.
Fix#78496
Allow `since="TBD"` for rustc_deprecated
Closes#78381.
This PR only affects `#[rustc_deprecated]`, not `#[deprecated]`, so there is no effect on any stable language feature.
Likewise this PR only implements `since="TBD"`, it does not actually tag any library functions with it, so there is no effect on any stable API.
Overview of changes:
* `rustc_middle/stability.rs`:
* change `deprecation_in_effect` function to return `false` when `since="TBD"`
* tidy up the compiler output when a deprecated item has `since="TBD"`
* `rustc_passes/stability.rs`:
* allow `since="TBD"` to pass the sanity check for stable_version < deprecated_version
* refactor the "invalid stability version" and "invalid deprecation version" error into separate errors
* rustdoc: make `since="TBD"` message on a deprecated item's page match the command-line deprecation output
* tests:
* test rustdoc output
* test that the `deprecated_in_future` lint fires when `since="TBD"`
* test the new "invalid deprecation version" error message
Implement if-let match guards
Implements rust-lang/rfcs#2294 (tracking issue: #51114).
I probably should do a few more things before this can be merged:
- [x] Add tests (added basic tests, more advanced tests could be done in the future?)
- [x] Add lint for exhaustive if-let guard (comparable to normal if-let statements)
- [x] Fix clippy
However since this is a nightly feature maybe it's fine to land this and do those steps in follow-up PRs.
Thanks a lot `@matthewjasper` ❤️ for helping me with lowering to MIR! Would you be interested in reviewing this?
r? `@ghost` for now
Take into account negative impls in "trait item not found" suggestions
This removes the suggestion to implement a trait for a type when that type already has a negative implementation for the trait, and replaces it with a note to point out that the trait is explicitely unimplemented, as suggested by `@scottmcm.`
Helps with #79683.
r? `@scottmcm` do you want to review this?
See
https://github.com/rust-lang/rust/issues/80045#issuecomment-745733339
Coverage statements are moved to the beginning of the BCB. This does
also affect what's counted before a panic, changing some results, but I
think these results may even be preferred? In any case, there are no
guarantees about what's counted when a panic occurs (by design).
Fixes reported bugs in Rust Coverage
Fixes: #79569Fixes: #79566Fixes: #79565
For the first issue (#79569), I got hit a `debug_assert!()` before
encountering the reported error message (because I have `debug = true`
enabled in my config.toml).
The assertion showed me that some `SwitchInt`s can have more than one
target pointing to the same `BasicBlock`.
I had thought that was invalid, but since it seems to be possible, I'm
allowing this now.
I added a new test for this.
----
In the last two cases above, both tests (intentionally) fail to compile,
but the `InstrumentCoverage` pass is invoked anyway.
The MIR starts with an `Unreachable` `BasicBlock`, which I hadn't
encountered before. (I had assumed the `InstrumentCoverage` pass
would only be invoked with MIRs from successful compilations.)
I don't have test infrastructure set up to test coverage on files that
fail to compile, so I didn't add a new test.
r? `@tmandry`
FYI: `@wesleywiser`
consider assignments of union field of ManuallyDrop type safe
Assigning to `Copy` union fields is safe because that assignment will never drop anything. However, with https://github.com/rust-lang/rust/pull/77547, unions may also have `ManuallyDrop` fields, and their assignments are currently still unsafe. That seems unnecessary though, as assigning `ManuallyDrop` does not drop anything either, and is thus safe even for union fields.
I assume this will at least require FCP.
[mir-opt] Allow debuginfo to be generated for a constant or a Place
Prior to this commit, debuginfo was always generated by mapping a name
to a Place. This has the side-effect that `SimplifyLocals` cannot remove
locals that are only used for debuginfo because their other uses have
been const-propagated.
To allow these locals to be removed, we now allow debuginfo to point to
a constant value. The `ConstProp` pass detects when debuginfo points to
a local with a known constant value and replaces it with the value. This
allows the later `SimplifyLocals` pass to remove the local.
Move intrinsics lowering pass from the optimization phase (where it
would not run if -Zmir-opt-level=0), to the drop lowering phase where it
runs unconditionally.
The implementation of those intrinsics in code generation and
interpreter is unnecessary. Remove it.
Properly capture trailing 'unglued' token
If we try to capture the `Vec<u8>` in `Option<Vec<u8>>`, we'll
need to capture a `>` token which was 'unglued' from a `>>` token.
The processing of unglueing a token for parsing purposes bypasses the
usual capturing infrastructure, so we currently lose the trailing `>`.
As a result, we fall back to the reparsed `TokenStream`, causing us to
lose spans.
This commit makes token capturing keep track of a trailing 'unglued'
token. Note that we don't need to care about unglueing except at the end
of the captured tokens - if we capture both the first and second unglued
tokens, then we'll end up capturing the full 'glued' token, which
already works correctly.
Recover on `const impl<> X for Y`
`@leonardo-m` mentioned that `const impl Foo for Bar` could be recovered from in #79287.
I'm not sure about the error strings as they are, I think it should probably be something like the error that `expected_one_of_not_found` makes + the suggestion to flip the keywords, but I'm not sure how exactly to do that. Also, I decided not to try to handle `const unsafe impl` or `unsafe const impl` cause I figured that `unsafe impl const` would be pretty rare anyway (if it's even valid?), and it wouldn't be worth making the code more messy.
Resolve enum field visibility correctly
Fixes#79593. 🎉
Previously, this code treated enum fields' visibility as if they were
struct fields. However, that's not correct because the visibility of a
struct field with `ast::VisibilityKind::Inherited` is private to the
module it's defined in, whereas the visibility of an *enum* field with
`ast::VisibilityKind::Inherited` is the visibility of the enum it
belongs to.
Previously, this code treated enum fields' visibility as if they were
struct fields. However, that's not correct because the visibility of a
struct field with `ast::VisibilityKind::Inherited` is private to the
module it's defined in, whereas the visibility of an *enum* field with
`ast::VisibilityKind::Inherited` is the visibility of the enum it
belongs to.
If we try to capture the `Vec<u8>` in `Option<Vec<u8>>`, we'll
need to capture a `>` token which was 'unglued' from a `>>` token.
The processing of unglueing a token for parsing purposes bypasses the
usual capturing infrastructure, so we currently lose the trailing `>`.
As a result, we fall back to the reparsed `TokenStream`, causing us to
lose spans.
This commit makes token capturing keep track of a trailing 'unglued'
token. Note that we don't need to care about unglueing except at the end
of the captured tokens - if we capture both the first and second unglued
tokens, then we'll end up capturing the full 'glued' token, which
already works correctly.
Fixes: #79569Fixes: #79566Fixes: #79565
For the first issue (#79569), I got hit a `debug_assert!()` before
encountering the reported error message (because I have `debug = true`
enabled in my config.toml).
The assertion showed me that some `SwitchInt`s can have more than one
target pointing to the same `BasicBlock`.
I had thought that was invalid, but since it seems to be possible, I'm
allowing this now.
I added a new test for this.
----
In the last two cases above, both tests (intentionally) fail to compile,
but the `InstrumentCoverage` pass is invoked anyway.
The MIR starts with an `Unreachable` `BasicBlock`, which I hadn't
encountered before. (I had assumed the `InstrumentCoverage` pass
would only be invoked with MIRs from successful compilations.)
I don't have test infrastructure set up to test coverage on files that
fail to compile, so I didn't add a new test.
Capture precise paths in THIR and MIR
This PR allows THIR and MIR to use the result of the new capture analysis to actually capture precise paths
To achieve we:
- Writeback min capture results to TypeckResults
- Move handling upvars to PlaceBuilder in mir_build
- Lower precise paths in THIR build by reading min_captures
- Search for ancestors in min_capture when trying to build a MIR place which starts off of an upvar
Closes: https://github.com/rust-lang/project-rfc-2229/issues/10
Partly implements: rust-lang/project-rfc-2229#18
Work that remains (not in this PR):
- [ ] [Known bugs when feature gate is enabled](https://github.com/rust-lang/project-rfc-2229/projects/1?card_filter_query=label%3Abug)
- [ ] Use min_capure_map for
- [ ] Liveness analysis
- [ ] rustc_mir/interpret/validity.rs
- [ ] regionck
- [ ] rust-lang/project-rfc-2229#8
- [ ] remove closure_captures and upvar_capture_map
r? `@ghost`
Apply `doc(cfg)` from parent items while collecting trait impls
Because trait impls bypass the standard `clean` hierarchy they do not participate in the `propagate_doc_cfg` pass, so instead we need to pre-collect all possible `doc(cfg)` attributes that will apply to them when cleaning.
fixes#79201
CTFE: tweak abort-on-uninhabited message
Having an "aborted execution:" makes it more consistent with the `Abort` terminator saying "the program aborted execution". Right now, at least one of the two errors will look weird in Miri.
r? `@oli-obk`
Use `def_path_hash_to_def_id` when re-using a `RawDefId`
Fixes#79890
Previously, we just copied a `RawDefId` from the 'old' map to the 'new'
map. However, the `RawDefId` for a given `DefPathHash` may be different
in the current compilation session. Using `def_path_hash_to_def_id`
ensures that the `RawDefId` we use is valid in the current session.
Clarify the 'default is only allowed on...' error
Code like
impl Foo {
default fn foo() {}
}
will trigger the error
error: `default` is only allowed on items in `impl` definitions
--> src/lib.rs:5:5
|
5 | default fn foo() {}
| -------^^^^^^^^^
| |
| `default` because of this
but that's very confusing! I *did* put it on an item in an impl!
So this commit changes the message to
error: `default` is only allowed on items in trait impls
--> src/lib.rs:5:5
|
5 | default fn foo() {}
| -------^^^^^^^^^
| |
| `default` because of this
Fixes#79890
Previously, we just copied a `RawDefId` from the 'old' map to the 'new'
map. However, the `RawDefId` for a given `DefPathHash` may be different
in the current compilation session. Using `def_path_hash_to_def_id`
ensures that the `RawDefId` we use is valid in the current session.
