HowardHinnant / date

A date and time library based on the C++11/14/17 <chrono> header
Other
3.16k stars 684 forks source link

Use feature test macros instead of __cplusplus version for feature detection #576

Open rigtorp opened 4 years ago

rigtorp commented 4 years ago

This code can be updated to use __cpp_lib_uncaught_exceptions and __cpp_lib_void_t feature test macros:

#ifndef HAS_UNCAUGHT_EXCEPTIONS
#  if __cplusplus > 201703 || (defined(_MSVC_LANG) && _MSVC_LANG > 201703L)
#    define HAS_UNCAUGHT_EXCEPTIONS 1
#  else
#    define HAS_UNCAUGHT_EXCEPTIONS 0
#  endif
#endif  // HAS_UNCAUGHT_EXCEPTIONS

#ifndef HAS_VOID_T
#  if __cplusplus >= 201703 || (defined(_MSVC_LANG) && _MSVC_LANG >= 201703L)
#    define HAS_VOID_T 1
#  else
#    define HAS_VOID_T 0
#  endif
#endif  // HAS_VOID_T
oviano commented 4 years ago

I am finding that when compiling a project in VS2019 with C++17 enabled, and /Zc:cplusplus defined (reference https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-cplusplus/) that it incorrectly defined HAS_UNCAUGHT_EXCEPTIONS to 0 and the build fails.

If I modify it so it is like this, with >= instead of >, it works:

#ifndef HAS_UNCAUGHT_EXCEPTIONS
#  if __cplusplus >= 201703 || (defined(_MSVC_LANG) && _MSVC_LANG >= 201703L)
#    define HAS_UNCAUGHT_EXCEPTIONS 1
#  else
#    define HAS_UNCAUGHT_EXCEPTIONS 0
#  endif
#endif  // HAS_UNCAUGHT_EXCEPTIONS
HowardHinnant commented 4 years ago

Build fails? Do you mean that you get a warning, and you have a setting to turn warnings into errors?

HowardHinnant commented 4 years ago

In any event use these directions to set HAS_UNCAUGHT_EXCEPTIONS=1.

oviano commented 4 years ago

Yes it gives a deprecation warning and I have warnings set to errors.

Change > to >= is my workaround which must be the correct fix anyway?

HowardHinnant commented 4 years ago

It is also correct to use a deprecated feature.

I purposefully set the default for C++17 to use the deprecated signature instead of the new one because many people also are in the position of compiling with C++17, yet shipping to OS versions where the new feature is not available. I.e. no matter what the default is on C++17 for this setting, 50% of my clients aren't going to like it.

The setting can be overridden by defining HAS_UNCAUGHT_EXCEPTIONS in your build environment prior to the preprocessor parsing date.h. date.h is purposefully set up to check for that possibility and respect your definition instead of defining the macro itself:

#ifndef HAS_UNCAUGHT_EXCEPTIONS

oviano commented 4 years ago

Got it, it makes sense and thanks for the explanation.

akorud commented 4 years ago

It seems that problem is that for C++17 __cplusplus equals to 201703 and check > looks for C++ version beyond C++17. IMO for C++17 correct check should be >=

oviano commented 4 years ago

Read @HowardHinnant comments on 14th June for the reason why it’s like it is.