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@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@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
