Skip to content
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

Explicitly include stdint #2558

Merged
merged 1 commit into from
Mar 22, 2023
Merged

Explicitly include stdint #2558

merged 1 commit into from
Mar 22, 2023

Conversation

kmilos
Copy link
Collaborator

@kmilos kmilos commented Mar 21, 2023

Fixes #2557

@kmilos kmilos added bug compilers Related with compiler options, definitions, support, etc. labels Mar 21, 2023
@ghost
Copy link

ghost commented Mar 21, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@neheb
Copy link
Collaborator

neheb commented Mar 21, 2023

Google search shows https://github.com/marketplace/actions/install-llvm-and-clang for testing newer versions.

@kmilos
Copy link
Collaborator Author

kmilos commented Mar 21, 2023

Don't think we need/want a new action per se, just better coverage of toolchains in already existing images (e.g. CLANG64 in MSYS2 and llvm-toolchain-15 in ubuntu-latest) instead of all the insane combinations of release/debug/shared/static...

@neheb
Copy link
Collaborator

neheb commented Mar 21, 2023

In #2553 I removed some CI. Maybe I should do more...

Actually, I tested clang64 with msys2 before. It didn't fail.

@kmilos
Copy link
Collaborator Author

kmilos commented Mar 21, 2023

Actually, I tested clang64 with msys2 before. It didn't fail.

I think for a while we had different warning/error flags for Windows builds, could be revisited perhaps...

@kmilos
Copy link
Collaborator Author

kmilos commented Mar 21, 2023

@neheb If I enable EXIV2_TEAM_WARNINGS_AS_ERRORS on CLANG64, I get these errors for a Debug build:

C:/msys64/home/kmilos/exiv2/src/helper_functions.cpp:23:30: error: 'codecvt_utf8<wchar_t, 1114111,
 0>' is deprecated [-Werror,-Wdeprecated-declarations]
  using convert_typeX = std::codecvt_utf8<wchar_t>;
                             ^
C:/msys64/clang64/include/c++/v1/codecvt:199:28: note: 'codecvt_utf8<wchar_t, 1114111, 0>' has been
explicitly marked deprecated here
class _LIBCPP_TEMPLATE_VIS _LIBCPP_DEPRECATED_IN_CXX17 codecvt_utf8
                           ^
C:/msys64/clang64/include/c++/v1/__config:800:41: note: expanded from macro '_LIBCPP_DEPRECATED_IN_C
XX17'
#    define _LIBCPP_DEPRECATED_IN_CXX17 _LIBCPP_DEPRECATED
                                        ^
C:/msys64/clang64/include/c++/v1/__config:773:49: note: expanded from macro '_LIBCPP_DEPRECATED'
#      define _LIBCPP_DEPRECATED __attribute__((deprecated))
                                                ^
C:/msys64/home/kmilos/exiv2/src/helper_functions.cpp:24:24: error: 'convert_typeX' is deprecated [
-Werror,-Wdeprecated-declarations]
  std::wstring_convert<convert_typeX, wchar_t> converterX;
                       ^
C:/msys64/clang64/include/c++/v1/codecvt:199:28: note: 'codecvt_utf8<wchar_t, 1114111, 0>' has been
explicitly marked deprecated here
class _LIBCPP_TEMPLATE_VIS _LIBCPP_DEPRECATED_IN_CXX17 codecvt_utf8
                           ^
C:/msys64/clang64/include/c++/v1/__config:800:41: note: expanded from macro '_LIBCPP_DEPRECATED_IN_C
XX17'
#    define _LIBCPP_DEPRECATED_IN_CXX17 _LIBCPP_DEPRECATED
                                        ^
C:/msys64/clang64/include/c++/v1/__config:773:49: note: expanded from macro '_LIBCPP_DEPRECATED'
#      define _LIBCPP_DEPRECATED __attribute__((deprecated))
                                                ^
