search

uazu / qcell

Statically-checked alternatives to RefCell and RwLock
Apache License 2.0
356 stars 22 forks source link

Enable generativity feature for `LCellOwner` #32

Closed CeleritasCelery closed 2 years ago

CeleritasCelery commented 2 years ago

Closes #31

I was able to avoid the issue of conflicting implementation of 'id by just reusing the lifetime in PhantomData. Let me know what you think of how I exported generativity. I added it as a namespace under qcell.

Also fixed a clippy lint.

CeleritasCelery commented 2 years ago

I updated based on feedback and renamed the feature to lcell_macro.

uazu commented 2 years ago

This looks fine, thanks -- and thanks to @geeklint for reviewing before I got to it. Re-exporting generativity is okay, to allow a caller to use exactly the same version that qcell is using (although it seems pretty stable).

Effectively you're just importing the lifetime from generativity's Guard and using qcell's implementation of Id, which should be fine.

I will add new doctests to test this more thoroughly just now.

uazu commented 2 years ago

I have doctests working, and everything seems okay. The error messages with generativity's approach if you misuse an owner are way more confusing (e.g. if you confuse which owner is for which cell). It says that "tag is dropped while still borrowed" (which is not really true), and that the tag does not live long enough -- and that's all. With the scope version, you can see that it is thrashing about trying to resolve some impossible lifetime situation, explaining that in depth, and it even points to the line causing the problem. So I guess generativity is slightly less beginner-friendly in terms of compiler errors.

uazu commented 2 years ago

It's published (version 0.5.1)