Re: [webkit-dev] [webkit-changes] [264332] trunk/Source

[Darin Adler]

We need to be cautious about adding header includes to other headers. Adding 
includes to .cpp files is likely fine and not a deeply consequential decision, 
but adding to headers is something that can have a massive effect on build 
times over time.

> On Jul 13, 2020, at 10:44 PM, hironori.fu...@sony.com wrote:
>  <>Modified: trunk/Source/WebCore/platform/graphics/ColorSerialization.h 
> (264331 => 264332)
> 
> --- trunk/Source/WebCore/platform/graphics/ColorSerialization.h   
> 2020-07-14 05:17:20 UTC (rev 264331)
> +++ trunk/Source/WebCore/platform/graphics/ColorSerialization.h   
> 2020-07-14 05:44:25 UTC (rev 264332)
> @@ -25,6 +25,8 @@
>  
>  #pragma once
>  
> +#include 
This change is wrong. The header to include here is Forward.h, not WTFString.h. 
Not super-harmful since WTFString.h is already so widely included, but we need 
to be really cautious in headers.

> Modified: trunk/Source/WebCore/svg/SVGParserUtilities.h (264331 => 264332)
> 
> --- trunk/Source/WebCore/svg/SVGParserUtilities.h 2020-07-14 05:17:20 UTC 
> (rev 264331)
> +++ trunk/Source/WebCore/svg/SVGParserUtilities.h 2020-07-14 05:44:25 UTC 
> (rev 264332)
> @@ -24,6 +24,7 @@
>  #include "ParsingUtilities.h"
>  #include 
>  #include 
> +#include 
Same mistake.

I’d really like to come up with some other system for reviewing adding headers 
to *headers*.

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

Re: [webkit-dev] [webkit-changes] [264332] trunk/Source

[Said Abou-Hallawa]

> On Jul 14, 2020, at 10:40 AM, Darin Adler  wrote:
> 
> We need to be cautious about adding header includes to other headers. Adding 
> includes to .cpp files is likely fine and not a deeply consequential 
> decision, but adding to headers is something that can have a massive effect 
> on build times over time.
> 
>> On Jul 13, 2020, at 10:44 PM, hironori.fu...@sony.com 
>>  wrote:
>>  <>Modified: trunk/Source/WebCore/platform/graphics/ColorSerialization.h 
>> (264331 => 264332)
>> 
>> --- trunk/Source/WebCore/platform/graphics/ColorSerialization.h  
>> 2020-07-14 05:17:20 UTC (rev 264331)
>> +++ trunk/Source/WebCore/platform/graphics/ColorSerialization.h  
>> 2020-07-14 05:44:25 UTC (rev 264332)
>> @@ -25,6 +25,8 @@
>>  
>>  #pragma once
>>  
>> +#include 
> This change is wrong. The header to include here is Forward.h, not 
> WTFString.h. Not super-harmful since WTFString.h is already so widely 
> included, but we need to be really cautious in headers.
> 
>> Modified: trunk/Source/WebCore/svg/SVGParserUtilities.h (264331 => 264332)
>> 
>> --- trunk/Source/WebCore/svg/SVGParserUtilities.h2020-07-14 05:17:20 UTC 
>> (rev 264331)
>> +++ trunk/Source/WebCore/svg/SVGParserUtilities.h2020-07-14 05:44:25 UTC 
>> (rev 264332)
>> @@ -24,6 +24,7 @@
>>  #include "ParsingUtilities.h"
>>  #include 
>>  #include 
>> +#include 
> Same mistake.
> 
> I’d really like to come up with some other system for reviewing adding 
> headers to *headers*.

I looked at the SVGParserUtilities.h and in fact we do not have to include 
HashSet.h or Vector.h either. HashSet and Vector need only to be forward 
declare in this header because they are referenced as return values. So 
 can replace all the three header files.


Maybe the policy can be: include  before trying to include any 
other wtf header file. 

I think we need to include the wtf header files only when the compiler needs to 
know the size. For example:

class X {
Vector m_list;
};

Thanks,
Said

