On Mon, Jul 22, 2013 at 12:46 PM, Benjamin Smedberg <benja...@smedbergs.us>wrote:
> On 7/22/2013 3:01 PM, Rob Arnold wrote: > >> >> Wouldn't disallowing this implicit conversion break code which does >> >> void UseFoo(nsIFoo* foo); >> >> nsCOMPtr<nsIFoo> foo; >> UseFoo(foo); >> >> ? That is an extremely common idiom in our code. >> >> That idiom seems dangerous by itself; the assumption it makes is that the >> reference will outlive the callee but that isn't actually enforced >> anywhere >> > Doesn't the nsCOMPtr<nsIFoo> foo; enforce it? I'm confused by what you're > arguing. In this case, yes. I'm saying that there are others where this cannot be enforced (see below). > (you could write UseFoo(nsRefPtr<T>(new T)) and Bad Things would happen; >> > What would be bad about it? The T stays alive during the call to UseFoo. 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. > there are less obvious ways to write this). I would rewrite UseFoo to take >> an const nsRefPtr<nsIFoo>& instead >> > Except that you'd basically be rewriting our entire tree, including all of > XPIDL. Let's not try to boil that ocean for marginal benefit. > 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. Thus, I am a bit skeptical that this change requires an ocean boil, but I am not the one doing the work so really it's just an armchair opinion (I did a similar migration on a 30kloc codebase without significant issues and Mozilla has better test coverage). I think improved memory safety and lower cognitive overhead for reasoning about the safety of the code is an important, not marginal, benefit for both security vulnerabilities and the project's hackability (in the code contribution sense). -Rob _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform