[Audacity-devel] How can we treat more compiler warnings as errors?

Discussion:

Paul Licameli

2017-07-11 17:17:56 UTC

It is good to help the compiler help you avoid all sorts of stupid little
mistakes. I make my share of them.

Some of the recent crashes on Windows that were my fault happened because a
non-void function (in fact, a lambda expression) had a path returning no
value, which on Windows debug but not on Mac, returned a garbage pointer.

Shouldn't the compiler have nabbed this early? It didn't, on Travis, or
Mac, or the Windows build. I see Windows IntelliSense noticed the error,
but the build did not fail.

At commit e3ea42ff7ec69c80d8c5dac40aa114996f68c9c2 I changed the Mac XCode
project so that this warning is treated as an error and breaks the build,
at least as I do it with XCode.

This does something to raise the bar for correct code.

Is there a gcc equivalent that can be added to Linux builds? Who, more
adept than I at Linux development, can make such a change?

Are there other warning conditions you want to nominate for treatment as
errors?

PRL

Steve the Fiddle

2017-07-11 17:38:45 UTC

Permalink

A good start on Linux would be to fix the compiler warnings that we
already have. That would make it much easier to spot and fix new
compiler warnings.

Steve

Post by Paul Licameli
It is good to help the compiler help you avoid all sorts of stupid little
mistakes. I make my share of them.
Some of the recent crashes on Windows that were my fault happened because a
non-void function (in fact, a lambda expression) had a path returning no
value, which on Windows debug but not on Mac, returned a garbage pointer.
Shouldn't the compiler have nabbed this early? It didn't, on Travis, or
Mac, or the Windows build. I see Windows IntelliSense noticed the error,
but the build did not fail.
At commit e3ea42ff7ec69c80d8c5dac40aa114996f68c9c2 I changed the Mac XCode
project so that this warning is treated as an error and breaks the build, at
least as I do it with XCode.
This does something to raise the bar for correct code.
Is there a gcc equivalent that can be added to Linux builds? Who, more
adept than I at Linux development, can make such a change?
Are there other warning conditions you want to nominate for treatment as
errors?
PRL
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

Paul Licameli

2017-07-11 17:48:27 UTC

Permalink

Post by Steve the Fiddle
A good start on Linux would be to fix the compiler warnings that we
already have. That would make it much easier to spot and fix new
compiler warnings.
Steve

Ditto XCode. I was preparing a sweep of certain common warnings for
2.1.3. But it didn't get in then, and I would have much rebasing work to
do now.

PRL

Post by Steve the Fiddle

Post by Paul Licameli
It is good to help the compiler help you avoid all sorts of stupid little
mistakes. I make my share of them.
Some of the recent crashes on Windows that were my fault happened

because a

Post by Paul Licameli
non-void function (in fact, a lambda expression) had a path returning no
value, which on Windows debug but not on Mac, returned a garbage pointer.
Shouldn't the compiler have nabbed this early? It didn't, on Travis, or
Mac, or the Windows build. I see Windows IntelliSense noticed the error,
but the build did not fail.
At commit e3ea42ff7ec69c80d8c5dac40aa114996f68c9c2 I changed the Mac

XCode

Post by Paul Licameli
project so that this warning is treated as an error and breaks the

build, at

Post by Paul Licameli
least as I do it with XCode.
This does something to raise the bar for correct code.
Is there a gcc equivalent that can be added to Linux builds? Who, more
adept than I at Linux development, can make such a change?
Are there other warning conditions you want to nominate for treatment as
errors?
PRL
------------------------------------------------------------

------------------

Post by Paul Licameli
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

------------------------------------------------------------
------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

Henric Jungheim

2017-07-11 19:08:34 UTC

Permalink

That goes for Windows too.

In the meantime, turning certain things into errors is one
way of making sure they aren't ignored. In the particular
example mentioned here, gcc should add -Werror=return-type
(IIRC) and MSVC should treat C4716 as an error (/We4716 or
add 4716 to the "Treat Specific Warnings as Errors" box).

Other warnings that should probably be errors:
-- shadowed names (C4458, C4457, C4456; -Wshadow was a bit
noisy in older versions of GCC)
-- possibly uninitialized variable (C4701, C4703)
-- Windows specific: inconsistent DLL linkage (C4273)
-- incompatible types (C4133)

Perhaps unreferenced variables should be errors as well
(C4101, C4189)? There will be some false positives, but it
is usually trivial to avoid them (set to 0 or nullptr).

Even if there is no intent at using or even supporting a
bleeding edge compiler, their diagnostics can be useful.
g++ 7.1 (or even the 8 preview) may have interesting things
to say about the code.

