Add ReferenceCountedThreadSafe class

[kring]

Like ReferenceCountedNonThreadSafe, this is a template class intended to be inherited from using CRTP to make it easy to use a class with IntrusivePointer. Unlike that class, though, the reference count is thread safe (and more expensive) because it is stored in a std::atomic<int32_t>.

[kring]

It would take some fairly advanced template trickery to pull that off, @csciguy8. I think we'd just be replacing two very simple classes with one incomprehensible one. But maybe you have an implementation in mind that would be simpler than the one I'm able to envison off the top of my head?

[csciguy8]

@kring How about this?

ReferenceCounted.txt

More template params is more complexity, sure, but hopefully not too incomprehensible?

std::conditional_t is probably the less known convention (I've never used it before). Some good related discussion on StackOverflow here

[kring]

Yeah that's pretty good @csciguy8!

It's much less self-documenting at the point of use:

class Whatever : public ReferenceCountedThreadSafe<Whatever>

versus:

class Whatever : public ReferenceCounted<Whatever, true>

But we can fix that with a couple of using declarations.

Only other suggestion is to use if constexpr (!isThreadSafe) so it gets compiled-out in the thread-safe version.

Is there a way to eliminate the std::thread::id _threadID from the thread-safe version where it isn't used? That's the only other cost of this approach, but it's relatively minor since it's only in debug builds.

[kring]

Ok I've merged the two classes using Brian's approach (and a couple of small tweaks). I couldn't see a way to eliminate the _threadID in the version that doesn't need it, but that's a minor problem at worst. This should be ready.

[kring]

Actually I did find a way to get rid of that field, using inheritance. In C++20 it'd be possible to do it without inheritance, by using [[no_unique_address]], but not in C++17 that we're currently targeting.

[csciguy8]

Very cool @kring!

I'm glad you like the direction I proposed. It's definitely getting more complex than I originally thought, with template specialization and the ThreadIdHolder class, but still hopefully not to the incomprehensible level. If the thread assertion fires or we get a reference counting crash in use, I think it's all still usable.

Overall, I still like it. We don't get the self documenting nature of having NonThreadSafe in the name of the class, like you pointed out, but we still do get a single class that is clearly our own "ReferenceCounted" take, with thread safety being a feature. There's no obvious code duplication and we can rework the implementation, or add even more features later if we need to.

Merging...