[lldb-dev] Passing std::atomics by value

[Zachary Turner via lldb-dev]

How about making it a std::unique_ptr<std::atomic<lldb::addr_t>>?  This way
there's no risk of re-introducing a potential race, and copying still works.

On Mon, Aug 29, 2016 at 8:49 AM Zachary Turner <zturner at google.com> wrote:

> My concern is that by not taking out the copy constructor, someone passes
> another Address by value later. If passing by value is unsafe (ignoring
> whether it generates a compiler error) it should be prevented by the
> compiler. That is doing due diligence.
>
> I will track down the commit that introduced this, but again, all you have
> to do is inspect the class to see that it is not thread aware at all.
> Anyone attempting to share an Address on multiple threads had better be
> using a mutex or they're already gonna have a bad time, in which case the
> atomic seems unnecessary
>
>
>
> On Mon, Aug 29, 2016 at 8:38 AM Greg Clayton <gclayton at apple.com> wrote:
>
>>
>> > On Aug 26, 2016, at 5:37 PM, Zachary Turner <zturner at google.com> wrote:
>> >
>> > How would you enforce that, other than to ask people to try to remember
>> not to do it?  It seems to me like std::atomic not being copy-constructible
>> is telling you that, well, you shouldn't be copying it.
>>
>> It just won't compile on platforms that have this issue. We are not
>> taking the Address copy constructor out just so we can enforce not passing
>> this object by value, that is not a viable solution. The copy constructor
>> of address currently works around this issue, but the solution is not to
>> take out the copy constructor of Address.
>>
>> > BTW, nobody has commented on my original concern that the atomic may
>> not even be necessary in the first place.
>>
>> Track down the original author and see what the commit message said. I
>> can see how this could cause problems, but maybe if we don't allow Address
>> to be passed by value the problem goes away. I am not sure. Just taking it
>> out because it doesn't compile in your platform is not the way forward
>> without doing due diligence.
>>
>> So just change the places where this gets passed by value and be done
>> with it, or take the time to make sure you don't introduce a threading
>> issue and decide to take out the std::atomic.
>>
>> Greg Clayton
>>
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20160829/0cc9f7ed/attachment.html>