Skip to content

Build fixes: Fewer warnings, safer scripting, no math.h

FeRD (Frank Dana) requested to merge ferdnyc/recoll:lib-fixes into master

This MR consists of mostly-minor quality-of-life build adjustments that I've made while compiling locally. Only one of which I expect may be controversial, so I'll cover that first:

Eliminate all use of the math.h header in C++

More than any other include file, the C legacy math.h tends to cause compile problems in various circumstances. In most implementations, it relies heavily on preprocessor macros for the symbols it defines, and those macros tend not to be compatible with any other implementations'. Which means that, if you end up pulling the wrong math.h into a build (which is easy to have happen when setting include paths like /usr/local/incude), you'll end up with crazy compiler errors about isabs, isfinite, etc.

And there's really no need to rely on math.h in C++ code. #include <cmath> is a far safer option for legacy math functions, and exports its symbols into the std:: namespace.

The C++ <cfoo> versions of the C foo.h headers also offer some other nice features. (Like, one librecoll call to abs() was passing an argument of a type not supported by abs(), for which std::abs() has an overload.) I'd personally be thrilled to replace all <foo.h> header includes with their <cfoo> versions, and to use the std:: versions of their functions exclusively. But other than that one abs() call that I did replace with std::abs(), the focus here is only on <math.h> as it's by far the biggest headache.

So, one commit in this MR (which I can of course drop or change, if requested) removes #include <math.h> from all C++ sources, replacing them with #include <cmath> (or with #include <cstdlib> in the case of that std::abs() call).

...Or with nothing, in the several cases where I couldn't find any function calls that would require #include <math.h>. (Some files that were including <math.h> contain calls to atoi() or atoll(), but (like abs()) those are actually defined in stdlib.h, and their C++ equivalents std::atoi and std::atoll are defined in <cstdlib>. I didn't touch any of those.)

Actual math.h functions like round(), sqrt(), ceil(), etc., I replaced with the std::-prefixed versions.

The one exception to this is src/python/pychm/recollchm/swig_chm.c, where floor() and ceil() are called by the SWIG_CanCastAsInteger function. Since it's C source generated by SWIG, it still includes <math.h> for those functions.

I won't lie by making any sort of promises that this change is completely safe, or that every math function throughout the source has definitely been corrected. I can't truthfully make any such claims. I can only say that my own builds on Linux were successful before and after. And on macOS they were successful only after, as that's where math.h was causing me troubles to begin with. Nevertheless, test builds are certainly warranted to confirm that this change doesn't cause any issues with certain platforms/compilers/features/etc.

Other commits

Replace deprecated constructs

  • The QMessageBox() overloads that take integer button arguments were deprecated long ago, message box arguments should be passed a bitwise-or'd set of QStandardButtons values.
  • QApplication::fontMetrics() is deprecated, the canonical way to determine a widget's font metrics is to use a call to QApplication::font(widget) to construct a QFontMetricsF, then read the metrics from that.
  • Using Qt::Key + Qt::Key for keyboard combinations has been deprecated for some time, Qt::Key | Qt::Key is the supported syntax.

Eliminate warnings

  • Calling QTranslator::load without checking the return value causes a compile warning, as it's been annotated [[nodiscard]] by Qt.
  • Uses of the moc helper macro Q_OBJECT (along with Q_WIDGET, etc.) shouldn't have a semicolon after
  • Wrap unused arguments in src/utils/ virtual method implementations with PRETEND_USE()

Safer scripting

  • The autogen.sh script was using git to restore a clobbered ylwrap file after the run, which assumes there's a .git directory for the tree. If it's not there (say, if the tree is a copy of the repo's src/ directory only), the script fails. So, instead of relying on git to restore the original ylwrap, autogen.sh just copies the file before it gets clobbered, and restores from that copy after.

Merge request reports