> 
> — Darin
> ___
> 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] New EWS queues for tvOS and watchOS

[Aakash Jain]

I just modified watchOS EWS queue to build both arm64_32 and armv7k 
architectures (in https://bugs.webkit.org/show_bug.cgi?id=214279). Thanks 
Yusuke for pointing it out.

Also, in the process, I figured out that the build for armv7k was in fact 
broken. Thanks Jonathan for fixing that (in https://trac.webkit.org/r264357).

As always, please let me know if you notice any issue or have any feedback.

Thanks
Aakash

> On Jul 13, 2020, at 12:36 AM, Keith Miller  wrote:
> 
> It looks like just arm64_32.
> 
> Cheers,
> Keith
> 
>> On Jul 11, 2020, at 8:41 PM, Yusuke Suzuki  wrote:
>> 
>> Nice,
>> 
>> Is watchOS build ARMv7k? Or is it ARM64_32?
>> 
>> -Yusuke
>> 
>>> On Jul 11, 2020, at 8:09 AM, Aakash Jain  wrote:
>>> 
>>> Hi Everyone,
>>> 
>>> I am happy to announce that we have added new EWS queues for tvOS and 
>>> watchOS. From now on, you would see four new status bubbles (on Bugzilla 
>>> bug) named: tv, tv-sim, watch, watch-sim (corresponding to device and 
>>> simulator builds for watchOS and tvOS). This should help in preventing any 
>>> accidental build breakage on these platforms.
>>> 
>>> Please let me know if you notice any issue.
>>> 
>>> Thanks
>>> Aakash
>>> ___
>>> 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 mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] [webkit-changes] [264332] trunk/Source

[Sam Weinig]

While I don’t want to take away from what Darin is saying here about correct 
usage of forward declaration and , I’d like to understand why we 
have two different compilation models supported in WebKit. Is there a reason 
both need to be supported? Can we remove one?

- Sam

> On Jul 14, 2020, at 10:40 AM, Darin Adler  wrote:
> 
> We need to be cautious about adding header includes to other headers. Adding 
> includes to .cpp files is likely fine and not a deeply consequential 
> decision, but adding to headers is something that can have a massive effect 
> on build times over time.
> 
>> On Jul 13, 2020, at 10:44 PM, hironori.fu...@sony.com 
>>  wrote:
>>  <>Modified: trunk/Source/WebCore/platform/graphics/ColorSerialization.h 
>> (264331 => 264332)
>> 
>> --- trunk/Source/WebCore/platform/graphics/ColorSerialization.h  
>> 2020-07-14 05:17:20 UTC (rev 264331)
>> +++ trunk/Source/WebCore/platform/graphics/ColorSerialization.h  
>> 2020-07-14 05:44:25 UTC (rev 264332)
>> @@ -25,6 +25,8 @@
>>  
>>  #pragma once
>>  
>> +#include 
> This change is wrong. The header to include here is Forward.h, not 
> WTFString.h. Not super-harmful since WTFString.h is already so widely 
> included, but we need to be really cautious in headers.
> 
>> Modified: trunk/Source/WebCore/svg/SVGParserUtilities.h (264331 => 264332)
>> 
>> --- trunk/Source/WebCore/svg/SVGParserUtilities.h2020-07-14 05:17:20 UTC 
>> (rev 264331)
>> +++ trunk/Source/WebCore/svg/SVGParserUtilities.h2020-07-14 05:44:25 UTC 
>> (rev 264332)
>> @@ -24,6 +24,7 @@
>>  #include "ParsingUtilities.h"
>>  #include 
>>  #include 
>> +#include 
> Same mistake.
> 
> I’d really like to come up with some other system for reviewing adding 
> headers to *headers*.
> 
> — Darin
> ___
> 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] [webkit-changes] [264332] trunk/Source

[Simon Fraser]

Could someone educate me about ? When should I use this instead 
of individual wtf headers?

Simon

