Leaving C++03: automation, part 2

Welcome back.

In this post, I continue to report on using clang-tidy to automate the modernization of the QuantLib code base.

Last time, I showed you a summary of the checks I had run so far, which I reproduce here for convenience. The files column contains the number of files touched, the lines column contains the number of lines touched, compiles shows whether or not the code compiled after the modernization, and fails PR merges shows if any of my sample set of pull requests (see this post) failed to merge against the modernized code.

I also described the reasons for the compilation errors reported in the third column. In one case, the reason was problems with boost::function, and after I published the post Daniel Duffy (thanks for the question, Daniel) asked if the same problems affect std::function: they don’t.

And now, I’ll continue posting my lab notes; this time, on the checks listed above that didn’t cause compilation problems. I won’t be showing a lot of examples of the changes performed; they’re described in the clang-tidy documentation, and in most cases they’re pretty straightforward. In a couple of cases, they’re so straightforward that I won’t comment on them.

Running the modernize-use-auto check didn’t change a lot of declaration, given the size of the code base; this is probably the reason why it didn’t cause any merge conflicts with existing pull requests. From what I see of the changes, it affected declarations which duplicated the type name, such as

    const IborCoupon* c = dynamic_cast<const IborCoupon*>(&coupon);

and declarations which involved iterators—even if a bit unevenly; I had one

    for (std::map<Date, Real>::const_iterator ii = quotes.begin(); // ...

turned into

    for (auto ii = quotes.begin(); // ...

but other similar cases remained unchanged.

If you belong to the Almost Always Auto camp (as I incline to), this might be a bit disappointing. Of course, I don’t suggest to go through the code and manually check all the other declarations where auto can be used; there are a whole lot of much better uses of one’s time. You can be a good scout instead, and clean up a bit every time you’re working on some piece of code. While you’re at it, you can also fix another side effect of the automated change: a declaration such as

     Leg::const_iterator cf = nextCashFlow(leg,
                                           includeSettlementDateFlows,
                                           settlementDate);

that we had lovingly aligned, will come out on the other side as

     auto cf = nextCashFlow(leg,
                                           includeSettlementDateFlows,
                                           settlementDate);

which irks me some. Here, using an automated formatting tool like clang-format would help; but on the one hand, we can’t enforce its use by all the contributors, and on the other hand, I’m probably getting softer and/or wiser in my old age and I no longer think it’s a big deal. I’ll hit a key and realign code like the above if I come across it, but I won’t go looking for it.

The figures for modernize-use-equals-default in the table above are for running the check alone on the code base. However, it should be run after modernize-use-default-member-init, since the latter can turn non-default constructors into default ones that can be converted. In fact, running the check afterwards increases the number of files touched from 182 to 199 and the lines changes from 237 to 256.

The modernize-use-emplace touched a lot of lines in the 87 files it touched; however, in a complete modernization of the code it would be less important than it seems. The reason is that a lot of those lines are in the test suite, and have to do with initialization of test data; that is, they belong to code like:

     std::vector<Foo> test_data;
     test_data.push_back(Foo(1));
     test_data.push_back(Foo(2));
     test_data.push_back(Foo(4));

The real modernization of the above wouldn’t be to replace push_back with emplace_back; it would be to sidestep both methods and write

     std::vector<Foo> test_data = { Foo(1), Foo(2), Foo(4) };

Unfortunately, moving to initialization lists will be manual work.

On a less important note, the replacement can turn the aligned

    discounters_.push_back(MarketModelDiscounter(cashFlowTimes[j],
                                                 rateTimes));

into the unaligned

    discounters_.emplace_back(cashFlowTimes[j],
                                                 rateTimes));

but I’ve already commented on this when talking of auto above.

The modernize-use-default-member-init check worked mostly as I expected, except for a few member initializers that it left unchanged; but my expectations might be at fault. Most of the cases are like

    class Foo {
      public:
        Foo() : x(Null<Real>()) {}
      private:
        Real x;
    };

which I expected to be transformed into

    class Foo {
      public:
        Foo() {}
      private:
        Real x = Null<Real>();
    };

Instead, the check has no effect on the code. The documentation does warn that only built-in types are affected; I’m not sure on which side of the fence the above falls, given that the data member is a Real which is a typedef to the built-in double type, but Null<Real> is a user-defined class that converts to Real.

The modernize-use-noexcept check only acted on a couple of lines; that’s because we don’t use exception specifications in our code base, so it didn’t find anything to convert. The one exception (no pun intended) was the destructor of our Error class, which inherits from the std::exception class and is used throughout the library as the type of the exceptions our code throws. We added a throw() specification to its destructor because it was required by one of the compilers we supported; the destructor of std::exception had the same specification in the implementation shipped with the compiler, and the latter would take exception (I’m on a roll tonight) if the destructor of our class didn’t.

In modern C++, however, noexcept specifications are a lot more important, since they allow to use move constructors (and thus to gain efficiency) in operations that require the string exception guarantee; see, for instance, this video for an example and more details. Unfortunately, adding them to the modernized code will be a manual operation.

Finally, a word on modernize-redundant-void-arg and modernize-deprecated-headers: they actually modernize the code to C++03, not C++11. Therefore, I backported their changes to the main code base; they were merged in pull requests #445 and #449, respectively.

That’s all for this post. I’m currently trying out the remaining checks on the library; I’ll report on the results in the next post in this series. As usual, I’ll be grateful for any feedback you’ll send my way; tweet or email me.

Follow me on Twitter or LinkedIn 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.