lemire
fast_double_parser.h polluts global namespace with macros
fast_double_parser.h does not properly undefine macros that it uses locally.
https://github.com/lemire/fast_double_parser/blob/master/include/fast_double_parser.h#L71-L115
WARN_UNUSED
really_inline
unlikely
These are pretty common, in other projects, and could have different semantics. They should either be prefixed to unsure uniqueness, or undefined at the end of the header file if possible. As far as I can tell, these are used only in the header file, and there are no macro definitions outliving inclusion of the file, that use them, so they can be safely undefined.
there are few more, but they are prefixed with FAST_DOUBLE_PARSER_, so they can be probably left as is, but it is still a good practice to undefined them at the end.
This was discovered when our project defined macro CHECK, that was using unlikely (as a namespaced function, not macro), but also was including lightgbm which had own definition ( https://github.com/microsoft/LightGBM/blob/master/include/LightGBM/utils/log.h#L33-L42 ) of CHECK, protected by #ifndef, which got skipped, but other macro #define CHECK_EQ(a, b) CHECK(a == b) still referenced. But because in-between fast_double_parser.h was included, this caused our CHECK macro to fail, as it was referencing ::myproject::unlikely(x), and with fast_double_parser.h defining unlikely, this caused it to be expanded to ::myproject::__builtin_expect(x, 0), which does not compile. It was not easy to debug, because problem was deeper in the dependency chain.
Because this is C++ code, I suggest changing the unlikely to be a namespaced function:
constexpr inline bool unlikely(const bool expr) {
#ifdef _MSC_VER
return expr;
#else
return __builtin_expect(expr, false);
#endif
}
I recommend using https://github.com/fastfloat/fast_float which does not have this issue. The fast_double_parser library is legacy code at this point, that I would like to retire. I only maintain it for bug fixes. New users should adopt fast_float.
