Fix LTO setting and a clippy::use_self finding #1973
Merged
Conversation
This file contains 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
Per upstream docs[0] and a warning in my IDE, the `lto` setting's valid options are: false, true, "fat", "thin", or "off". This commit changes our "yes" value to true.
```
warning: unnecessary structure name repetition
--> rustls/src/msgs/handshake.rs:1385:52
|
1385 | pub(crate) fn new(cert: CertificateDer<'a>) -> CertificateEntry<'a> {
| ^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#use_self
note: the lint level is defined here
--> rustls/src/lib.rs:310:5
|
310 | clippy::use_self,
| ^^^^^^^^^^^^^^^^
```
ctz
approved these changes
May 28, 2024
| @@ -25,4 25,4 @@ resolver = "2" | |||
|
|
|||
| [profile.bench] | |||
| codegen-units = 1 | |||
| lto = "yes" | |||
| lto = true | |||
There was a problem hiding this comment.
lol, i got "yes" from https://doc.rust-lang.org/rustc/codegen-options/index.html#lto which is actually documenting the rustc CLI :lolsob:
Benchmark resultsInstruction countsSignificant differencesClick to expand
Other differencesClick to expand
Wall-timeSignificant differencesThere are no significant wall-time differences Other differencesClick to expand
Additional informationCheckout details:
|
So it looks like there's a couple things going on with nightly clippy. Looking at a recent main job: a) we're throwing a lot of "rustc-check-cfg" warnings (also seeing these locally). I think my "fix" from #1942 was sufficient to fix stable, but isn't appeasing nightly. If I had to guess, I'd say it's related to the workaround I mentioned:
b) there are a lot of warnings that aren't failing the build, including the one fixed in this branch. It looks like we use I would be in favour of fixing B, but it will be blocked on figuring out A. I can take a crack at both in a little while after more pressing work. |
@djc This feels like something you are likely to have thoughts on :-) |
|
Yes, that was intentional. The stable ones only change every six weeks but nightly clippy can change everyday and can contain regressions (like false positives) that will get fixed before release. So getting warnings from nightly clippy feels like the right trade-off to me. |
|
That's fair, I just think they're hard to notice in practice unless you search them out. That's probably still the right trade-off. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
cargo: fix bench profile LTO setting
Per upstream docs and a warning in my IDE, the
ltosetting's valid options are: false, true, "fat", "thin", or "off". This commit changes our "yes" value to true.msgs: fix clippy::use_self finding
I'm not sure yet why CI didn't flag this, but I see it locally: