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.

Sign up for GitHub

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

don't rely on rustc bug #27

Merged
merged 2 commits into from
Aug 1, 2022
Merged

don't rely on rustc bug #27

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
918794c
don't rely on rustc bug
aliemjay Jul 29, 2022
dcf31c6
link to the compiler issue
aliemjay Jul 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ async fn get_frames(
}
}

async fn get_images<'a>(
async fn get_images(
frames: &Option<Frames>,
client: &Client,
file_id: &str,
Expand Down Expand Up @@ -304,7 +304,11 @@ async fn get_images<'a>(

let mut futures = vec![];

let mut add_future_images = |image_ids: &'a str, format, scale| {
let mut add_future_images = |image_ids, format, scale| {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally, the required change would be to only remove the lifetime annotation:

-       let mut add_future_images = |image_ids: &'a str, format, scale| {
+      let mut add_future_images = |image_ids: &str, format, scale| {

but this unfortunately exposes another rust bug that requires such a hacky solution. See rust-lang/rust#58052 rust-lang/rust#70263

Copy link
Owner

Choose a reason for hiding this comment

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

@aliemjay thank you for this! could you add the explanation in the comments in line 308 or probably the link to this PR so it's clearer for people reading the code and easier to understand the motivations that led to this particular hack?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

created a new issue rust-lang/rust#100002

Copy link
Owner

Choose a reason for hiding this comment

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

That’s great! Thank you very much!

// hack: annotate type inside the closure to avoid rustc bug
// https://github.com/rust-lang/rust/issues/100002
let image_ids: &str = image_ids;

if !image_ids.is_empty() {
futures.push(
get_images_url_collection(image_ids, client, file_id, scale, format)
Expand Down