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
std::shared_ptr for our code? In other
words: we’re going to have a piece of code somewhere that says
but what should we write inside the branches of the
I didn’t really consider a macro. Sure, I could define a
macro to either
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
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:
Thus, you could import the header above, use
and have it mean what you want via the define. Usually, you don’t
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
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
QuantLib namespace, reasonable-looking code such as:
wasn’t compiling. The reason was that the
std pointers were
make_shared above was resolving to the
version due to argument-dependent lookup (ADL) because the first
argument was from namespace
std, and the returned
couldn’t be assigned to a
Therefore, the code had to be reconsidered. What I came up with was to use a nested namespace:
With the above in place, everything compiles cleanly after replacing
boost qualifications with
ext. The name
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
ext, 13 of them
would need to fix some conflicts before merging. Another 8 pull
requests would merge cleanly, but they would introduce new
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
Second: there are probably some places where we used
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
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.)
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.
Follow me on Twitter if you want to be notified of new posts, or subscribe via RSS: the buttons for 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.