Re: [lmi] Warnings with gcc-8.2

[Vadim Zeitlin]

On Tue, 19 Mar 2019 19:41:37 +0000 Greg Chicares <address@hidden> wrote:

GC> Debian "buster" now has MinGW-w64 gcc-8.2,

 Yes, but unfortunately it still doesn't seem to include libstdc++fs.a,
which is needed when using std::filesystem. Apparently we'll have to wait
for gcc 9 for it.

GC> ---------
GC> 
GC> wxWidgets rebuilt with no warnings at all except this:
GC> 
GC> i686-w64-mingw32-gcc -c -o wxtiff_tif_predict.o -DNDEBUG 
-I/cache_for_lmi/vcs/wxWidgets/src/zlib -I/cache_for_lmi/vcs/wxWidgets/src/jpeg 
-I/opt/lmi/wx-scratch/lmi-gcc-8.2-win32/src/tiff/libtiff 
-I/cache_for_lmi/vcs/wxWidgets/src/tiff/libtiff  -D_FILE_OFFSET_BITS=64 
-I/opt/lmi/wx-scratch/lmi-gcc-8.2-win32/lib/wx/include/i686-w64-mingw32-msw-unicode-3.1
 -I/cache_for_lmi/vcs/wxWidgets/include -I/opt/lmi/local/include -mthreads 
