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

[Zachary Turner via lldb-dev]

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/20fe6cdc/attachment.html>