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.
|files||lines||compiles||fails PR merges|
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
and declarations which involved iterators—even if a bit unevenly; I had one
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
that we had lovingly aligned, will come out on the other side as
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:
The real modernization of the above wouldn’t be to replace
emplace_back; it would be to sidestep both methods and write
Unfortunately, moving to initialization lists will be manual work.
On a less important note, the replacement can turn the aligned
into the unaligned
but I’ve already commented on this when talking of
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
which I expected to be transformed into
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
which is a typedef to the built-in
double type, but
a user-defined class that converts to
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
std::exception class and is used throughout the library as the
type of the exceptions our code throws. We added a
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 if you want to be notified of new posts, or add me to your Google+ circles, or subscribe via RSS or email: the buttons for that are in the footer. Also, make sure to check my Training page.