Allow specialisation of vsg::Inherit for sublasses #1609
Conversation
This allows default implementations of members to be provided, prevents copying and pasting of lots of trivial functions, and makes it possible to generate a compiler error if a subclass fails to provide a particular member without making it a pure virtual function. This is a pretty close equivalent to how in the OSG, there was a META_Object macro that did roughly what the VSG's Inherit template does, but also there were specialised macros like META_Node and META_StateAttribute to do extra things for particular kinds of object. I've only used this for vsg::ArrayState so far as that was the place where the problem came up in my work (I needed to add extra cloneArrayState overloads and ensure subclasses overrode them). I think it's a fairly safe bet that there are other places where using this would be helpful, as I know I've been annoyed by the lack of such a feature in the past, although I can't remember when or why. This isn't the prettiest code in the world, and has a bonus extra annoying feature that it makes the Visual Studio debugger add an extra level of indentation to loads of things, but it's the best that can be done until P3469 https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3469r0.pdf is ratified and becomes part of C++ (it's too late to get into C++26) and finally makes CRTP fully obsolete.
|
Huh, apparently it doesn't work on Windows as a DLL build. I'll see why. |
MSVC apparently got confused by the mixed approaches for inheriting constructors, so switched InheritBase to a using declaration as that's the simpler syntax. This then upset GCC because 'an inherited constructor is not a candidate for initialization from an expression of the same or derived type', so the explicit from-ArrayState constructors required for cloneArrayState stopped being available until they were explicitly reimplemented, and then that meant that DisplacementMapArrayState had to expose it with the copyop.
|
I have done a first pass review and my first reaction is urgg, this feels cludgy. From my first pass reading it seems like you want to avoid the local cloneArrayState() method implementations in subclasses, instead having a CRTP solution like vsg::Inherit. While I can see that this approach is a solution, but it doesn't immediately jump out at me as a good solution. I'll be reviewing your intersection code today so this will be give me a bit more insight into where you are coming from. As I do this I'll ponder on the problem you are trying to solve w.r.t cloneArrayState() and see if there might be a cleaner approach. |
|
The changes that made this approach less bad than not using this approach aren't checked into a public branch yet, but the gist is that more overloads of I agree that this is kludgy, but it's kind of inevitable - it uses things that have become more or less obsolete in the eight years since C++17 was standardised, e.g. the SFINAE parts could be done more clearly with C++20 concepts, and CRTP was always a kludge, which C++23 was supposed to replace with deducing Since last night, I've thought of an alternative kludge that might work, and might look like less of a kludge than this does. I'll give it a try and see if it turns out to be prettier. Implementing this isn't a strict requirement for the optimisation I used it for, and I originally implemented it before making this in an attempt to tidy it up and remove a footgun. If we can't come up with something sufficiently nice, we can do without it, but I expect that this or something similar will end up paying for itself even if it doesn't end up being especially pretty. |
2 participants
This allows default implementations of members to be provided, prevents copying and pasting of lots of trivial functions, and makes it possible to generate a compiler error if a subclass fails to provide a particular member without making it a pure virtual function.
This is a pretty close equivalent to how in the OSG, there was a
META_Objectmacro that did roughly what the VSG's Inherit template does, but also there were specialised macros likeMETA_NodeandMETA_StateAttributeto do extra things for particular kinds of object.I've only used this for
vsg::ArrayStateso far as that was the place where the problem came up in my work (I needed to add extracloneArrayStateoverloads and ensure subclasses overrode them). I think it's a fairly safe bet that there are other places where using this would be helpful, as I know I've been annoyed by the lack of such a feature in the past, although I can't remember when or why.This isn't the prettiest code in the world, and has a bonus extra annoying feature that it makes the Visual Studio debugger add an extra level of indentation to loads of things, but it's the best that can be done until P3469 https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3469r0.pdf is ratified and becomes part of C++ (it's too late to get into C++26) and finally makes CRTP fully obsolete.