Warn about unused `pub` fields in non-`pub` structs
This pull request fixes#85255. The current implementation of dead code analysis is too prudent because it marks all `pub` fields of structs as live, even though they cannot be accessed from outside of the current crate if the struct itself only has restricted or private visibility.
I have changed this behavior to take the containing struct's visibility into account when looking at field visibility and liveness. This also makes dead code warnings more consistent; consider the example given in #85255:
```rust
struct Foo {
a: i32,
pub b: i32,
}
struct Bar;
impl Bar {
fn a(&self) -> i32 { 5 }
pub fn b(&self) -> i32 { 6 }
}
fn main() {
let _ = Foo { a: 1, b: 2 };
let _ = Bar;
}
```
Current nightly already warns about `Bar::b()`, even though it is `pub` (but `Bar` is not). It should therefore also warn about `Foo::b`, which it does with the changes in this PR.
Generate not more docs than necessary
This is something that `@Nemo157` was talking about: they wanted that when using `x.py doc std`, it only generated `std` (and the crates "before" it).
r? `@Mark-Simulacrum`
Convert rustfmt from a submodule to a subtree
r? `@calebcartwright` cc `@Manishearth` `@Mark-Simulacrum`
The motivation is that submodule updates cause rustfmt to not be available on nightly a lot; most recently it was unavailable for over 10 days, causing the beta release to be delayed. Additionally this is much less work on the part of the rustfmt maintainers to keep the rustfmt compiling, since now people making breaking changes will be responsible for fixing them.
I kept the rustfmt git history so it looks like there are thousands of commits. The important commits are 851dee3af9~..pull/82208/head. This adds about 10 MB of git history, which is not terribly much compared to the 702 MB that already exist.
- Add `src/tools/rustfmt` to `x.py check`
- Fix CRLF issues with rustfmt tests (see commit for details)
- Use `rustc_private` instead of crates.io dependencies
This was already switched upstream and would have landed in the next submodule bump anyway. This just updates Cargo.lock for rust-lang/rust.
- Add `yansi-term` to the list of allowed dependencies.
This is a false positive - rustc doesn't actually use it, only rustfmt, but because it's activated by the cargo feature of a dependency, tidy gets confused. It's fairly innocuous in any case, it's used for color printing.
This would have happened in the next submodule bump.
- Remove rustfmt from the list of toolstate tools.
- Give a hard error if testing or building rustfmt fails.
- Update log to 0.4.14
This avoids a warning about semicolons in macros; see the commit for details.
- Don't add tools to the sysroot when they finish building.
This is the only change that could be considered a regression - this avoids a "colliding StableCrateId" error due to a bug in resolve (https://github.com/rust-lang/rust/issues/56935). The regression is that this rebuilds dependencies more often than strictly necessary. See the commit for details.
Fixes https://github.com/rust-lang/rust/issues/85226 (permanently). Closes https://github.com/rust-lang/rust/issues/82385. Helps with https://github.com/rust-lang/rust/issues/70651. Helps with https://github.com/rust-lang/rust/issues/80639.
swap function order for better read flow
I was reading this error message for the first time.
I was a little bit confused when reading that part:
```
foo.bar(); // we can now use this method since i32 implements the Foo trait
```
At the time I was reading `// we can now use this method` I wasn't sure why. It only made sense when reading on. So swapping these parts results in a better read flow.
coverage bug fixes and some refactoring
This replaces the relevant commits (2 and 3) from PR #85082, and also corrects an error querying for coverageinfo.
1. `coverageinfo` query needs to use the same MIR as codegen
I ran into an error trying to fix dead block coverage and realized the
`coverageinfo` query is getting a different MIR compared to the
codegenned MIR, which can sometimes be a problem during mapgen.
I changed that query to use the `InstandeDef` (which includes the
generic parameter substitutions, prosibly specific to const params)
instead of the `DefId` (without unknown/default const substitutions).
2. Simplified body_span and filtered span code
Some code cleanup extracted from future (but unfinished) commit to fix
coverage in attr macro functions.
3. Spanview needs the relevant body_span used for coverage
The coverage body_span doesn't always match the function body_span.
r? ```@tmandry```
Minimize amount of fake `DefId`s used in rustdoc
Follow up from #84707, which minimizes the amount of fake defids to the smallest amount possible. Every `FakeDefId` that is now used in the rustdoc library must be preserved and can not be replaced with a normal `DefId`.
Insignificant destructors rfc 2229
- Adds new attribute `rustc_insignificant_dtor` to annotate the drop method.
- Adds a query to check if a type has a significant drop.
- Updates closure analysis to check for significant drops rather than just drop.
A type marked with the attribute `rustc_insignificant_dtor` is considered to not be significant. A drop is significant if it is implemented by the user or does anything that will have any observable behavior (other than freeing up memory).
https://github.com/rust-lang/project-rfc-2229/issues/35
Box `Impl.blanket_impl` to reduce size
Blanket impls are probably not super common, and `Type` is a fairly
large type, so this should reduce `Impl`'s size by a lot: `Option<Type>`
is around 96 bytes, and `Option<Box<Type>>` is 8 bytes, so it's a big
difference!
Error out if a PR is sent to the wrong channel
It happened multiple times that a PR meant to go on beta ends up being opened (and occasionally merged) to master. This PR does two things:
* Moves the definition of the channel in `src/ci/channel` so it's easier for tools to read it. I was not sure whether to move it to `src/channel` (like `src/version`): ended up with `src/ci` as it's currently only used for CI, but I'm open to moving it to `src`. We'll need to update the release process after this.
* Adds a check on **non-bors** builds that errors out if the base branch is not the expected one for the currently defined channel. This will not cause problems for promotion PRs, as those PRs are meant to also update the channel name.
r? `@Mark-Simulacrum`
This avoids the following warning:
```
warning: trailing semicolon in macro used in expression position
--> /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/log-0.4.11/src/macros.rs:152:45
|
147 | / macro_rules! debug {
148 | | (target: $target:expr, $($arg:tt)+) => (
149 | | log!(target: $target, $crate::Level::Debug, $($arg)+);
150 | | );
151 | | ($($arg:tt)+) => (
152 | | log!($crate::Level::Debug, $($arg)+);
| | ^
153 | | )
154 | | }
| |_- in this expansion of `debug!`
|
::: src/tools/rustfmt/src/modules/visitor.rs:36:23
|
36 | Err(e) => debug!("{}", e),
| --------------- in this macro invocation
|
= note: requested on the command line with `-W semicolon-in-expressions-from-macros`
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #79813 <https://github.com/rust-lang/rust/issues/79813>
```
This fixes the following error:
```
error: found crates (`serde_derive` and `serde_derive`) with colliding StableCrateId values.
--> /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/cargo_metadata-0.8.2/src/lib.rs:162:1
|
162 | extern crate serde_derive;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
```
This is a bug in resolve (https://github.com/rust-lang/rust/issues/56935)
but it will be difficult to fix in the near future. This works around it
in the meantime by not copying serde_derive and other dependencies to
the sysroot when they're built for other tools. This rebuilds the
dependencies slightly more often than necessary, but avoids the crate
conflicts.
This can be reverted once #56935 is fixed.
- Add rustfmt to `x.py check`
- Update Cargo.lock
- Remove rustfmt from the toolstate list
- Make rustfmt an in-tree tool
- Give an error on `x.py test rustfmt` if rustfmt fails to build or if tests fail
- Don't call `save_toolstate` when testing rustfmt
Blanket impls are probably not super common, and `Type` is a fairly
large type, so this should reduce `Impl`'s size by a lot: `Option<Type>`
is around 96 bytes, and `Option<Box<Type>>` is 8 bytes, so it's a big
difference!
Preserve `SyntaxContext` for invalid/dummy spans in crate metadata
Fixes#85197
We already preserved the `SyntaxContext` for invalid/dummy spans in the
incremental cache, but we weren't doing the same for crate metadata.
If an invalid (lo/hi from different files) span is written to the
incremental cache, we will decode it with a 'dummy' location, but keep
the original `SyntaxContext`. Since the crate metadata encoder was only
checking for `DUMMY_SP` (dummy location + root `SyntaxContext`),
the metadata encoder would treat it as a normal span, encoding the
`SyntaxContext`. As a result, the final span encoded to the metadata
would change across sessions, even if the crate itself was unchanged.
This could lead to an 'unstable fingerprint' ICE under the following conditions:
1. We compile a crate with an invalid span using incremental compilation. The metadata encoder discards the `SyntaxContext` since the span is invalid, while the incremental cache encoder preserves the `SyntaxContext`
2. From another crate, we execute a foreign query, decoding the invalid span from the metadata as `DUMMY_SP` (e.g. with `SyntaxContext::root()`). This span gets hashed into the query fingerprint. So far, this has always happened through the `optimized_mir` query.
3. We recompile the first crate using our populated incremental cache, without changing anything. We load the (previously) invalid span from our incremental cache - it gets converted to a span with a dummy (but valid) location, along with the original `SyntaxContext`. This span gets written out to the crate metadata - since it now has a valid location, we preserve its `SyntaxContext`.
4. We recompile the second crate, again using a populated incremental cache. We now re-run the foreign query `optimized_mir` - the foreign crate hash is unchanged, but we end up decoding a different span (it now ha a non-root `SyntaxContext`). This results in the fingerprint changing, resulting in an ICE.
This PR updates our encoding of spans in the crate metadata to mirror
the encoding of spans into the incremental cache. We now always encode a
`SyntaxContext`, and encode location information for spans with a
non-dummy location.