C:/msys64/home/kmilos/exiv2/src/helper_functions.cpp:24:8: error: 'wstring_convert<std::codecvt_ut
f8<wchar_t, 1114111, 0>>' is deprecated [-Werror,-Wdeprecated-declarations]
  std::wstring_convert<convert_typeX, wchar_t> converterX;
       ^
C:/msys64/clang64/include/c++/v1/locale:3621:28: note: 'wstring_convert<std::codecvt_utf8<wchar_t, 1
114111, 0>>' has been explicitly marked deprecated here
class _LIBCPP_TEMPLATE_VIS _LIBCPP_DEPRECATED_IN_CXX17 wstring_convert
                           ^
C:/msys64/clang64/include/c++/v1/__config:800:41: note: expanded from macro '_LIBCPP_DEPRECATED_IN_C
XX17'
#    define _LIBCPP_DEPRECATED_IN_CXX17 _LIBCPP_DEPRECATED
                                        ^
C:/msys64/clang64/include/c++/v1/__config:773:49: note: expanded from macro '_LIBCPP_DEPRECATED'
#      define _LIBCPP_DEPRECATED __attribute__((deprecated))
                                                ^
3 errors generated.

This is even before trying to fix this header reorganization, so it is a separate (Windows specific?) issue.

@kmilos
Copy link
Collaborator Author

kmilos commented Mar 21, 2023

@neheb If I turn that off, but turn samples on, CLANG64 fails w/

