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

Add closing delimiter that was missing in rust generated code #249

Conversation

muralisoundararajan
Copy link

Why this change is required?

When we try adding index range in flatdata like below example and generate the rust code, the generated rust code is invalid.

struct Node {
    @range(edges_range)
    @optional( NO_EDGES_REF )
    first_edge_ref : u32;
}

When we use the generated code, compilation fails with below reason:

    |
483 |     pub fn edges_range(&self) -> std::ops::Range<Option<u32>> {
    |                                                               - closing delimiter possibly meant for this
...
486 |         let check = |x| Some(x).filter(|&x| x != super::test::NO_EDGES_REF;
    |                                       ^ unclosed delimiter
487 |         check(start)..check(end)
488 |     }
    |     ^ mismatched closing delimiter

What this change does?

  • Add closing delimiter to jinja template

@VeaaC
Copy link
Collaborator

VeaaC commented Nov 14, 2024

Thanks for finding this. It looks like this combination of attributes was never tested.

Conceptually it should not work at all, though: @range is combining values from the current index and the next one. If any one is optional things break.

E.g. imagine 5 entries for Node (5th one being a sentinel)

0, 2, NO_EDGE_REF, 5, 10

That means the 4 nodes have the ranges:

0..2, 2..NO_EDGE_REF, NO_EDGE_REF..5, 5..10

The correct thing to do in this case would be to store identical start/end values:

0, 2, 2, 5, 10

2..2 already encodes an empty range, no need for @optional

We should add an error handler for this combination, though, and give the user a proper error message.

@muralisoundararajan
Copy link
Author

muralisoundararajan commented Nov 14, 2024

@VeaaC agreed encoding identical start/end values seems better approach in this case.

For the flatdata generator, it would be better to fail in the validation itself. I would close this pull request

@VeaaC
Copy link
Collaborator

VeaaC commented Nov 14, 2024

See #250

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