[webkit-dev] Set once pointer types and static analysis

[Jean-Yves Avenard]

Hi

It’s a very strong +1 for me ; I find the usage of foo->protectedBar()->method() or Ref { foo->bar() }->method()
particularly unsightly (and points more to the inability of the static analyser to deal with some use case than the code being inherently unsafe)

Having a way to reduce the unsightly pattern where we are 100% certain it’s not needed is a benefit.
I assume also that the static analyser will not complain with `const` members either 

(For the sake of disclosure, I asked Ryosuke for that feature after discussing the need with Youenn) 

Jean-Yves

> On 29 Oct 2024, at 5:48 am, Ryosuke Niwa via webkit-dev <webkit-dev at lists.webkit.org> wrote:
> 
> Hi all,
> 
> In WebKit, it’s fairly common to write a member variable as RefPtr or std::unique_ptr that later gets lazily initialized to some value but never unset or assigned of a different value after that.
> 
> e.g.
> 
> class Foo {
>     Bar& bar() {
>         if (!m_bar)
>             m_bar = Bar::create();
>         return *m_bar;
>     }
>     Ref<Bar> protectedBar() { return bar(); }
> 
>     RefPtr<Bar> m_bar;
> }
> 
> Assuming there is no other code modifying m_bar, foo->bar()->method() is always safe to call even if method wasn’t a trivial function. Right now, static analyzer doesn’t recognize this pattern so we’d be forced to write code like this: foo->protectedBar()->method() where ensureProtectedBar is a wrapper around ensureBar which returns Ref<Bar>.
> 
> A suggestion was made that static analyzer can recognize this patten. Specifically, if we introduced a new smart pointer types that only allow setting the value once, static analyzer can allow foo->bar()->method()and avoid ref-churn in some cases:
> 
> e.g.
> 
> class Foo {
>     Bar& bar() {
>         if (!m_bar)
>             m_bar = Bar::create();
>         return *m_bar;
>     }
> 
>     SetOnceRefPtr<Bar> m_bar;
> }
> 
> SetOnceRefPtr::operator=(T*) release asserts that m_ptr isn’t set, and doesn’t have a move constructor, operator=(nullptr_t), leakRef(), releaseNonNull(), etc… which can override the value of m_ptr after setting the value via operator= or in constructor.
> 
> We could create various variants: SetOnceRef, SetOnceUniquePtr, SetOnceUniqueRef.
> 
> Do you think this will be useful? 
> 
> - R. Niwa
> 
> _______________________________________________
> 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/20241029/7f5fe8ca/attachment.htm>