Instead of introducing a new smart pointer type like SetOnceRefPtr, we’re going 
to use `const RefPtr`, `const Ref`, `const unique_ptr`, and `const UniqueRef`. 
`const Ref` and `const UniqueRef` are used for values that are initialized in 
the constructor and never changed. `const RefPtr` and `const unique_ptr` are 
used for lazily initialized values. We introduce a `initializeOnce(ptr, v)` 
function which takes `const RefPtr` or `const unique_ptr` ptr and sets it to v. 
The static analyzer will be updated to recognize this pattern.

https://bugs.webkit.org/show_bug.cgi?id=283038
https://github.com/llvm/llvm-project/pull/115594

- R. Niwa

> On Oct 29, 2024, at 2:49 AM, youenn fablet <youe...@gmail.com> wrote:
> 
> FWIW, there are many classes with Ref<> members that are initialized in the 
> constructor and are never intended to be changed.
> I was hoping we would cover this case at least. The RefPtr lazy 
> initialization is a welcome addition as well.
> 
> I think this would be useful to improve readability:
> - Too many protected in the same line of code makes code harder to read for 
> me.
> - It is easier to reason about "stable" member variables. Having an explicit 
> type or some annotation instead of having to look at the whole code is an 
> improvement.
> 
> Le mar. 29 oct. 2024 à 03:10, Jean-Yves Avenard via webkit-dev 
> <webkit-dev@lists.webkit.org> a écrit :
>> 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

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

Reply via email to