Hi Mike,
thank you for your comments. My original goal was to completely get rid
of auto_ptr (since depricated), therefore I removed the constructor
taking an auto_ptr. From what you write below I understand that we
should keep it, but I do not quite get why. Could you explain this point
again please?
I do get your two points about the Clone(std::unique_ptr<T>&)
constructor though. If I write
Clone(std::unique_ptr<T> p);
and passing a std::unique_ptr<T> to it the compiler complains that I am
using a deleted function which I understand is because unique_ptr can
not be copied, i.e. example 1 would be illegal. Therefore one would then
have to change the client code passing std::move(p) instead of p
directly, as in example 2. E.g. line 3202 in test-suite/marketmodel.cpp would
then change as follows
PathwiseVegasAccountingEngine accountingengine(boost::shared_ptr<LogNormalFwdRateEuler>(new LogNormalFwdRateEuler(evolver)), // method relies heavily on LMM Euler
- productToUse,
+ std::move(productToUse),
marketModel, // we need pseudo-roots and displacements
vegaBumps,
initialNumeraireValue);
Example 3 would also work.
Is that what you propose ?
regards
Peter
Mike Sharpe <[hidden email]> writes:
> Hi Peter,
>
> First off, thanks for taking a look at getting C++11 into a major project!
>
> The line
> Clone(std::unique_ptr<T>& p);
>
> looks suspicious to me. It doesn't make it clear from the client side that the
> pointer ownership was changed. Compare the following three examples:
> // example 1
> std::unique_ptr<T> ptr(new T());
> Clone<T> newObject(ptr);
> // example 2
> std::unique_ptr<T> ptr(new T());
> Clone<T> newObject(std::move(ptr));
> //example 3
> Clone<T> newObject(std::unique_ptr<T>(new T()); // 3
>
> Example 2 makes it obvious that the constructor will retain ownership of the
> unique_ptr, which isn't obvious in example 1 even though the current
> implementation means example 1 and example 2 are the same. In addition, the
> current code will remove the ability for clients to use code like example 3
> completely, as you can't bind a temporary object to a non-const lvalue
> reference. It's possible clients could run into problems even if they don't
> upgrade from auto_ptr to unique_ptr if they have code like this.
>
> I propose that, instead of changing the function definition to incorporate
> either auto_ptr or unique_ptr, that another Clone constructor is added, so that
> there will be two constructors:
>
> Clone(std::auto_ptr<T> ptr);
> #ifdef QL_UNIQUE_PTR_ENABLED
> Clone(std::unique_ptr<T> ptr);
> #endif
>
> The implementation of the auto_ptr constructor will need to make sure to
> initialize the unique_ptr calling uniquePtr(std::move(autoPtr)) in the
> initializer class if the internal class uses unique_ptr. This is
> because unique_ptr has a constructor that accepts auto_ptr by rvalue reference
> only, and rvalue references cannot bind to lvalues. But, you can base the
> private data stored in clone off object internals simply off the
> QL_UNIQUE_PTR_ENABLED #define, without clients having to change their code.
> The decision to remove the auto_ptr constructor can be made later.
>
> Hopefully this helps. Let me know if an updated Clone.hpp would make things a
> little bit clearer and I can put something together when I have some time.
>
> Mike
>
>
> On Thu, May 30, 2013 at 9:13 AM, Peter Caspers <[hidden email]> wrote:
>
> following up the topic below: I missed to look at the test-suite, where
> some more
> auto_ptr would have to be replaced by unique_ptr.
>
> https://github.com/pcaspers/quantlib/commit/
> 466fbfbc423312eb7c4536ab59d861689cde70a0
>
> Furthermore the header ql/utilities/clone.hpp has to be changed.
>
> https://github.com/pcaspers/quantlib/commit/
> fe5a87dcfb60c10f1d8600b1511037e200915e4c
> https://github.com/pcaspers/quantlib/commit/
> 7a12fc4c62b44d175dc794b487db0582803ef6a5
>
> The test-suite now compiles and runs using g++ (with and without -std=
> c++0x)
> which is the main thing I guess. The whole thing does not look very
> nice though. Also I am not c++ expert enough to say that the upgrade of
> clone.hpp to unique_ptr should be done like this. Luigi ?
>
> Thank you
> Peter
>
> Peter Caspers <[hidden email]> writes:
>
> > Hi Luigi, all,
> >
> > we discussed compiling the core lib under the c++11 standard (e.g. with
> > g++ -std=c++0x). This produces some errors which can be fixed easily
> > (keeping backward compatibility):
> >
> > https://github.com/pcaspers/quantlib/commit/
> 5b32b7705530264551e0622d0a2673813067be5b
> >
> > What is left is a bunch of warnings saying that std::auto_ptr is
> > depricated under c++11, the replacement being std::unique_ptr. A
> > possible solution is to replace the auto_ptr in the source by unique_ptr
> > (thus upgrading the code to c++11 already) and replace the unique_ptr by
> > auto_ptr again in case that compilation is done under c++ versions <
> > 11. I have done this here
> >
> > https://github.com/pcaspers/quantlib/commit/
> 3e47a82a936112f12e3bb502292833d860f5aac9
> >
> > with a #define in qldefines.hpp resetting the pointers to auto_ptr again
> > based on the boost macro BOOST_NO_CXX_SMART_PTR. Unfortunately this
> > macro is available only in later boost versions so if one uses an older
> > one and provides no c++11 support he or she will get errors. Therefore
> > the following solution might be better
> >
> > https://github.com/pcaspers/quantlib/commit/
> 6db965375b02094a7ea0da4e48efd7e0a1b8fd85
> >
> > using the __cplusplus macro to identify c++11. However since gcc sets
> > this macro simply to 1 in versions 4.6 (should be solved starting in
> > 4.7), I had to add another criterion for gcc based on
> > __GXX_EXPERIMENTAL_CXX0X_.
> >
> > For msvc 2010 the __cplusplus is also not 201103L but still 199711L. This
> > seems also to be the case in 2012 and this specific case was reported as
> a bug to
> > Microsoft,
> >
> > http://connect.microsoft.com/VisualStudio/feedback/details/763051/
> a-value-of-predefined-macro-cplusplus-is-still-199711l
> >
> > though not solved yet. Therefore I added a direct test on msvc 2010 or
> > higher versions based on _MSC_VER
> >
> > https://github.com/pcaspers/quantlib/commit/
> 796beb89e2a707f951b77869bf869694e67ac769
> >
> > This final solution should work for all versions of msvc and gcc. I can
> not check other
> > compilers however.
> >
> > Since the #define seems a bit like a dirty hack we also thought of a
> > typedef. But then we would need a template typedef which is available
> only in
> > c++11 again and a metaprogramming-like workaround a la
> >
> > template<class T> struct PTR {
> > typedef std::unique_ptr<T> Type;
> > };
> >
> > PTR<double>::Type a(new double(0.0));
> >
> > does not seem to improve the code either.
> >
> > Do you think we should do the upgrade to c++11 like propsed above or is
> there
> > maybe a better solution (I bet there is...). Or should we defer the
> > upgrade until a later release ? I personally would very much like to be
> > able to compile under c++11 without warnings already now.
> >
> > Thank you
> > Peter
> >
> >
> >
> >
>
> ------------------------------------------------------------------------------
> Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
> Get 100% visibility into your production application - at no cost.
> Code-level diagnostics for performance bottlenecks with <2% overhead
> Download for free and get started troubleshooting in minutes.
> http://p.sf.net/sfu/appdyn_d2d_ap1
> _______________________________________________
> QuantLib-dev mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/quantlib-dev
| Free forum by Nabble | Disable Popup Ads | Edit this page |