Hi, sorry, but this wasn't the only error:

configure:16437: checking ext/hash_set usability configure:16437: /usr/lib/ccache/g++ -c [...] conftest.cpp >&5 In file included from /usr/include/c++/4.8/ext/hash_set:60:0, from conftest.cpp:59: /usr/include/c++/4.8/backward/backward_warning.h:32:2: error: #warning This file includes at least one deprecated or antiquated header which may be removed without further notice at a future date. Please use a non-deprecated interface with equivalent functionality instead. For a listing of replacement headers and interfaces, consult the file backward_warning.h . To disable this warning use -Wno-deprecated. [-Werror=cpp] #warning \ ^ cc1plus: all warnings being treated as errors

Here's the next one.

Regards, Markus

-----Ursprüngliche Nachricht----- Von: Johan Engelen [mailto:jbc.engelen@...2592...] Gesendet: Freitag, 21. März 2014 19:40 An: Inkscape-Devel Betreff: [Inkscape-devel] Warnings check

Hi all, I'd like people to check what compiler errors are reported for trunk on their system (Windows I can do myself), with these flags:

-Wall -Wformat -Werror=format-security -W -Wpointer-arith -Wcast-align -Wsign-compare -Woverloaded-virtual -Werror=switch -Werror=return-type -Werror -Wno-error=pointer-sign -Wno-error=unused-parameter -Wno-error=unused-but-set-variable -Wno-error=strict-overflow -Wno-error=write-strings -Wno-error=format -Wno-error=format-extra-args

Please no comments about whether you like these flags or not, just what build *errors* you get. (don't send me the warnings please!)

(On Windows, with these flags we are down to 3 errorring files, and I'm working on them)

Thanks a lot, Johan

---------------------------------------------------------------------------- -- Learn Graph Databases - Download FREE O'Reilly Book "Graph Databases" is the definitive new guide to graph databases and their applications. Written by three acclaimed leaders in the field, this first edition is now available. Download your free book today! http://p.sf.net/sfu/13534_NeoTech _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel

On 2014-03-21 19:39 +0100, Johan Engelen wrote:

Hi all, I'd like people to check what compiler errors are reported for trunk on their system (Windows I can do myself), with these flags:

-Wall -Wformat -Werror=format-security -W -Wpointer-arith -Wcast-align -Wsign-compare -Woverloaded-virtual -Werror=switch -Werror=return-type -Werror -Wno-error=pointer-sign -Wno-error=unused-parameter -Wno-error=unused-but-set-variable -Wno-error=strict-overflow -Wno-error=write-strings -Wno-error=format -Wno-error=format-extra-args

Please no comments about whether you like these flags or not, just what build *errors* you get. (don't send me the warnings please!)

System: OS X 10.7.5, Xcode 4.3.2

$ llvm-gcc-4.2 --version i686-apple-darwin11-llvm-gcc-4.2 (GCC) 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2336.9.00)

$ clang --version Apple clang version 3.1 (tags/Apple/clang-318.0.58) (based on LLVM 3.1svn)

1a) llvm-gcc-4.2: fails during configure:

checking whether the C++ compiler works... no

config.log: cc1plus: error: -Werror=unused-but-set-variable: No option -Wunused-but-set-variable

1b) if '-Werror=unused-but-set-variable' is removed from $CPPFLAGS, build fails here:

CXX gc.o cc1plus: warnings being treated as errors ../../src/gc.cpp: In function ‘void Inkscape::GC::<unnamed>::do_init()’: ../../src/gc.cpp:31: warning: ‘GC_no_dls’ is deprecated (declared at /Volumes/magenta/mp-trunk/quartz/include/gc/gc.h:257) ../../src/gc.cpp:32: warning: ‘GC_all_interior_pointers’ is deprecated (declared at /Volumes/magenta/mp-trunk/quartz/include/gc/gc.h:143) ../../src/gc.cpp:33: warning: ‘GC_finalize_on_demand’ is deprecated (declared at /Volumes/magenta/mp-trunk/quartz/include/gc/gc.h:159) make[1]: *** [gc.o] Error 1

1c) Running 'make -k' (not a reliable check, I know) to continue:

