Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
On Fri, Jan 27 2023 at 10:52:52 AM -0600, Michael Catanzaro wrote: There is probably a relatively high cost compared to WTF::WeakPtr, so I'd say it should be used only when it provides valuable safety (e.g. in member variables) rather than spammed (e.g. in local variables). Another good use for GWeakPtr: callbacks ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
On Tue, Jan 24 2023 at 02:53:42 PM -0600, Michael Catanzaro wrote: E.g. for GObjects, we could write GWeakPtr, but this would not be very ergonomic, and it won't work for arbitrary types. So Carlos Garcia added this in https://commits.webkit.org/259482@main. There is a downside to GWeakPtr: global locking. Even though GWeakPtr is not threadsafe and each GObject keeps its own list of weak locations, all GObjects nonetheless share the same global lock for weak pointer locations. There is probably a relatively high cost compared to WTF::WeakPtr, so I'd say it should be used only when it provides valuable safety (e.g. in member variables) rather than spammed (e.g. in local variables). Michael ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
On Wed, Jan 25, 2023 at 5:54 AM Michael Catanzaro via webkit-dev <
webkit-dev@lists.webkit.org> wrote:
>
> Thinking about this more, I'm not sure this plan works for WeakPtrs?
> Say we have:
>
> WeakPtr f = /* initialized somehow */;
> if (Foo* f = f.get())
> {
> // do something
> }
>
> Then we already broke the rule against using a raw pointer in a local
> variable. That's the only way to use a WeakPtr, so we kind of have to,
> but as long as you have it stored in a raw pointer, then we gain no
> additional safety from the WeakPtr. CheckedPtr would work better in
> local variables, but again that's not an option for types we don't
> control.
>
We need to keep a RefPtr to do something.
if (RefPtr f = f.get())
It's not a problem of the circular dependency in this case because the
local variable will be destructed soon.
The problem of WeakPtr is that it may return a pointer of half-destructed
Foo object if this code is called during the Foo destructor.
This is being discussed in https://github.com/WebKit/WebKit/pull/8748 .
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
On Tue, Jan 24 2023 at 11:33:45 AM -0800, Ryosuke Niwa via webkit-dev
wrote:
That’s also the semantics of Ref/RefPtr in WebKit. But we’re
expanding the use of Ref/RefPtr to be beyond just owners for memory
safety. I don’t see how the situation is any different with
GRefPtr/GUniquePtr. If an explicit ownership isn’t appropriate,
then CheckedPtr/CheckedRef should be used instead.
The difference is we can modify WebKit C++ types to inherit from
CanMakeCheckedPtr or CanMakeThreadSafeCheckedPtr, but we cannot modify
types we don't control or types that are not even written in C++, so
the smart pointer would have to do all the work of tracking ownership
itself. std::shared_ptr and std::weak_ptr can do this for types that
don't implement their own refcounting already. For types that *do* have
their own refcounting, then I guess it's only doable if the type
supports weak pointers. E.g. for GObjects, we could write GWeakPtr, but
this would not be very ergonomic, and it won't work for arbitrary types.
Thinking about this more, I'm not sure this plan works for WeakPtrs?
Say we have:
WeakPtr f = /* initialized somehow */;
if (Foo* f = f.get())
{
// do something
}
Then we already broke the rule against using a raw pointer in a local
variable. That's the only way to use a WeakPtr, so we kind of have to,
but as long as you have it stored in a raw pointer, then we gain no
additional safety from the WeakPtr. CheckedPtr would work better in
local variables, but again that's not an option for types we don't
control.
Michael
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
> On Jan 24, 2023, at 5:30 AM, Alicia Boya García via webkit-dev > wrote: > > On 12/01/2023 06.21, Ryosuke Niwa via webkit-dev wrote: >> I suggest we stop using raw pointers and references in any local or heap >> stored variable. > I don't think this is feasible for code using non-WebKit libraries, and would > expect it to not apply to such cases. > > For instance, many GLib and GStreamer objects are refcounted, and we even > have smart pointers for them in WebKit: GRefPtr, which refs/unref using RAII, > and GUniquePtr, which frees GLib non-refcounted objects. > > We use both GRefPtr/GUniquePtr and raw pointers, but the meaning of each is > different: > > GRefPtr and GUniquePtr mean the code owns a reference, and will unref/free > when the variable goes out of scope. Raw pointer means the code is borrowing > a reference. The code will do something with the object, then leave it, > without establishing ownership of a reference to it. This is especially the > case for const raw pointers, which you are meant to use within scope and are > not allowed to free() them, as you are just borrowing them. > That’s also the semantics of Ref/RefPtr in WebKit. But we’re expanding the use of Ref/RefPtr to be beyond just owners for memory safety. I don’t see how the situation is any different with GRefPtr/GUniquePtr. If an explicit ownership isn’t appropriate, then CheckedPtr/CheckedRef should be used instead. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
Hm, I see you're right because CheckedRef/CheckedPtr are intrusive pointers that require modification of the type to be pointed to, so they are not going to work in general as they cannot be used for types that do not inherit from CanMakeCheckedRef/CanMakeCheckedPtr. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
On 12/01/2023 06.21, Ryosuke Niwa via webkit-dev wrote: I suggest we *stop using raw pointers and references in any local or heap stored variable*. I don't think this is feasible for code using non-WebKit libraries, and would expect it to not apply to such cases. For instance, many GLib and GStreamer objects are refcounted, and we even have smart pointers for them in WebKit: GRefPtr, which refs/unref using RAII, and GUniquePtr, which frees GLib non-refcounted objects. We use both GRefPtr/GUniquePtr and raw pointers, but the meaning of each is different: GRefPtr and GUniquePtr mean the code owns a reference, and will unref/free when the variable goes out of scope. Raw pointer means the code is borrowing a reference. The code will do something with the object, then leave it, without establishing ownership of a reference to it. This is especially the case for const raw pointers, which you are meant to use within scope and are not allowed to free() them, as you are just borrowing them. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
Is the idea that a checker failure should block landing a patch, or just that it should start a conversation with your patch reviewer about why this case is an exception? Maybe the ’should’ clause in the error message could clarify. In general, I do think it’s helpful to flag any new raw pointer usage in a member variable. But I also agree that it might not be practical to get that number to true zero. Thanks, Geoff > On Jan 21, 2023, at 12:12 PM, Michael Catanzaro via webkit-dev > wrote: > > On Fri, Jan 20 2023 at 06:15:38 PM -0800, Ryosuke Niwa via webkit-dev > wrote: >> Here´s a PR to make the style checker look for raw pointers & references in >> data members: https://github.com/WebKit/WebKit/pull/8907 >> I suggest we land this PR in 5 business days from now on unless someone >> objects. > > I'm not sure how this would work as a style check rule since there's always > going to be exceptions. E.g. we probably don't want to convert GObject priv > pointers to use CheckedRef: that would be silly. > > I think we can follow this rule in most of WebCore and maybe most of WebKit > and WTF as well. Probably not going to work for bmalloc. Not sure about JSC. > > Michael > > > ___ > 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
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
On Fri, Jan 20 2023 at 06:15:38 PM -0800, Ryosuke Niwa via webkit-dev wrote: Here’s a PR to make the style checker look for raw pointers & references in data members: https://github.com/WebKit/WebKit/pull/8907 I suggest we land this PR in 5 business days from now on unless someone objects. I'm not sure how this would work as a style check rule since there's always going to be exceptions. E.g. we probably don't want to convert GObject priv pointers to use CheckedRef: that would be silly. I think we can follow this rule in most of WebCore and maybe most of WebKit and WTF as well. Probably not going to work for bmalloc. Not sure about JSC. Michael ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
> On Jan 13, 2023, at 11:06 AM, Ryosuke Niwa via webkit-dev > wrote: > >> On Jan 13, 2023, at 6:39 AM, Darin Adler wrote: >> >> I don’t think I can notice these patterns. I would be able to program this >> way if I had an analysis tool, but otherwise this simply seems too >> complicated. > > Yeah, I agree. > >> I can’t see the types of intermediate values to know if they are CheckedPtr >> or RefPtr or raw pointers or whatever. A variation on the first line in >> Element::clientLeft is a good example: >> >> document()->updateLayoutIgnorePendingStylesheets(); >> >> That looks perfectly good to me, nothing makes me think “the result of >> document is an unsafe reference to a heap-allocated object and only trivial >> functions can be called on that”; I should not need to be an expert on each >> function to be able to tell if code is correct or not. How can we choose a >> programming style where something simple like that is subtly wrong and >> dangerous and requires reviewers to look out for it? >> >> I understand that if properly supported by a tool we could adapt to this and >> write code a whole new way. Moving to this style without a tool would almost >> literally require me to stop working on WebKit because I couldn’t correctly >> write or review even a short function. > > I acknowledge that we can’t move to this new model today. So what I’m > suggesting is to adopt a smaller scope now: Use smart pointers in all local > variables and heap allocated values. This will not be 100% security proof > (because of the function return values) but it’s a much better point to get > to than where we are now. Here’s a PR to make the style checker look for raw pointers & references in data members: https://github.com/WebKit/WebKit/pull/8907 I suggest we land this PR in 5 business days from now on unless someone objects. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
> On Jan 13, 2023, at 6:39 AM, Darin Adler wrote: > > I don’t think I can notice these patterns. I would be able to program this > way if I had an analysis tool, but otherwise this simply seems too > complicated. Yeah, I agree. > I can’t see the types of intermediate values to know if they are CheckedPtr > or RefPtr or raw pointers or whatever. A variation on the first line in > Element::clientLeft is a good example: > > document()->updateLayoutIgnorePendingStylesheets(); > > That looks perfectly good to me, nothing makes me think “the result of > document is an unsafe reference to a heap-allocated object and only trivial > functions can be called on that”; I should not need to be an expert on each > function to be able to tell if code is correct or not. How can we choose a > programming style where something simple like that is subtly wrong and > dangerous and requires reviewers to look out for it? > > I understand that if properly supported by a tool we could adapt to this and > write code a whole new way. Moving to this style without a tool would almost > literally require me to stop working on WebKit because I couldn’t correctly > write or review even a short function. I acknowledge that we can’t move to this new model today. So what I’m suggesting is to adopt a smaller scope now: Use smart pointers in all local variables and heap allocated values. This will not be 100% security proof (because of the function return values) but it’s a much better point to get to than where we are now. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
I don’t think I can notice these patterns. I would be able to program this way if I had an analysis tool, but otherwise this simply seems too complicated. I can’t see the types of intermediate values to know if they are CheckedPtr or RefPtr or raw pointers or whatever. A variation on the first line in Element::clientLeft is a good example: document()->updateLayoutIgnorePendingStylesheets(); That looks perfectly good to me, nothing makes me think “the result of document is an unsafe reference to a heap-allocated object and only trivial functions can be called on that”; I should not need to be an expert on each function to be able to tell if code is correct or not. How can we choose a programming style where something simple like that is subtly wrong and dangerous and requires reviewers to look out for it? I understand that if properly supported by a tool we could adapt to this and write code a whole new way. Moving to this style without a tool would almost literally require me to stop working on WebKit because I couldn’t correctly write or review even a short function. — Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
> On Jan 12, 2023, at 9:05 PM, Ryosuke Niwa via webkit-dev
> wrote:
>
>
>> On Jan 12, 2023, at 8:37 PM, Darin Adler wrote:
>>
>>> On Jan 12, 2023, at 9:35 PM, Ryosuke Niwa wrote:
>>>
>>> One alternative is to make bar() return RefPtr although that would be a bit
>>> heavy handed in the case of trivial function calls like this:
>>> document().frame()->ownerElement()
>>
>> I don’t quite follow. You just said that all arguments including this need
>> to have RefPtr or something like it.
>
> Right. The caller should keep an object alive.
>
>> What makes it OK to call ownerElement without a RefPtr? What is a “trivial
>> function” and how can we tell which functions are the trivial ones?
>
> We made an exception to “trivial functions” to allow chaining like the one
> above possible. A trivial function is any inlined accessor of a member
> variable the form: Document& document() { return m_document.get(); } or
> Frame* frame() { return m_frame.get(); }
>
> Anything more complicated than that, or for any out-of-lined functions, we
> should be storing “this” on a smart pointer type.
>
>
> Here’s an example:
> static void printNavigationErrorMessage(Frame& frame, const URL& activeURL,
> const char* reason)
> {
> String message = "Unsafe JavaScript attempt to initiate navigation for
> frame with URL '" + frame.document()->url().string() + "' from frame with URL
> '" + activeURL.string() + "'. " + reason + "\n";
>
> // FIXME: should we print to the console of the document performing the
> navigation instead?
> frame.document()->domWindow()->printErrorMessage(message);
> }
>
> This function, under our rule, should be rewritten like this:
> static void printNavigationErrorMessage(Frame& frame, const URL& activeURL,
> const char* reason)
> {
> String message = "Unsafe JavaScript attempt to initiate navigation for
> frame with URL '" + frame.document()->url().string() + "' from frame with URL
> '" + activeURL.string() + "'. " + reason + "\n";
>
> // FIXME: should we print to the console of the document performing the
> navigation instead?
> RefPtr { frame.document()->domWindow() }->printErrorMessage(message);
> }
>
> Here, it’s okay to call document() without first storing “frame” in a smart
> pointer because it is a function argument. By transitive property, the caller
> of this function should be keeping the frame object alive. It’s okay to call
> domWindow() because it’s a trivial accessor function of the form:
>
> DOMWindow* domWindow() const { return m_domWindow.get(); }
>
> But it’s not okay to call printErrorMessage without first storing DOMWindow
> in a RefPtr because it’s a non-trivial function, and domWindow() was not an
> argument to this function so there is no caller to guarantee the lifetime of
> it.
>
>
> Here’s another example:
> int Element::clientLeft()
> {
> document().updateLayoutIgnorePendingStylesheets();
>
> if (auto* renderer = renderBox()) {
> auto clientLeft = LayoutUnit { roundToInt(renderer->clientLeft()) };
> return
> convertToNonSubpixelValue(adjustLayoutUnitForAbsoluteZoom(clientLeft,
> *renderer).toDouble());
> }
> return 0;
> }
>
> This function should be rewritten to something like this assuming
> RenderObject is updated to support CheckedPtr:
> int Element::clientLeft()
> {
> Ref { document() }->updateLayoutIgnorePendingStylesheets();
>
> if (CheckedPtr renderer = renderBox()) {
> auto clientLeft = LayoutUnit { roundToInt(renderer->clientLeft()) };
> return
> convertToNonSubpixelValue(adjustLayoutUnitForAbsoluteZoom(clientLeft,
> *renderer).toDouble());
> }
> return 0;
> }
>
> Here, we must store document() in Ref before calling
> updateLayoutIgnorePendingStylesheets because
> updateLayoutIgnorePendingStylesheets is a non-trivial function, and
> document() is not one of the arguments. Similarly, we need to store
> renderBox() in a smart pointer since clientLeft() is a non-trivial function.
> But we don’t have to store “this” in Ref/RefPtr before calling
> convertToNonSubpixelValue because “this” is an implicit function argument.
Oops, correction. convertToNonSubpixelValue is a static function which takes
double & LegacyCSSOMElementMetricsRoundingStrategy so this is irrelevant there.
- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
> On Jan 12, 2023, at 8:37 PM, Darin Adler wrote:
>
>> On Jan 12, 2023, at 9:35 PM, Ryosuke Niwa wrote:
>>
>> One alternative is to make bar() return RefPtr although that would be a bit
>> heavy handed in the case of trivial function calls like this:
>> document().frame()->ownerElement()
>
> I don’t quite follow. You just said that all arguments including this need to
> have RefPtr or something like it.
Right. The caller should keep an object alive.
> What makes it OK to call ownerElement without a RefPtr? What is a “trivial
> function” and how can we tell which functions are the trivial ones?
We made an exception to “trivial functions” to allow chaining like the one
above possible. A trivial function is any inlined accessor of a member variable
the form: Document& document() { return m_document.get(); } or Frame* frame() {
return m_frame.get(); }
Anything more complicated than that, or for any out-of-lined functions, we
should be storing “this” on a smart pointer type.
Here’s an example:
static void printNavigationErrorMessage(Frame& frame, const URL& activeURL,
const char* reason)
{
String message = "Unsafe JavaScript attempt to initiate navigation for
frame with URL '" + frame.document()->url().string() + "' from frame with URL
'" + activeURL.string() + "'. " + reason + "\n";
// FIXME: should we print to the console of the document performing the
navigation instead?
frame.document()->domWindow()->printErrorMessage(message);
}
This function, under our rule, should be rewritten like this:
static void printNavigationErrorMessage(Frame& frame, const URL& activeURL,
const char* reason)
{
String message = "Unsafe JavaScript attempt to initiate navigation for
frame with URL '" + frame.document()->url().string() + "' from frame with URL
'" + activeURL.string() + "'. " + reason + "\n";
// FIXME: should we print to the console of the document performing the
navigation instead?
RefPtr { frame.document()->domWindow() }->printErrorMessage(message);
}
Here, it’s okay to call document() without first storing “frame” in a smart
pointer because it is a function argument. By transitive property, the caller
of this function should be keeping the frame object alive. It’s okay to call
domWindow() because it’s a trivial accessor function of the form:
DOMWindow* domWindow() const { return m_domWindow.get(); }
But it’s not okay to call printErrorMessage without first storing DOMWindow in
a RefPtr because it’s a non-trivial function, and domWindow() was not an
argument to this function so there is no caller to guarantee the lifetime of it.
Here’s another example:
int Element::clientLeft()
{
document().updateLayoutIgnorePendingStylesheets();
if (auto* renderer = renderBox()) {
auto clientLeft = LayoutUnit { roundToInt(renderer->clientLeft()) };
return
convertToNonSubpixelValue(adjustLayoutUnitForAbsoluteZoom(clientLeft,
*renderer).toDouble());
}
return 0;
}
This function should be rewritten to something like this assuming RenderObject
is updated to support CheckedPtr:
int Element::clientLeft()
{
Ref { document() }->updateLayoutIgnorePendingStylesheets();
if (CheckedPtr renderer = renderBox()) {
auto clientLeft = LayoutUnit { roundToInt(renderer->clientLeft()) };
return
convertToNonSubpixelValue(adjustLayoutUnitForAbsoluteZoom(clientLeft,
*renderer).toDouble());
}
return 0;
}
Here, we must store document() in Ref before calling
updateLayoutIgnorePendingStylesheets because
updateLayoutIgnorePendingStylesheets is a non-trivial function, and document()
is not one of the arguments. Similarly, we need to store renderBox() in a smart
pointer since clientLeft() is a non-trivial function. But we don’t have to
store “this” in Ref/RefPtr before calling convertToNonSubpixelValue because
“this” is an implicit function argument.
- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
> On Jan 12, 2023, at 9:35 PM, Ryosuke Niwa wrote: > > One alternative is to make bar() return RefPtr although that would be a bit > heavy handed in the case of trivial function calls like this: > document().frame()->ownerElement() I don’t quite follow. You just said that all arguments including this need to have RefPtr or something like it. What makes it OK to call ownerElement without a RefPtr? What is a “trivial function” and how can we tell which functions are the trivial ones? — Darin___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
> On Jan 12, 2023, at 6:50 PM, Michael Catanzaro wrote:
>
> On Thu, Jan 12 2023 at 12:35:09 PM -0800, Ryosuke Niwa via webkit-dev
> wrote:
>> So… instead of:
>> foo(bar());
>> do:
>> foo(RefPtr { bar() }.get());
>
> What's the value of creating a temporary RefPtr just to get at the underlying
> raw pointer? Isn't this overkill?
The benefit is that bar() will be kept alive while the duration of call to foo.
Without, whatever bar() returns can be dead before foo() finishes running,
which can result in use-after-free.
An obvious alternative is to use smart pointer types on each function argument.
But this has a few drawbacks:
The same rule can’t be applied to “this” since passing of “this" pointer is
implicit in C++.
Ref churn when multiple functions are called with the same object; e.g.
foo = foo()
bar(foo); // ref/deref here
baz(foo); // ref/deref here again
Ref churn when a function argument is passed to another function; e.g.
void foo(RefPtr&& obj)
{
bar(obj); // ref/deref here even though obj is guaranteed to be alive
throughout this function
}
- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
On Thu, Jan 12 2023 at 12:35:09 PM -0800, Ryosuke Niwa via webkit-dev
wrote:
So… instead of:
foo(bar());
do:
foo(RefPtr { bar() }.get());
What's the value of creating a temporary RefPtr just to get at the
underlying raw pointer? Isn't this overkill?
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
> On Jan 12, 2023, at 1:27 PM, Darin Adler wrote:
>
>> On Jan 12, 2023, at 3:35 PM, Ryosuke Niwa via webkit-dev
>> wrote:
>>
>>> On Jan 12, 2023, at 6:13 AM, Darin Adler wrote:
>>>
On Jan 12, 2023, at 12:21 AM, Ryosuke Niwa via webkit-dev
wrote:
assuming every local variable / variable in stack is stored in a smart
pointer, function arguments are safe to be raw pointers / references via
transitive property
>>>
>>> What about the case where the function argument is the return value from
>>> another function?
>>
>> In those cases, the value should be stored in a local variable using a smart
>> pointer first.
>>
>> So… instead of:
>> foo(bar());
>>
>> do:
>> foo(RefPtr { bar() }.get());
>
> This seems impractical. How will I remember to do this?
The smart pointer analysis tool I've been working with the clang team should be
able to tell you that once it becomes available. Until then, we probably need
to rely on reviewers to notice these patterns.
One alternative is to make bar() return RefPtr although that would be a bit
heavy handed in the case of trivial function calls like this:
document().frame()->ownerElement()
- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
> On Jan 12, 2023, at 3:35 PM, Ryosuke Niwa via webkit-dev
> wrote:
>
>> On Jan 12, 2023, at 6:13 AM, Darin Adler wrote:
>>
>>> On Jan 12, 2023, at 12:21 AM, Ryosuke Niwa via webkit-dev
>>> wrote:
>>>
>>> assuming every local variable / variable in stack is stored in a smart
>>> pointer, function arguments are safe to be raw pointers / references via
>>> transitive property
>>
>> What about the case where the function argument is the return value from
>> another function?
>
> In those cases, the value should be stored in a local variable using a smart
> pointer first.
>
> So… instead of:
> foo(bar());
>
> do:
> foo(RefPtr { bar() }.get());
This seems impractical. How will I remember to do this?
— Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
> On Jan 12, 2023, at 6:13 AM, Darin Adler wrote:
>
>> On Jan 12, 2023, at 12:21 AM, Ryosuke Niwa via webkit-dev
>> wrote:
>>
>> assuming every local variable / variable in stack is stored in a smart
>> pointer, function arguments are safe to be raw pointers / references via
>> transitive property
>
> What about the case where the function argument is the return value from
> another function?
In those cases, the value should be stored in a local variable using a smart
pointer first.
So… instead of:
foo(bar());
do:
foo(RefPtr { bar() }.get());
- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Stop Using Raw Pointers & References in New Code
> On Jan 12, 2023, at 12:21 AM, Ryosuke Niwa via webkit-dev > wrote: > > assuming every local variable / variable in stack is stored in a smart > pointer, function arguments are safe to be raw pointers / references via > transitive property What about the case where the function argument is the return value from another function? — Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
