[PATCH] configure: enable warnings for shadowed variables
These can easily lead to incredibly confusing errors, and should practically never happen. I'd have loved to make this a -Werror even, but sadly there is way too many instances in the codebase right now that first needs to be weeded out. --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index a8e67d230c..547b67565d 100755 --- a/configure +++ b/configure @@ -7406,6 +7406,7 @@ check_cflags -Wundef check_cflags -Wmissing-prototypes check_cflags -Wstrict-prototypes check_cflags -Wempty-body +check_cflags -Wshadow if enabled extra_warnings; then check_cflags -Wcast-qual -- 2.44.2
On Sat, Sep 07, 2024 at 08:52:41PM +0200, Timo Rothenpieler wrote:
These can easily lead to incredibly confusing errors, and should practically never happen. I'd have loved to make this a -Werror even, but sadly there is way too many instances in the codebase right now that first needs to be weeded out. --- configure | 1 + 1 file changed, 1 insertion(+)
diff --git a/configure b/configure index a8e67d230c..547b67565d 100755 --- a/configure +++ b/configure @@ -7406,6 +7406,7 @@ check_cflags -Wundef check_cflags -Wmissing-prototypes check_cflags -Wstrict-prototypes check_cflags -Wempty-body +check_cflags -Wshadow
can you provide a list of warnings this produces ? but in principle this seems a good idea thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Does the universe only have a finite lifespan? No, its going to go on forever, its just that you wont like living in it. -- Hiranya Peiri
On 07.09.2024 21:07, Michael Niedermayer wrote:
On Sat, Sep 07, 2024 at 08:52:41PM +0200, Timo Rothenpieler wrote:
These can easily lead to incredibly confusing errors, and should practically never happen. I'd have loved to make this a -Werror even, but sadly there is way too many instances in the codebase right now that first needs to be weeded out. --- configure | 1 + 1 file changed, 1 insertion(+)
diff --git a/configure b/configure index a8e67d230c..547b67565d 100755 --- a/configure +++ b/configure @@ -7406,6 +7406,7 @@ check_cflags -Wundef check_cflags -Wmissing-prototypes check_cflags -Wstrict-prototypes check_cflags -Wempty-body +check_cflags -Wshadow
can you provide a list of warnings this produces ?
List is attached.
Timo Rothenpieler:
These can easily lead to incredibly confusing errors, and should practically never happen. I'd have loved to make this a -Werror even, but sadly there is way too many instances in the codebase right now that first needs to be weeded out.
IMO these instances would need to be weeded out before this is enabled even as a warning.
--- configure | 1 + 1 file changed, 1 insertion(+)
diff --git a/configure b/configure index a8e67d230c..547b67565d 100755 --- a/configure +++ b/configure @@ -7406,6 +7406,7 @@ check_cflags -Wundef check_cflags -Wmissing-prototypes check_cflags -Wstrict-prototypes check_cflags -Wempty-body +check_cflags -Wshadow
if enabled extra_warnings; then check_cflags -Wcast-qual
On 08.09.2024 10:15, Andreas Rheinhardt wrote:
Timo Rothenpieler:
These can easily lead to incredibly confusing errors, and should practically never happen. I'd have loved to make this a -Werror even, but sadly there is way too many instances in the codebase right now that first needs to be weeded out.
IMO these instances would need to be weeded out before this is enabled even as a warning.
There is almost 1000 of them without enabling much of any dependencies. So I think that's next to impossible.
On Sep 8, 2024, at 20:39, Timo Rothenpieler <timo@rothenpieler.org> wrote:
On 08.09.2024 10:15, Andreas Rheinhardt wrote:
Timo Rothenpieler:
These can easily lead to incredibly confusing errors, and should practically never happen. I'd have loved to make this a -Werror even, but sadly there is way too many instances in the codebase right now that first needs to be weeded out. IMO these instances would need to be weeded out before this is enabled even as a warning.
There is almost 1000 of them without enabling much of any dependencies. So I think that's next to impossible.
Then this patch will make it harder to see new warnings within thousands of warnings, isn’t it? It’s annoying to enable this warning before fix those code first.
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 08.09.2024 18:44, Zhao Zhili wrote:
On Sep 8, 2024, at 20:39, Timo Rothenpieler <timo@rothenpieler.org> wrote:
On 08.09.2024 10:15, Andreas Rheinhardt wrote:
Timo Rothenpieler:
These can easily lead to incredibly confusing errors, and should practically never happen. I'd have loved to make this a -Werror even, but sadly there is way too many instances in the codebase right now that first needs to be weeded out. IMO these instances would need to be weeded out before this is enabled even as a warning.
There is almost 1000 of them without enabling much of any dependencies. So I think that's next to impossible.
Then this patch will make it harder to see new warnings within thousands of warnings, isn’t it? It’s annoying to enable this warning before fix those code first.
Well, the reality is, that without printing all these warnings, nobody will ever fix them. Unless someone sits down and spends potentially multiple days if not weeks of work to fix all the current ones. If we print all the warnings instead, it can get fixed gradually.
Hi, Le 8 septembre 2024 19:57:38 GMT+03:00, Timo Rothenpieler <timo@rothenpieler.org> a écrit :
Well, the reality is, that without printing all these warnings, nobody will ever fix them. Unless someone sits down and spends potentially multiple days if not weeks of work to fix all the current ones.
If we print all the warnings instead, it can get fixed gradually.
I can only agree with Zhao and Andreas here. If you want to turn a warning flag on, you have to eliminate the warnings first. Otherwise you're just drowning the existing and typically more serious warnings in a sea of new warnings. The result is obvious: people will just ignore *all* warnigns then, and that's worse.
Quoting Rémi Denis-Courmont (2024-09-09 09:32:07)
Hi,
Le 8 septembre 2024 19:57:38 GMT+03:00, Timo Rothenpieler <timo@rothenpieler.org> a écrit :
Well, the reality is, that without printing all these warnings, nobody will ever fix them. Unless someone sits down and spends potentially multiple days if not weeks of work to fix all the current ones.
If we print all the warnings instead, it can get fixed gradually.
I can only agree with Zhao and Andreas here. If you want to turn a warning flag on, you have to eliminate the warnings first.
Otherwise you're just drowning the existing and typically more serious warnings in a sea of new warnings. The result is obvious: people will just ignore *all* warnigns then, and that's worse.
+1 -- Anton Khirnov
On 09/09/2024 12:02, Anton Khirnov wrote:
Quoting Rémi Denis-Courmont (2024-09-09 09:32:07)
Hi,
Le 8 septembre 2024 19:57:38 GMT+03:00, Timo Rothenpieler <timo@rothenpieler.org> a écrit :
Well, the reality is, that without printing all these warnings, nobody will ever fix them. Unless someone sits down and spends potentially multiple days if not weeks of work to fix all the current ones.
If we print all the warnings instead, it can get fixed gradually.
I can only agree with Zhao and Andreas here. If you want to turn a warning flag on, you have to eliminate the warnings first.
Otherwise you're just drowning the existing and typically more serious warnings in a sea of new warnings. The result is obvious: people will just ignore *all* warnigns then, and that's worse.
+1
I don't have time to fix 1000+ warnings, so this is just blocked then. Great.
On Mon, Sep 9, 2024 at 1:23 PM Timo Rothenpieler <timo@rothenpieler.org> wrote:
On 09/09/2024 12:02, Anton Khirnov wrote:
Quoting Rémi Denis-Courmont (2024-09-09 09:32:07)
Hi,
Le 8 septembre 2024 19:57:38 GMT+03:00, Timo Rothenpieler < timo@rothenpieler.org> a écrit :
Well, the reality is, that without printing all these warnings, nobody
will ever fix them.
Unless someone sits down and spends potentially multiple days if not weeks of work to fix all the current ones.
If we print all the warnings instead, it can get fixed gradually.
I can only agree with Zhao and Andreas here. If you want to turn a warning flag on, you have to eliminate the warnings first.
Otherwise you're just drowning the existing and typically more serious warnings in a sea of new warnings. The result is obvious: people will just ignore *all* warnigns then, and that's worse.
+1
I don't have time to fix 1000+ warnings, so this is just blocked then.
Maybe this could be a task to be sponsored with the STF funds? -- Vittorio
participants (7)
-
Andreas Rheinhardt -
Anton Khirnov -
Michael Niedermayer -
Rémi Denis-Courmont -
Timo Rothenpieler -
Vittorio Giovara -
Zhao Zhili