-
Notifications
You must be signed in to change notification settings - Fork 281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix preprocessor warnings about undefined _MSVC_LANG #2676
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2676 +/- ##
=======================================
Coverage 63.89% 63.89%
=======================================
Files 103 103
Lines 22313 22313
Branches 10799 10799
=======================================
Hits 14258 14258
Misses 5830 5830
Partials 2225 2225
|
Sorry about the broken formatting in the commit message... apparently one line from the warning paste got lost as well. I'll push again with a fixed commit log. |
Stricter compiler/settings, such as found during a build on FreeBSD with clang 14, issue warnings of the kind below. /usr/local/include/exiv2/value.hpp:1272:31: warning: '_MSVC_LANG' is not defined, evaluates to 0 [-Wundef] Fix: Guard use of _MSVC_LANG by a check. Personally, I found that MSVC has several feature-specific checks in predefined macros which might allow for one standards-based check that matches GCC/clang/MSVC rather than the split check for C++ standard and MSVC language version settings. See https://en.cppreference.com/w/cpp/feature_test I am not building Exiv2 on MSVC, so I cannot test/suggest anything here.
aaa8761
to
2f2a926
Compare
Pull request has been modified.
Thanks.
So can we potentially remove this instead of patching, since we require C++17 now anyway? (I can't check MSVC either...) |
@kmilos no this is in the headers. We're keeping them C++11 compatible. The whole reason for using MSVC_LANG is because its __cplusplus macro is broken. Unfortunately for the _t/_v stuff, there's no standard feature test macro AFAIK. |
@Mergifyio backport 0.28.x |
✅ Backports have been created
|
Stricter compiler/settings, such as found during a build on FreeBSD with clang 14, issue warnings of the kind below.
/usr/local/include/exiv2/value.hpp:1272:31: warning: '_MSVC_LANG' is not defined, evaluates to 0 [-Wundef] fixed-width font helps here-- ^
Fix: Guard use of _MSVC_LANG by a check.
Personally, I found that MSVC has several feature-specific checks in predefined macros which might allow for one standards-based check that matches GCC/clang/MSVC rather than the split check for C++ standard and MSVC language version settings.
See https://en.cppreference.com/w/cpp/feature_test
I am not building Exiv2 on MSVC, so I cannot test/suggest anything here.