Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Appease clippy's new rules #1023

Merged
merged 3 commits into from
Dec 2, 2024

Conversation

micahscopes
Copy link

@micahscopes micahscopes commented Nov 30, 2024

What was wrong?

Clippy has been complaining about unnecessary explicit lifetimes. In order to please clippy I found that I had to fix quite a few other things as well.

I'm not sure that they're actually unnecessary though: rust-lang/rust-clippy#12908

I'm opening this PR to find out.

How was it fixed?

clippy --fix ...

@micahscopes micahscopes force-pushed the fix-lifetime-elision-lint branch 2 times, most recently from 8ceb15d to d6c0f71 Compare November 30, 2024 05:51
@micahscopes micahscopes changed the title Attempt to please clippy by removing "needless lifetimes" Appease clippy's new rules Nov 30, 2024
@@ -306,58 +358,6 @@ impl<'db> TyCheckerFinalizer<'db> {
}

fn check_unknown_types(&mut self) {
impl<'db> Visitor<'db> for TyCheckerFinalizer<'db> {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clippy didn't like this trait being implemented inside of this function.

) {
(reason, output)
} else {
panic!("EVM trap during test")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clippy complained that this else block never gets reached and that the surrounding if let was unnecessary

@@ -110,6 +110,7 @@ impl<'db> ModuleTree<'db> {
/// top level modules. This function only depends on an ingot structure and
/// external ingot dependency, and not depends on file contents.
#[salsa::tracked(return_ref)]
#[allow(elided_named_lifetimes)]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clippy and the rust compiler were battling back and forth, on the one hand Clippy would complain about needless_lifetimes and change it to the elided version, but then the rust compiler would complain about elided_named_lifetimes. So I added this allow rule here and one other place.

@@ -117,6 +117,7 @@ impl<'db> ModuleAnalysisPass<'db> for ParsingPass<'db> {
// The reason why this function is not a public API is that we want to prohibit users of `HirDb` to
// access `InputIngot` directly.
#[salsa::tracked(return_ref)]
#[allow(elided_named_lifetimes)]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another place where clippy was battling the compiler

Copy link
Collaborator

@sbillig sbillig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

@sbillig sbillig merged commit 6767314 into ethereum:fe-v2 Dec 2, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants