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

Fix iterator lifetimes #201

Merged
merged 2 commits into from
Mar 24, 2022
Merged

Conversation

aleokdev
Copy link
Contributor

This PR fixes some small lifetime issues with moderately big implications. Currently, code like this fails:

use tiled::Object;

let spawnpoints: Vec<Object> = map
    .layers()
    .filter_map(|layer| match layer.layer_type() {
        tiled::LayerType::ObjectLayer(layer) => Some(layer),
        _ => None,
    })
    .flat_map(|layer| layer.objects())
    .filter(|object| object.obj_type == "spawn")
    .collect();

dbg!(spawnpoints);

The compiler complains with:

error[E0515]: cannot return reference to function parameter `layer`
  --> src/layers/object.rs:88:23
   |
17 |     .flat_map(|layer| layer.objects())
   |                       ^^^^^^^^^^^^^^^ returns a reference to data owned by the current function

error: aborting due to previous error

This is due to the declaration of impl ExactSizeIterator functions:

pub fn objects(&self) -> impl ExactSizeIterator<Item = Object>

Which, expanded, look like this:

pub fn objects<'s>(&'s self) -> impl ExactSizeIterator<Item = Object<'s>> + 's

Which limit the lifetime of the iterator and its items to the mapwrapper object. This is not required because the mapwrapper object is just a reference tuple, and so these functions should be like this:

pub fn objects(&self) -> impl ExactSizeIterator<Item = Object<'map>> + 'map

This PR fixes these issues and enforces the fix with some doctests.

@aleokdev aleokdev added the bug label Mar 24, 2022
@aleokdev aleokdev merged commit c5c2ee6 into mapeditor:master Mar 24, 2022
@aleokdev aleokdev deleted the bigger-lifetimes branch March 24, 2022 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants