On 4/2/2014 5:30 PM, Ehsan Akhgari wrote:
I have no reservations against making them member functions with a clear name that do not return T/T*/etc. I was trying to not be super inconsistent with std::atomic, but if you think that's ok, that's fine by me.
std::atomic has the fetch_* methods on them. I'm actually surprised that C++11 has global memory functions here, considering that they only accept atomic<T>* in arguments and don't support T*.
I'm kind of regretting using the example of reference counts for this discussion. I just wanted to give a real example of something that actually happened and went unnoticed until it hit a user, but I think it ultimately just muddied the waters here. None of what I've said is restricted to reference counting. I don't dare predict what people are going to do with the Atomic type, and I find assertions such as "mostly limited to the first pattern" very hard to believe. The underlying issue is that the syntax is hiding the atomicity semantics. As long as you have that syntax around, any code that uses this type is prone to this problem
My (admittedly limited) experience of atomics suggests that the cases where it wouldn't be obvious from context that the variable in question is atomic (and that the variable could be accidentally misused atomically--mozilla::Atomic<bool> is rather hard to misuse) are comparatively limited: I can't think of a situation outside of reference counts where people could plausibly use an atomic without realizing it's an atomic.
Using the clang plugin to detect this will make the problem non-detectible in places where the code is b2g/OSX/Windows specific. We have a fair bit of those.
The only reason the clang plugin isn't running on OS X is because I don't use OS X and thus don't know how to make it work with clang there, and no one else has offered to step in and add support. There's no reason it couldn't be extended to work on Windows or B2G in the long run (or even making other custom plugins for msvc/gcc), although that would have the ancillary effect of making Clang builds effectively tier 1 there.
Can you please help me understand why do you resist changing mozilla::Atomic? Is it just to hold on to the shorter syntax that these operators give you? It seems like you're OK with at least some degree of incompatibility with <atomic>?
To me, it's a matter of reducing the usability and fundamental design to bring about illusions of safety in what I contend is a corner case of API usage.
-- Joshua Cranmer Thunderbird and DXR developer Source code archæologist _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