-Wall -Wundef -O2 -mthreads -fno-omit-frame-pointer  
/cache_for_lmi/vcs/wxWidgets/src/tiff/libtiff/tif_predict.c
GC> 
GC> /cache_for_lmi/vcs/wxWidgets/src/tiff/libtiff/tif_predict.c:281:1: warning: 
'unsigned-integer-overflow' attribute directive ignored [-Wattributes]
GC>  {
GC>  ^
GC> 
GC> Presumably that's harmless, especially because AFAIK we don't actually
GC> use TIFF.

 It does seem harmless, but I wonder why I don't get it here. Using exactly
the same (modulo the paths) command line doesn't produce any warnings. I'm
using this compiler version from Sid:

% i686-w64-mingw32-gcc -v
Using built-in specs.
COLLECT_GCC=i686-w64-mingw32-gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/i686-w64-mingw32/8.2-win32/lto-wrapper
Target: i686-w64-mingw32
Configured with: ../../src/configure --build=x86_64-linux-gnu --prefix=/usr 
--includedir='/usr/include' --mandir='/usr/share/man' 
--infodir='/usr/share/info' --sysconfdir=/etc --localstatedir=/var 
--disable-silent-rules --libdir='/usr/lib/x86_64-linux-gnu' 
--libexecdir='/usr/lib/x86_64-linux-gnu' --disable-maintainer-mode 
--disable-dependency-tracking --prefix=/usr --enable-shared --enable-static 
--disable-multilib --with-system-zlib --libexecdir=/usr/lib 
--without-included-gettext --libdir=/usr/lib --enable-libstdcxx-time=yes 
--with-tune=generic --with-headers=/usr/i686-w64-mingw32/include 
--enable-version-specific-runtime-libs --enable-fully-dynamic-string 
--enable-libgomp --enable-languages=c,c++,fortran,objc,obj-c++,ada --enable-lto 
--with-plugin-ld --enable-threads=win32 --program-suffix=-win32 
--program-prefix=i686-w64-mingw32- --target=i686-w64-mingw32 
--with-as=/usr/bin/i686-w64-mingw32-as --with-ld=/usr/bin/i686-w64-mingw32-ld 
--enable-libatomic
Thread model: win32
gcc version 8.2-win32 20190215 (GCC)

 Perhaps it's later than the one from Buster and a spurious warning got
fixed in this newer version already?

GC> ---------
GC> 
GC> Turning to lmi itself, '-Wno-useless-cast' suppresses diagnostics like this:
GC> 
GC> In file included from /opt/lmi/third_party/include/boost/mpl/int.hpp:20,
GC>                  from 
/opt/lmi/third_party/include/boost/mpl/lambda_fwd.hpp:23,
GC>                  from 
/opt/lmi/third_party/include/boost/mpl/aux_/na_spec.hpp:18,
GC>                  from /opt/lmi/third_party/include/boost/mpl/or.hpp:24,
GC>                  from 
/opt/lmi/third_party/include/boost/iterator/interoperable.hpp:11,
GC>                  from 
/opt/lmi/third_party/include/boost/iterator/iterator_facade.hpp:11,
GC>                  from 
/opt/lmi/third_party/include/boost/filesystem/path.hpp:16,
GC>                  from 
/opt/lmi/third_party/include/boost/filesystem/operations.hpp:18,
GC>                  from /opt/lmi/src/lmi/global_settings.hpp:30,
GC>                  from /opt/lmi/src/lmi/data_directory.cpp:27:
GC> /opt/lmi/third_party/include/boost/mpl/aux_/static_cast.hpp:24:66: error: 
useless cast to type 'int' [-Werror=useless-cast]
GC>  #   define BOOST_MPL_AUX_STATIC_CAST(T, expr) static_cast<T>(expr)
GC>                                                                   ^
GC> /opt/lmi/third_party/include/boost/mpl/aux_/integral_wrapper.hpp:36:95: 
note: in definition of macro 'AUX_WRAPPER_INST'
GC>  #       define AUX_WRAPPER_INST(value) 
BOOST_MPL_AUX_ADL_BARRIER_NAMESPACE::AUX_WRAPPER_NAME< value >
GC>                                                                             
                   ^~~~~
GC> /opt/lmi/third_party/include/boost/mpl/aux_/integral_wrapper.hpp:72:31: 
note: in expansion of macro 'BOOST_MPL_AUX_STATIC_CAST'
GC>      typedef AUX_WRAPPER_INST( 
BOOST_MPL_AUX_STATIC_CAST(AUX_WRAPPER_VALUE_TYPE, (value + 1)) ) next;
GC>                                ^~~~~~~~~~~~~~~~~~~~~~~~~
GC> 
GC> Years ago, I might have patched 'boost/mpl/aux_/static_cast.hpp' to avoid 
that,
GC> but I'm hoping that, with gcc-8.2, we'll be able to remove every dependency 
on
GC> the boost filesystem library, so that this old legacy code will no longer be
GC> used at all. In the meantime, it's okay to suppress the useless-cast 
warning.

 Yes, it looks like there could easily be other occurrences of it in the
generic code, which won't be trivial to get rid of. OTOH this warning could
be useful in "normal" code, as it could indicate a typo or even a more
fundamental mistake, so it would be nice to disable it only locally, i.e.
using pragmas around this code, if possible, rather than globally in the
compiler options.


GC> ---------
GC> 
GC> '-Wno-cast-function-type' suppresses diagnostics like this:
GC> 
GC> /opt/lmi/src/lmi/wx_utility.hpp: In instantiation of 'To c_cast(From) [with 
To = void (wxEvtHandler::*)(wxEvent&); From = void 
(wxEvtHandler::*)(wxKeyEvent&)]':
GC> /opt/lmi/src/lmi/wx_utility.hpp:97:20:   required from 'void 
Connect(wxEvtHandler*, wxEventType, Return (Class::*)(Argument), int, 
wxEvtHandler*) [with Return = void; Class = {anonymous}::InputSequenceTextCtrl; 
Argument = wxKeyEvent&; wxEventType = int]'
GC> /opt/lmi/src/lmi/input_sequence_entry.cpp:1359:13:   required from here
GC> /opt/lmi/src/lmi/wx_utility.hpp:52:12: error: cast between incompatible 
pointer to member types from 'void (wxEvtHandler::*)(wxKeyEvent&)' to 'void 
(wxEvtHandler::*)(wxEvent&)' [-Werror=cast-function-type]

 The compiler is right here, of course, and the cast is indeed invalid. The
problem is that wx relies on such invalid casts to store the event handlers
of all the different types inside a container whose element type is the
same "void (wxEvtHandler::*)(wxEvent&)" and changing this is not easily (or
maybe even at all) possible. It is also "actually safe", as much as relying
on undefined behaviour may be safe, because each handler is only ever
invoked with the correct parameter type, so it's not really crucial... but
quite ugly, of course.

GC> IIRC, I added 'c_cast' to encapsulate usage of C-style casts in code that
GC> used wx facilities in what was an idiomatic way, at least at the time. It
GC> would seem that C-style casts used to work with pointers-to-member, but no
GC> longer do. Vadim, do you see an easy way to rewrite lmi's function template:
GC> 
GC>   /// Function template c_cast() performs an explicit type conversion
GC>   /// (C++98 5.4), which is inherently unsafe. Its sole virtue is that
GC>   /// it is easily searched for.
GC> 
GC>   template<typename To, typename From>
GC>   To c_cast(From z)
GC>   {
GC>       return (To)(z);
GC>   }
GC> 
GC> ...so that this sort of cast compiles cleanly with this warning enabled?

 I haven't tested this yet (should I?), but I'm afraid the only way to
