Change unused_labels from allow to warn #66325
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @varkor (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive added the S-waiting-on-review 
| I personally think a crater run is unnecessary here; unused labels do not seem like a widespread problem that would cause compilation failures to trigger due to |
This comment has been minimized.
This comment has been minimized.
| I already fixed that. :) |
| The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
| There you have it. Your lint triggered ^^ rust/src/librustc/ty/layout.rs Line 2560 in e3d9984 |
| There's a second one at Can you please rebase your (three) commits and then force push them? |
| @hellow554 I seem to be current with rust-lang/rust/master. Did you want me to squash my commits? I maybe will abandon this if there's a third and try local builds until I have them all sorted. :-( |
Yes 😅 sorry.
I'm doing a local build right now, that's why I knew that there will be a second error. If you like you can wait, I will tell you either more places or that it's done |
| Yes, I'll finish this tomorrow, as it's bedtime. Thanks much for your help with this! |
| The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
| This is the last one: rust/src/librustc_codegen_ssa/mir/block.rs Line 709 in e3d9984 |
| Chose to split the label deletions from the default change. That way if it is decided not to change the default we'll have at least got rid of the unused labels. 🙂 |
| The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
| Sorry, I only did the build itself, not the tests :/ But should be easy for you to add |
| @BartMassey: it would be easier for you to run all the tests locally with |
| Working. Thanks! |
| OK, I've managed to reproduce the test failures. Is it better to just stick |
I don't have a strong preference (slightly towards removing unnecessary labels, as long as they don't seem to be the point of the test) — so whichever is more convenient. |
| OK, I have completed fixes for |
| @BartMassey the CI must be green in order to merge your PR, so everything must be fixed :) |
| @hellow554 Yes; was just wondering how much the CI checks? Rustdoc? Rustfmt? Clippy? Guess I'll try again and see where we are. Software development is hard. 😅 Thanks for all the help! |
| The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
| Sigh. There must be a better way. The rustdoc for |
| @BartMassey: you can run the entire test suite locally, which will mean you don't rely on the CI to find out if you've found all the erroneous labels. |
| Sorry, discussion of underscore was hidden by github, found the issue and PR! I'm fine with this. |
Ah woops; missed the FCP; @bors r- |
bors added S-waiting-on-author 
Centril added S-waiting-on-team 
| I agree with the analysis of the crater results. Due to cap-lints, we don't need to worry about reverse dependencies. We're allowed to add new warnings, and people using As soon as we have team consensus on this, I think we can merge it. |
| @joshtriplett The @withoutboats Thanks! Appreciate your looking at this. If you could add your vote to the FCP I think we will be good to go… If you think it's important for me to put back some of the labels I removed from the compiler with underscores, I can do that also: let me know. I left the ones in the documentation for the reason you mentioned: it's probably clearer that way. |
rfcbot added final-comment-period 
| 🔔 This is now entering its final comment period, as per the review above. 🔔 |
| @BartMassey Congratulations on your first change to rustc! |
rfcbot added finished-final-comment-period | The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. The RFC will be merged soon. |
| @bors r+ |
| 📌 Commit 34a45a5 has been approved by |
bors added S-waiting-on-bors Centril added a commit to Centril/rust that referenced this pull request Dec 7, 2019
Change unused_labels from allow to warn Fixes rust-lang#66324, making the unused_labels lint warn instead of allow by default. I'm told @rust-lang/lang will need to review this, and perhaps will want to do a crater run.
Centril added a commit to Centril/rust that referenced this pull request Dec 8, 2019
Change unused_labels from allow to warn Fixes rust-lang#66324, making the unused_labels lint warn instead of allow by default. I'm told @rust-lang/lang will need to review this, and perhaps will want to do a crater run.
bors added a commit that referenced this pull request Dec 8, 2019
Rollup of 5 pull requests Successful merges: - #66325 (Change unused_labels from allow to warn) - #66991 (Cleanup BodyCache) - #67101 (use `#[allow(unused_attributes)]` to paper over incr.comp problem) - #67114 (Make `ForeignItem` an alias of `Item`.) - #67129 (Fixes typo) Failed merges: - #66886 (Remove the borrow check::nll submodule) r? @ghost
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Feb 17, 2020
Version 1.41.0 (2020-01-30) =========================== Language -------- - [You can now pass type parameters to foreign items when implementing traits.][65879] E.g. You can now write `impl<T> From<Foo> for Vec<T> {}`. - [You can now arbitrarily nest receiver types in the `self` position.][64325] E.g. you can now write `fn foo(self: Box<Box<Self>>) {}`. Previously only `Self`, `&Self`, `&mut Self`, `Arc<Self>`, `Rc<Self>`, and `Box<Self>` were allowed. - [You can now use any valid identifier in a `format_args` macro.][66847] Previously identifiers starting with an underscore were not allowed. - [Visibility modifiers (e.g. `pub`) are now syntactically allowed on trait items and enum variants.][66183] These are still rejected semantically, but can be seen and parsed by procedural macros and conditional compilation. Compiler -------- - [Rustc will now warn if you have unused loop `'label`s.][66325] - [Removed support for the `i686-unknown-dragonfly` target.][67255] - [Added tier 3 support\* for the `riscv64gc-unknown-linux-gnu` target.][66661] - [You can now pass an arguments file passing the `@path` syntax to rustc.][66172] Note that the format differs somewhat from what is found in other tooling; please see [the documentation][argfile-docs] for more information. - [You can now provide `--extern` flag without a path, indicating that it is available from the search path or specified with an `-L` flag.][64882] \* Refer to Rust's [platform support page][forge-platform-support] for more information on Rust's tiered platform support. [argfile-docs]: https://doc.rust-lang.org/nightly/rustc/command-line-arguments.html#path-load-command-line-flags-from-a-path Libraries --------- - [The `core::panic` module is now stable.][66771] It was already stable through `std`. - [`NonZero*` numerics now implement `From<NonZero*>` if it's a smaller integer width.][66277] E.g. `NonZeroU16` now implements `From<NonZeroU8>`. - [`MaybeUninit<T>` now implements `fmt::Debug`.][65013] Stabilized APIs --------------- - [`Result::map_or`] - [`Result::map_or_else`] - [`std::rc::Weak::weak_count`] - [`std::rc::Weak::strong_count`] - [`std::sync::Weak::weak_count`] - [`std::sync::Weak::strong_count`] Cargo ----- - [Cargo will now document all the private items for binary crates by default.][cargo/7593] - [`cargo-install` will now reinstall the package if it detects that it is out of date.][cargo/7560] - [Cargo.lock now uses a more git friendly format that should help to reduce merge conflicts.][cargo/7579] - [You can now override specific dependencies's build settings][cargo/7591] E.g. `[profile.dev.overrides.image] opt-level = 2` sets the `image` crate's optimisation level to `2` for debug builds. You can also use `[profile.<profile>.build_overrides]` to override build scripts and their dependencies. Misc ---- - [You can now specify `edition` in documentation code blocks to compile the block for that edition.][66238] E.g. `edition2018` tells rustdoc that the code sample should be compiled the 2018 edition of Rust. - [You can now provide custom themes to rustdoc with `--theme`, and check the current theme with `--check-theme`.][54733] - [You can use `#[cfg(doc)]` to compile an item when building documentation.][61351] Compatibility Notes ------------------- - [As previously announced 1.41.0 will be the last tier 1 release for 32-bit Apple targets.][apple-32bit-drop] This means that the source code is still available to build, but the targets are no longer being tested and release binaries for those platforms will no longer be distributed by the Rust project. Please refer to the linked blog post for more information. [54733]: rust-lang/rust#54733 [61351]: rust-lang/rust#61351 [67255]: rust-lang/rust#67255 [66661]: rust-lang/rust#66661 [66771]: rust-lang/rust#66771 [66847]: rust-lang/rust#66847 [66238]: rust-lang/rust#66238 [66277]: rust-lang/rust#66277 [66325]: rust-lang/rust#66325 [66172]: rust-lang/rust#66172 [66183]: rust-lang/rust#66183 [65879]: rust-lang/rust#65879 [65013]: rust-lang/rust#65013 [64882]: rust-lang/rust#64882 [64325]: rust-lang/rust#64325 [cargo/7560]: rust-lang/cargo#7560 [cargo/7579]: rust-lang/cargo#7579 [cargo/7591]: rust-lang/cargo#7591 [cargo/7593]: rust-lang/cargo#7593 [`Result::map_or_else`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.map_or_else [`Result::map_or`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.map_or [`std::rc::Weak::weak_count`]: https://doc.rust-lang.org/std/rc/struct.Weak.html#method.weak_count [`std::rc::Weak::strong_count`]: https://doc.rust-lang.org/std/rc/struct.Weak.html#method.strong_count [`std::sync::Weak::weak_count`]: https://doc.rust-lang.org/std/sync/struct.Weak.html#method.weak_count [`std::sync::Weak::strong_count`]: https://doc.rust-lang.org/std/sync/struct.Weak.html#method.strong_count [apple-32bit-drop]: https://blog.rust-lang.org/2020/01/03/reducing-support-for-32-bit-apple-targets.html markan added a commit to habitat-sh/builder that referenced this pull request Aug 12, 2020
(replaced by compiler change rust-lang/rust#66325) Signed-off-by: Mark Anderson <mark@chef.io>
markan added a commit to habitat-sh/builder that referenced this pull request Aug 13, 2020
(replaced by compiler change rust-lang/rust#66325) Signed-off-by: Mark Anderson <mark@chef.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
12 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Fixes #66324, making the unused_labels lint warn instead of allow by default. I'm told @rust-lang/lang will need to review this, and perhaps will want to do a crater run.