New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import shared_ptr into namespace QuantLib. #406

Merged
merged 20 commits into from Jun 25, 2018

Conversation

3 participants
@lballabio
Copy link
Owner

lballabio commented Feb 5, 2018

Discussion of this is encouraged, and comments are welcome.

More details on the rationale are available at https://www.implementingquantlib.com/2018/04/going-to-11-shared-ptr.html.

The idea is to allow the user to choose (at compile time) whether to use boost::shared_ptr or std::shared_ptr. To do this, we need to avoid hard-coding either implementation.

The steps were:

  • import shared_ptr into an inner namespace of QuantLib, so that it can be used without hard-coding the implementation in QuantLib code; see ql/shared_ptr.hpp.
  • replace any inclusion of <boost/shared_ptr.hpp> and <boost/make_shared.hpp> with the newly created <ql/shared_ptr.hpp>;
  • replace the namespace specification in all uses of boost::shared_ptr, boost::make_shared and the casts;
  • add configuration flags to enable the switch (--enable-std-pointers in configure.ac and QL_USE_STD_SHARED_PTR in userconfig.hpp).

The inner namespace is required to avoid ambiguous name resolutions, either due to using namespace directives or ADL.

Client code is not required to use the new namespace right away: using boost::shared_ptr etc. is ok if the Boost implementation is chosen. This saves backward compatibility.

lballabio added some commits Feb 5, 2018

Import shared_ptr into namespace QuantLib.
This avoids hard-coding whether we'll use the Boost or std
implementation.  To complete this change, we should also:

- replace any inclusion of <boost/shared_ptr.hpp> and
  <boost/make_shared.hpp> with <ql/shared_ptr.hpp>;
- remove the namespace specification from all uses of
  boost::shared_ptr and boost::make_shared;
- add configuration flags to enable the switch.
Create inner namespace to contain imported names.
Without a specification, the names would sometime resolve
to the std version because of ADL.
@tomwhoiscontrary

This comment has been minimized.

Copy link
Contributor

tomwhoiscontrary commented Apr 11, 2018

Is there a configure flag to enable this? Or do we just set -DQL_USE_STD_SHARED_PTR in CXXFLAGS?

@lballabio

This comment has been minimized.

Copy link
Owner

lballabio commented Apr 12, 2018

There will be a configure flag. Right now, the whole thing doesn't really work anyway--I put it out here for discussion, but I've only replaced the namespace in one file to increase the signal/noise ratio of the PR.

@pcaspers

This comment has been minimized.

Copy link
Contributor

pcaspers commented Apr 14, 2018

I didn't know, but actually we have a few weak_ptr<T> in the library (in the thread safe observer patter implementation). I guess it's straightforward to take care of them in the same way?

@lballabio

This comment has been minimized.

Copy link
Owner

lballabio commented Apr 16, 2018

Yes, I think so.

lballabio added some commits May 19, 2018

@lballabio lballabio added this to the 1.14 release milestone May 25, 2018

lballabio added some commits May 28, 2018

Revert transformation not valid with C++03.
The C++03 version of make_shared takes at most 10 parameters.
Avoid compilation errors with VC++2010.
I didn't figure them out.  Not sure it's worth the time.

@lballabio lballabio merged commit fad0423 into master Jun 25, 2018

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

lballabio added a commit that referenced this pull request Jun 25, 2018

@lballabio lballabio deleted the shared_ptr branch Jun 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment