search

BlockstreamResearch / rust-simplicity

Creative Commons Zero v1.0 Universal
58 stars 12 forks source link

types: introduce `TypeInner` making `Type` the only public "incomplete Simplicity type" type #233

Closed apoelstra closed 3 months ago

apoelstra commented 3 months ago

Previously we had two types, Type and Bound, which were mutually recursive. Type was also exposed in the public API as a type representing an incomplete Simplicity type. Bound was exposed in the public API for a couple of reasons which this PR eliminates.

However, because Type is both public and unusable without a live type inference context, this means that it needs to contain an Arc<Context>. But this means that if we have a self-referential type, this leaks to a somewhat-convoluted reference cycle in which a Type holds an Arc<Context> which owns a set of Bounds which contain the original Type. If you are skeptical of this, try running the test harness in valgrind before and after the last commit of this PR -- you will see that previously we were leaking memory in the memory_leak unit test but now we are not.

This completes the "type inference overhaul" needed to eliminate the memory leak in infinitely-sized types.

apoelstra commented 3 months ago

Addressed the two actionable comments. Should be good to go.