Re: [lmi] Warnings with gcc-8.2

[Greg Chicares]

On 2019-03-19 23:50, Vadim Zeitlin wrote:
> 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.

Alas. But it's still easier to upgrade as each new release becomes available
than to delay and try to apply multiple upgrades years later.

> 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?

No, the 'gcc -v' output is exactly the same: I pasted mine (below)
and yours (above) into two separate files and compared them, and
the only difference was the prompt, "% " vs. "$":

$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) 

However, your source file may very well differ from mine, because I'm
using a version of wx that's almost a year old:

/cache_for_lmi/vcs/wxWidgets[0]$git log -1
commit e38866d3a603f600f87016458260f73593627348 (HEAD)
Merge: 457ba4ace52 deee5c19eb7
Author: Vadim Zeitlin <address@hidden>
Date:   2018-04-06T13:40:20+00:00

    Merge branch 'lzma'
    
    Add support for using externally available liblzma via new
    wxLZMA{Input,Output}Stream classes.
    
    Closes https://github.com/wxWidgets/wxWidgets/pull/771

We'll just ignore this inconsequential warning. I don't plan to upgrade wx
at this moment--it's better to upgrade the compiler separately. If upgrading
wx can help with the '-Wno-noexcept' issue below, then we can upgrade wx
separately, later.

> 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> /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> 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.

Yes, I'll look into that, now that I know MinGW-w64 gcc-8.2 doesn't have
std::filesystem yet and we'll therefore have to keep boost for a while longer.

> 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.

Oh. Okay.

> 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
[...'c_cast' function template...]
> 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.

Thanks, I think I'll look into disabling it with a pragma instead.

> 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> /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]
[...]
>  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.

Here's the entire command for compiling 'previewframe_ex.o', with all
of its output:

i686-w64-mingw32-g++ -MMD -MP -MT previewframe_ex.o -MF previewframe_ex.d  -c 
-I /opt/lmi/src/lmi -I /opt/lmi/src/lmi/tools/pete-2.1.1 -I 
/opt/lmi/local/lib/wx/include/i686-w64-mingw32-msw-unicode-3.1 -I 
/opt/lmi/local/include/wx-3.1 -I /opt/lmi/third_party/include -I 
/opt/lmi/third_party/src -I /opt/lmi/local/include -I 
/opt/lmi/local/include/libxml2 -DLMI_WX_NEW_USE_SO  -DLIBXML_USE_DLL -DSTRICT   
 -D_FILE_OFFSET_BITS=64 -DWXUSINGDLL -D__WXMSW__ -DBOOST_NO_AUTO_PTR 
-DBOOST_STRICT_CONFIG -DBOOST_STATIC_ASSERT_HPP   -frounding-math -std=c++17 
-pedantic-errors -Werror -Wall -Walloc-zero -Walloca -Wcast-align -Wconversion 
-Wdangling-else -Wdeprecated-declarations -Wdisabled-optimization 
-Wdouble-promotion -Wduplicated-branches -Wduplicated-cond -Wextra 
-Wformat-nonliteral -Wformat-security -Wformat-signedness -Wformat-y2k -Wimport 
-Winit-self -Winvalid-pch -Wlogical-op -Wmissing-include-dirs -Wmultichar 
-Wpacked -Wpointer-arith -Wredundant-decls -Wrestrict -Wshadow -Wsign-compare 
-Wstack-protector -Wtrampolines -Wundef -Wunreachable-code -Wunused-macros 
-Wvector-operation-performance -Wwrite-strings -Wno-parentheses  -Wc++11-compat 
-Wc++14-compat -Wc++1z-compat -Wconditionally-supported -Wctor-dtor-privacy 
-Wdelete-non-virtual-dtor -Wdeprecated -Wnoexcept -Wnoexcept-type 
-Wnon-template-friend -Woverloaded-virtual -Wpmf-conversions -Wregister 
-Wreorder -Wstrict-null-sentinel -Wsynth -Wuseless-cast  -Wcast-qual    
-D'BOOST_STATIC_ASSERT(A)=static_assert((A))'   -ggdb -O2 
-fno-omit-frame-pointer    /opt/lmi/src/lmi/previewframe_ex.cpp 
-opreviewframe_ex.o
In file included from /opt/lmi/local/include/wx-3.1/wx/clntdata.h:16,
                 from /opt/lmi/local/include/wx-3.1/wx/event.h:17,
                 from /opt/lmi/src/lmi/wx_utility.hpp:29,
                 from /opt/lmi/src/lmi/previewframe_ex.hpp:38,
                 from /opt/lmi/src/lmi/previewframe_ex.cpp:32:
/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 unsigned int&}]':
/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 unsigned int&>((std::__result_of_success<unsigned 
int, std::__invoke_other>::__invoke_type{}, std::__result_of_success<unsigned 
int, std::__invoke_other>::__invoke_type()))]'
/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 unsigned int&>, const wxIntegerHash&, const long 
unsigned int&>'
/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 unsigned int&>, std::__call_is_nothrow<std::__invoke_result<const 
wxIntegerHash&, const long unsigned int&>, const wxIntegerHash&, const long 
unsigned int&> >'
/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 unsigned int&>'
/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 unsigned int&> >'
/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 unsigned int&> > >'
/usr/lib/gcc/i686-w64-mingw32/8.2-win32/include/c++/bits/unordered_map.h:104:66:
   required from 'class std::unordered_map<long unsigned int, 
wxImageHistogramEntry, wxIntegerHash, wxIntegerEqual>'
/opt/lmi/local/include/wx-3.1/wx/image.h:193:50:   required from here
/opt/lmi/local/include/wx-3.1/wx/hashmap.h:510:12: error: but 'size_t 
wxIntegerHash::operator()(long unsigned int) const' does not throw; perhaps it 
should be declared 'noexcept' [-Werror=noexcept]
     size_t operator()( unsigned long x ) const { return ulongHash( x ); }
            ^~~~~~~~
cc1plus: all warnings being treated as errors
make[1]: *** [/opt/lmi/src/lmi/workhorse.make:916: previewframe_ex.o] Error 1

As above, I'm using lmi HEAD, with a year-old wx.

> 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.

Yes, that sounds like the right thing to do. Thanks.