Open
Bug 1506441
Opened 5 years ago
Updated 8 months ago
MOZ_CAN_RUN_SCRIPT may require unnecessary refcount if a method returns pointer to itself as different type
Categories
(Developer Infrastructure :: Source Code Analysis, enhancement, P4)
Developer Infrastructure
Source Code Analysis
Tracking
(Not tracked)
NEW
People
(Reporter: masayuki, Unassigned)
References
(Blocks 1 open bug)
Details
In some classes, we have methods as following:
> Bar* Foo::AsBar() const { return static_cast<Bar*>(this); }
If Foo (and Bar) is refcountable class and you do like as following, you'd see compile error, "arguments must all be strong refs or parent parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument)".
class Foo {
Bar* AsBar() const { return IsBar() ? static_cast<Bar*>(this) : nullptr; }
};
class Baz {
RefPtr<Foo> mFoo;
MOZ_CAN_RUN_SCRIPT void DoSomethingDangerous()
{
mFoo->AsBar()->DoSomethingDangerous();
}
This method needs to be written as:
{
RefPtr<Bar> bar = mFoo->AsBar();
bar->DoSomethingDangerous();
}
This redundant adding-reference may make damage to the performance if it's in a hot path.
So, perhaps, there is another attribute for mFoo::AsBar() which indicates that when it returns non-nullptr, it's point to same instance and MOZ_CAN_RUN_SCRIPT check should check whether the mFoo is grabbed with RefPtr or nsCOMPtr, or not.
Reporter | ||
Comment 1•4 years ago
|
||
If we won't add a template class like already_AddRefed
, using MOZ_KnownLive()
in each caller is the solution.
What do you think, bz?
Flags: needinfo?(bzbarsky)
![]() |
||
Comment 2•4 years ago
|
||
We can use MOZ_KnownLive for now, yes, but it would be better if this were handled automatically somehow. The problem is that automatic handling would need to catch cases like this:
auto* bar = foo->AsBar();
foo = baz;
bar->DoSomethingDangerous();
with the "foo = baz" possibly hidden inside other function calls, etc.
I don't have any bright ideas for automating this so far.
Flags: needinfo?(bzbarsky)
Updated•2 years ago
|
Priority: -- → P4
Updated•10 months ago
|
Product: Firefox Build System → Developer Infrastructure
Updated•8 months ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•