On 2014-04-01, 7:32 PM, Martin Thomson wrote:
On 2014-04-01, at 16:17, Ehsan Akhgari <ehsan.akhg...@gmail.com> wrote:
Just to clarify my position a bit more, I think criticizing my position by
pretending that I'm advocating for a brain-off way of programming with atomics
is a bit contrived. I definitely understand that atomics require special
feeding and care. What's under debate is whether we should make that obvious
to authors and reviewers by not conflating things such as operator++ etc. to
work on both atomic and non-atomic types.
I don’t think that the pushback is based on the fact that code using
Atomic<uint32_t> is at least as thread safe as code using uint32_t.
As is, someone reading code is likely to see threading errors that are actually safe
due to use of Atomic<>. The opposite - missing a real error - happens because
we are human. It doesn’t seem more or less likely if you require the more explicit
syntax.
So are you suggesting that if the code in my original patch looked like
this:
// (a)
if (mozilla::AtomicFetchSub(&mRefCnt, -1) == 1) {
delete this;
}
I would still go ahead and rewrite it as:
// (b)
mozilla::AtomicFetchSub(&mRefCnt, -1);
if (mozilla::AtomicLoad(&mRefCnt) == 0) {
delete this;
}
My contention is that it is obvious enough by looking at (a) to tell
that mRefCnt is an atomic value which should be handled with the
necessary care, so the pattern (b) would be caught either at code
writing time or at code review time.
Cheers,
Ehsan
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
