Today we worked on finishing yesterday’s refactor (at least the most critical areas, like all the uses of unsafe I made throughout the development process.) This has lead into a more neatly organized public interface exposed by the core library, that does not divide responsibilities between associated functions and traits, but solely centralizes traversal of constants (and potential mutation) to the methods offered by the Visit and VisitMut traits.
To this extent, one of the most notable changes has been the removal of the BorrowedContainer::deprecate and BorrowedContainer::undeprecate methods, as the same functionality was already being implemented for a subset of the non-owning view through the afore mentioned traits. This meant implementing equivalent trait methods for the VisitMut trait, as well as cleaning up uses of it in the binary. Now there’s two new methods, VisitMut::deprecate and VisitMut::select_deprecate, in charge of deprecating either one of all constants in the BorrowedContainer (fitting the toggle action while in normal mode) or a subset of those symbols (in accordance with the toggling action while in select mode.) This is still pending testing, but I don’t think there should be any issues arising from this specific change, as the implementation is fundamentally the same, only now it’s all centralized under the traversal trait.
Beyond that, we cleaned up all uses of unsafe in the core library and in the binary except for the Spinner type, which is still pending and may be left for next week. This was actually motivated by another refactor of the effect_changes routine that attempted to shorten and further use tasks to parallelize its workflow. That refactor enforced a 'static lifetime on the items that were borrowed by the future the task drove, and that just wouldn’t do it with a mutable reference to Self as the method receiver. At first, I implemented a dirty little fix with some unsafe as I was sure I didn’t really need to enforce those lifetime bounds in this instance. But then I thought better and realized it would be best if I put in the time to change the overarching infrastructure around uses of ConstContainer in the binary to have it wrapped in an Arc pointer, and thus be capable of using self: Arc<Self> as the method receiver. Of course, this spiraled out into converting all inner types of the ConstContainer type into concurrency-friendly types that I could mutate through a pattern of interior mutability, such that I never required a mutable receiver of type ConstContainer in its interface. Up to that point, the filter method had been needing that because the hashmap of cached regexes that the container keeps needed modification there. That ended up being replaced with a DashMap (to which I took the liberty of changing the default hasher to that exposed in the rustc-hash crate.)
Beyond that, the refactor also modified the type of I/O-bound operation being performed within the routine that actually saved each bucket of same-file constants to disk. Previously, it was using blocking operations as it had been running within a sync iterator with no synchronization involved. That has been modified such that the iterator is transformed into a Stream, from which we yield items outside a closure and within an async context (the routine in which this is taking place) to use async I/O (which was the whole point of spawning separate tasks in the first place.)
This refactor also lead to solving the less-than-ideal solution I had fabricated for the purposes of having a 'static ConstContainer moved to the task that effected changes to disk in the binary. It also voids uses of ThreadePtr, the quick and unsafe solution I implemented for this initially.
The two last notable changes included caching the number of regex-matched elements in the non-owning view within the binary’s running state. This was an expensive operation that was being computed in way too many places without need. Beyond that, the ConstContainer::filter method got refactored to be the only filtering operation possible. Previously, both filter_with and filter were offered to either produce a new BorrowedContainer or reuse one. This stopped being useful a few patches ago when this type stopped being used as a resizable container, so there’s that.
None at present.
Yesterday’s plans for the refactor have been completed and a few more changes have also been implemented. I would like to keep refactoring some more stuff, but tomorrow I will be solely focusing on improving path display of constants in the TUI, as that was the last (revised) goal of the week. Next week, I will continue doing refactors and actually deprecating items to see ways this tool can be improved for wider use.