- All C files produces new warnings: CC libcroco/cr-utils.o cc1: warning: command line option "-Woverloaded-virtual" is valid for C++/ObjC++ but not for C

- Don't see this error with regular builds, why now? In file included from /Volumes/magenta/mp-trunk/quartz/include/glibmm-2.4/glibmm.h:89, from /Volumes/magenta/mp-trunk/quartz/include/gdkmm-2.4/gdkmm/pixbuf.h:9, from ../../src/libdepixelize/kopftracer2011.h:32, from ../../src/libdepixelize/kopftracer2011.cpp:36: /Volumes/magenta/mp-trunk/quartz/include/glibmm-2.4/glibmm/threads.h:201: error: field ‘gobject_’ has incomplete type /Volumes/magenta/mp-trunk/quartz/include/glibmm-2.4/glibmm/threads.h: In member function ‘GThread* Glib::Threads::Thread::gobj()’: /Volumes/magenta/mp-trunk/quartz/include/glibmm-2.4/glibmm/threads.h:197: error: ‘gobject_’ was not declared in this scope /Volumes/magenta/mp-trunk/quartz/include/glibmm-2.4/glibmm/threads.h: In member function ‘const GThread* Glib::Threads::Thread::gobj() const’: /Volumes/magenta/mp-trunk/quartz/include/glibmm-2.4/glibmm/threads.h:198: error: ‘gobject_’ was not declared in this scope make[3]: *** [libdepixelize/kopftracer2011.o] Error 1

- another warning treated as error: cc1plus: warnings being treated as errors ../../src/display/nr-filter-gaussian.cpp: In function ‘void Inkscape::Filters::filter2D_IIR(PT*, int, int, const PT*, int, int, int, int, const IIRValue*, const double*, IIRValue* const*, int) [with PT = unsigned char, unsigned int PC = 1u, bool PREMULTIPLIED_ALPHA = false]’: ../../src/display/nr-filter-gaussian.cpp:503: instantiated from here ../../src/display/nr-filter-gaussian.cpp:342: warning: comparison of unsigned expression < 0 is always false ../../src/display/nr-filter-gaussian.cpp:503: instantiated from here ../../src/display/nr-filter-gaussian.cpp:357: warning: comparison of unsigned expression < 0 is always false make[3]: *** [display/nr-filter-gaussian.o] Error 1

2a) clang: boost warning during configure:

checking boost/unordered_set.hpp presence... yes configure: WARNING: boost/unordered_set.hpp: present but cannot be compiled <snip> checking for boost/unordered_set.hpp... no

2b) default build on OS X with lcms1: clang errors out here:

In file included from ../../src/color-profile.cpp:36: /Volumes/magenta/mp-trunk/quartz/include/lcms.h:1418:14: error: comparison of unsigned expression < 0 is always false [-Werror,-Wtautological-compare] if (size < 0) return NULL; // Prevent signed size_t exploits ~~~~ ^ ~

2c) if configured to use lcms2 to avoid above error, clang errors out a couple of files later:

../../src/libcroco/cr-enc-handler.c:92:32: error: cast from 'enum CREncoding *' to 'CREncHandler *' (aka 'struct _CREncHandler *') increases required alignment from 4 to 8 [-Werror,-Wcast-align] return (CREncHandler *) ^~~~~~~~~~~~~~~~

There are tons of cast-align warnings with clang and current boost 1.55, like these (from regular clang build):

In file included from ../../src/display/drawing.h:21: In file included from ../../src/display/drawing-item.h:19: In file included from /Volumes/magenta/mp-trunk/quartz/include/boost/intrusive/list.hpp:20: In file included from /Volumes/magenta/mp-trunk/quartz/include/boost/intrusive/list_hook.hpp:19: In file included from /Volumes/magenta/mp-trunk/quartz/include/boost/intrusive/detail/utilities.hpp:33: In file included from /Volumes/magenta/mp-trunk/quartz/include/boost/functional/hash.hpp:6: In file included from /Volumes/magenta/mp-trunk/quartz/include/boost/functional/hash/hash.hpp:15: /Volumes/magenta/mp-trunk/quartz/include/boost/functional/hash/detail/hash_float.hpp:71:25: warning: cast from 'char *' to 'std::size_t *' (aka 'unsigned long *') increases required alignment from 1 to 8 [-Wcast-align] seed = *(std::size_t*) ptr; ^~~~~~~~~~~~~~~~~~

