[lldb-dev] Passing std::atomics by value
Zachary Turner via lldb-dev
lldb-dev at lists.llvm.org
Mon Aug 29 08:56:21 PDT 2016
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...
More information about the lldb-dev