Re: [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 



> On Sep 13, 2017, at 12:49, JF Bastien  wrote:
> 
> 
> 
>> On Sep 13, 2017, at 11:07, Maciej Stachowiak > > wrote:
>> 
>> 
>> 
>>> On Sep 13, 2017, at 8:11 AM, JF Bastien >> > 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*);
>>> 
>>> 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>> 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>> CrashOnOverflow, size_t minCapacity = 16, typename Malloc = FastMalloc>
>>> class Vector : private VectorBuffer {
>>> 
>>> 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

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Forward.h's Vector

[JF Bastien]

> On Sep 13, 2017, at 11:07, Maciej Stachowiak  wrote:
> 
> 
> 
>> On Sep 13, 2017, at 8:11 AM, JF Bastien > > 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*);
>> 
>> 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> 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> CrashOnOverflow, size_t minCapacity = 16, typename Malloc = FastMalloc>
>> class Vector : private VectorBuffer {
>> 
>> 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

Re: [webkit-dev] Forward.h's Vector

[Maciej Stachowiak]

> On Sep 13, 2017, at 8:11 AM, JF Bastien  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*);
> 
> 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 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 CrashOnOverflow, size_t minCapacity = 16, typename Malloc = FastMalloc>
> class Vector : private VectorBuffer {
> 
> 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.

 - Maciej



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Forward.h's Vector

[Myles C. Maxfield]

> On Sep 13, 2017, at 8:11 AM, JF Bastien  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*);
> 
> 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 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 CrashOnOverflow, size_t minCapacity = 16, typename Malloc = FastMalloc>
> class Vector : private VectorBuffer {
> 
> 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.
When looking for default parameters, I would expect to find them next to the 
Vector definition. If we do this, #4 should be required.
> 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?
> 
> JF
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Forward.h's Vector

[JF Bastien]

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*);

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 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
class Vector : private VectorBuffer {

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?

JF

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev