RetainPtr isn’t supported by static analyzer at the moment. > On Nov 13, 2024, at 4:28 AM, Jean-Yves Avenard <jean-yves.aven...@apple.com> > wrote: > > Hi > > That sounds great. > > Can we also do the same for const RetainPtr ? > > Thanks > > >> On 13 Nov 2024, at 4:49 PM, Ryosuke Niwa via webkit-dev >> <webkit-dev@lists.webkit.org> wrote: >> >> 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 >
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
