Welcome back.

As I mentioned in last post, I’m doing an experiment where I’m migrating our C++03 code to C++11. One obvious step would be to use the smart pointers now available in the standard library. How can we do it while keeping backward compatibility and not breaking client code, though?

Now, we’re not savages. Even if we’re still using C++03 in QuantLib, we’ve been using smart pointers for almost 15 years; mostly boost::shared_ptr. Given that std::shared_ptr has the same interface, it could be a simple replacement.

Except that we can’t simply fire up sed and go s/boost::shared_ptr/std::shared_ptr/g on the source code. As I mentioned in last post, you might be able to do it: for instance, you might be migrating proprietary code and controlling its client code, if any. If that’s the case, sure, go ahead. But part of the experiment is also to see whether we can start using some of the features in the official library, and std::shared_ptr is one of the candidates given that Visual C++ 2010 will soon be the earliest supported version. In our case, people have been writing code that depends on QuantLib and expects to pass and receive Boost shared pointers. We can’t just switch pointers and break their code.

(Sure, we wouldn’t push the new release to their servers, so we wouldn’t actually break their code since they can choose not to upgrade; but saying “you can’t upgrade until you convert your code” is something we have been avoiding for years, and we’d like to continue).

Therefore, it would be nice to provide a migration path; that is, a mechanism for client code to choose which pointer to use. As an aside, this would be useful for my experiment as well, since it would make it possible to keep integrating changes from master into the C++11 branch.

So, what kind of switch will allow us to choose either boost::shared_ptr or std::shared_ptr for our code? In other words: we’re going to have a piece of code somewhere that says

    #if defined(QL_USE_STD_POINTERS)
        ... // something to use std::shared_ptr
    #else
        ... // something else to use boost::shared_ptr
    #endif

but what should we write inside the branches of the #if?

I didn’t really consider a macro. Sure, I could define a PTRNS macro to either std or boost, and use PTRNS::shared_ptr in the code; but that would mean filling the code to the brim with macros. For me personally, it would mean making faces like Disgust from Inside Out every time I open a file. Not really an option.

Creating a typedef and having it point at the desired class is not possible, either, because shared_ptr is a class template and we can’t use typedefs with those (remember, we’re keeping backward compatibility with the C++03 code). Also, I’m only talking about shared_ptr for brevity, but we also need to manage make_shared, dynamic_pointer_cast and the likes, and those are not types.

The thing I tried first, and which (spoiler) almost worked, was to import the desired name into the QuantLib namespace. The relevant code was:

    #if defined(QL_USE_STD_POINTERS)
    #include <memory>
    #else
    #include <boost/shared_ptr.hpp>
    #include <boost/make_shared.hpp>
    #endif

    namespace QuantLib {

    #if defined(QL_USE_STD_POINTERS)
        using std::shared_ptr;
        using std::make_shared;
    #else
        using boost::shared_ptr;
        using boost::make_shared;
    #endif

    }

Thus, you could import the header above, use QuantLib::shared_ptr, and have it mean what you want via the define. Usually, you don’t want a using directive in a header file, in order to avoid importing names into your namespace (or worse, in the global one); but in this case, importing names is exactly what we want.

Unfortunately, I had a couple of problems with this. The first was that we had a few source files in which we used the directive using namespace std; for convenience. Sure, it’s our fault, you’re supposed to only import specific names from namespaces, but people do that (the language allows it, after all) so we’re possibly breaking their code. This way, an unqualified shared_ptr could be flagged as ambiguous if we used the Boost one and std::shared_ptr was included indirectly via some standard header (or the other way around). Sure, it’s an easy fix, but it’s still a breakage.

The second problem was more subtle. With the Boost pointers imported into the QuantLib namespace, reasonable-looking code such as:

    namespace QuantLib {
        ...
        std::vector<int> v = ...;
        ...
        shared_ptr<T> p = make_shared<T>(v, /* whatever else */);
    }

wasn’t compiling. The reason was that the std pointers were imported indirectly, make_shared above was resolving to the std version due to argument-dependent lookup (ADL) because the first argument was from namespace std, and the returned std::shared_ptr couldn’t be assigned to a boost::shared_ptr.

Therefore, the code had to be reconsidered. What I came up with was to use a nested namespace:

    #if defined(QL_USE_STD_POINTERS)
    #include <memory>
    #else
    #include <boost/shared_ptr.hpp>
    #include <boost/make_shared.hpp>
    #endif

    namespace QuantLib {

        namespace ext {

        #if defined(QL_USE_STD_POINTERS)
            using std::shared_ptr;
            using std::make_shared;
            using std::static_pointer_cast;
            using std::dynamic_pointer_cast;
        #else
            using boost::shared_ptr;
            using boost::make_shared;
            using boost::static_pointer_cast;
            using boost::dynamic_pointer_cast;
        #endif

        }
    }

With the above in place, everything compiles cleanly after replacing all relevant boost qualifications with ext. The name ext (for external, of course) is the least bad name I could find, but I’m not fond of it. I’d be glad to hear of a better suggestion.

This is still not the end of the story. For one thing, replacing the types doesn’t happen in a vacuum. At any time, we have a number of pull requests open, and the change is invasive enough that it’s going to conflict with some of them. As an experimental sample, I took 43 of the pull requests that went into our 1.12 release and rebased them against 1.11 so that they all merged cleanly.

After performing the replacement of boost with ext, 13 of them would need to fix some conflicts before merging. Another 8 pull requests would merge cleanly, but they would introduce new boost:: qualifications that need to be replaced; this would also hold for 5 of the former ones, after resolving their conflicts. Only 22, or about half, could be merged without any changes. In summary: if we do it (or if you do it in your project) it will likely disrupt the workflow for a while, and would introduce the need to check any new pull request for old boost:: specifications.

Second: there are probably some places where we used shared_ptr out of laziness or inexperience, but unique_ptr would have been a better choice. Obviously, there’s no way to go about this in an automated way. We’ll have to keep an eye for these cases as we happen to look at different parts of the code. We did use boost::scoped_ptr in a few places, and those should be turned into unique_ptr; but we’ll have to think about those, since their names are different (thus making it impossible to use the namespace trick above) and so are the semantics. The same goes for the few places where we used std::auto_ptr. I’ll keep you posted.

Finally, switching from boost to std causes a change in behavior, at least in our case. It’s probably on us: we’ve been lazy, and we set a compilation flag for boost::shared_ptr that causes access to an empty pointer to be checked and to cause an exception rather than a crash. (This allowed us not to worry that using the QuantLib module could crash the whole Python interpreter, for instance. Python users don’t take kindly to that.)

However, std::shared_ptr doesn’t have such a flag. This is in line with the philosophy of not paying for what you don’t use: if you’re accessing a shared pointer inside a loop, you want to check it only once before the loop, and not at each access. In our case, though, this means that we’ll need to try and find unchecked accesses and add the relevant preconditions. This would be a lot simpler if we let a static analyzer do it, but I couldn’t make clang detect them. If you have any suggestions, I’ll be glad to hear them.

Speaking of which: so far, I didn’t find the time (or, indeed, the motivation) to research, install and moderate a comment system. If you want to reach out with suggestions or questions, send me a tweet or an email. In this particular case, it might be even better if you commented on the pull request that I opened on GitHub and that includes a sketch of the changes I discussed above. I’ll report on your feedback in future posts.

Subscribe to my Substack to receive my posts in your inbox, or follow me on Twitter or LinkedIn if you want to be notified of new posts, or subscribe via RSS if you’re the tech type: the buttons for all that are in the footer. Also, I’m available for training, both online and (when possible) on-site: visit my Training page for more information.