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

[Said Abou-Hallawa]

> On Jul 14, 2020, at 10:40 AM, Darin Adler <darin at apple.com> 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.Fujii at sony.com <mailto:Hironori.Fujii at 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 <wtf/text/WTFString.h>
> 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 <wtf/HashSet.h>
>>  #include <wtf/Vector.h>
>> +#include <wtf/text/WTFString.h>
> 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 <wtf/Forward.h> can replace all the three header files.


Maybe the policy can be: include <wtf/Forward.h> 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<String> m_list;
};

Thanks,
Said

> 
> — Darin
> _______________________________________________
> 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/20200714/b049925e/attachment.htm>