C:/msys64/home/kmilos/exiv2/samples/easyaccess-test.cpp:11:23: error: constexpr variable 'easyAcce
ss' must be initialized by a constant expression
static constexpr auto easyAccess = std::array{
                      ^            ~~~~~~~~~~~
1 error generated.

@neheb
Copy link
Collaborator

neheb commented Mar 21, 2023

Codecvt being deprecated is a C++17 thing.

I tested the meson build, which does not have samples yet. I'll look into it.

edit: I looked into codecvt. Seems to do the same as iconv. Maybe we should make iconv required on non Windows. Thoughts?

edit2: or add utf8-cpp and remove a bunch of code instead.

@neheb
Copy link
Collaborator

neheb commented Mar 21, 2023

I just looked at that code. I have no idea why it wouldn't be constexpr friendly.

edit: apply

--- a/samples/easyaccess-test.cpp
+++ b/samples/easyaccess-test.cpp
@@ -8,7 +8,7 @@
 // Type for an Exiv2 Easy access function
 using EasyAccessFct = Exiv2::ExifData::const_iterator (*)(const Exiv2::ExifData&);
 
-static constexpr auto easyAccess = std::array{
+static constexpr std::array easyAccess{
     std::make_tuple("Orientation", &Exiv2::orientation, "Orientation"),
     std::make_tuple("ISO speed", &Exiv2::isoSpeed, "ISOspeed"),
     std::make_tuple("Date & time original", &Exiv2::dateTimeOriginal, "DateTimeOriginal"),

alternatively:

--- a/samples/easyaccess-test.cpp
+++ b/samples/easyaccess-test.cpp
@@ -8,41 +8,45 @@
 // Type for an Exiv2 Easy access function
 using EasyAccessFct = Exiv2::ExifData::const_iterator (*)(const Exiv2::ExifData&);
 
-static constexpr auto easyAccess = std::array{
-    std::make_tuple("Orientation", &Exiv2::orientation, "Orientation"),
-    std::make_tuple("ISO speed", &Exiv2::isoSpeed, "ISOspeed"),
-    std::make_tuple("Date & time original", &Exiv2::dateTimeOriginal, "DateTimeOriginal"),
-    std::make_tuple("Flash bias", &Exiv2::flashBias, "FlashBias"),
-    std::make_tuple("Exposure mode", &Exiv2::exposureMode, "ExposureMode"),
-    std::make_tuple("Scene mode", &Exiv2::sceneMode, "SceneMode"),
-    std::make_tuple("Macro mode", &Exiv2::macroMode, "MacroMode"),
-    std::make_tuple("Image quality", &Exiv2::imageQuality, "ImageQuality"),
-    std::make_tuple("White balance", &Exiv2::whiteBalance, "WhiteBalance"),
-    std::make_tuple("Lens name", &Exiv2::lensName, "LensName"),
-    std::make_tuple("Saturation", &Exiv2::saturation, "Saturation"),
-    std::make_tuple("Sharpness", &Exiv2::sharpness, "Sharpness"),
-    std::make_tuple("Contrast", &Exiv2::contrast, "Contrast"),
-    std::make_tuple("Scene capture type", &Exiv2::sceneCaptureType, "SceneCaptureType"),
-    std::make_tuple("Metering mode", &Exiv2::meteringMode, "MeteringMode"),
-    std::make_tuple("Camera make", &Exiv2::make, "Make"),
-    std::make_tuple("Camera model", &Exiv2::model, "Model"),
-    std::make_tuple("Exposure time", &Exiv2::exposureTime, "ExposureTime"),
-    std::make_tuple("FNumber", &Exiv2::fNumber, "FNumber"),
-    std::make_tuple("Shutter speed value", &Exiv2::shutterSpeedValue, "ShutterSpeed"),
-    std::make_tuple("Aperture value", &Exiv2::apertureValue, "Aperture"),
-    std::make_tuple("Brightness value", &Exiv2::brightnessValue, "Brightness"),
-    std::make_tuple("Exposure bias", &Exiv2::exposureBiasValue, "ExposureBias"),
-    std::make_tuple("Max aperture value", &Exiv2::maxApertureValue, "MaxAperture"),
-    std::make_tuple("Subject distance", &Exiv2::subjectDistance, "SubjectDistance"),
-    std::make_tuple("Light source", &Exiv2::lightSource, "LightSource"),
-    std::make_tuple("Flash", &Exiv2::flash, "Flash"),
-    std::make_tuple("Camera serial number", &Exiv2::serialNumber, "SerialNumber"),
-    std::make_tuple("Focal length", &Exiv2::focalLength, "FocalLength"),
-    std::make_tuple("Subject location/area", &Exiv2::subjectArea, "SubjectArea"),
-    std::make_tuple("Flash energy", &Exiv2::flashEnergy, "FlashEnergy"),
-    std::make_tuple("Exposure index", &Exiv2::exposureIndex, "ExposureIndex"),
-    std::make_tuple("Sensing method", &Exiv2::sensingMethod, "SensingMethod"),
-    std::make_tuple("AF point", &Exiv2::afPoint, "AFpoint"),
+static constexpr struct {
+  const char* l;
+  EasyAccessFct f;
+  const char* n;
+} easyAccess[] = {
+    {"Orientation", &Exiv2::orientation, "Orientation"},
+    {"ISO speed", &Exiv2::isoSpeed, "ISOspeed"},
+    {"Date & time original", &Exiv2::dateTimeOriginal, "DateTimeOriginal"},
+    {"Flash bias", &Exiv2::flashBias, "FlashBias"},
+    {"Exposure mode", &Exiv2::exposureMode, "ExposureMode"},
+    {"Scene mode", &Exiv2::sceneMode, "SceneMode"},
+    {"Macro mode", &Exiv2::macroMode, "MacroMode"},
+    {"Image quality", &Exiv2::imageQuality, "ImageQuality"},
+    {"White balance", &Exiv2::whiteBalance, "WhiteBalance"},
+    {"Lens name", &Exiv2::lensName, "LensName"},
+    {"Saturation", &Exiv2::saturation, "Saturation"},
+    {"Sharpness", &Exiv2::sharpness, "Sharpness"},
+    {"Contrast", &Exiv2::contrast, "Contrast"},
+    {"Scene capture type", &Exiv2::sceneCaptureType, "SceneCaptureType"},
+    {"Metering mode", &Exiv2::meteringMode, "MeteringMode"},
+    {"Camera make", &Exiv2::make, "Make"},
+    {"Camera model", &Exiv2::model, "Model"},
+    {"Exposure time", &Exiv2::exposureTime, "ExposureTime"},
+    {"FNumber", &Exiv2::fNumber, "FNumber"},
+    {"Shutter speed value", &Exiv2::shutterSpeedValue, "ShutterSpeed"},
+    {"Aperture value", &Exiv2::apertureValue, "Aperture"},
+    {"Brightness value", &Exiv2::brightnessValue, "Brightness"},
+    {"Exposure bias", &Exiv2::exposureBiasValue, "ExposureBias"},
+    {"Max aperture value", &Exiv2::maxApertureValue, "MaxAperture"},
+    {"Subject distance", &Exiv2::subjectDistance, "SubjectDistance"},
+    {"Light source", &Exiv2::lightSource, "LightSource"},
+    {"Flash", &Exiv2::flash, "Flash"},
+    {"Camera serial number", &Exiv2::serialNumber, "SerialNumber"},
+    {"Focal length", &Exiv2::focalLength, "FocalLength"},
+    {"Subject location/area", &Exiv2::subjectArea, "SubjectArea"},
+    {"Flash energy", &Exiv2::flashEnergy, "FlashEnergy"},
+    {"Exposure index", &Exiv2::exposureIndex, "ExposureIndex"},
+    {"Sensing method", &Exiv2::sensingMethod, "SensingMethod"},
+    {"AF point", &Exiv2::afPoint, "AFpoint"},
 };
 
 static void printFct(EasyAccessFct fct, Exiv2::ExifData ed, const char* label) {

my first guess is a CMake bug where samples are not being compiled as C++17.

edit: applied second version to #2556

@kmilos kmilos merged commit 18e11ac into main Mar 22, 2023
@kmilos kmilos deleted the fix_2557 branch March 22, 2023 10:56
@kmilos
Copy link
Collaborator Author

kmilos commented Mar 22, 2023

@neheb Thanks for looking into it.

my first guess is a CMake bug where samples are not being compiled as C++17

Nope, -std=gnu++17 is in there...

[118/208] Building CXX object samples/CMakeFiles/easyaccess-test.dir/easyaccess-test.cpp.obj
FAILED: samples/CMakeFiles/easyaccess-test.dir/easyaccess-test.cpp.obj
C:\msys64\clang64\bin\c++.exe -DEXV_LOCALEDIR=\"../share/locale\" -DWIN32_LEAN_AND_MEAN -IC:/msys64/
home/kmilos/exiv2/build -IC:/msys64/home/kmilos/exiv2/src -IC:/msys64/home/kmilos/exiv2/includ
e/exiv2 -IC:/msys64/home/kmilos/exiv2/include -g3 -gstrict-dwarf -O0 -std=gnu++17 -fvisibility=hid
den -fvisibility-inlines-hidden -Wp,-D_GLIBCXX_ASSERTIONS -Wall -Wcast-align -Wpointer-arith -Wforma
t-security -Wmissing-format-attribute -Woverloaded-virtual -W -Wno-error=format-nonliteral -municode
 -MD -MT samples/CMakeFiles/easyaccess-test.dir/easyaccess-test.cpp.obj -MF samples\CMakeFiles\easya
ccess-test.dir\easyaccess-test.cpp.obj.d -o samples/CMakeFiles/easyaccess-test.dir/easyaccess-test.c
pp.obj -c C:/msys64/home/kmilos/exiv2/samples/easyaccess-test.cpp
C:/msys64/home/kmilos/exiv2/samples/easyaccess-test.cpp:11:23: error: constexpr variable 'easyAcce
ss' must be initialized by a constant expression
static constexpr auto easyAccess = std::array{
                      ^            ~~~~~~~~~~~
1 error generated.

@kmilos
Copy link
Collaborator Author

kmilos commented Mar 22, 2023

I looked into codecvt. Seems to do the same as iconv. Maybe we should make iconv required on non Windows. Thoughts?

Definitely worth exploring.

@neheb
Copy link
Collaborator

neheb commented Mar 23, 2023

#2553 is a small fix for MSYS2. Well, less compiled code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug compilers Related with compiler options, definitions, support, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code in main does not compile on F38
3 participants