Lifetime added to type signature
Closed this issue · 8 comments
When updating from 1.0.1 to 1.0.4 I've had to change my code to add lifetimes:
kornelski/lodepng-rust@f7bc8b8
It's not a big deal (I have to fix a lot of stuff for almost every nightly Rust release), but it was unexpected and I haven't found documentation for it.
Hum... I see your point. You were forced to look at commits history. Not very convenient. What do you think would be best :
- I add some kind of history in the README file which explains quickly what changed
- I just change the major version (from 1.0.x to 2.0.0 for example)
?
I think the point is that the purpose of the lifetime parameter isn't documented.
Yes, a note in the readme or release information would be great (not sure where releases should be documented with Cargo — creates.io doens't have a good place for it). Something like "lifetime added to CVec, add 'static if you're upgrading from previous version."
And Cargo uses semver, so major version bump would be appropriate here.
@gkoz: I don't think it's relevant to say that the Thunk struct now needs a lifetime parameter. I think what's relevant would be to just say that now CVec does need a lifetime parameter.
@pornel: There is nothing like that unfortunalety, except putting a warning at compile time, but that's not an acceptable solution for me. For the version, I don't really like this solution since I only (or nearly) update c_vec when there is a breaking change in rust. It would be the 4.0.0 or 5.0.0 version. There is too many breaking changes to accept a solution like that I think.
@GuillaumeGomez
I'd expect Thunk to just disappear from libstd one day. It's not marked as stable and not documented and it's like 6 lines of code. CVec hardly needs it and the F: Send bound doesn't make sense to me in the context of CVec. Thunk might as well be Box.
Would you like a PR that would drop Thunk and let the destructor closure take the pointer as an argument so it wouldn't have to capture it from somewhere pointlessly? It wouldn't make the lifetime parameter go away though :)
@gkoz: That's what I was expecting too but it's still here... I didn't have time to take a deeper look at it but if you think you can make a PR to remove it (and the lifetime parameter at the same time), it would be awesome !
Alright. As for the lifetime parameter it does seem out of place for the destructor to have captured some borrowed references. So unless there's a use case I'd prefer to lose the lifetime parameter too.
This issue is resolved with Unique I guess. I close it. If there's anything else, I'll reopen it.