avoid the warning is to use memcpy(). Compiler (at least gcc) will
recognize such trivial use of memcpy() and optimize it away.

GC> ---------
GC> 
GC> '-Wno-noexcept' suppresses diagnostics like this:
GC> 
GC> /usr/lib/gcc/i686-w64-mingw32/8.2-win32/include/c++/type_traits: In 
instantiation of 'constexpr bool std::__call_is_nt(std::__invoke_other) [with 
_Fn = const wxIntegerHash&; _Args = {const long int&}]':
GC> /usr/lib/gcc/i686-w64-mingw32/8.2-win32/include/c++/type_traits:2663:34:   
required by substitution of 'template<bool __v> using __bool_constant = 
std::integral_constant<bool, __v> [with bool __v = std::__call_is_nt<const 
wxIntegerHash&, const long int&>((std::__result_of_success<unsigned int, 
std::__invoke_other>::__invoke_type{}, std::__result_of_success<unsigned int, 
std::__invoke_other>::__invoke_type()))]'
GC> /usr/lib/gcc/i686-w64-mingw32/8.2-win32/include/c++/type_traits:2661:12:   
required from 'struct std::__call_is_nothrow<std::__invoke_result<const 
wxIntegerHash&, const long int&>, const wxIntegerHash&, const long int&>'
GC> /usr/lib/gcc/i686-w64-mingw32/8.2-win32/include/c++/type_traits:131:12:   
required from 'struct std::__and_<std::__is_invocable<const wxIntegerHash&, 
const long int&>, std::__call_is_nothrow<std::__invoke_result<const 
wxIntegerHash&, const long int&>, const wxIntegerHash&, const long int&> >'
GC> /usr/lib/gcc/i686-w64-mingw32/8.2-win32/include/c++/type_traits:2673:12:   
required from 'struct std::__is_nothrow_invocable<const wxIntegerHash&, const 
long int&>'
GC> /usr/lib/gcc/i686-w64-mingw32/8.2-win32/include/c++/type_traits:131:12:   
required from 'struct std::__and_<std::__is_fast_hash<wxIntegerHash>, 
std::__is_nothrow_invocable<const wxIntegerHash&, const long int&> >'
GC> /usr/lib/gcc/i686-w64-mingw32/8.2-win32/include/c++/type_traits:142:31:   
required from 'struct 
std::__not_<std::__and_<std::__is_fast_hash<wxIntegerHash>, 
std::__is_nothrow_invocable<const wxIntegerHash&, const long int&> > >'
GC> 
/usr/lib/gcc/i686-w64-mingw32/8.2-win32/include/c++/bits/unordered_map.h:104:66:
   required from 'class std::unordered_map<long int, long int, wxIntegerHash, 
wxIntegerEqual>'
GC> /opt/lmi/local/include/wx-3.1/wx/generic/grid.h:2007:25:   required from 
here
GC> /opt/lmi/local/include/wx-3.1/wx/hashmap.h:509:12: error: but 'size_t 
wxIntegerHash::operator()(long int) const' does not throw; perhaps it should be 
declared 'noexcept' [-Werror=noexcept]
GC> 
GC> Usually I'd expect a gcc diagnostic like that to have a capitalized initial
GC> fragment of a sentence some lines before "error: but [...] does not throw",
GC> but I can't find the beginning of that sentence anywhere.

 The warning is clear enough, but I'd like to see the compiler command line
resulting in it, so that I could run it here to check that I see it and to
fix it. Because I thought I'd see it by just compiling some wx source
including the same header, but for some reason I don't see it neither,
which starts getting suspicious.

 I'll upgrade MinGW in my lmi VM and test it later, but if you already have
the command which produced the warning above at hand, could you please copy
and paste it here?

GC> Anyway, I guess it's pretty clear what the compiler is saying; Vadim,
GC> is that something that should (and readily can) be addressed in wx?

 Yes, we should define wxNOEXCEPT macro (similar to wxOVERRIDE and
something I actually thought already existed, but it seems I've once again
only thought about adding it, but without actually doing it) and liberally
sprinkle the code with it. Doing this should be quite straightforward, but
I'd just like to check that it really fixes the problem by confirming that
I see it in the first place, which might actually take more time than
fixing it, but is still probably worth doing.

 Best regards,
VZ