Re: [webkit-dev] Stop Using Raw Pointers & References in New Code

[Ryosuke Niwa via webkit-dev]

> On Jan 13, 2023, at 11:06 AM, Ryosuke Niwa via webkit-dev 
>  wrote:
> 
>> On Jan 13, 2023, at 6:39 AM, Darin Adler  wrote:
>> 
>> I don’t think I can notice these patterns. I would be able to program this 
>> way if I had an analysis tool, but otherwise this simply seems too 
>> complicated.
> 
> Yeah, I agree.
> 
>> I can’t see the types of intermediate values to know if they are CheckedPtr 
>> or RefPtr or raw pointers or whatever. A variation on the first line in 
>> Element::clientLeft is a good example:
>> 
>>  document()->updateLayoutIgnorePendingStylesheets();
>> 
>> That looks perfectly good to me, nothing makes me think “the result of 
>> document is an unsafe reference to a heap-allocated object and only trivial 
>> functions can be called on that”; I should not need to be an expert on each 
>> function to be able to tell if code is correct or not. How can we choose a 
>> programming style where something simple like that is subtly wrong and 
>> dangerous and requires reviewers to look out for it?
>> 
>> I understand that if properly supported by a tool we could adapt to this and 
>> write code a whole new way. Moving to this style without a tool would almost 
>> literally require me to stop working on WebKit because I couldn’t correctly 
>> write or review even a short function.
> 
> I acknowledge that we can’t move to this new model today. So what I’m 
> suggesting is to adopt a smaller scope now: Use smart pointers in all local 
> variables and heap allocated values. This will not be 100% security proof 
> (because of the function return values) but it’s a much better point to get 
> to than where we are now.

Here’s a PR to make the style checker look for raw pointers & references in 
data members: https://github.com/WebKit/WebKit/pull/8907

I suggest we land this PR in 5 business days from now on unless someone objects.

- R. Niwa

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

Re: [webkit-dev] Compile times and class-scoped enums

[Darin Adler via webkit-dev]

> On Jan 20, 2023, at 9:50 AM, Jer Noble via webkit-dev 
>  wrote:
> 
> I attempted to address this in , 
> which (on this machine) reduces the total compile time of Document.h in the 
> WebCore project from about 1000s to about 200s.

That’s good.

> However, this change requires moving class-scoped enums out into the 
> namespace scope.

Seems worthwhile. Doesn’t seem to me like it would have far reaching effects.

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

Re: [webkit-dev] Compile times and class-scoped enums

[Michael Catanzaro via webkit-dev]

On Fri, Jan 20 2023 at 09:50:05 AM -0800, Jer Noble via webkit-dev 
 wrote:
However, this requires a significant coding style change, both to 
existing code and new code, and as such, it should probably be 
discussed here. So, what do people think? Is the change in coding 
style (moving class-scoped enums out into namespace scope) worth 
doing if it means a significant increases in compile speeds?


My $0.02: it's awful but worth it. That's a ridiculous speedup. Nice.


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

[webkit-dev] Compile times and class-scoped enums

[Jer Noble via webkit-dev]

Hi all!

I’ve noticed that compile times for the WebKit project have started creeping up 
again, so I and a few other WebKit contributors started looking into possible 
compile time improvements using the tools listed in 
. One cause of long 
compile times is when a commonly-included header (like Document.h) includes 
other headers (which include other headers, ad nauseam). Whereas much of the 
time those includes can be avoided by forward declaring types, for types 
(specifically enums) scoped within classes, this is impossible.

I attempted to address this in , 
which (on this machine) reduces the total compile time of Document.h in the 
WebCore project from about 1000s to about 200s.

However, this change requires moving class-scoped enums out into the namespace 
scope. E.g.:

> diff --git a/Source/WebCore/Modules/audiosession/DOMAudioSession.h 
> b/Source/WebCore/Modules/audiosession/DOMAudioSession.h
> index 01bf6960d3a4..d84e1eae78d5 100644
> --- a/Source/WebCore/Modules/audiosession/DOMAudioSession.h
> +++ b/Source/WebCore/Modules/audiosession/DOMAudioSession.h
> @@ -36,14 +36,17 @@
>  
>  namespace WebCore {
>  
> +enum class DOMAudioSessionType : uint8_t { Auto, Playback, Transient, 
> TransientSolo, Ambient, PlayAndRecord };
> +enum class DOMAudioSessionState : uint8_t { Inactive, Active, Interrupted };
> +
>  class DOMAudioSession final : public RefCounted, public 
> ActiveDOMObject, public EventTarget, public 
> AudioSession::InterruptionObserver {
>  WTF_MAKE_ISO_ALLOCATED(DOMAudioSession);
>  public:
>  static Ref create(ScriptExecutionContext*);
>  ~DOMAudioSession();
>  
> -enum class Type : uint8_t { Auto, Playback, Transient, TransientSolo, 
> Ambient, PlayAndRecord };
> -enum class State : uint8_t { Inactive, Active, Interrupted };
> +using Type = DOMAudioSessionType;
> +using State = DOMAudioSessionState;
>  
>  ExceptionOr setType(Type);
>  Type type() const;

So that these enums can be forward declared in Document.h, rather than 
including the header wholesale.

However, this requires a significant coding style change, both to existing code 
and new code, and as such, it should probably be discussed here. So, what do 
people think? Is the change in coding style (moving class-scoped enums out into 
namespace scope) worth doing if it means a significant increases in compile 
speeds?

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