On Mon, Nov 4, 2019 at 11:55 PM John Chapman via Digitalmars-d-announce <[email protected]> wrote: > > On Tuesday, 5 November 2019 at 06:44:29 UTC, Manu wrote: > > On Mon., 4 Nov. 2019, 2:05 am John Chapman via > > Digitalmars-d-announce, < [email protected]> > > wrote: > > > >> Something has changed with core.atomic.cas - it used to work > >> with `null` as the `ifThis` argument, now it throws an AV. Is > >> this intentional? > >> > >> If I use `cast(shared)null` it doesn't throw but if the change > >> was deliberate shouldn't it be mentioned? > >> > > > > Changes were made because there were a lot of problems with > > that module... > > but the (reasonably comprehensive) unit tests didn't reveal any > > such > > regressions. We also build+test many popular OSS projects via > > buildkite, > > and there weren't problems. > > Can you show the broken code? > > Sure - this AVs on DMD 2.088 Windows: > > import core.atomic; > void main() { > Object a, b = new Object; > cas(cast(shared)&a, null, cast(shared)b); > }
Oh... a class. Yeah, that's an interesting case that I actually noted had a low testing surface area. It's also theoretically broken; despite what's practical, I think it's improperly spec-ed that shared classes can be used with atomics. With a struct, you can declare `shared(T)* s_ptr`, but with classes you can only `shared(C) c_ptr`, where the difference is that `s_ptr` can be read/written... but `c_ptr` is typed such that the pointer itself is shared (because classes are implicitly a pointer), so that `c_ptr` can't be safely read/write-able... So, I actually think that atomic API is mal-formed, and it should not support `shared` arguments, but I tried to preserve existing behaviour, while being more strict about what is valid. I obviously missed something with `null` here. Incidentally, in your sample above there, `a` and `b` are not shared... why not just write: `cas(&a, null, b);` ?? If source data is not shared, you shouldn't cast to shared.
