Rework CFCppBase to remove Impl classes and the Details namespace.#1313
Conversation
* Adds support for calling non-nullary constructors on the new CF Type. * Removes the _CFFinalize chain. * Removes the _CFInitialize chain. * Uses some dodgy methods to work around the undefined behaviour inherent in placement-newing a block of partially-initialized memory
bbowman
left a comment
There was a problem hiding this comment.
This is super awesome nifty sauce.
| }; | ||
|
|
||
| struct __CFSubLifetimeCounter : CoreFoundation::CppBase<__CFSubLifetimeCounter, __CFLifetimeCounter> { | ||
| __CFSubLifetimeCounter(int& ctorCounter, int& dtorCounter) : Parent(ctorCounter, dtorCounter) { |
There was a problem hiding this comment.
Seems a little silly to be using int references here #ByDesign
There was a problem hiding this comment.
I guess it makes sense for the dtor count.
There was a problem hiding this comment.
This tests a few things: Members expressible only through C++ semantics work properly, constructors and destructors get called properly, and inherited members do not lose their magic.
|
|
||
| new (ret) T(std::forward<Args>(args)...); | ||
|
|
||
| // This, however, may be ill-defined. |
There was a problem hiding this comment.
can this be a static_cast? Every T here should work out for that and we should maybe static_assert or enable_if with is_base_of to validate that CreateInstance only gets called on __CFRuntimeBase things. (Would be super nifty to know it was the root base (not a multi inheritance weirdness) which is maybe doable with template magic?) #WontFix
There was a problem hiding this comment.
You shouldn't be able to call T::CreateInstance with anything but T; and I'd like to use reinterpret to circumvent any chicanery the compiler may pull on layout adjustment. I don't expect it to do any, but the block of memory we get back from _CFRuntimeCreateInstance is definitely normally a __CFRuntimeBase first and foremost.
| // | ||
| // To work around that, we save the __CFRuntimeBase at the head of ret, call the constructor, and replace/reinitialize | ||
| // it. As no constructor has been invoked on ret by the time we save temp, this is not ill-defined. | ||
| __CFRuntimeBase temp = *reinterpret_cast<__CFRuntimeBase*>(ret); |
There was a problem hiding this comment.
__CFRuntimeBase temp = *re [](start = 8, length = 26)
what all are we copying here, and then again below? #Resolved
|
|
inherent in placement-newing a block of partially-initialized memory
This change is