... so even fixing all cast-align warnings from Inkscape (lots of them with clang e.g. from src/extension/internal/emf* src/extension/internal/wmf* ) would not help.

It seems that this older version of clang doesn't interpret the order or type of the flags as expected - AFAIU cast-align warnings are not meant to be treated as errors?

Regards, V

P.S. I am aware that I use an outdated version of of clang from an outdated version of Xcode (4.3.2) on an outdated version of OS X (10.7.5) and that sooner or later Inkscape (trunk) will no longer support building on system like these.

If I append the flags like this:

CPPFLAGS="-Werror=format-security -Werror=switch -Werror=return-type $CPPFLAGS -Wall -Wformat -Werror=format-security -W -Wpointer-arith -Wcast-align -Wsign-compare -Woverloaded-virtual -Werror=switch -Werror=return-type -Werror -Wno-error=pointer-sign -Wno-error=unused-parameter -Wno-error=unused-but-set-variable -Wno-error=strict-overflow -Wno-error=write-strings -Wno-error=format -Wno-error=format-extra-args"

I'm not sure, though, whether this is the correct solution to this problem. The comment above says that this only checks whether the compiler supports these flags, doesn't it? It configures without errors. When compiling I get the same errors as su_v (kopftracer and ‘gobject_’ has incomplete type...).

Regards, Markus

-----Ursprüngliche Nachricht----- Von: Johan Engelen [mailto:jbc.engelen@...2592...] Gesendet: Freitag, 21. März 2014 23:26 An: Markus Engel; 'Inkscape-Devel' Betreff: Re: AW: [Inkscape-devel] Warnings check

On 21-3-2014 21:14, Markus Engel wrote:

Hi, on Ubuntu 13.10 x64 I can't even run configure without errors:

configure:23320: checking glibmm/threads.h usability configure:23320: /usr/lib/ccache/g++ -c [...] conftest.cpp >&5 conftest.cpp:75:0: error: "ENABLE_NLS" redefined [-Werror] #define ENABLE_NLS /**/ ^ conftest.cpp:25:0: note: this is the location of the previous definition #define ENABLE_NLS 1 ^ cc1plus: all warnings being treated as errors

Ouch..... It'd be great if some people could work on at least getting configure to work with -Werror. Does it help if you add the flags to configure.ac line 99 ?

Thanks a bunch, Johan

On Sat, Mar 22, 2014 at 05:42:34AM +0100, Krzysztof Kosiński wrote:

2014-03-21 23:25 GMT+01:00 Johan Engelen <jbc.engelen@...2592...>:

...

Ouch..... It'd be great if some people could work on at least getting configure to work with -Werror.

-Werror should NEVER be enabled by default, as it is equivalent to explicitly disabling forward compatibility with compilers and libraries.

1. The compiler might add a new warning, and it so happens that

existing code triggers it. Build broken. 2. An external library we're using might deprecate a function. Build broken. 3. An external library might add warning-generating code to one of its headers. Build broken.

In other words, -Werror should only be enabled when someone explicitly requests it in configure. Only specific warnings should be converted to errors.

If it helps, I'm not against having -Werror set in the codebase at this particular time, since we're trying to focus the project on QA more tightly than usual. I do agree we should not ship actual packages that have this specified though; we don't want to make it any harder for users to get up and running.

Bryce

On Sat, Mar 22, 2014 at 3:55 AM, Johan Engelen <jbc.engelen@...2592...> wrote:

Does it help if you add the flags to configure.ac line 99 ?

Similar configure errors again.

On 22-3-2014 5:42, Krzysztof Kosiński wrote:

2014-03-21 23:25 GMT+01:00 Johan Engelen <jbc.engelen@...2592...>:

...

Ouch..... It'd be great if some people could work on at least getting configure to work with -Werror.

-Werror should NEVER be enabled by default, as it is equivalent to explicitly disabling forward compatibility with compilers and libraries.

I think it should definitely be default during development. And then you simply disable it for release.

