softdevteam / yksom

Other
8 stars 6 forks source link

Fix yksom after libgc changes #200

Closed jacob-hughes closed 3 years ago

jacob-hughes commented 3 years ago

This updates various places in yksom to work after the following breaking changes to libgc:

Please can you pay special attention to the changes I've made in UpVars. The first couple of times I'd tried this, I had broken the block.som and escape{n}.som tests. I seem to have got this working now, I'd got very confused over whether in places I needed to be taking the on-stack-reference, or the usize field inside Val.

ltratt commented 3 years ago

At first I wasn't sure about this, but having looked a bit more carefully, I now think it's OK. We could, perhaps, change the Cell in SOMStack to AtomicUsize but the practical effect would be zero. I think this is OK as-is.

bors r+

bors[bot] commented 3 years ago

Build succeeded:

jacob-hughes commented 3 years ago

Yes I think so. Cell doesn't implement Sync, so even though the SOMStack can be passed between threads, its data can't be accessed by another thread. An AtomicUsize (or Mutex) would work if we wanted its access to be synchronised, but it looks like it's only used by the main-thread at the moment.

ltratt commented 3 years ago

yksom is single threaded so no problem there!