Skip to content

Fix sync lock let binding#152

Closed
cyqsimon wants to merge 1 commit intothecoshman:masterfrom
cyqsimon:fix-build
Closed

Fix sync lock let binding#152
cyqsimon wants to merge 1 commit intothecoshman:masterfrom
cyqsimon:fix-build

Conversation

@cyqsimon
Copy link
Copy Markdown
Contributor

Current master is rejected by rust 1.68.0:

    Checking https v1.12.5 (/foo/bar/http)
error: non-binding let on a synchronization lock
   --> src/main.rs:185:9
    |
185 |     let _ = end_handler.wait(mx.lock().unwrap()).unwrap();
    |         ^   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this binding will immediately drop the value assigned to it
    |         |
    |         this lock is not assigned to a binding and is immediately dropped
    |
    = note: `#[deny(let_underscore_lock)]` on by default
help: consider binding to an unused variable to avoid immediately dropping the value
    |
185 |     let _unused = end_handler.wait(mx.lock().unwrap()).unwrap();
    |         ~~~~~~~
help: consider immediately dropping the value
    |
185 |     drop(end_handler.wait(mx.lock().unwrap()).unwrap());
    |     ~~~~~                                             +

error: could not compile `https` due to previous error

This is a simple patch for it.

@nabijaczleweli
Copy link
Copy Markdown
Collaborator

Duplicate of #151.

@nabijaczleweli
Copy link
Copy Markdown
Collaborator

Still, why the hell is this a New Lint that refuses old, fully correct code?

@cyqsimon
Copy link
Copy Markdown
Contributor Author

cyqsimon commented Mar 21, 2023

Ah okay sorry. I only checked latest tag not master develop branch.

Still, why the hell is this a New Lint that refuses old, fully correct code?

I guess this lint makes more sense in the more general case when the internal type isn't (). Although I do agree that they should have made it a warning instead of a hard error.

@cyqsimon cyqsimon deleted the fix-build branch March 21, 2023 11:33
@nabijaczleweli
Copy link
Copy Markdown
Collaborator

Fix released in v1.12.6.

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