[webkit-dev] Forward.h's Vector

[JF Bastien]

Having not heard any objections, here’s a patch which does this: https://bugs.webkit.org/show_bug.cgi?id=176984 <https://bugs.webkit.org/show_bug.cgi?id=176984>


> On Sep 13, 2017, at 12:49, JF Bastien <jfbastien at apple.com> wrote:
> 
> 
> 
>> On Sep 13, 2017, at 11:07, Maciej Stachowiak <mjs at apple.com <mailto:mjs at apple.com>> wrote:
>> 
>> 
>> 
>>> On Sep 13, 2017, at 8:11 AM, JF Bastien <jfbastien at apple.com <mailto:jfbastien at 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 at lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20170915/9d89fd2e/attachment.html>