Beyond the normal compiler yammering, Coverity has a long
list of things that is probably worth looking at (I sent a
note to this list about one issue that I believe is legit,
but I'm haven't looked at how to fix) and enabling the
full-on Visual Studio Code Analysis threw up over 13k
warnings (no, I haven't read through them).

Dealing with this in the Audacity source is difficult enough,
but what about the various lib-src libraries? The
"audacity-patches.txt" file seems like the place for any
such per-library support policy notes (there is already
some stuff along those lines).

BTW, since the lame patents are no longer a problem, should
the implementation be in lib-src?

Post by Paul Licameli

Post by Steve the Fiddle
A good start on Linux would be to fix the compiler warnings that we
already have. That would make it much easier to spot and fix new
compiler warnings.
Steve

Ditto XCode. I was preparing a sweep of certain common warnings for
2.1.3. But it didn't get in then, and I would have much rebasing work to
do now.
PRL

Post by Steve the Fiddle

Post by Paul Licameli
It is good to help the compiler help you avoid all sorts of stupid little
mistakes. I make my share of them.
Some of the recent crashes on Windows that were my fault happened

because a

XCode

Post by Paul Licameli
project so that this warning is treated as an error and breaks the

build, at

------------------

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

Robert Hänggi

2017-07-11 20:41:07 UTC

Permalink

Hi Henric
Your link to the
http://www.audioblog.iis.fraunhofer.com/mp3-software-patents-licenses/

is great news.
I think that Audacity should as soon as possible integrate the
encoding capabilities into the core.

Would this affect FFmpeg as well or are there other licences looming behind it?

Robert

Post by Henric Jungheim
That goes for Windows too.
In the meantime, turning certain things into errors is one
way of making sure they aren't ignored. In the particular
example mentioned here, gcc should add -Werror=return-type
(IIRC) and MSVC should treat C4716 as an error (/We4716 or
add 4716 to the "Treat Specific Warnings as Errors" box).
-- shadowed names (C4458, C4457, C4456; -Wshadow was a bit
noisy in older versions of GCC)
-- possibly uninitialized variable (C4701, C4703)
-- Windows specific: inconsistent DLL linkage (C4273)
-- incompatible types (C4133)
Perhaps unreferenced variables should be errors as well
(C4101, C4189)? There will be some false positives, but it
is usually trivial to avoid them (set to 0 or nullptr).
Even if there is no intent at using or even supporting a
bleeding edge compiler, their diagnostics can be useful.
g++ 7.1 (or even the 8 preview) may have interesting things
to say about the code.
Beyond the normal compiler yammering, Coverity has a long
list of things that is probably worth looking at (I sent a
note to this list about one issue that I believe is legit,
but I'm haven't looked at how to fix) and enabling the
full-on Visual Studio Code Analysis threw up over 13k
warnings (no, I haven't read through them).
Dealing with this in the Audacity source is difficult enough,
but what about the various lib-src libraries? The
"audacity-patches.txt" file seems like the place for any
such per-library support policy notes (there is already
some stuff along those lines).
BTW, since the lame patents are no longer a problem, should
the implementation be in lib-src?

On Tue, Jul 11, 2017 at 1:38 PM, Steve the Fiddle

Post by Steve the Fiddle
A good start on Linux would be to fix the compiler warnings that we
already have. That would make it much easier to spot and fix new
compiler warnings.
Steve

Ditto XCode. I was preparing a sweep of certain common warnings for
2.1.3. But it didn't get in then, and I would have much rebasing work to
do now.
PRL

Post by Steve the Fiddle

Post by Paul Licameli
It is good to help the compiler help you avoid all sorts of stupid little
mistakes. I make my share of them.
Some of the recent crashes on Windows that were my fault happened

because a

XCode

Post by Paul Licameli
project so that this warning is treated as an error and breaks the

build, at

------------------

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

Steve the Fiddle

2017-07-11 22:08:13 UTC

Permalink

There are many license issues looming behind FFmpeg.
FFmpeg's take on such issues are here: https://www.ffmpeg.org/legal.html

Steve

Post by Robert HÃ¤nggi
Hi Henric
Your link to the
http://www.audioblog.iis.fraunhofer.com/mp3-software-patents-licenses/
is great news.
I think that Audacity should as soon as possible integrate the
encoding capabilities into the core.
Would this affect FFmpeg as well or are there other licences looming behind it?
Robert

On Tue, Jul 11, 2017 at 1:38 PM, Steve the Fiddle

Post by Steve the Fiddle
A good start on Linux would be to fix the compiler warnings that we
already have. That would make it much easier to spot and fix new
compiler warnings.
Steve

Ditto XCode. I was preparing a sweep of certain common warnings for
2.1.3. But it didn't get in then, and I would have much rebasing work to
do now.
PRL

Post by Steve the Fiddle

Post by Paul Licameli
It is good to help the compiler help you avoid all sorts of stupid little
mistakes. I make my share of them.
Some of the recent crashes on Windows that were my fault happened

because a

XCode

Post by Paul Licameli
project so that this warning is treated as an error and breaks the

build, at

------------------

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

Paul Licameli

2017-07-11 22:53:09 UTC

Permalink

Ah yes, one of my old unmerged branches was about eliminating the
"shadows." That is, variable names in an inner scope that are the same as
those in an outer scope. Or local variables in a function named the same
as arguments. Unclarity about the intent.

PRL

Post by Henric Jungheim
-- possibly uninitialized variable (C4701, C4703)
-- Windows specific: inconsistent DLL linkage (C4273)
-- incompatible types (C4133)
Perhaps unreferenced variables should be errors as well
(C4101, C4189)? There will be some false positives, but it
is usually trivial to avoid them (set to 0 or nullptr).
Even if there is no intent at using or even supporting a
bleeding edge compiler, their diagnostics can be useful.
g++ 7.1 (or even the 8 preview) may have interesting things
to say about the code.
Beyond the normal compiler yammering, Coverity has a long
list of things that is probably worth looking at (I sent a
note to this list about one issue that I believe is legit,
but I'm haven't looked at how to fix) and enabling the
full-on Visual Studio Code Analysis threw up over 13k
warnings (no, I haven't read through them).
Dealing with this in the Audacity source is difficult enough,
but what about the various lib-src libraries? The
"audacity-patches.txt" file seems like the place for any
such per-library support policy notes (there is already
some stuff along those lines).
BTW, since the lame patents are no longer a problem, should
the implementation be in lib-src?
fedoraproject.org/thread/KM557DP7OR2UEEPYQRNHJU7T45XDSXYJ/

On Tue, Jul 11, 2017 at 1:38 PM, Steve the Fiddle <

Post by Steve the Fiddle
A good start on Linux would be to fix the compiler warnings that we
already have. That would make it much easier to spot and fix new
compiler warnings.
Steve

Ditto XCode. I was preparing a sweep of certain common warnings for
2.1.3. But it didn't get in then, and I would have much rebasing work to
do now.
PRL

Post by Steve the Fiddle

Post by Paul Licameli
It is good to help the compiler help you avoid all sorts of stupid

little

Post by Steve the Fiddle

Post by Paul Licameli
mistakes. I make my share of them.
Some of the recent crashes on Windows that were my fault happened

because a

Post by Paul Licameli
non-void function (in fact, a lambda expression) had a path

returning no

Post by Steve the Fiddle

Post by Paul Licameli
value, which on Windows debug but not on Mac, returned a garbage

pointer.

Post by Steve the Fiddle

Post by Paul Licameli
Shouldn't the compiler have nabbed this early? It didn't, on

Travis, or

Post by Steve the Fiddle

Post by Paul Licameli
Mac, or the Windows build. I see Windows IntelliSense noticed the

error,

Post by Steve the Fiddle

Post by Paul Licameli
but the build did not fail.
At commit e3ea42ff7ec69c80d8c5dac40aa114996f68c9c2 I changed the Mac

XCode

Post by Paul Licameli
project so that this warning is treated as an error and breaks the

build, at

Post by Paul Licameli
least as I do it with XCode.
This does something to raise the bar for correct code.
Is there a gcc equivalent that can be added to Linux builds? Who,

Post by Steve the Fiddle

Post by Paul Licameli
adept than I at Linux development, can make such a change?
Are there other warning conditions you want to nominate for

treatment as

Post by Steve the Fiddle

Post by Paul Licameli
errors?
PRL
------------------------------------------------------------

------------------

------------------------------------------------------------

------------------

Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

Henric Jungheim

2017-07-12 03:54:40 UTC

Permalink

Post by Henric Jungheim
-- shadowed names (C4458, C4457, C4456; -Wshadow was a bit
noisy in older versions of GCC)
-- possibly uninitialized variable (C4701, C4703)
-- Windows specific: inconsistent DLL linkage (C4273)
-- incompatible types (C4133)
Perhaps unreferenced variables should be errors as well
(C4101, C4189)? There will be some false positives, but it
is usually trivial to avoid them (set to 0 or nullptr).

That last sentence is regarding uninitialized, not
unreferenced variables.

Steve the Fiddle

2017-07-12 10:35:24 UTC

Permalink

I'm unsure what the etiquette is when encountering compiler warnings.
For example:
* Developer A commits some code
* Developer B notices a "unused variable" or "uninitialised variable"
compiler warning,

Should developer B
a) fix the warning,
b) inform developer A
c) comment the issue
d) do nothing
e) something else

Option (a) is simplest, but could be annoying for developer A if
developer A is still working on that code.

Steve

Post by Henric Jungheim

That last sentence is regarding uninitialized, not
unreferenced variables.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel