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

> On Aug 29, 2016, at 8:56 AM, Zachary Turner wrote: > > How about making it a std::unique_ptr? This way > there's no risk of re-introducing a potential race, and copying still works. lldb_private::Address is a member variable in lldb_private::Symbol and it

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

How about making it a std::unique_ptr? 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 wrote: > My concern is that by not taking out the copy constructor, someone passes >

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

> On Aug 26, 2016, at 5:37 PM, Zachary Turner 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

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

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. BTW, nobody has commented on my original concern that the atomic may not even be necessary in

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

There is no need to delete the lldb_private::Address copy constructor. Just stop functions from passing it by value. > On Aug 26, 2016, at 1:13 PM, Zachary Turner wrote: > > IOW, marking it =delete() is no different than deleting the copy constructor > above, but at least

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

> On Aug 26, 2016, at 1:13 PM, Zachary Turner wrote: > > IOW, marking it =delete() is no different than deleting the copy constructor > above, but at least if you mark it delete, maybe someone will read the > comment above it that explains why it's deleted :) Got it, make

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

IOW, marking it =delete() is no different than deleting the copy constructor above, but at least if you mark it delete, maybe someone will read the comment above it that explains why it's deleted :) On Fri, Aug 26, 2016 at 1:13 PM Zachary Turner wrote: > I think so. But in

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

I think so. But in this case lldb::Address explicitly supplied a copy constructor that looked like this: Address (const Address& rhs) : m_section_wp (rhs.m_section_wp), m_offset(rhs.m_offset.load()) // this is the std::atomic<> { } circumventing the problem. On

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

> On Aug 26, 2016, at 1:02 PM, Zachary Turner via lldb-dev > wrote: > > It seems to be. I will also make the copy constructor =delete() to make sure > this cannot happen again. Just curious: if a member has a deleted copy-ctor (like std::atomic right?), isn’t the

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

It seems to be. I will also make the copy constructor =delete() to make sure this cannot happen again. I'm still concerned that the std::atomic is unnecessary, but at that point at least it just becomes a performance problem and not a bug. On Fri, Aug 26, 2016 at 1:00 PM Greg Clayton

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

M > To: Zachary Turner <ztur...@google.com> > Cc: LLDB <lldb-dev@lists.llvm.org> > Subject: Re: [lldb-dev] Passing std::atomics by value > > That seems to me a compiler bug, not: "You can't pass structures with > std::atomic" elements in them by value. It wo

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

Subject: Re: [lldb-dev] Passing std::atomics by value That seems to me a compiler bug, not: "You can't pass structures with std::atomic" elements in them by value. It would crazy to add a type to the C++ language that you couldn't use everywhere. There doesn't seem to be any

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

That seems to me a compiler bug, not: "You can't pass structures with std::atomic" elements in them by value. It would crazy to add a type to the C++ language that you couldn't use everywhere. There doesn't seem to be any official restriction. And anecdotally there's a reference to the

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

It could, in theory, it just doesn't. I compiled a quick program using i686-darwin as the target and the generated assembly makes no attempt to pad the arguments. I'll post some code later On Fri, Aug 26, 2016 at 11:42 AM Jim Ingham wrote: > > > On Aug 26, 2016, at 11:36 AM,

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

> On Aug 26, 2016, at 11:44 AM, Adrian McCarthy via lldb-dev > wrote: > > Methods like Address::SetOffset and Address::Slide seem to have data races > despite m_offset being atomic. Callers of those methods would have to > guarantee that nothing else is trying to

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

Methods like Address::SetOffset and Address::Slide seem to have data races despite m_offset being atomic. Callers of those methods would have to guarantee that nothing else is trying to write to m_offset. And if they're doing that, then there doesn't appear to be a need for std::atomic on that

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

> On Aug 26, 2016, at 11:36 AM, Zachary Turner via lldb-dev > wrote: > > The thing is, the code is already full of data races. I think the > std::atomic is actually used incorrectly and is not even doing anything. > > That said, consider darwin on 32-bit, where I

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

The thing is, the code is already full of data races. I think the std::atomic is actually used incorrectly and is not even doing anything. That said, consider darwin on 32-bit, where I believe each stack frame is 4-byte aligned. I dont' think there's any way the compiler can guarantee that a

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

> On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev > wrote: > >> >> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev >> wrote: >> >> I recently updated to Visual Studio 2015 Update 3, which has improved its >> diagnostics.

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

I recently updated to Visual Studio 2015 Update 3, which has improved its diagnostics. As a result of this, LLDB is uncompilable due to a slew of errors of the following nature: D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 'default_stop_addr': formal parameter with