-
Notifications
You must be signed in to change notification settings - Fork 316
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
Bump rust-toolchain to 1.59.0 #1607
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few things that I think are not fully correct.
.unwrap_or_else(|err| panic_any(err)); | ||
|
||
let cache_dir_path = format!("{:?}", self.cache_dir.path()); | ||
let session = match spawn_bash_with_retries(10, Some(self.session_timeout_ms)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same comment as above applies here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Refactored in in 9af5c14
@@ -4,44 +4,41 @@ use std::slice::{self, ChunksExactMut}; | |||
/// A slice type which can be shared between threads, but must be fully managed by the caller. | |||
/// Any synchronization must be ensured by the caller, which is why all access is `unsafe`. | |||
#[derive(Debug)] | |||
pub struct UnsafeSlice<'a, T> { | |||
// holds the data to ensure lifetime correctness | |||
data: UnsafeCell<&'a mut [T]>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure about this one, but this doesn't seem right. As the comment suggests, I think we still want to have a reference to the data for correctness purpose. Hence I would just rename it to _data
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, keeping this lifetime is important, to make sure it is tracked properly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Returned lifetime and renamed data
to _data
in 9af5c14. Could You please elaborate more on proper tracking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that if we just use the raw pointer the compiler won't be able to track the lifetime for us. if we keep it in here we get the tracking for it.
7c6c74a
to
0f535fb
Compare
This PR fixes #1605. As it is stated in its title it bumps toolchain to 1.59.0 as
async-global-executor v2.1.0
which is one of dependencies now requires mentioned version of rust compiler.One can be noticed that
cargo clippy
becomes more aggressive and for the new version ofrustc
it treats many of previous warnings as errors, so I had to fix all that issues in order to achieve green CI mark. More specifically, I had to