On 7/22/2013 6:05 PM, Rob Arnold wrote:
Ah, you're right. Bad example. 'UseFoo(new T)' compiles but leaks,
'nsRefPtr<T> foo() { nsRefPtr<T> bar(...); ...; return bar; }' is a
use-after-tree in cases like 'T* p = foo();' and there are probably a
number of other cases that the compiler allows and/or writes for you where
issues can arise.
The end goal that I am promoting is to never deal with raw pointers because
the ownership semantics aren't clear; instead, shove the pointer into a
strong container like RefPtr or std::unique_ptr and let the strong
containers/compiler handle the safety. I know that with earlier
compilers/editions of C++, it was not possible to do this efficiently, but
I think it is worth revisiting this in a discussion of modernizing the
codebase with C++11. I believe that it's mozilla style to never return an
nsRefPtr from a function/method but with C++11 move semantics and
optimizations like RVO, you can do this efficiently in a number of common
cases.
I understand the goal. I'm not sure I agree with it. Bare pointers are a
way of life in C/C++. We can and continue to abstract away some aspects
of them. But every smart pointer class adds cost in a bunch of different
ways:
* compilation time: the compiler takes a lot longer to inline
smart-pointer classes than raw pointers
* cognitive overhead: this can vary depending on whether we use C++
standard types or Mozilla-specific types, but everyone learning the
codebase needs to learn how to use each new container class, and that
learning curve is hard.
* debugging complexity: tooling can help with this a lot, but we don't
even have any/good debugger visualizers or shortcuts for our existing
comptr/refptr classes, so you have to jump in using .mRawPtr in order to
see them in a debugger.
* Invisible behaviors: a key required optimization is avoiding
refcounting when passing pointers around. As noted in the thread, we can
solve this using class patterns. But you not only need to make sure that
your patterns are correct, but you need to make sure that coders and
reviewers can actually verify that they are doing the correct thing in
each case. It's easy to die a death by a thousand unnecessary refcounts
which end up costing 10% or more on important benchmarks.
Given these costs, how do we decide whether the end-state of having few
raw pointers is worth it? Especially while we're doing the rust/servo
research on a memory-safe language which could moot the entire discussion.
If there are common patterns of unsafe code which we can identify as a
result of this thread, we can instead of rewriting the entire tree to
use refpointers write analysis passes for the particular unsafe code
patterns. I'm particularly interested in the "calling a method on a
member pointer which might change as a result of a nested call"
analysis, since we've hit that a number of times in plugin-land and it
doesn't sound like an analysis which would be too hard to write or cause
too many false positives.
If we can solve most of this problem with a few targeted analyses, we
should do that instead.
I haven't been very involved in Mozilla development for a while but my
understanding based on when I stopped contributing code is that a) we've
done large code modifications before, b) our static analysis tools make it
easier to maintain these and c) I am optimistic that these changes can be
done incrementally if need be.
Our static tools were pretty good at analysis, but only one tool for
rewriting (outparamdel) was ever useful, and most of that framework is
no longer usable. All of our recent treewide changes have been done
using grep/sed and a little bit of scripted reindentation. That's not
going to work for a pointer-type change of this sort.
Rewriting by hand or machine can work if you can do individual
signatures at a time, but given the way our signatures are often
dictated by XPIDL, doing a refactoring of this sort is still a pretty
huge undertaking. The opportunity cost is high, and overall I don't
think the change is worth it at least until we can make a definitive
decision about rust/servo.
--BDS
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
