> On Sep 13, 2017, at 11:07, Maciej Stachowiak <m...@apple.com> wrote: > > > >> On Sep 13, 2017, at 8:11 AM, JF Bastien <jfbast...@apple.com >> <mailto:jfbast...@apple.com>> wrote: >> >> Hello WebCritters, >> >> I’m moving some code around, and one particular header I have is included >> everywhere in JSC so I’d like it to be lightweight and include as few other >> headers as possible. It unfortunately uses WTF::Vector, but it only does so >> as a pointer: >> >> void ohNoes(Vector<Noms>*); >> >> It seems like something a forward declaration would fix nicely, and oh joy >> and wonder we have Forward.h just for this! Here’s what it does: >> >> template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t >> minCapacity, typename Malloc> class Vector; >> >> That’s nice and great for T, but all the other template parameters are SOL >> because Vector is actually declared with default template parameters: >> >> template<typename T, size_t inlineCapacity = 0, typename OverflowHandler = >> CrashOnOverflow, size_t minCapacity = 16, typename Malloc = FastMalloc> >> class Vector : private VectorBuffer<T, inlineCapacity, Malloc> { >> >> The extra painful thing is that, contrary to what I originally thought, one >> cannot declare Vector in Forward.h and then define it in Vector.h with the >> same defaults twice! Ideally the compiler would just yell at a mismatch, but >> instead it says error: template parameter redefines default argument (thanks >> clang, that’s exactly what I’m trying to do, just tell me if you catch a >> mismatch and ODR away otherwise). >> >> Here’s what I propose: >> >> Change the forward declaration in Forward.h to contain the default template >> parameters (and forward-declare CrashOnOverflow). >> Remove these default template parameters from Vector.h. >> Include Forward.h in Vector.h. >> Optionally, if the WebCritters think it useful, leave a comment just above >> the Vector definition redirecting to Forward.h for defaults (or more fancy, >> use size_t inlineCapacity /*=0*/ style like LLVM’s codebase does, and have a >> tool that checks for consistency). >> Optionally, try to fix C++20 so this isn’t A Thing anymore. Note that my >> hopes are low because of modules (why fix it if modules will magically make >> this not a thing). >> >> Alternatively, I could just include Vector.h and be done with it. >> >> Thoughts? > > Is there anything in Forward.h that should not be included everywhere you > include Vector.h, whether for efficiency or any other reasons? That's the > only potential thing I can think of that may be wrong with this plan. In that > case, the simple fix would be to have a separate VectorForward.h which can be > included by both.
I thought about that, and it seems like Forward.h is so tiny that it shouldn’t be an issue. If it ever starts including other WTF things then we should definitely do what you say, but I don’t think we should at the moment. Or put another way, if Forward.h doesn’t purely forward-declare things then it’s doing it wrong. :-)
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