> On Jul 14, 2020, at 2:32 PM, Sam Weinig  wrote:
> 
> While I don’t want to take away from what Darin is saying here about correct 
> usage of forward declaration and , I’d like to understand why 
> we have two different compilation models supported in WebKit. Is there a 
> reason both need to be supported? Can we remove one?
> 
> - Sam
> 
>> On Jul 14, 2020, at 10:40 AM, Darin Adler > > wrote:
>> 
>> We need to be cautious about adding header includes to other headers. Adding 
>> includes to .cpp files is likely fine and not a deeply consequential 
>> decision, but adding to headers is something that can have a massive effect 
>> on build times over time.
>> 
>>> On Jul 13, 2020, at 10:44 PM, hironori.fu...@sony.com 
>>>  wrote:
>>>  <>Modified: trunk/Source/WebCore/platform/graphics/ColorSerialization.h 
>>> (264331 => 264332)
>>> 
>>> --- trunk/Source/WebCore/platform/graphics/ColorSerialization.h 
>>> 2020-07-14 05:17:20 UTC (rev 264331)
>>> +++ trunk/Source/WebCore/platform/graphics/ColorSerialization.h 
>>> 2020-07-14 05:44:25 UTC (rev 264332)
>>> @@ -25,6 +25,8 @@
>>>  
>>>  #pragma once
>>>  
>>> +#include 
>> This change is wrong. The header to include here is Forward.h, not 
>> WTFString.h. Not super-harmful since WTFString.h is already so widely 
>> included, but we need to be really cautious in headers.
>> 
>>> Modified: trunk/Source/WebCore/svg/SVGParserUtilities.h (264331 => 264332)
>>> 
>>> --- trunk/Source/WebCore/svg/SVGParserUtilities.h   2020-07-14 05:17:20 UTC 
>>> (rev 264331)
>>> +++ trunk/Source/WebCore/svg/SVGParserUtilities.h   2020-07-14 05:44:25 UTC 
>>> (rev 264332)
>>> @@ -24,6 +24,7 @@
>>>  #include "ParsingUtilities.h"
>>>  #include 
>>>  #include 
>>> +#include 
>> Same mistake.
>> 
>> I’d really like to come up with some other system for reviewing adding 
>> headers to *headers*.
>> 
>> — Darin
>> ___
>> 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 mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] [webkit-changes] [264332] trunk/Source

[Darin Adler]

> On Jul 14, 2020, at 2:38 PM, Simon Fraser  wrote:
> 
> Could someone educate me about ? When should I use this 
> instead of individual wtf headers?

Forward.h is analogous to forward-declaring a class ('class IntPoint;' instead 
of ‘#include “IntPoint.h”'), but it works for many often-used classes and class 
templates in the WTF namespace, including class templates that would be 
difficult to correctly forward-declare due to their many arguments, such as 
WTF::Vector. And it includes “using WTF::String” and the like, as well, to 
import WTF namespace things into the global namespace.

We can use it any time we need a forward-declaration, not an entire definition, 
of one of the items. For example, to compile a header that just takes and 
returns String objects, we only need a forward declaration of String. The 
easiest way to correctly do that is to include . Including 
 pulls in a lot more. For the specific case of String, I think 
you might be able to go further and write this instead:

namespace WTF {
class String;
}
using WTF::String;

But I have never tried it, and there might be some problem with that.

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

Re: [webkit-dev] [webkit-changes] [264332] trunk/Source

[Fujii Hironori]

On Wed, Jul 15, 2020 at 6:32 AM Sam Weinig  wrote:

> While I don’t want to take away from what Darin is saying here about
> correct usage of forward declaration and , I’d like to
> understand why we have two different compilation models supported in
> WebKit. Is there a reason both need to be supported? Can we remove one?
>


I also want to know that. Does anyone still need non-unified builds?

I introduced other unnecessary header inclusion, and Darin asked me to fix
it.
https://bugs.webkit.org/show_bug.cgi?id=214204#c25
Reducing header inclusion can easily cause build breakages for non-unified
builds.
So, I fixed non-unified build breakage in r264332 and r264333 as the
preparation for that.
Non-unified builds was more broken than I expected. Does anyone still need
it?
Should we maintain non-unified builds until C++20 modules will nullify
unified builds?
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev