Make traits / trait methods detected by the dead code lint#118257
Merged
bors merged 2 commits intorust-lang:masterfrom Feb 7, 2024
Merged
Make traits / trait methods detected by the dead code lint#118257bors merged 2 commits intorust-lang:masterfrom
bors merged 2 commits intorust-lang:masterfrom
Conversation
Collaborator
|
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
This comment was marked as outdated.
This comment was marked as outdated.
Member
Does it manly just affect tests? If that's the case we can just put In any case could you post (via pastebin or something else, since GitHub will probably limit you) the full output of the new warnings. So we can get a sense of scale of new warnings. |
cjgillot
reviewed
Nov 25, 2023
Contributor
Ui tests allow unused code lint by default. So there is nothing to do on most. |
Member
In #92790 / #95977, I also caused a lot of new warnings to show up :-) There we started the lint as allow-by-default (and appear to still treat it as such... I should look into that). Perhaps we start the same way here? |
This comment has been minimized.
This comment has been minimized.
9fc34e3 to
f10c6a8
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
6bdbc3a to
a6f59a5
Compare
Collaborator
|
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor Some changes might have occurred in exhaustiveness checking cc @Nadrieril |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
254bb8a to
412c984
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
|
Could you extract the cleanups you do to the rest of the compiler to a standalone PR? |
cjgillot
reviewed
Nov 25, 2023
tests/codegen-units/item-collection/instantiation-through-vtable.rs
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
Merged
6cf287f to
d609000
Compare
This comment has been minimized.
This comment has been minimized.
Member
Collaborator
Collaborator
Collaborator
|
💔 Test failed - checks-actions |
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Contributor
Author
|
This failure looks like an infra error. Same with #120238 (comment) And I re-tried it and it succeeded, https://github.com/rust-lang/rust/actions/runs/7792380624/job/21250255712?pr=118257 |
Member
|
@bors retry |
Collaborator
|
☔ The latest upstream changes (presumably #120722) made this pull request unmergeable. Please resolve the merge conflicts. |
Member
Collaborator
Collaborator
Collaborator
|
☀️ Test successful - checks-actions |
Collaborator
|
Finished benchmarking commit (d4f6f9e): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis 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.
CyclesResultsThis 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.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 663.003s -> 664.334s (0.20%) |
Member
|
High-level check on the perf results...
|
Contributor
|
Regression: #120770. |
This was referenced Feb 11, 2024
Member
|
The compiler now does more work, so some small regressions are expected. The large regression is from initializing diagnostics (there were no warning produced before for that stress test), so that is fine, as it's a one-time cost. @rustbot label: +perf-regression-triaged |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
15 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 #118139 and #41883