std::unique_ptr, std::move, and std::forward (was Re: Using C++0x auto)
On Fri, Jul 19, 2013 at 4:46 AM, Mike Hommey m...@glandium.org wrote: Note that STL is another story. We're not using libstdc++ that comes with the compiler on android and b2g. We use STLport instead, and STLport has, afaik, no support for C++11 STL types. So, while we can now fix nsAutoPtr to use move semantics instead of copy semantics, we can't use std::unique_ptr. I saw bug 896100 [1] wants to add mozilla::Move and mozilla::Forward. Obviously, that is a clear improvement that we can build on. But, shouldn't we just name these std::move and std::forward and use these implementations only when we're using STLPort? I know we're not supposed to add stuff to the std:: namespace normally, but that's exactly what STLPort is doing. And, more to the point, shouldn't we just add std::unique_ptr to STLPort for Android so we can use std::unique_ptr everywhere? And/or just backport the libstdc++ version to GCC 4.4. Isn't it all just templates? Cheers, Brian [1] https://bugzilla.mozilla.org/show_bug.cgi?id=896100 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: std::unique_ptr, std::move, and std::forward (was Re: Using C++0x auto)
On Tue, Jul 30, 2013 at 7:40 PM, Brian Smith br...@briansmith.org wrote: On Fri, Jul 19, 2013 at 4:46 AM, Mike Hommey m...@glandium.org wrote: Note that STL is another story. We're not using libstdc++ that comes with the compiler on android and b2g. We use STLport instead, and STLport has, afaik, no support for C++11 STL types. So, while we can now fix nsAutoPtr to use move semantics instead of copy semantics, we can't use std::unique_ptr. I saw bug 896100 [1] wants to add mozilla::Move and mozilla::Forward. Obviously, that is a clear improvement that we can build on. Agreed. But, shouldn't we just name these std::move and std::forward and use these implementations only when we're using STLPort? I know we're not supposed to add stuff to the std:: namespace normally, but that's exactly what STLPort is doing. We've avoided doing this so far in MFBT for everything in the language or the standard library that we had to reimplement ourselves. I'm not aware of any practical problems in putting things in the std namespace (besides watching out for name clashes, which most standard library implementations avoid by either using nested namespaces for their implementation helpers, or symbols with underscore at the beginning of their name which are supposed to be reserved for implementations -- but real code in the while violates that all the time.) But it still feels a bit unclean to put things into namespace std. Is there a good reason why we should do that in this case? (Also remember that STLport is an STL implementation, so it is entirely ok for them to put things into namespace std!) And, more to the point, shouldn't we just add std::unique_ptr to STLPort for Android so we can use std::unique_ptr everywhere? And/or just backport the libstdc++ version to GCC 4.4. Isn't it all just templates? I believe that std::unique_ptr can be implemented in C++ without any compiler magics, and this sounds like a good idea (but I still think we should call ours mozilla::UniquePtr.) Cheers, -- Ehsan http://ehsanakhgari.org/ ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: std::unique_ptr, std::move, and std::forward (was Re: Using C++0x auto)
On Tue, Jul 30, 2013 at 10:50:39PM -0400, Ehsan Akhgari wrote: On Tue, Jul 30, 2013 at 7:40 PM, Brian Smith br...@briansmith.org wrote: On Fri, Jul 19, 2013 at 4:46 AM, Mike Hommey m...@glandium.org wrote: Note that STL is another story. We're not using libstdc++ that comes with the compiler on android and b2g. We use STLport instead, and STLport has, afaik, no support for C++11 STL types. So, while we can now fix nsAutoPtr to use move semantics instead of copy semantics, we can't use std::unique_ptr. I saw bug 896100 [1] wants to add mozilla::Move and mozilla::Forward. Obviously, that is a clear improvement that we can build on. Agreed. But, shouldn't we just name these std::move and std::forward and use these implementations only when we're using STLPort? I know we're not supposed to add stuff to the std:: namespace normally, but that's exactly what STLPort is doing. We've avoided doing this so far in MFBT for everything in the language or the standard library that we had to reimplement ourselves. I'm not aware of any practical problems in putting things in the std namespace (besides watching out for name clashes, which most standard library implementations avoid by either using nested namespaces for their implementation helpers, or symbols with underscore at the beginning of their name which are supposed to be reserved for implementations -- but real code in the while violates that all the time.) But it still feels a bit unclean to put things into namespace std. Is there a good reason why we should do that in this case? (Also remember that STLport is an STL implementation, so it is entirely ok for them to put things into namespace std!) Note that if STLport is the only thing preventing us from using some C++11 std:: features, we can implement them and contribute them upstream. That being said, it does look like the STLport git has unique_ptr, forward and move. It's not part of any release, though, and I'm not sure how good the current trunk is. The version we're currently using is 5.2.1 + some Android patches (pristine from the android NDK) Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: std::unique_ptr, std::move, and std::forward (was Re: Using C++0x auto)
On Wed, Jul 31, 2013 at 4:50 AM, Ehsan Akhgari ehsan.akhg...@gmail.comwrote: On Tue, Jul 30, 2013 at 7:40 PM, Brian Smith br...@briansmith.orgwrote: But, shouldn't we just name these std::move and std::forward and use these implementations only when we're using STLPort? I know we're not supposed to add stuff to the std:: namespace normally, but that's exactly what STLPort is doing. We've avoided doing this so far in MFBT for everything in the language or the standard library that we had to reimplement ourselves. I'm not aware of any practical problems in putting things in the std namespace (besides watching out for name clashes, which most standard library implementations avoid by either using nested namespaces for their implementation helpers, or symbols with underscore at the beginning of their name which are supposed to be reserved for implementations -- but real code in the while violates that all the time.) But it still feels a bit unclean to put things into namespace std. Is there a good reason why we should do that in this case? Yes: Then we can use std::unique_ptr in parts of Gecko that are intended to be buildable without MFBT (see below). (Also remember that STLport is an STL implementation, so it is entirely ok for them to put things into namespace std!) To be clear, I am not proposing that we add std::move/forward/unique_ptr to MFBT. I am suggesting that we add them to STLPort. We could even eventually upstream them. EDIT: I just saw Mike's post that STLPort upstream already has unique_ptr/move/forward. Perhaps we can backport them into our STLPort tree. FWIW, we have created a new certificate verification library written in C++. One of my goals is to eventually make it so that it can be embedded in server-side software to support things like OCSP stapling, short-lived certificates, and auto-fixing of certificate chains in servers, which are things that make SSL faster and easier to deploy. Basically, the idea is that the server can (pre-)build/verify their certificate chain exactly as Firefox would. There are also some security researchers interested in using a real browser's certificate processing logic in studies they are doing. This kind of research directly benefits my work on Gecko and I'm intending to share this library with them so they can use it in their research. For this sub-project, I've been trying to avoid any Gecko (including MFBT) dependencies and I will be cutting down (removing?) the NSPR and NSS dependencies over time. In order to avoid the MFBT dependency, I created my own ScopedPtr class and cviecco added a hack for GCC 4.4's nullptr. We also have been doing the typical hacky/dangerous stuff to deal with a world without std::move()/forward() and without cstdint. Now we can use cstdint (or at least stdint.h?), and I'm eager to fix these last mile issues. Besides that, in general I'd like to continue making Gecko's code less foreign to C++ coders. In particular, I'd like to get rid of nsAutoPtrT and mozilla::ScopedPtrT completely. Cheers, Brian ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Mon, Jul 22, 2013 at 10:01 PM, Rob Arnold tell...@gmail.com wrote: 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 (you could write UseFoo(nsRefPtrT(new T)) and Bad Things would happen; there are less obvious ways to write this). I would rewrite UseFoo to take an const nsRefPtrnsIFoo instead and then you don't have to worry about these issues. It is dangerous. See discussion here, which Ralph Giles linked to: https://bugzilla.mozilla.org/show_bug.cgi?id=767178 In particular, I've fixed a crash bug and a security bug caused by this idiom, and I'm sure there are many more like them. (Although getting rid of mutation events would help a lot!) However, just making the argument be a refptr means the function call does addref and release on every call even if the caller is already holding a reference, which has been deemed unacceptable from a perf standpoint. A couple of suggestions were made in the linked-to bug to ameliorate the problem, but nobody has come up with a scheme that gives the same safety as nsRefPtr-as-param-type without unacceptable runtime cost. Most of the problem could be solved if you required that nsCOMPtr/nsRefPtr only be used as a local variable (in particular, not member variables) and introduced a type COMParam/RefParam or something that implicitly converted from nsCOMPtr/nsRefPtr without addref/release but didn't convert implicitly from raw pointer at all, so the caller would have to wrap it in something that did the addref/release. Then -- in theory -- the caller should hold a reference throughout the function call, unless you do something really obnoxious like store a non-const reference to the local variable somewhere that the callee can get at. Callees that were really super duper sure they didn't need a strong reference could still use the raw pointer for parameters if they wanted to save the addref/release. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
It seems really dangerous that there is an implicit conversion from a strong ref ptr to a weak pointer. With C++11, you can thankfully require this conversion to be explicit which should alleviate your concern. Wouldn't disallowing this implicit conversion break code which does void UseFoo(nsIFoo* foo); nsCOMPtrnsIFoo foo; UseFoo(foo); ? That is an extremely common idiom in our code. Like I say, maybe there's a way to allow lvalue nsRefPtrT to convert to T*, but disallow the conversion for rvalue nsRefPtrT. That would be reasonable, I think. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
Robert O'Callahan wrote: On Mon, Jul 22, 2013 at 3:58 PM, L. David Baron dba...@dbaron.org wrote: Is the idea here that nsRefPtr/nsCOMPtr/etc. would have move constructors, and we'd just return them, and the move constructors plus return value optimizations would take care of avoiding excess reference counting? That, plus forget() returns a nsRefPtr. Historically people used refptr.forget().get(); to get a raw addrefed pointer. MXR suggests that we still have 72 uses like this. (But as an interim stage I assume you could keep the addrefed wrapper type and define a move constructor from it to refptr.) -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 2013-07-22 2:59 AM, Justin Lebar wrote: It seems really dangerous that there is an implicit conversion from a strong ref ptr to a weak pointer. With C++11, you can thankfully require this conversion to be explicit which should alleviate your concern. Wouldn't disallowing this implicit conversion break code which does void UseFoo(nsIFoo* foo); nsCOMPtrnsIFoo foo; UseFoo(foo); ? That is an extremely common idiom in our code. Yeah... Like I say, maybe there's a way to allow lvalue nsRefPtrT to convert to T*, but disallow the conversion for rvalue nsRefPtrT. That would be reasonable, I think. That's not possible in C++11. Thinking about this more, the reason that this works fine for std::shared_ptr is the fact that it does _not_ have a T* implicit conversion operator. Therefore, we can't switch to rvalue references to replace already_AddRefed. :( Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Sun, Jul 21, 2013 at 11:59 PM, Justin Lebar justin.le...@gmail.comwrote: It seems really dangerous that there is an implicit conversion from a strong ref ptr to a weak pointer. With C++11, you can thankfully require this conversion to be explicit which should alleviate your concern. Wouldn't disallowing this implicit conversion break code which does void UseFoo(nsIFoo* foo); nsCOMPtrnsIFoo 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 (you could write UseFoo(nsRefPtrT(new T)) and Bad Things would happen; there are less obvious ways to write this). I would rewrite UseFoo to take an const nsRefPtrnsIFoo instead and then you don't have to worry about these issues. -Rob ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
Justin Lebar wrote: Maybe we should call ours mozilla::move and mozilla::forward so that we can change to std::move and std::forward with minimal pain? Won't that cause confusion if someone accidentally has both using namespace mozilla; and using namespace std; at the same time? -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 21/07/2013 17:56, Neil wrote:
Won't that cause confusion if someone accidentally has both using
namespace mozilla; and using namespace std; at the same time?
Possibly but our coding guidelines suggest that we avoid using ...;
statements preferring to wrap code into namespace ... { ... };
constructs. This seems to have been enforced well within our code as I
could find only 55 instances of using namespace std; in
mozilla-central and a significant number of them appear in files that we
have imported.
Gabriele
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
Maybe we should call ours mozilla::move and mozilla::forward so that we can change to std::move and std::forward with minimal pain? Won't that cause confusion if someone accidentally has both using namespace mozilla; and using namespace std; at the same time? That's a fair point. Maybe we should call ours mozilla::Move and mozilla::Forward instead. That way we can still easily change them to std::move and std::forward, but there's no possibility of a collision. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 07/21/2013 11:44 AM, Justin Lebar wrote: That's a fair point. Maybe we should call ours mozilla::Move and mozilla::Forward instead. That way we can still easily change them to std::move and std::forward, but there's no possibility of a collision. This is pretty much always what we've done with the type traits stuff, so this is what we'd do here, I think. Jeff ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Friday 2013-07-19 12:15 +1200, Robert O'Callahan wrote: On Fri, Jul 19, 2013 at 3:34 AM, Ehsan Akhgari ehsan.akhg...@gmail.comwrote: On 2013-07-18 5:48 AM, mscl...@googlemail.com wrote: r-value references 4.3@10.0! Yes This is very useful. I believe the JS engine already rolls their own tricks to implement this semantics. With this we can get rid of already_AddRefed and just pass nsRefPtr/nsCOMPtr/RefPtr around, right? Is the idea here that nsRefPtr/nsCOMPtr/etc. would have move constructors, and we'd just return them, and the move constructors plus return value optimizations would take care of avoiding excess reference counting? Or does it involve something more complicated like returning rvalue references? (Is such a thing possible?) -David -- 턞 L. David Baron http://dbaron.org/ 턂 턢 Mozilla http://www.mozilla.org/ 턂 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Mon, Jul 22, 2013 at 3:58 PM, L. David Baron dba...@dbaron.org wrote: Is the idea here that nsRefPtr/nsCOMPtr/etc. would have move constructors, and we'd just return them, and the move constructors plus return value optimizations would take care of avoiding excess reference counting? That, plus forget() returns a nsRefPtr. Rob -- Jtehsauts tshaei dS,o n Wohfy Mdaon yhoaus eanuttehrotraiitny eovni le atrhtohu gthot sf oirng iyvoeu rs ihnesa.rt sS?o Whhei csha iids teoa stiheer :p atroa lsyazye,d 'mYaonu,r sGients uapr,e tfaokreg iyvoeunr, 'm aotr atnod sgaoy ,h o'mGee.t uTph eann dt hwea lmka'n? gBoutt uIp waanndt wyeonut thoo mken.o w * * ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
AIUI the new constructor would be something like nsRefPtrT(nsRefPtrT aOther) where means r-value, which means temporary, so moveable. But I'm not totally sure about being able to return nsRefPtr. Right now, if I do already_AddRefedT GetFoo(); Foo* foo = GetFoo(); that's a compile error. In order to leak the reference, I'd have to do foo = GetFoo().forget();, which is relatively easy for reviewers to catch. But if we return nsRefPtr, then if I have nsRefPtrFoo GetFoo(); Foo* = GetFoo(); that compiles fine (I think?) because nsRefPtrT has an implicit conversion to T*. I'd wager that this is probably pretty hard for reviewers to catch; at least, I don't have confidence I'd catch this mistake. Maybe there's a way to delete the nsRefPtrT conversion to T* if the nsRefPtr is an r-value. -Justin On Sun, Jul 21, 2013 at 9:08 PM, Robert O'Callahan rob...@ocallahan.org wrote: On Mon, Jul 22, 2013 at 3:58 PM, L. David Baron dba...@dbaron.org wrote: Is the idea here that nsRefPtr/nsCOMPtr/etc. would have move constructors, and we'd just return them, and the move constructors plus return value optimizations would take care of avoiding excess reference counting? That, plus forget() returns a nsRefPtr. Rob -- Jtehsauts tshaei dS,o n Wohfy Mdaon yhoaus eanuttehrotraiitny eovni le atrhtohu gthot sf oirng iyvoeu rs ihnesa.rt sS?o Whhei csha iids teoa stiheer :p atroa lsyazye,d 'mYaonu,r sGients uapr,e tfaokreg iyvoeunr, 'm aotr atnod sgaoy ,h o'mGee.t uTph eann dt hwea lmka'n? gBoutt uIp waanndt wyeonut thoo mken.o w * * ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Mon, Jul 22, 2013 at 5:39 PM, Justin Lebar justin.le...@gmail.comwrote: But if we return nsRefPtr, then if I have nsRefPtrFoo GetFoo(); Foo* = GetFoo(); that compiles fine (I think?) because nsRefPtrT has an implicit conversion to T*. It doesn't leak, though. OTOH you probably get a use-after-free, which is worse. So I dunno, maybe this is a bad idea. Rob -- Jtehsauts tshaei dS,o n Wohfy Mdaon yhoaus eanuttehrotraiitny eovni le atrhtohu gthot sf oirng iyvoeu rs ihnesa.rt sS?o Whhei csha iids teoa stiheer :p atroa lsyazye,d 'mYaonu,r sGients uapr,e tfaokreg iyvoeunr, 'm aotr atnod sgaoy ,h o'mGee.t uTph eann dt hwea lmka'n? gBoutt uIp waanndt wyeonut thoo mken.o w * * ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Sun, Jul 21, 2013 at 10:39 PM, Justin Lebar justin.le...@gmail.comwrote: AIUI the new constructor would be something like nsRefPtrT(nsRefPtrT aOther) where means r-value, which means temporary, so moveable. Yes, this is why you want to be careful when returning them. The rules should be more conservative here than returning lvalue references. But I'm not totally sure about being able to return nsRefPtr. Right now, if I do already_AddRefedT GetFoo(); Foo* foo = GetFoo(); that's a compile error. In order to leak the reference, I'd have to do foo = GetFoo().forget();, which is relatively easy for reviewers to catch. But if we return nsRefPtr, then if I have nsRefPtrFoo GetFoo(); Foo* = GetFoo(); that compiles fine (I think?) because nsRefPtrT has an implicit conversion to T*. I'd wager that this is probably pretty hard for reviewers to catch; at least, I don't have confidence I'd catch this mistake. Maybe there's a way to delete the nsRefPtrT conversion to T* if the nsRefPtr is an r-value. It seems really dangerous that there is an implicit conversion from a strong ref ptr to a weak pointer. With C++11, you can thankfully require this conversion to be explicit which should alleviate your concern. On Sun, Jul 21, 2013 at 9:08 PM, Robert O'Callahan rob...@ocallahan.org wrote: On Mon, Jul 22, 2013 at 3:58 PM, L. David Baron dba...@dbaron.org wrote: Is the idea here that nsRefPtr/nsCOMPtr/etc. would have move constructors, and we'd just return them, and the move constructors plus return value optimizations would take care of avoiding excess reference counting? This has been my experience with RefPtrs and C++11 though I have not used it in code as performance critical as Gecko. I have built a typesafe RefPtr class with I think very similar semantics and requirements to Gecko's so I think you will be able to do the same. -Rob ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 2013-07-18 10:46 PM, Mike Hommey wrote: On Thu, Jul 18, 2013 at 08:54:02PM -0500, Joshua Cranmer ? wrote: On 7/18/2013 7:15 PM, Robert O'Callahan wrote: On Fri, Jul 19, 2013 at 3:34 AM, Ehsan Akhgari ehsan.akhg...@gmail.comwrote: On 2013-07-18 5:48 AM, mscl...@googlemail.com wrote: r-value references 4.3@10.0! Yes This is very useful. I believe the JS engine already rolls their own tricks to implement this semantics. With this we can get rid of already_AddRefed and just pass nsRefPtr/nsCOMPtr/RefPtr around, right? I believe so. We can also add a non-broken variant of nsAutoPtr modeled after std::unique_ptr (allows moves but not copies). Note that STL is another story. We're not using libstdc++ that comes with the compiler on android and b2g. We use STLport instead, and STLport has, afaik, no support for C++11 STL types. So, while we can now fix nsAutoPtr to use move semantics instead of copy semantics, we can't use std::unique_ptr. Does this mean that we can't use std::move and std::forward either? In that case, we need MFBT versions of those, because working with rvalue references without those two functions is a pain! Good news is that implementing these functions should be really easy. Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 2013-07-19 3:26 PM, Ehsan Akhgari wrote: On 2013-07-18 10:46 PM, Mike Hommey wrote: On Thu, Jul 18, 2013 at 08:54:02PM -0500, Joshua Cranmer ? wrote: On 7/18/2013 7:15 PM, Robert O'Callahan wrote: On Fri, Jul 19, 2013 at 3:34 AM, Ehsan Akhgari ehsan.akhg...@gmail.comwrote: On 2013-07-18 5:48 AM, mscl...@googlemail.com wrote: r-value references 4.3@10.0! Yes This is very useful. I believe the JS engine already rolls their own tricks to implement this semantics. With this we can get rid of already_AddRefed and just pass nsRefPtr/nsCOMPtr/RefPtr around, right? I believe so. We can also add a non-broken variant of nsAutoPtr modeled after std::unique_ptr (allows moves but not copies). Note that STL is another story. We're not using libstdc++ that comes with the compiler on android and b2g. We use STLport instead, and STLport has, afaik, no support for C++11 STL types. So, while we can now fix nsAutoPtr to use move semantics instead of copy semantics, we can't use std::unique_ptr. Does this mean that we can't use std::move and std::forward either? In that case, we need MFBT versions of those, because working with rvalue references without those two functions is a pain! Good news is that implementing these functions should be really easy. Also, is there a reason that we don't build on Android with the libstdc++ that ships with the toolchain? Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Fri, Jul 19, 2013 at 03:45:13PM -0400, Ehsan Akhgari wrote: On 2013-07-19 3:26 PM, Ehsan Akhgari wrote: On 2013-07-18 10:46 PM, Mike Hommey wrote: On Thu, Jul 18, 2013 at 08:54:02PM -0500, Joshua Cranmer ? wrote: On 7/18/2013 7:15 PM, Robert O'Callahan wrote: On Fri, Jul 19, 2013 at 3:34 AM, Ehsan Akhgari ehsan.akhg...@gmail.comwrote: On 2013-07-18 5:48 AM, mscl...@googlemail.com wrote: r-value references 4.3@10.0! Yes This is very useful. I believe the JS engine already rolls their own tricks to implement this semantics. With this we can get rid of already_AddRefed and just pass nsRefPtr/nsCOMPtr/RefPtr around, right? I believe so. We can also add a non-broken variant of nsAutoPtr modeled after std::unique_ptr (allows moves but not copies). Note that STL is another story. We're not using libstdc++ that comes with the compiler on android and b2g. We use STLport instead, and STLport has, afaik, no support for C++11 STL types. So, while we can now fix nsAutoPtr to use move semantics instead of copy semantics, we can't use std::unique_ptr. Does this mean that we can't use std::move and std::forward either? Indeed In that case, we need MFBT versions of those, because working with rvalue references without those two functions is a pain! Good news is that implementing these functions should be really easy. Also, is there a reason that we don't build on Android with the libstdc++ that ships with the toolchain? For one, the fact that it actually doesn't work currently (at least, it didn't last i heard someone tried) doesn't help. And I think that when stlport was chosen, using the GNU stl made the android package significantly larger. But then, we weren't statically linking stlport, so this may have changed. Also note that because we're building for specific targets that are not exactly those for which we have prebuilt libraries coming in the NDK, we're actually building stlport ourselves. We'd have to do the same for the GNU stl if we chose to use it, and we have no current support for that in the build system (the build system only knows how to build against the prebuilt one, and that's what doesn't even work). I don't know how easy it is to compile the GNU stl outside of building gcc. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 2013-07-19 7:04 PM, Mike Hommey wrote: On Fri, Jul 19, 2013 at 03:45:13PM -0400, Ehsan Akhgari wrote: On 2013-07-19 3:26 PM, Ehsan Akhgari wrote: On 2013-07-18 10:46 PM, Mike Hommey wrote: On Thu, Jul 18, 2013 at 08:54:02PM -0500, Joshua Cranmer ? wrote: On 7/18/2013 7:15 PM, Robert O'Callahan wrote: On Fri, Jul 19, 2013 at 3:34 AM, Ehsan Akhgari ehsan.akhg...@gmail.comwrote: On 2013-07-18 5:48 AM, mscl...@googlemail.com wrote: r-value references 4.3@10.0! Yes This is very useful. I believe the JS engine already rolls their own tricks to implement this semantics. With this we can get rid of already_AddRefed and just pass nsRefPtr/nsCOMPtr/RefPtr around, right? I believe so. We can also add a non-broken variant of nsAutoPtr modeled after std::unique_ptr (allows moves but not copies). Note that STL is another story. We're not using libstdc++ that comes with the compiler on android and b2g. We use STLport instead, and STLport has, afaik, no support for C++11 STL types. So, while we can now fix nsAutoPtr to use move semantics instead of copy semantics, we can't use std::unique_ptr. Does this mean that we can't use std::move and std::forward either? Indeed Sadface. I filed bug 896100 about this, and I'll write a patch for it soon. In that case, we need MFBT versions of those, because working with rvalue references without those two functions is a pain! Good news is that implementing these functions should be really easy. Also, is there a reason that we don't build on Android with the libstdc++ that ships with the toolchain? For one, the fact that it actually doesn't work currently (at least, it didn't last i heard someone tried) doesn't help. Heh, yeah it doesn't, indeed! And I think that when stlport was chosen, using the GNU stl made the android package significantly larger. But then, we weren't statically linking stlport, so this may have changed. Also note that because we're building for specific targets that are not exactly those for which we have prebuilt libraries coming in the NDK, we're actually building stlport ourselves. We'd have to do the same for the GNU stl if we chose to use it, and we have no current support for that in the build system (the build system only knows how to build against the prebuilt one, and that's what doesn't even work). I don't know how easy it is to compile the GNU stl outside of building gcc. Ah, I have feeling that doing that is actually very hard... Let's declare defeat on this for now (unless people want to do all of this work!) and move on with bug 896100. Once we have that, I will try to find some time to see if we can kill already_AddRefed. That is one class I won't miss. :-) Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
Maybe we should call ours mozilla::move and mozilla::forward so that we can change to std::move and std::forward with minimal pain? On Jul 19, 2013 4:36 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On 2013-07-19 7:04 PM, Mike Hommey wrote: On Fri, Jul 19, 2013 at 03:45:13PM -0400, Ehsan Akhgari wrote: On 2013-07-19 3:26 PM, Ehsan Akhgari wrote: On 2013-07-18 10:46 PM, Mike Hommey wrote: On Thu, Jul 18, 2013 at 08:54:02PM -0500, Joshua Cranmer ? wrote: On 7/18/2013 7:15 PM, Robert O'Callahan wrote: On Fri, Jul 19, 2013 at 3:34 AM, Ehsan Akhgari ehsan.akhg...@gmail.com**wrote: On 2013-07-18 5:48 AM, mscl...@googlemail.com wrote: r-value references 4.3@10.0! Yes This is very useful. I believe the JS engine already rolls their own tricks to implement this semantics. With this we can get rid of already_AddRefed and just pass nsRefPtr/nsCOMPtr/RefPtr around, right? I believe so. We can also add a non-broken variant of nsAutoPtr modeled after std::unique_ptr (allows moves but not copies). Note that STL is another story. We're not using libstdc++ that comes with the compiler on android and b2g. We use STLport instead, and STLport has, afaik, no support for C++11 STL types. So, while we can now fix nsAutoPtr to use move semantics instead of copy semantics, we can't use std::unique_ptr. Does this mean that we can't use std::move and std::forward either? Indeed Sadface. I filed bug 896100 about this, and I'll write a patch for it soon. In that case, we need MFBT versions of those, because working with rvalue references without those two functions is a pain! Good news is that implementing these functions should be really easy. Also, is there a reason that we don't build on Android with the libstdc++ that ships with the toolchain? For one, the fact that it actually doesn't work currently (at least, it didn't last i heard someone tried) doesn't help. Heh, yeah it doesn't, indeed! And I think that when stlport was chosen, using the GNU stl made the android package significantly larger. But then, we weren't statically linking stlport, so this may have changed. Also note that because we're building for specific targets that are not exactly those for which we have prebuilt libraries coming in the NDK, we're actually building stlport ourselves. We'd have to do the same for the GNU stl if we chose to use it, and we have no current support for that in the build system (the build system only knows how to build against the prebuilt one, and that's what doesn't even work). I don't know how easy it is to compile the GNU stl outside of building gcc. Ah, I have feeling that doing that is actually very hard... Let's declare defeat on this for now (unless people want to do all of this work!) and move on with bug 896100. Once we have that, I will try to find some time to see if we can kill already_AddRefed. That is one class I won't miss. :-) Ehsan __**_ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/**listinfo/dev-platformhttps://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Thursday, 18 July 2013 10:32:35 UTC+1, Mike Hommey wrote: On Wed, Jul 17, 2013 at 05:09:22PM +0900, Mike Hommey wrote: On Tue, Jul 16, 2013 at 04:17:50PM +0900, Mike Hommey wrote: On Sat, Jul 13, 2013 at 01:15:31PM -0700, Kyle Huey wrote: We've dropped support for versions of MSVC prior to 2010, and we're requiring at least GCC 4.4. According to [0] that means we should be able to use *auto*. Anybody know any reasons why we can't start using it? Filed bug 894242. (double 42!) This almost stuck, but B2G desktop builds are, guess what, using gcc 4.4, and a bug was already on file to upgrade that (bug 770625). Hopefully this will go forward now that there are patches. And it's now done. Thanks to Ted Mielczarek and Ben Hearsum for the quick reviews, and Aki Sasaki for the buildbot reconfig. Now we can start discussing what specific features we want to start using. Bug 895322 has already been filed to use static_assert instead of MOZ_STATIC_ASSERT. A good resource is http://wiki.apache.org/stdcxx/C++0xCompilerSupport Our base versions are 4.4 for gcc, and 10.0 for MSVC. I'm not sure what our lowest supported version of clang is. From that table, using the gcc and msvc versions, that gives: gcc msvcclang auto4.4 10.0* Yes decltype4.3@10.0@ 2.9 extern template 3.3 6.0Yes new fn syntax 4.4 10.02.9 right angle brackets4.3 8.0Yes r-value references 4.3@10.0! Yes static_assert 4.3 10.02.9 built-in type traits4.3 8.03.0 * v0.9 @ v1.0 ! v2.0 Which means a minimum clang version of 2.9 without built-in type traits, or 3.0 with. - Reece ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 7/18/2013 4:32 AM, Mike Hommey wrote: And it's now done. Thanks to Ted Mielczarek and Ben Hearsum for the quick reviews, and Aki Sasaki for the buildbot reconfig. Now we can start discussing what specific features we want to start using. Bug 895322 has already been filed to use static_assert instead of MOZ_STATIC_ASSERT. Bug 895047 and bug 523156 are about changing PRUnichar and jschar respectively to char16_t [wchar_t on Windows], which would eventually give us the ability to do things like stop using NS_LITERAL_STRING().get() for a const PRUnichar *... A good resource is http://wiki.apache.org/stdcxx/C++0xCompilerSupport Our base versions are 4.4 for gcc, and 10.0 for MSVC. I'm not sure what our lowest supported version of clang is. I've generally considered this to be 3.1. -- Joshua Cranmer Thunderbird and DXR developer Source code archæologist ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 7/18/2013 9:44 AM, Ehsan Akhgari wrote: On 2013-07-18 5:32 AM, Mike Hommey wrote: A good resource is http://wiki.apache.org/stdcxx/C++0xCompilerSupport Our base versions are 4.4 for gcc, and 10.0 for MSVC. I'm not sure what our lowest supported version of clang is. As of bug 870173, it's 3.3. I'm hoping that we can follow the latest clang as it's released in the future. That's not the lowest-supported version of Clang (for example, the DXR builds still use 3.2), it's just what the buildbot slaves are using. -- Joshua Cranmer Thunderbird and DXR developer Source code archæologist ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 2013-07-18 5:48 AM, mscl...@googlemail.com wrote: From that table, using the gcc and msvc versions, that gives: gcc msvcclang auto4.4 10.0* Yes Yes, please! decltype4.3@10.0@ 2.9 Yes, please! extern template 3.3 6.0Yes It's not exactly clear to me what the implications of using extern templates are. They promise faster build times. Has anybody tried to use them? new fn syntax 4.4 10.02.9 This seems like it might be useful in some cases, but it also seems like something which should not be overused. right angle brackets4.3 8.0Yes OMG, YES PLEASE! r-value references 4.3@10.0! Yes This is very useful. I believe the JS engine already rolls their own tricks to implement this semantics. static_assert 4.3 10.02.9 Yes, please! built-in type traits4.3 8.03.0 Yes, please! Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Fri, Jul 19, 2013 at 3:34 AM, Ehsan Akhgari ehsan.akhg...@gmail.comwrote: On 2013-07-18 5:48 AM, mscl...@googlemail.com wrote: r-value references 4.3@10.0! Yes This is very useful. I believe the JS engine already rolls their own tricks to implement this semantics. With this we can get rid of already_AddRefed and just pass nsRefPtr/nsCOMPtr/RefPtr around, right? Rob -- Jtehsauts tshaei dS,o n Wohfy Mdaon yhoaus eanuttehrotraiitny eovni le atrhtohu gthot sf oirng iyvoeu rs ihnesa.rt sS?o Whhei csha iids teoa stiheer :p atroa lsyazye,d 'mYaonu,r sGients uapr,e tfaokreg iyvoeunr, 'm aotr atnod sgaoy ,h o'mGee.t uTph eann dt hwea lmka'n? gBoutt uIp waanndt wyeonut thoo mken.o w * * ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Thu, Jul 18, 2013 at 11:34:53AM -0400, Ehsan Akhgari wrote: On 2013-07-18 5:48 AM, mscl...@googlemail.com wrote: From that table, using the gcc and msvc versions, that gives: gcc msvcclang auto 4.4 10.0* Yes Yes, please! decltype4.3@10.0@ 2.9 Yes, please! extern template 3.3 6.0Yes It's not exactly clear to me what the implications of using extern templates are. They promise faster build times. Has anybody tried to use them? More than faster build times, they also promise smaller objdir. The downside is that extern template class FooBar; may require Bar to be defined. Which potentially means adding #includes. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 7/18/2013 7:15 PM, Robert O'Callahan wrote: On Fri, Jul 19, 2013 at 3:34 AM, Ehsan Akhgari ehsan.akhg...@gmail.comwrote: On 2013-07-18 5:48 AM, mscl...@googlemail.com wrote: r-value references 4.3@10.0! Yes This is very useful. I believe the JS engine already rolls their own tricks to implement this semantics. With this we can get rid of already_AddRefed and just pass nsRefPtr/nsCOMPtr/RefPtr around, right? I believe so. We can also add a non-broken variant of nsAutoPtr modeled after std::unique_ptr (allows moves but not copies). -- Joshua Cranmer Thunderbird and DXR developer Source code archæologist ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Thu, Jul 18, 2013 at 08:54:02PM -0500, Joshua Cranmer ? wrote: On 7/18/2013 7:15 PM, Robert O'Callahan wrote: On Fri, Jul 19, 2013 at 3:34 AM, Ehsan Akhgari ehsan.akhg...@gmail.comwrote: On 2013-07-18 5:48 AM, mscl...@googlemail.com wrote: r-value references 4.3@10.0! Yes This is very useful. I believe the JS engine already rolls their own tricks to implement this semantics. With this we can get rid of already_AddRefed and just pass nsRefPtr/nsCOMPtr/RefPtr around, right? I believe so. We can also add a non-broken variant of nsAutoPtr modeled after std::unique_ptr (allows moves but not copies). Note that STL is another story. We're not using libstdc++ that comes with the compiler on android and b2g. We use STLport instead, and STLport has, afaik, no support for C++11 STL types. So, while we can now fix nsAutoPtr to use move semantics instead of copy semantics, we can't use std::unique_ptr. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Tue, Jul 16, 2013 at 04:17:50PM +0900, Mike Hommey wrote: On Sat, Jul 13, 2013 at 01:15:31PM -0700, Kyle Huey wrote: We've dropped support for versions of MSVC prior to 2010, and we're requiring at least GCC 4.4. According to [0] that means we should be able to use *auto*. Anybody know any reasons why we can't start using it? Filed bug 894242. (double 42!) This almost stuck, but B2G desktop builds are, guess what, using gcc 4.4, and a bug was already on file to upgrade that (bug 770625). Hopefully this will go forward now that there are patches. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Sun, Jul 14, 2013 at 10:50:55PM -0700, Justin Lebar wrote: We can't require any c++11 feature until we drop support for gcc 4.4. [...] there are problems in the gcc 4.4 system headers that make using c++11 mode impossible (except on b2g/android). Is there any reason to support gcc 4.4 outside of B2G/Android? That's what I'm saying. I don't see a reason to after 24. It's better to still support gcc 4.4 for 24, though. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Sat, Jul 13, 2013 at 01:15:31PM -0700, Kyle Huey wrote: We've dropped support for versions of MSVC prior to 2010, and we're requiring at least GCC 4.4. According to [0] that means we should be able to use *auto*. Anybody know any reasons why we can't start using it? Filed bug 894242. (double 42!) Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 7/13/2013 3:15 PM, Kyle Huey wrote: We've dropped support for versions of MSVC prior to 2010, and we're requiring at least GCC 4.4. According to [0] that means we should be able to use /auto/. Anybody know any reasons why we can't start using it? We don't yet require C++11 mode when building Mozilla. That said, all platforms on the buildbot farms now compile with C++11, this should be feasible (though, as Mike points out, it effectively means requiring Desktop Linux builds to go to gcc 4.5 or newer). Forcing C++11 mode to build Mozilla will also enable us to use the following features: mozilla/Char16.h [I may start tackling replacing PRUnichar with char16_t after I finish atomic conversion] auto rvalue references in limited fashion -- Beware of bugs in the above code; I have only proved it correct, not tried it. -- Donald E. Knuth ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On 7/14/13 10:50 PM, Justin Lebar wrote: We can't require any c++11 feature until we drop support for gcc 4.4. [...] there are problems in the gcc 4.4 system headers that make using c++11 mode impossible (except on b2g/android). Is there any reason to support gcc 4.4 outside of B2G/Android? btw, Android builders default to gcc 4.7 (bug 877503 and bug 892361). chris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
We can't require any c++11 feature until we drop support for gcc 4.4. [...] there are problems in the gcc 4.4 system headers that make using c++11 mode impossible (except on b2g/android). Is there any reason to support gcc 4.4 outside of B2G/Android? If we dropped support for gcc 4.4 on desktop, would we get auto (and potentially other C++11 niceties)? That seems like a pretty easy win... On Sat, Jul 13, 2013 at 5:00 PM, Mike Hommey m...@glandium.org wrote: On Sat, Jul 13, 2013 at 01:15:31PM -0700, Kyle Huey wrote: We've dropped support for versions of MSVC prior to 2010, and we're requiring at least GCC 4.4. According to [0] that means we should be able to use *auto*. Anybody know any reasons why we can't start using it? We can't require any c++11 feature until we drop support for gcc 4.4. Or remove support for non c++11. Because there are problems in the gcc 4.4 system headers that make using c++11 mode impossible (except on b2g/android). I'm happy with removing support for non-c++11 mode now that m-c is past next esr. But starting to use C++11 features can be dangerous, as it will make backporting harder. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Using C++0x auto
On Sat, Jul 13, 2013 at 01:15:31PM -0700, Kyle Huey wrote: We've dropped support for versions of MSVC prior to 2010, and we're requiring at least GCC 4.4. According to [0] that means we should be able to use *auto*. Anybody know any reasons why we can't start using it? We can't require any c++11 feature until we drop support for gcc 4.4. Or remove support for non c++11. Because there are problems in the gcc 4.4 system headers that make using c++11 mode impossible (except on b2g/android). I'm happy with removing support for non-c++11 mode now that m-c is past next esr. But starting to use C++11 features can be dangerous, as it will make backporting harder. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
