Discussion:
How can we treat more compiler warnings as errors?
(too old to reply)
Paul Licameli
2017-07-11 17:17:56 UTC
Permalink
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
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
------------------------------------------------------------------------------
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
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
------------------------------------------------------------------------------
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
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
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
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
------------------------------------------------------------------------------
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
------------------------------------------------------------------------------
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
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)
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,
more
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
------------------------------------------------------------
------------------
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
------------------------------------------------------------
------------------
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-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
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.
------------------------------------------------------------------------------
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
Loading...