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
