[lmi] PATCH: C++20 and clang

[Vadim Zeitlin]

 Hello,

 First of all, the TL;DR summary: I've created

        https://github.com/let-me-illustrate/lmi/pull/179

which fixes the CI build for clang now that C++20 is used. If you're fine
with the changes there, please feel free to merge them without reading the
rest. However, if, as I suspect, you'd like to know more about this, please
find all the details below:


 Enabling support for designated initializers did have one unexpected
consequence: enabling C++20 for clang also enables the previously disabled
warnings about using enums in potentially suspicious way, resulting in
problems like this:

md5sum.cpp:95:58: error: arithmetic between different enumeration types 
('(anonymous enum at md5sum.hpp:40:1)' and '(anonymous enum at 
md5sum.hpp:38:1)') is deprecated 
[-Werror,-Wdeprecated-anon-enum-enum-conversion]
ihs_basicval.cpp:893:75: error: arithmetic between floating-point type 'double' 
and enumeration type 'mcenum_mode' is deprecated 
[-Werror,-Wdeprecated-enum-float-conversion]
[...many more similar errors in the same file snipped...]
about_dialog.cpp:87:39: error: bitwise operation between different enumeration 
types ('wxDirection' and 'wxAlignment') is deprecated 
[-Werror,-Wdeprecated-enum-enum-conversion]
[...a few more in this one...]
In file included from transferor.cpp:48:
In file included from ~wx/include/wx/filepicker.h:18:
~wx/include/wx/pickerbase.h:150:40: error: bitwise operation between different 
enumeration types ('wxAlignment' and 'wxDirection') is deprecated 
[-Werror,-Wdeprecated-enum-enum-conversion]
pdf_command_wx.cpp:2212:47: error: comparison of enumeration type 
'oenum_premium_flexibility' with floating-point type 'double' is deprecated 
[-Werror,-Wdeprecated-enum-float-conversion]
[...several other similar errors in the same file...]
test_coding_rules.cpp:256:14: error: bitwise operation between different 
enumeration types ('enum_kingdom' and 'enum_phylum') is deprecated 
[-Werror,-Wdeprecated-enum-enum-conversion]
illustration_view.cpp:348:23: error: bitwise operation between different 
enumeration types ('(anonymous enum at ~wx/include/wx/docview.h:47:1)' and 
'(anonymous enum at ~illustration_document.hpp:43:1)') is deprecated 
[-Werror,-Wdeprecated-anon-enum-enum-conversion]

 I think at least some of these warnings should be fixed, e.g. the first
one is fixed by

https://github.com/let-me-illustrate/lmi/commit/9a4888100bbf4029c72fc9c598122079674dcb9f

which is clearly a good idea anyhow, as using enum for defining constants
is a C-ism not useful with contemporary C++ any more.

 Similarly, the ones in about_dialog.cpp are fixed by

https://github.com/let-me-illustrate/lmi/commit/48ef5370a50e6e0d656916506c09ee3eb890d2c2

which is a good idea anyhow and gets rid of the last use of wxSizer::Add()
not using wxSizerFlags in lmi code.

 However the other ones are more problematic. I do agree with clang that
converting mcenum_mode to double or comparing oenum_premium_flexibility
with double is iffy, but I'm not sure if you'd consider doing anything
about this. Personally I think we should add some function converting
double to enum and throwing if the double value doesn't correspond to any
of the enum values, but this won't be a trivial change as it would
introduce a possibility of throwing an exception from places where it
couldn't happen before. The minimum change sufficient to fix this warning
that I see is to convert enum values to double, possibly via an explicitly
named function rather than a cast, but this seems ugly and I'm still not
sure if you'd be willing to do it.

 The warning in wx header won't happen in a build using lmi makefiles
(even if you use it with clang) because wx headers are behind -isystem in
it but I'm going to fix it in wx itself anyhow, so it shouldn't happen
after the next wx update in lmi.

 The one in test_coding_rules.cpp is clearly intentional, i.e. we use
different enums because they are related, and I don't see how to avoid it
without making the code less readable.

 Similarly, the one in illustration_view.cpp is technically correct, but
LMI_WX_CHILD_DOCUMENT is supposed to be used in this way and I don't see
how to avoid it without affecting readability, e.g. by adding a cast.

 So, for now, I've simply disabled all the warnings above in the PR.
However I still think it would be nice to avoid at least
-Wdeprecated-enum-float-conversion, please let me know if you'd be willing
to discuss this further.


 Additionally, the PR also includes the changes needed to make the code
compile with __cplusplus value corresponding to C++20 by avoiding the
#errors in the existing code, please see

https://github.com/let-me-illustrate/lmi/commit/f2023190695640d4ef0e887fab46852f0338680b

These changes are mostly trivial and the code in rate_table.cpp could
probably be improved by judicious use of if constexpr instead of checking
whether WORDS_BIGENDIAN is defined, but I don't think supporting big endian
systems is really a priority and I'd rather avoid spending time on this now
(but please let me know if I should).


 And, finally, I've also disabled a couple of other warnings for clang. In

https://github.com/let-me-illustrate/lmi/pull/179/commits/d703b4b42f826507e0b3651a55615d25357af1b9

I've disabled -Wunused-parameter in Boost.Numeric headers which,
surprisingly, wasn't given before, and still isn't with my clang version
which is slightly older than the one used in the CI build. I have no idea
why does the warning appear with -std=c++20 only, it looks like a bug to me
(or as a bug fix, activated in C++20 mode only), but disabling it looks
like the right thing to do anyhow.

 Disabling the warning about volatile in

https://github.com/let-me-illustrate/lmi/pull/179/commits/104ef04feb7e037769ec22905ca39b7ddbc9c83a

also seems like the right thing to do, at least for now. Dealing with
"volatile" is never obvious (which is one of the reasons it was deprecated
in C++20), so I'd rather not do it for now and just fix the build, but,
again, please let me know if you'd like to discuss this further.



 To summarize, together these changes should be enough to make the CI
builds pass again (in addition to autotools-specific changes which fixed
the gcc build that I've already pushed), which is why it would be nice if
they could be merged relatively soon.

 Thanks in advance!
VZ

pgpJPNCVM4c09.pgp
Description: PGP signature