1. The compiler might add a new warning, and it so happens that

existing code triggers it. Build broken.

Great! Compiler builders are very careful when adding new warnings to -Wall. When a new warning is triggered, it very likely means our code is bad and needs improvement. Breaking the build is (in our project) the only reliable way to get people to fix things.

2. An external library we're using might deprecate a function. Build broken.

Again, great! We don't want to release code that will be broken in the future because it uses a deprecated function, right?

3. An external library might add warning-generating code to one of its

headers. Build broken.

This is a valid concern, and is why I added -Wno-error=... flags. For example, libcroco (our copy in any case) is broken as it uses guchar* where it calls external lib functions that expect gchar*. And our devlibs version of boost::make_shared also contains a cast-alignment warning. Disabling erroring on specific warnings is easy, and much better in my opinion than not errorring on warnings at all. I'm pretty sure that the devs of libraries worth depending on try hard to resolve any warnings. In lib2geom we *really* should build with -Werror.

In other words, -Werror should only be enabled when someone explicitly requests it in configure. Only specific warnings should be converted to errors.

In my opinion, -Werror should be default for our development. Only specific warnings should be allowed to not error. (this set is much smaller than it's inverse)

-Johan

Thanks su_v! This is massively useful. I'll start working on clearing some of the warnings, so some of the no-error flags can be removed.

-Johan

On 22-3-2014 12:42, su_v wrote:

On 2014-03-21 19:39 +0100, Johan Engelen wrote:

...

Hi all, I'd like people to check what compiler errors are reported for trunk on their system (Windows I can do myself), with these flags:

-Wall -Wformat -Werror=format-security -W -Wpointer-arith -Wcast-align -Wsign-compare -Woverloaded-virtual -Werror=switch -Werror=return-type -Werror -Wno-error=pointer-sign -Wno-error=unused-parameter -Wno-error=unused-but-set-variable -Wno-error=strict-overflow -Wno-error=write-strings -Wno-error=format -Wno-error=format-extra-args

Please no comments about whether you like these flags or not, just what build *errors* you get. (don't send me the warnings please!)

Attached are two diffs which allowed to compile successfully with:

• clang 3.1 (based on LLVM 3.1svn)
• GCC 4.2.1 (llvm-gcc-4.2)

(don't use both at the same time)

Notes:

• clang diff was tested with dbus enabled, llvm-gcc-4.2 without.

• cast-align warnings from boost headers are suppressed with #pragma

• I failed to disable Werror for the warnings from lcms1 with

llvm-gcc-4.2, so the build was allowed to use lcms2 instead.

• No tests have been run with experimental GTK3.

• The warnings which had to be turned off have comments referring to the

files where the error occurred. If you need more detailed error logs, let me now.

• Ideally, the section in configure.ac would be converted into a loop so

that each flag gets tested individually whether the compiler supports it or not. I didn't spend time trying to figure out how to do that. For testing, those that fail can be individually commented out.

On 2014-03-22 12:42 +0100, su_v wrote:

On 2014-03-21 19:39 +0100, Johan Engelen wrote:

...

Hi all, I'd like people to check what compiler errors are reported for trunk on their system (Windows I can do myself), with these flags:

-Wall -Wformat -Werror=format-security -W -Wpointer-arith -Wcast-align -Wsign-compare -Woverloaded-virtual -Werror=switch -Werror=return-type -Werror -Wno-error=pointer-sign -Wno-error=unused-parameter -Wno-error=unused-but-set-variable -Wno-error=strict-overflow -Wno-error=write-strings -Wno-error=format -Wno-error=format-extra-args

Please no comments about whether you like these flags or not, just what build *errors* you get. (don't send me the warnings please!)

Attached are two diffs which allowed to compile successfully with:

• clang 3.1 (based on LLVM 3.1svn)
• GCC 4.2.1 (llvm-gcc-4.2)

The diff for GCC was faulty, I'll share a new one in a couple of minutes..

(don't use both at the same time)

Notes:

• clang diff was tested with dbus enabled, llvm-gcc-4.2 without.

• cast-align warnings from boost headers are suppressed with #pragma

• I failed to disable Werror for the warnings from lcms1 with

llvm-gcc-4.2, so the build was allowed to use lcms2 instead.

• No tests have been run with experimental GTK3.

• The warnings which had to be turned off have comments referring to the

files where the error occurred. If you need more detailed error logs, let me now.

• Ideally, the section in configure.ac would be converted into a loop so

that each flag gets tested individually whether the compiler supports it or not. I didn't spend time trying to figure out how to do that. For testing, those that fail can be individually commented out.

-Wno-error=self-assign

more errors:

../../src/libuemf/uwmf_endian.c:285:10: error: explicitly assigning a variable of type 'int' to itself [-Werror,-Wself-assign] ../../src/libuemf/uwmf_endian.c:310:10: error: explicitly assigning a variable of type 'int' to itself [-Werror,-Wself-assign] ../../src/libuemf/uwmf_endian.c:703:10: error: explicitly assigning a variable of type 'int' to itself [-Werror,-Wself-assign]

On 2014-03-22 13:25 +0100, Johan Engelen wrote:

With clang, can you try again without (after bzr up) -Wno-error=return-type -Wno-error=self-assign -Wno-error=parentheses-equality

Thanks, Johan

On 22-3-2014 12:42, su_v wrote:

...

On 2014-03-21 19:39 +0100, Johan Engelen wrote:

...

Hi all, I'd like people to check what compiler errors are reported for trunk on their system (Windows I can do myself), with these flags:

-Wall -Wformat -Werror=format-security -W -Wpointer-arith -Wcast-align -Wsign-compare -Woverloaded-virtual -Werror=switch -Werror=return-type -Werror -Wno-error=pointer-sign -Wno-error=unused-parameter -Wno-error=unused-but-set-variable -Wno-error=strict-overflow -Wno-error=write-strings -Wno-error=format -Wno-error=format-extra-args

Please no comments about whether you like these flags or not, just what build *errors* you get. (don't send me the warnings please!)

Attached are two diffs which allowed to compile successfully with:

• clang 3.1 (based on LLVM 3.1svn)
• GCC 4.2.1 (llvm-gcc-4.2)

(don't use both at the same time)

Notes:

• clang diff was tested with dbus enabled, llvm-gcc-4.2 without.

• cast-align warnings from boost headers are suppressed with #pragma

• I failed to disable Werror for the warnings from lcms1 with

llvm-gcc-4.2, so the build was allowed to use lcms2 instead.

• No tests have been run with experimental GTK3.

• The warnings which had to be turned off have comments referring to the

files where the error occurred. If you need more detailed error logs, let me now.

• Ideally, the section in configure.ac would be converted into a loop so

that each flag gets tested individually whether the compiler supports it or not. I didn't spend time trying to figure out how to do that. For testing, those that fail can be individually commented out.

On Fri, 21 Mar 2014 22:29:53 -0700 Jon Cruz <jon@...18...> wrote:

On Mar 21, 2014, at 9:42 PM, Krzysztof Kosiński wrote:

...

2014-03-21 23:25 GMT+01:00 Johan Engelen <jbc.engelen@...2592...>:

...

Ouch..... It'd be great if some people could work on at least getting configure to work with -Werror.

-Werror should NEVER be enabled by default, as it is equivalent to explicitly disabling forward compatibility with compilers and libraries.

Actually...

Warnings as errors is generally considered as a very good practice for professional software houses. The recommendations and standard industry practices are usually to have it on by default and then turn it off for the specific cases where it is required.

In practice future-proofing the code is usually achieved by writing code that complies with the standards, and that works cross-platform. Occasionally third-party libs can get their headers wrapped up in other headers to proof them against warnings.

Given that I've seen this work with code originally written for Netware, it definitely can work with future proofing.

Dear Jon,

At Johan's suggestion the Makefile I wrote recently allows:

- Ordinary system builders to build without -Werror - Developers to build with -Werror on all files apart from 'naughty' ones. Those files are compiled without -Werror. This makes it hard to add new warnings to the code base.

I believe this approach, which is somewhat more aggressive than Johan's current approach will encourage good development practice like you say, whilst not impinging on others. The Makefile nags developers to address the 'naughty' files to help keep the numbers down.

Krzysztof re-iterates the points I raised with Johan about platform compatibility. When I was writing the Makefiles for 0.48.4 I tried -Werror and I noticed significantly more errors with newer gccs, with newer glibcs, with -D_FORTIFY_SOURCE=2 and with newer libraries. Going from Debian 7.4 to my normal platform added many more which wouldn't compile with -Werror. Getting to zero errors is only going to be possible on a reference platform.

I like the use of -Werror -Wno-error=XYZ, but I respectfully suggest that to maintain code quality these flags are only applied to certain 'naughty' files. In future on the reference platform all other files should be compiled with just -Werror.

You note that other projects wrap libraries which generate compiler warnings. This might work in our case but I believe even this is vulnerable to the vagaries of compiler and library versions.

In my forthcoming re-hash of the Naughty Step as I called it, I intend to support different options for different platforms. Although this doesn't help us to fix the warnings it might help us get an overview of the warnings people on different platforms/with different build options experience. At the moment I suspect many people with different platforms wouldn't know whether a warning is specific to them or if it is a general problem.

Regards,

Is.

PS. Johan, on gcc at least:

1. Change -W to -Wextra (-W is old-hat). 2. Remove -Werror=switch and -Werror=return-type (-Werror makes them redundant).

http://inkscape.13.x6.nabble.com/Incomplete-switch-statement-tp4969823.html

On 22-3-2014 18:02, Markus Engel wrote:

Now even our automatic builds by Launchpad fail: CXX ui/dialog/input.o ../../src/ui/dialog/input.cpp: In member function 'bool Inkscape::UI::Dialog::InputDialogImpl::eventSnoop(GdkEvent*)': ../../src/ui/dialog/input.cpp:1917:16: error: enumeration value 'GDK_SOURCE_KEYBOARD' not handled in switch [-Werror=switch] ../../src/ui/dialog/input.cpp:1917:16: error: enumeration value 'GDK_SOURCE_TOUCHSCREEN' not handled in switch [-Werror=switch] ../../src/ui/dialog/input.cpp:1917:16: error: enumeration value 'GDK_SOURCE_TOUCHPAD' not handled in switch [-Werror=switch] cc1plus: some warnings being treated as errors

Did you check this in before fixing it? Receiving tons of mail by the build bots is annoying.

Regards, Markus

-----Ursprüngliche Nachricht----- Von: Johan Engelen [mailto:jbc.engelen@...2592...] Gesendet: Samstag, 22. März 2014 13:25 An: su_v; Inkscape-Devel Betreff: Re: [Inkscape-devel] Warnings check

With clang, can you try again without (after bzr up) -Wno-error=return-type -Wno-error=self-assign -Wno-error=parentheses-equality

Thanks, Johan

On 22-3-2014 12:42, su_v wrote:

...

On 2014-03-21 19:39 +0100, Johan Engelen wrote:

...

Hi all, I'd like people to check what compiler errors are reported for trunk on their system (Windows I can do myself), with these flags:

-Wall -Wformat -Werror=format-security -W -Wpointer-arith -Wcast-align -Wsign-compare -Woverloaded-virtual -Werror=switch -Werror=return-type -Werror -Wno-error=pointer-sign -Wno-error=unused-parameter -Wno-error=unused-but-set-variable -Wno-error=strict-overflow -Wno-error=write-strings -Wno-error=format -Wno-error=format-extra-args

Please no comments about whether you like these flags or not, just what build *errors* you get. (don't send me the warnings please!)

Attached are two diffs which allowed to compile successfully with:

• clang 3.1 (based on LLVM 3.1svn)
• GCC 4.2.1 (llvm-gcc-4.2)

(don't use both at the same time)

Notes:

• clang diff was tested with dbus enabled, llvm-gcc-4.2 without.

• cast-align warnings from boost headers are suppressed with #pragma

• I failed to disable Werror for the warnings from lcms1 with

llvm-gcc-4.2, so the build was allowed to use lcms2 instead.

• No tests have been run with experimental GTK3.

• The warnings which had to be turned off have comments referring to

the files where the error occurred. If you need more detailed error logs, let me now.

• Ideally, the section in configure.ac would be converted into a loop

so that each flag gets tested individually whether the compiler supports it or not. I didn't spend time trying to figure out how to do that. For testing, those that fail can be individually commented out.

---

-- Learn Graph Databases - Download FREE O'Reilly Book "Graph Databases" is the definitive new guide to graph databases and their applications. Written by three acclaimed leaders in the field, this first edition is now available. Download your free book today! http://p.sf.net/sfu/13534_NeoTech _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel

Does the compiler warn with

void U_WMRCORE_SIZE16_swap(char *record, int /*torev*/){

as well? I've seen this several times.

Regards, Markus

-----Ursprüngliche Nachricht----- Von: mathog [mailto:mathog@...1176...] Gesendet: Dienstag, 25. März 2014 00:20 An: su_v Cc: Inkscape Devel Betreff: Re: [Inkscape-devel] Warnings check

On 22-Mar-2014 06:16, su_v wrote:

...

-Wno-error=self-assign

more errors:

../../src/libuemf/uwmf_endian.c:285:10: error: explicitly assigning a variable of type 'int' to itself [-Werror,-Wself-assign]

Sometimes you can't win.

The line with the warning is the second one below:

void U_WMRCORE_SIZE16_swap(char *record, int torev){ torev = torev; // shuts up compiler warnings about unused parameters U_swap4(record, 1); /* Size16_4 is at offset 0 in U_METARECORD */ }

where this is one of those situations where there are a large number of functions all use the same argument list, but some of them don't actually use some of the arguments. The

torev = torev

trick is one way to silence the "unused argument" warning in gcc and last time I checked it worked for several other compilers as well, like Sun's compiler. Apparently not for clang though. Note this is in a C module, so the C++ syntax to indicate an unused argument will not work. AFAIK the C language specification provides no standard way to eliminate this warning or its evil twin "statement with no effect", which is what you get if you try to finesse this situation with:

(void) torev;

Note, the other similar clang warnings are all for the same thing.

Regards,

David Mathog mathog@...1176... Manager, Sequence Analysis Facility, Biology Division, Caltech

---------------------------------------------------------------------------- -- Learn Graph Databases - Download FREE O'Reilly Book "Graph Databases" is the definitive new guide to graph databases and their applications. Written by three acclaimed leaders in the field, this first edition is now available. Download your free book today! http://p.sf.net/sfu/13534_NeoTech _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel

On 25-3-2014 0:24, Markus Engel wrote:

Does the compiler warn with

void U_WMRCORE_SIZE16_swap(char *record, int /*torev*/){

as well? I've seen this several times.

It's C code Markus. (I'll stop there...)

Regards, Markus

-----Ursprüngliche Nachricht----- Von: mathog [mailto:mathog@...1176...] Gesendet: Dienstag, 25. März 2014 00:20 An: su_v Cc: Inkscape Devel Betreff: Re: [Inkscape-devel] Warnings check

On 22-Mar-2014 06:16, su_v wrote:

...

...

-Wno-error=self-assign

more errors:

../../src/libuemf/uwmf_endian.c:285:10: error: explicitly assigning a variable of type 'int' to itself [-Werror,-Wself-assign]

Sometimes you can't win.

The line with the warning is the second one below:

void U_WMRCORE_SIZE16_swap(char *record, int torev){ torev = torev; // shuts up compiler warnings about unused parameters U_swap4(record, 1); /* Size16_4 is at offset 0 in U_METARECORD */ }

where this is one of those situations where there are a large number of functions all use the same argument list, but some of them don't actually use some of the arguments. The

 torev = torev

trick is one way to silence the "unused argument" warning in gcc and last time I checked it worked for several other compilers as well, like Sun's compiler. Apparently not for clang though. Note this is in a C module, so the C++ syntax to indicate an unused argument will not work. AFAIK the C language specification provides no standard way to eliminate this warning or its evil twin "statement with no effect", which is what you get if you try to finesse this situation with:

 (void) torev;

Note, the other similar clang warnings are all for the same thing.

Regards,

David Mathog mathog@...1176... Manager, Sequence Analysis Facility, Biology Division, Caltech

---

-- Learn Graph Databases - Download FREE O'Reilly Book "Graph Databases" is the definitive new guide to graph databases and their applications. Written by three acclaimed leaders in the field, this first edition is now available. Download your free book today! http://p.sf.net/sfu/13534_NeoTech _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel

---

Learn Graph Databases - Download FREE O'Reilly Book "Graph Databases" is the definitive new guide to graph databases and their applications. Written by three acclaimed leaders in the field, this first edition is now available. Download your free book today! http://p.sf.net/sfu/13534_NeoTech _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel

On 24-Mar-2014 16:28, Johan Engelen wrote:

On 25-3-2014 0:20, mathog wrote:

...

On 22-Mar-2014 06:16, su_v wrote:

...

...

-Wno-error=self-assign

more errors:

../../src/libuemf/uwmf_endian.c:285:10: error: explicitly assigning a variable of type 'int' to itself [-Werror,-Wself-assign]

Sometimes you can't win.

The line with the warning is the second one below:

void U_WMRCORE_SIZE16_swap(char *record, int torev){ torev = torev; // shuts up compiler warnings about unused parameters U_swap4(record, 1); /* Size16_4 is at offset 0 in U_METARECORD */ }

where this is one of those situations where there are a large number of functions all use the same argument list, but some of them don't actually use some of the arguments. The

 torev = torev

trick is one way to silence the "unused argument" warning in gcc and last time I checked it worked for several other compilers as well, like Sun's compiler. Apparently not for clang though. Note this is in a C module, so the C++ syntax to indicate an unused argument will not work. AFAIK the C language specification provides no standard way to eliminate this warning or its evil twin "statement with no effect", which is what you get if you try to finesse this situation with:

 (void) torev;

Note, the other similar clang warnings are all for the same thing.

The problem is that torev = torev is not instructive as to what it is doing. [*]

Really? To me it clearly does nothing at all! When the programmer wonders why there is a line that does nothing at all they can read the comment I always try to include next to it.

I do wish that the C language, as opposed to a few C compiler extensions, supported the C++ syntax for ignoring function arguments, but sadly it does not.

The self-assignment may actually have side-effects for C++ objects, so for us such a line looks very suspicious.

Understood, but finding it in a .c file should tell you that it has nothing to do with C++ objects.

A much better way (and more generally accepted I believe) is what I already did in a number of places in libuemf: http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/13182

Unfortunately, as I indicated above, that is not actually any better, as it generates

"statement with no effect"

warnings with other compilers and/or compiler versions.

I don't have clang, nor do I have Markus's scanner, nor do I have any of the Microsoft compilers, if anybody is using those on Windows. If somebody who does have these can send me:

1. the syntax that suppresses all warnings associated with an unused function argument 2. the preprocessor Macro that identifies that environment

I will set up the appropriate tests in one of the header files so that libUEMF compiles silently in all of our build environments. Until, most likely, we hit some future compiler or compiler revision that wants yet another syntax. My experience has been that it is basically impossible to write anything more than a toy program that will continue to build cleanly forever into the future, since the compiler developers are always adding new warnings, and unlike error messages, warning messages often do not actually indicate bugs. As in this case.

Regards,

David Mathog mathog@...1176... Manager, Sequence Analysis Facility, Biology Division, Caltech

On Mar 24, 2014, at 4:20 PM, mathog wrote:

torev = torev

trick is one way to silence the "unused argument" warning in gcc and last time I checked it worked for several other compilers as well, like Sun's compiler. Apparently not for clang though. Note this is in a C module, so the C++ syntax to indicate an unused argument will not work. AFAIK the C language specification provides no standard way to eliminate this warning or its evil twin "statement with no effect", which is what you get if you try to finesse this situation with:

(void) torev;

Quite interesting... my experience has been some of the opposite.

That is, the self assignment usually would only work for MSVC and not gcc. But perhaps that is combined with C vs C++ mode/flags on the compiler.

Using the void cast would usually be preferable, and was happy on many systems including compilers for Netware. :-)

For C++, of course, we can just leave the name off the parameter. But as you mention, these files should be C so we can't do that there. I think the main alternatives are to use some compiler specific decorations on the params with macro magic, or an UNUSED_PARAM() or REF_PARAM() macro that can do the 'correct' thing in most cases and then maybe have one or two edge case variants.