public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
@ 2022-06-08  7:17 Felix Willgerodt
  2022-06-08 12:58 ` Bruno Larsen
  2022-07-15 18:40 ` Tom Tromey
  0 siblings, 2 replies; 13+ messages in thread
From: Felix Willgerodt @ 2022-06-08  7:17 UTC (permalink / raw)
  To: gdb-patches

This patch fixes two issues with callfuncs.exp, which are both related
to new Clang warnings:

1) Clang 15.0.0 added a new warning for deprecated non-prototype functions:
https://reviews.llvm.org/D122895
Callfuncs.exp is impacted and won't run due to new warnings:

callfuncs.c:339:5: warning: a function declaration without a prototype is
deprecated in all versions of C and is not supported in C2x
[-Wdeprecated-non-prototype]
int t_float_values (float_arg1, float_arg2)

This patch disables those warnings with -Wno-deprecated-non-prototype.
Removing the test for deprecated syntax would also be an option. But I will
leave that for others to decide.

2) The other new warnings are about comparing a define with floats and doubles:

callfuncs.c:518:1: warning: floating-point comparison is always true; constant
cannot be represented exactly in type 'float' [-Wliteral-range]
DEF_FUNC_VALUES_3(fc, float, crealf, cimagf)

This can be fixed by making the define a float.
---
 gdb/testsuite/gdb.base/callfuncs.c   | 2 +-
 gdb/testsuite/gdb.base/callfuncs.exp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/callfuncs.c b/gdb/testsuite/gdb.base/callfuncs.c
index d18d0dba073..cf7c75c1360 100644
--- a/gdb/testsuite/gdb.base/callfuncs.c
+++ b/gdb/testsuite/gdb.base/callfuncs.c
@@ -96,7 +96,7 @@ long double _Complex ldc3 = 3.0L + 3.0Li;
 long double _Complex ldc4 = 4.0L + 4.0Li;
 #endif /* TEST_COMPLEX */
 
-#define DELTA (0.001)
+#define DELTA (0.001f)
 
 char *string_val1 = (char *)"string 1";
 char *string_val2 = (char *)"string 2";
diff --git a/gdb/testsuite/gdb.base/callfuncs.exp b/gdb/testsuite/gdb.base/callfuncs.exp
index 4448cc127ba..d74357b8b48 100644
--- a/gdb/testsuite/gdb.base/callfuncs.exp
+++ b/gdb/testsuite/gdb.base/callfuncs.exp
@@ -18,7 +18,7 @@
 
 standard_testfile
 
-set compile_flags {debug}
+set compile_flags {debug additional_flags=-Wno-deprecated-non-prototype}
 if [support_complex_tests] {
     lappend compile_flags "additional_flags=-DTEST_COMPLEX"
 }
-- 
2.34.3

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-06-08  7:17 [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings Felix Willgerodt
@ 2022-06-08 12:58 ` Bruno Larsen
  2022-06-08 15:11   ` Willgerodt, Felix
  2022-07-15 18:40 ` Tom Tromey
  1 sibling, 1 reply; 13+ messages in thread
From: Bruno Larsen @ 2022-06-08 12:58 UTC (permalink / raw)
  To: Felix Willgerodt, gdb-patches


On 6/8/22 04:17, Felix Willgerodt via Gdb-patches wrote:
> This patch fixes two issues with callfuncs.exp, which are both related
> to new Clang warnings:

Hi Felix!

Thanks for this!

> 
> 1) Clang 15.0.0 added a new warning for deprecated non-prototype functions:
> https://reviews.llvm.org/D122895
> Callfuncs.exp is impacted and won't run due to new warnings:
> 
> callfuncs.c:339:5: warning: a function declaration without a prototype is
> deprecated in all versions of C and is not supported in C2x
> [-Wdeprecated-non-prototype]
> int t_float_values (float_arg1, float_arg2)
> 
> This patch disables those warnings with -Wno-deprecated-non-prototype.
> Removing the test for deprecated syntax would also be an option. But I will
> leave that for others to decide.

I like your solution with -Wno-deprecated-non-prototype.

I think it is important (at least for now) to keep this test, since we have to support very old setups.

> 
> 2) The other new warnings are about comparing a define with floats and doubles:
> 
> callfuncs.c:518:1: warning: floating-point comparison is always true; constant
> cannot be represented exactly in type 'float' [-Wliteral-range]
> DEF_FUNC_VALUES_3(fc, float, crealf, cimagf)
> 
> This can be fixed by making the define a float.

Genuine question, would this not cause a problem for the times where the parameters are doubles and DELTA is a float? If it isn't a problem, I'm fine with this.

With all that said, I can't approve patches for pushing. Let's hope a maintainer shows up to approve :-)

Cheers!
Bruno Larsen
> ---
>   gdb/testsuite/gdb.base/callfuncs.c   | 2 +-
>   gdb/testsuite/gdb.base/callfuncs.exp | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/callfuncs.c b/gdb/testsuite/gdb.base/callfuncs.c
> index d18d0dba073..cf7c75c1360 100644
> --- a/gdb/testsuite/gdb.base/callfuncs.c
> +++ b/gdb/testsuite/gdb.base/callfuncs.c
> @@ -96,7 +96,7 @@ long double _Complex ldc3 = 3.0L + 3.0Li;
>   long double _Complex ldc4 = 4.0L + 4.0Li;
>   #endif /* TEST_COMPLEX */
>   
> -#define DELTA (0.001)
> +#define DELTA (0.001f)
>   
>   char *string_val1 = (char *)"string 1";
>   char *string_val2 = (char *)"string 2";
> diff --git a/gdb/testsuite/gdb.base/callfuncs.exp b/gdb/testsuite/gdb.base/callfuncs.exp
> index 4448cc127ba..d74357b8b48 100644
> --- a/gdb/testsuite/gdb.base/callfuncs.exp
> +++ b/gdb/testsuite/gdb.base/callfuncs.exp
> @@ -18,7 +18,7 @@
>   
>   standard_testfile
>   
> -set compile_flags {debug}
> +set compile_flags {debug additional_flags=-Wno-deprecated-non-prototype}
>   if [support_complex_tests] {
>       lappend compile_flags "additional_flags=-DTEST_COMPLEX"
>   }


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-06-08 12:58 ` Bruno Larsen
@ 2022-06-08 15:11   ` Willgerodt, Felix
  2022-07-18 13:37     ` Andrew Burgess
  0 siblings, 1 reply; 13+ messages in thread
From: Willgerodt, Felix @ 2022-06-08 15:11 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches

> -----Original Message-----
> From: Bruno Larsen <blarsen@redhat.com>
> Sent: Mittwoch, 8. Juni 2022 14:59
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
> new clang warnings.
> 
> 
> On 6/8/22 04:17, Felix Willgerodt via Gdb-patches wrote:
> > This patch fixes two issues with callfuncs.exp, which are both related
> > to new Clang warnings:
> 
> Hi Felix!
> 
> Thanks for this!
> 
> >
> > 1) Clang 15.0.0 added a new warning for deprecated non-prototype
> functions:
> > https://reviews.llvm.org/D122895
> > Callfuncs.exp is impacted and won't run due to new warnings:
> >
> > callfuncs.c:339:5: warning: a function declaration without a prototype is
> > deprecated in all versions of C and is not supported in C2x
> > [-Wdeprecated-non-prototype]
> > int t_float_values (float_arg1, float_arg2)
> >
> > This patch disables those warnings with -Wno-deprecated-non-prototype.
> > Removing the test for deprecated syntax would also be an option. But I will
> > leave that for others to decide.
> 
> I like your solution with -Wno-deprecated-non-prototype.
> 
> I think it is important (at least for now) to keep this test, since we have to
> support very old setups.
> 
> >
> > 2) The other new warnings are about comparing a define with floats and
> doubles:
> >
> > callfuncs.c:518:1: warning: floating-point comparison is always true;
> constant
> > cannot be represented exactly in type 'float' [-Wliteral-range]
> > DEF_FUNC_VALUES_3(fc, float, crealf, cimagf)
> >
> > This can be fixed by making the define a float.
> 
> Genuine question, would this not cause a problem for the times where the
> parameters are doubles and DELTA is a float? If it isn't a problem, I'm fine
> with this.
> 

I was wondering about that as well, but it still passes with GCC, clang and Intel
compilers and there are no compiler warnings about it.
I couldn't really figure it out if it would actually a problem somewhere. I could only
test on linux x86 though. My best guess was, as it is the "smaller precision" the
compiler will do the right thing. But we could always add a DELTA that is double
as well. Hopefully someone else can chime in.

Thanks,
Felix

> With all that said, I can't approve patches for pushing. Let's hope a maintainer
> shows up to approve :-)
> 
> Cheers!
> Bruno Larsen
> > ---
> >   gdb/testsuite/gdb.base/callfuncs.c   | 2 +-
> >   gdb/testsuite/gdb.base/callfuncs.exp | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/gdb/testsuite/gdb.base/callfuncs.c
> b/gdb/testsuite/gdb.base/callfuncs.c
> > index d18d0dba073..cf7c75c1360 100644
> > --- a/gdb/testsuite/gdb.base/callfuncs.c
> > +++ b/gdb/testsuite/gdb.base/callfuncs.c
> > @@ -96,7 +96,7 @@ long double _Complex ldc3 = 3.0L + 3.0Li;
> >   long double _Complex ldc4 = 4.0L + 4.0Li;
> >   #endif /* TEST_COMPLEX */
> >
> > -#define DELTA (0.001)
> > +#define DELTA (0.001f)
> >
> >   char *string_val1 = (char *)"string 1";
> >   char *string_val2 = (char *)"string 2";
> > diff --git a/gdb/testsuite/gdb.base/callfuncs.exp
> b/gdb/testsuite/gdb.base/callfuncs.exp
> > index 4448cc127ba..d74357b8b48 100644
> > --- a/gdb/testsuite/gdb.base/callfuncs.exp
> > +++ b/gdb/testsuite/gdb.base/callfuncs.exp
> > @@ -18,7 +18,7 @@
> >
> >   standard_testfile
> >
> > -set compile_flags {debug}
> > +set compile_flags {debug additional_flags=-Wno-deprecated-non-
> prototype}
> >   if [support_complex_tests] {
> >       lappend compile_flags "additional_flags=-DTEST_COMPLEX"
> >   }

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-06-08  7:17 [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings Felix Willgerodt
  2022-06-08 12:58 ` Bruno Larsen
@ 2022-07-15 18:40 ` Tom Tromey
  2022-07-18  9:41   ` Willgerodt, Felix
  1 sibling, 1 reply; 13+ messages in thread
From: Tom Tromey @ 2022-07-15 18:40 UTC (permalink / raw)
  To: Felix Willgerodt via Gdb-patches

>>>>> "Felix" == Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org> writes:

Felix> This patch disables those warnings with -Wno-deprecated-non-prototype.

What happens when running the test against gcc?

Tom

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-07-15 18:40 ` Tom Tromey
@ 2022-07-18  9:41   ` Willgerodt, Felix
  2022-07-18 13:28     ` Andrew Burgess
  0 siblings, 1 reply; 13+ messages in thread
From: Willgerodt, Felix @ 2022-07-18  9:41 UTC (permalink / raw)
  To: Tom Tromey, Felix Willgerodt via Gdb-patches

> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Freitag, 15. Juli 2022 20:40
> To: Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Willgerodt, Felix <felix.willgerodt@intel.com>
> Subject: Re: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
> new clang warnings.
> 
> >>>>> "Felix" == Felix Willgerodt via Gdb-patches <gdb-
> patches@sourceware.org> writes:
> 
> Felix> This patch disables those warnings with -Wno-deprecated-non-
> prototype.
> 
> What happens when running the test against gcc?
> 

The test continues to pass.
I ran it with gcc 9.4, 10.3.1 and 11.3.1 on somewhat latest Ubuntu/Fedora.
There is also this comment in gdb.exp already:

    # Some C/C++ testcases unconditionally pass -Wno-foo as additional
    # options to disable some warning.  That is OK with GCC, because
    # by design, GCC accepts any -Wno-foo option, even if it doesn't
    # support -Wfoo.  Clang however warns about unknown -Wno-foo by
    # default, unless you pass -Wno-unknown-warning-option as well.
    # We do that here, so that individual testcases don't have to
    # worry about it.

That said, I just tested it again with the latest Intel compilers. And GDB only
adds the "additional_flags=-Wno-unknown-warning-option" for clang, not
for icx/icc. I will write a new patch for that soon. But that is a separate
issue.

Regards,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-07-18  9:41   ` Willgerodt, Felix
@ 2022-07-18 13:28     ` Andrew Burgess
  2022-07-18 13:42       ` Willgerodt, Felix
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2022-07-18 13:28 UTC (permalink / raw)
  To: Willgerodt, Felix, Tom Tromey, Felix Willgerodt via Gdb-patches

"Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org> writes:

>> -----Original Message-----
>> From: Tom Tromey <tom@tromey.com>
>> Sent: Freitag, 15. Juli 2022 20:40
>> To: Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Willgerodt, Felix <felix.willgerodt@intel.com>
>> Subject: Re: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
>> new clang warnings.
>> 
>> >>>>> "Felix" == Felix Willgerodt via Gdb-patches <gdb-
>> patches@sourceware.org> writes:
>> 
>> Felix> This patch disables those warnings with -Wno-deprecated-non-
>> prototype.
>> 
>> What happens when running the test against gcc?
>> 
>
> The test continues to pass.
> I ran it with gcc 9.4, 10.3.1 and 11.3.1 on somewhat latest Ubuntu/Fedora.
> There is also this comment in gdb.exp already:
>
>     # Some C/C++ testcases unconditionally pass -Wno-foo as additional
>     # options to disable some warning.  That is OK with GCC, because
>     # by design, GCC accepts any -Wno-foo option, even if it doesn't
>     # support -Wfoo.  Clang however warns about unknown -Wno-foo by
>     # default, unless you pass -Wno-unknown-warning-option as well.
>     # We do that here, so that individual testcases don't have to
>     # worry about it.
>
> That said, I just tested it again with the latest Intel compilers. And GDB only
> adds the "additional_flags=-Wno-unknown-warning-option" for clang, not
> for icx/icc. I will write a new patch for that soon. But that is a separate
> issue.

Doesn't that mean adding this patch will can cause a test to regress
with the Intel compiler?  Shouldn't adding -Wno-unknown-warning-option
for Intel be a prerequisite for this patch?

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-06-08 15:11   ` Willgerodt, Felix
@ 2022-07-18 13:37     ` Andrew Burgess
  2022-07-19  9:06       ` Willgerodt, Felix
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2022-07-18 13:37 UTC (permalink / raw)
  To: Willgerodt, Felix, Bruno Larsen, gdb-patches

"Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org> writes:

>> -----Original Message-----
>> From: Bruno Larsen <blarsen@redhat.com>
>> Sent: Mittwoch, 8. Juni 2022 14:59
>> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
>> patches@sourceware.org
>> Subject: Re: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
>> new clang warnings.
>> 
>> 
>> On 6/8/22 04:17, Felix Willgerodt via Gdb-patches wrote:
>> > This patch fixes two issues with callfuncs.exp, which are both related
>> > to new Clang warnings:
>> 
>> Hi Felix!
>> 
>> Thanks for this!
>> 
>> >
>> > 1) Clang 15.0.0 added a new warning for deprecated non-prototype
>> functions:
>> > https://reviews.llvm.org/D122895
>> > Callfuncs.exp is impacted and won't run due to new warnings:
>> >
>> > callfuncs.c:339:5: warning: a function declaration without a prototype is
>> > deprecated in all versions of C and is not supported in C2x
>> > [-Wdeprecated-non-prototype]
>> > int t_float_values (float_arg1, float_arg2)
>> >
>> > This patch disables those warnings with -Wno-deprecated-non-prototype.
>> > Removing the test for deprecated syntax would also be an option. But I will
>> > leave that for others to decide.
>> 
>> I like your solution with -Wno-deprecated-non-prototype.
>> 
>> I think it is important (at least for now) to keep this test, since we have to
>> support very old setups.
>> 
>> >
>> > 2) The other new warnings are about comparing a define with floats and
>> doubles:
>> >
>> > callfuncs.c:518:1: warning: floating-point comparison is always true;
>> constant
>> > cannot be represented exactly in type 'float' [-Wliteral-range]
>> > DEF_FUNC_VALUES_3(fc, float, crealf, cimagf)
>> >
>> > This can be fixed by making the define a float.
>> 
>> Genuine question, would this not cause a problem for the times where the
>> parameters are doubles and DELTA is a float? If it isn't a problem, I'm fine
>> with this.
>> 
>
> I was wondering about that as well, but it still passes with GCC, clang and Intel
> compilers and there are no compiler warnings about it.
> I couldn't really figure it out if it would actually a problem somewhere. I could only
> test on linux x86 though. My best guess was, as it is the "smaller precision" the
> compiler will do the right thing.

Indeed, my understanding of type promotion is that the compiler will
promote the float argument to double or long double as needed.

But then, prior to this patch, when DELTA was just (0.001), and would be
considered a double, I would have expected, in any comparison between a
float and DELTA, for the float to be converted to double, so I don't
really understand that part of the original patch.

Thanks,
Andrew

>                                    But we could always add a DELTA that is double
> as well. Hopefully someone else can chime in.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-07-18 13:28     ` Andrew Burgess
@ 2022-07-18 13:42       ` Willgerodt, Felix
  0 siblings, 0 replies; 13+ messages in thread
From: Willgerodt, Felix @ 2022-07-18 13:42 UTC (permalink / raw)
  To: Andrew Burgess, Tom Tromey, Felix Willgerodt via Gdb-patches

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Montag, 18. Juli 2022 15:28
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; Tom Tromey
> <tom@tromey.com>; Felix Willgerodt via Gdb-patches <gdb-
> patches@sourceware.org>
> Subject: RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
> new clang warnings.
> 
> "Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org> writes:
> 
> >> -----Original Message-----
> >> From: Tom Tromey <tom@tromey.com>
> >> Sent: Freitag, 15. Juli 2022 20:40
> >> To: Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org>
> >> Cc: Willgerodt, Felix <felix.willgerodt@intel.com>
> >> Subject: Re: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
> >> new clang warnings.
> >>
> >> >>>>> "Felix" == Felix Willgerodt via Gdb-patches <gdb-
> >> patches@sourceware.org> writes:
> >>
> >> Felix> This patch disables those warnings with -Wno-deprecated-non-
> >> prototype.
> >>
> >> What happens when running the test against gcc?
> >>
> >
> > The test continues to pass.
> > I ran it with gcc 9.4, 10.3.1 and 11.3.1 on somewhat latest Ubuntu/Fedora.
> > There is also this comment in gdb.exp already:
> >
> >     # Some C/C++ testcases unconditionally pass -Wno-foo as additional
> >     # options to disable some warning.  That is OK with GCC, because
> >     # by design, GCC accepts any -Wno-foo option, even if it doesn't
> >     # support -Wfoo.  Clang however warns about unknown -Wno-foo by
> >     # default, unless you pass -Wno-unknown-warning-option as well.
> >     # We do that here, so that individual testcases don't have to
> >     # worry about it.
> >
> > That said, I just tested it again with the latest Intel compilers. And GDB only
> > adds the "additional_flags=-Wno-unknown-warning-option" for clang, not
> > for icx/icc. I will write a new patch for that soon. But that is a separate
> > issue.
> 
> Doesn't that mean adding this patch will can cause a test to regress
> with the Intel compiler?  Shouldn't adding -Wno-unknown-warning-option
> for Intel be a prerequisite for this patch?

My thought process was that there are already other tests that added
a "Wno-foo" option and these (and the testsuite) have therefore already
regressed against icc/icx in that regard.
Of course this particular test would stop running on older icc/icx versions.
Yet it would start running again for newer clang and icx versions.

That said, I don't think we need to discuss this any further. I have already
written the patch and am just running some more extensive testing.
I will post it soon. I am fine to make it a prerequisite.

Regards,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-07-18 13:37     ` Andrew Burgess
@ 2022-07-19  9:06       ` Willgerodt, Felix
  2022-07-19  9:14         ` Andrew Burgess
  0 siblings, 1 reply; 13+ messages in thread
From: Willgerodt, Felix @ 2022-07-19  9:06 UTC (permalink / raw)
  To: Andrew Burgess, Bruno Larsen, gdb-patches

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Montag, 18. Juli 2022 15:37
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; Bruno Larsen
> <blarsen@redhat.com>; gdb-patches@sourceware.org
> Subject: RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
> new clang warnings.
> 
> "Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org> writes:
> 
> >> -----Original Message-----
> >> From: Bruno Larsen <blarsen@redhat.com>
> >> Sent: Mittwoch, 8. Juni 2022 14:59
> >> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> >> patches@sourceware.org
> >> Subject: Re: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
> >> new clang warnings.
> >>
> >>
> >> On 6/8/22 04:17, Felix Willgerodt via Gdb-patches wrote:
> >> > This patch fixes two issues with callfuncs.exp, which are both related
> >> > to new Clang warnings:
> >>
> >> Hi Felix!
> >>
> >> Thanks for this!
> >>
> >> >
> >> > 1) Clang 15.0.0 added a new warning for deprecated non-prototype
> >> functions:
> >> > https://reviews.llvm.org/D122895
> >> > Callfuncs.exp is impacted and won't run due to new warnings:
> >> >
> >> > callfuncs.c:339:5: warning: a function declaration without a prototype is
> >> > deprecated in all versions of C and is not supported in C2x
> >> > [-Wdeprecated-non-prototype]
> >> > int t_float_values (float_arg1, float_arg2)
> >> >
> >> > This patch disables those warnings with -Wno-deprecated-non-
> prototype.
> >> > Removing the test for deprecated syntax would also be an option. But I
> will
> >> > leave that for others to decide.
> >>
> >> I like your solution with -Wno-deprecated-non-prototype.
> >>
> >> I think it is important (at least for now) to keep this test, since we have to
> >> support very old setups.
> >>
> >> >
> >> > 2) The other new warnings are about comparing a define with floats and
> >> doubles:
> >> >
> >> > callfuncs.c:518:1: warning: floating-point comparison is always true;
> >> constant
> >> > cannot be represented exactly in type 'float' [-Wliteral-range]
> >> > DEF_FUNC_VALUES_3(fc, float, crealf, cimagf)
> >> >
> >> > This can be fixed by making the define a float.
> >>
> >> Genuine question, would this not cause a problem for the times where
> the
> >> parameters are doubles and DELTA is a float? If it isn't a problem, I'm fine
> >> with this.
> >>
> >
> > I was wondering about that as well, but it still passes with GCC, clang and
> Intel
> > compilers and there are no compiler warnings about it.
> > I couldn't really figure it out if it would actually a problem somewhere. I
> could only
> > test on linux x86 though. My best guess was, as it is the "smaller precision"
> the
> > compiler will do the right thing.
> 
> Indeed, my understanding of type promotion is that the compiler will
> promote the float argument to double or long double as needed.
> 
> But then, prior to this patch, when DELTA was just (0.001), and would be
> considered a double, I would have expected, in any comparison between a
> float and DELTA, for the float to be converted to double, so I don't
> really understand that part of the original patch.
> 
> Thanks,
> Andrew

Thanks for your input.
What I didn't quite realize when writing the patch, is that float literals without
suffix are of type double. I double checked the ISO C11 draft and it is mentioned
there as well.

So to me this is likely a compiler bug. Or did I miss anything?

In that case, I would open an issue with LLVM/clang and drop that part from
this patch for now. Clang 15.0.0 isn't released yet.

Regards,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-07-19  9:06       ` Willgerodt, Felix
@ 2022-07-19  9:14         ` Andrew Burgess
  2022-07-19  9:20           ` Willgerodt, Felix
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2022-07-19  9:14 UTC (permalink / raw)
  To: Willgerodt, Felix, Bruno Larsen, gdb-patches

"Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org> writes:

>> -----Original Message-----
>> From: Andrew Burgess <aburgess@redhat.com>
>> Sent: Montag, 18. Juli 2022 15:37
>> To: Willgerodt, Felix <felix.willgerodt@intel.com>; Bruno Larsen
>> <blarsen@redhat.com>; gdb-patches@sourceware.org
>> Subject: RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
>> new clang warnings.
>> 
>> "Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org> writes:
>> 
>> >> -----Original Message-----
>> >> From: Bruno Larsen <blarsen@redhat.com>
>> >> Sent: Mittwoch, 8. Juni 2022 14:59
>> >> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
>> >> patches@sourceware.org
>> >> Subject: Re: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
>> >> new clang warnings.
>> >>
>> >>
>> >> On 6/8/22 04:17, Felix Willgerodt via Gdb-patches wrote:
>> >> > This patch fixes two issues with callfuncs.exp, which are both related
>> >> > to new Clang warnings:
>> >>
>> >> Hi Felix!
>> >>
>> >> Thanks for this!
>> >>
>> >> >
>> >> > 1) Clang 15.0.0 added a new warning for deprecated non-prototype
>> >> functions:
>> >> > https://reviews.llvm.org/D122895
>> >> > Callfuncs.exp is impacted and won't run due to new warnings:
>> >> >
>> >> > callfuncs.c:339:5: warning: a function declaration without a prototype is
>> >> > deprecated in all versions of C and is not supported in C2x
>> >> > [-Wdeprecated-non-prototype]
>> >> > int t_float_values (float_arg1, float_arg2)
>> >> >
>> >> > This patch disables those warnings with -Wno-deprecated-non-
>> prototype.
>> >> > Removing the test for deprecated syntax would also be an option. But I
>> will
>> >> > leave that for others to decide.
>> >>
>> >> I like your solution with -Wno-deprecated-non-prototype.
>> >>
>> >> I think it is important (at least for now) to keep this test, since we have to
>> >> support very old setups.
>> >>
>> >> >
>> >> > 2) The other new warnings are about comparing a define with floats and
>> >> doubles:
>> >> >
>> >> > callfuncs.c:518:1: warning: floating-point comparison is always true;
>> >> constant
>> >> > cannot be represented exactly in type 'float' [-Wliteral-range]
>> >> > DEF_FUNC_VALUES_3(fc, float, crealf, cimagf)
>> >> >
>> >> > This can be fixed by making the define a float.
>> >>
>> >> Genuine question, would this not cause a problem for the times where
>> the
>> >> parameters are doubles and DELTA is a float? If it isn't a problem, I'm fine
>> >> with this.
>> >>
>> >
>> > I was wondering about that as well, but it still passes with GCC, clang and
>> Intel
>> > compilers and there are no compiler warnings about it.
>> > I couldn't really figure it out if it would actually a problem somewhere. I
>> could only
>> > test on linux x86 though. My best guess was, as it is the "smaller precision"
>> the
>> > compiler will do the right thing.
>> 
>> Indeed, my understanding of type promotion is that the compiler will
>> promote the float argument to double or long double as needed.
>> 
>> But then, prior to this patch, when DELTA was just (0.001), and would be
>> considered a double, I would have expected, in any comparison between a
>> float and DELTA, for the float to be converted to double, so I don't
>> really understand that part of the original patch.
>> 
>> Thanks,
>> Andrew
>
> Thanks for your input.
> What I didn't quite realize when writing the patch, is that float literals without
> suffix are of type double. I double checked the ISO C11 draft and it is mentioned
> there as well.
>
> So to me this is likely a compiler bug. Or did I miss anything?

I did wonder the same, but I don't claim to be an expert on language
details.  I don't have immediate access to clang 15, but it might be
interesting to try and reduce the test program down to just that
error/warning case, and see if the code makes sense.

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-07-19  9:14         ` Andrew Burgess
@ 2022-07-19  9:20           ` Willgerodt, Felix
  2022-07-19 10:15             ` Andrew Burgess
  0 siblings, 1 reply; 13+ messages in thread
From: Willgerodt, Felix @ 2022-07-19  9:20 UTC (permalink / raw)
  To: Andrew Burgess, Bruno Larsen, gdb-patches



> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Dienstag, 19. Juli 2022 11:14
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; Bruno Larsen
> <blarsen@redhat.com>; gdb-patches@sourceware.org
> Subject: RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
> new clang warnings.
> 
> "Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org> writes:
> 
> >> -----Original Message-----
> >> From: Andrew Burgess <aburgess@redhat.com>
> >> Sent: Montag, 18. Juli 2022 15:37
> >> To: Willgerodt, Felix <felix.willgerodt@intel.com>; Bruno Larsen
> >> <blarsen@redhat.com>; gdb-patches@sourceware.org
> >> Subject: RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
> >> new clang warnings.
> >>
> >> "Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org>
> writes:
> >>
> >> >> -----Original Message-----
> >> >> From: Bruno Larsen <blarsen@redhat.com>
> >> >> Sent: Mittwoch, 8. Juni 2022 14:59
> >> >> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> >> >> patches@sourceware.org
> >> >> Subject: Re: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp
> for
> >> >> new clang warnings.
> >> >>
> >> >>
> >> >> On 6/8/22 04:17, Felix Willgerodt via Gdb-patches wrote:
> >> >> > This patch fixes two issues with callfuncs.exp, which are both related
> >> >> > to new Clang warnings:
> >> >>
> >> >> Hi Felix!
> >> >>
> >> >> Thanks for this!
> >> >>
> >> >> >
> >> >> > 1) Clang 15.0.0 added a new warning for deprecated non-prototype
> >> >> functions:
> >> >> > https://reviews.llvm.org/D122895
> >> >> > Callfuncs.exp is impacted and won't run due to new warnings:
> >> >> >
> >> >> > callfuncs.c:339:5: warning: a function declaration without a prototype
> is
> >> >> > deprecated in all versions of C and is not supported in C2x
> >> >> > [-Wdeprecated-non-prototype]
> >> >> > int t_float_values (float_arg1, float_arg2)
> >> >> >
> >> >> > This patch disables those warnings with -Wno-deprecated-non-
> >> prototype.
> >> >> > Removing the test for deprecated syntax would also be an option.
> But I
> >> will
> >> >> > leave that for others to decide.
> >> >>
> >> >> I like your solution with -Wno-deprecated-non-prototype.
> >> >>
> >> >> I think it is important (at least for now) to keep this test, since we have
> to
> >> >> support very old setups.
> >> >>
> >> >> >
> >> >> > 2) The other new warnings are about comparing a define with floats
> and
> >> >> doubles:
> >> >> >
> >> >> > callfuncs.c:518:1: warning: floating-point comparison is always true;
> >> >> constant
> >> >> > cannot be represented exactly in type 'float' [-Wliteral-range]
> >> >> > DEF_FUNC_VALUES_3(fc, float, crealf, cimagf)
> >> >> >
> >> >> > This can be fixed by making the define a float.
> >> >>
> >> >> Genuine question, would this not cause a problem for the times where
> >> the
> >> >> parameters are doubles and DELTA is a float? If it isn't a problem, I'm
> fine
> >> >> with this.
> >> >>
> >> >
> >> > I was wondering about that as well, but it still passes with GCC, clang and
> >> Intel
> >> > compilers and there are no compiler warnings about it.
> >> > I couldn't really figure it out if it would actually a problem somewhere. I
> >> could only
> >> > test on linux x86 though. My best guess was, as it is the "smaller
> precision"
> >> the
> >> > compiler will do the right thing.
> >>
> >> Indeed, my understanding of type promotion is that the compiler will
> >> promote the float argument to double or long double as needed.
> >>
> >> But then, prior to this patch, when DELTA was just (0.001), and would be
> >> considered a double, I would have expected, in any comparison between
> a
> >> float and DELTA, for the float to be converted to double, so I don't
> >> really understand that part of the original patch.
> >>
> >> Thanks,
> >> Andrew
> >
> > Thanks for your input.
> > What I didn't quite realize when writing the patch, is that float literals
> without
> > suffix are of type double. I double checked the ISO C11 draft and it is
> mentioned
> > there as well.
> >
> > So to me this is likely a compiler bug. Or did I miss anything?
> 
> I did wonder the same, but I don't claim to be an expert on language
> details.  I don't have immediate access to clang 15, but it might be
> interesting to try and reduce the test program down to just that
> error/warning case, and see if the code makes sense.
> 
> Thanks,
> Andrew

I did this experiment:


$ cat float.c
#define DELTA 0.001

int main() {
float a = 0.3;

if (a == DELTA)
  return 1;
else
  return 0;
}
$ clang float.c 
float.c:7:7: warning: floating-point comparison is always false; constant
cannot be represented exactly in type 'float' [-Wliteral-range]
if (a == DELTA)
    ~ ^  ~~~~~
1 warning generated.
$


When I make the define a variable, there is no warning:


$ cat double.c 
double DELTA = 0.001;

int main() {
float a = 0.3;

if (a == DELTA)
  return 1;
else
  return 0;
}
$ clang double.c 
$

Thanks,
Felix

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-07-19  9:20           ` Willgerodt, Felix
@ 2022-07-19 10:15             ` Andrew Burgess
  2022-07-25 11:17               ` Willgerodt, Felix
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2022-07-19 10:15 UTC (permalink / raw)
  To: Willgerodt, Felix, Bruno Larsen, gdb-patches

"Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org> writes:

>> -----Original Message-----
>> From: Andrew Burgess <aburgess@redhat.com>
>> Sent: Dienstag, 19. Juli 2022 11:14
>> To: Willgerodt, Felix <felix.willgerodt@intel.com>; Bruno Larsen
>> <blarsen@redhat.com>; gdb-patches@sourceware.org
>> Subject: RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
>> new clang warnings.
>> 
>> "Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org> writes:
>> 
>> >> -----Original Message-----
>> >> From: Andrew Burgess <aburgess@redhat.com>
>> >> Sent: Montag, 18. Juli 2022 15:37
>> >> To: Willgerodt, Felix <felix.willgerodt@intel.com>; Bruno Larsen
>> >> <blarsen@redhat.com>; gdb-patches@sourceware.org
>> >> Subject: RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
>> >> new clang warnings.
>> >>
>> >> "Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org>
>> writes:
>> >>
>> >> >> -----Original Message-----
>> >> >> From: Bruno Larsen <blarsen@redhat.com>
>> >> >> Sent: Mittwoch, 8. Juni 2022 14:59
>> >> >> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
>> >> >> patches@sourceware.org
>> >> >> Subject: Re: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp
>> for
>> >> >> new clang warnings.
>> >> >>
>> >> >>
>> >> >> On 6/8/22 04:17, Felix Willgerodt via Gdb-patches wrote:
>> >> >> > This patch fixes two issues with callfuncs.exp, which are both related
>> >> >> > to new Clang warnings:
>> >> >>
>> >> >> Hi Felix!
>> >> >>
>> >> >> Thanks for this!
>> >> >>
>> >> >> >
>> >> >> > 1) Clang 15.0.0 added a new warning for deprecated non-prototype
>> >> >> functions:
>> >> >> > https://reviews.llvm.org/D122895
>> >> >> > Callfuncs.exp is impacted and won't run due to new warnings:
>> >> >> >
>> >> >> > callfuncs.c:339:5: warning: a function declaration without a prototype
>> is
>> >> >> > deprecated in all versions of C and is not supported in C2x
>> >> >> > [-Wdeprecated-non-prototype]
>> >> >> > int t_float_values (float_arg1, float_arg2)
>> >> >> >
>> >> >> > This patch disables those warnings with -Wno-deprecated-non-
>> >> prototype.
>> >> >> > Removing the test for deprecated syntax would also be an option.
>> But I
>> >> will
>> >> >> > leave that for others to decide.
>> >> >>
>> >> >> I like your solution with -Wno-deprecated-non-prototype.
>> >> >>
>> >> >> I think it is important (at least for now) to keep this test, since we have
>> to
>> >> >> support very old setups.
>> >> >>
>> >> >> >
>> >> >> > 2) The other new warnings are about comparing a define with floats
>> and
>> >> >> doubles:
>> >> >> >
>> >> >> > callfuncs.c:518:1: warning: floating-point comparison is always true;
>> >> >> constant
>> >> >> > cannot be represented exactly in type 'float' [-Wliteral-range]
>> >> >> > DEF_FUNC_VALUES_3(fc, float, crealf, cimagf)
>> >> >> >
>> >> >> > This can be fixed by making the define a float.
>> >> >>
>> >> >> Genuine question, would this not cause a problem for the times where
>> >> the
>> >> >> parameters are doubles and DELTA is a float? If it isn't a problem, I'm
>> fine
>> >> >> with this.
>> >> >>
>> >> >
>> >> > I was wondering about that as well, but it still passes with GCC, clang and
>> >> Intel
>> >> > compilers and there are no compiler warnings about it.
>> >> > I couldn't really figure it out if it would actually a problem somewhere. I
>> >> could only
>> >> > test on linux x86 though. My best guess was, as it is the "smaller
>> precision"
>> >> the
>> >> > compiler will do the right thing.
>> >>
>> >> Indeed, my understanding of type promotion is that the compiler will
>> >> promote the float argument to double or long double as needed.
>> >>
>> >> But then, prior to this patch, when DELTA was just (0.001), and would be
>> >> considered a double, I would have expected, in any comparison between
>> a
>> >> float and DELTA, for the float to be converted to double, so I don't
>> >> really understand that part of the original patch.
>> >>
>> >> Thanks,
>> >> Andrew
>> >
>> > Thanks for your input.
>> > What I didn't quite realize when writing the patch, is that float literals
>> without
>> > suffix are of type double. I double checked the ISO C11 draft and it is
>> mentioned
>> > there as well.
>> >
>> > So to me this is likely a compiler bug. Or did I miss anything?
>> 
>> I did wonder the same, but I don't claim to be an expert on language
>> details.  I don't have immediate access to clang 15, but it might be
>> interesting to try and reduce the test program down to just that
>> error/warning case, and see if the code makes sense.
>> 
>> Thanks,
>> Andrew
>
> I did this experiment:
>
>
> $ cat float.c
> #define DELTA 0.001
>
> int main() {
> float a = 0.3;
>
> if (a == DELTA)
>   return 1;
> else
>   return 0;
> }
> $ clang float.c 
> float.c:7:7: warning: floating-point comparison is always false; constant
> cannot be represented exactly in type 'float' [-Wliteral-range]
> if (a == DELTA)
>     ~ ^  ~~~~~
> 1 warning generated.

Sure, but in gdb.base/callfuncs.c we seem to take great care to ensure
we always check delta like:

  if ( (a - b) < DELTA )

Is there a macro that actually expands to use DELTA with '==' ?

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.
  2022-07-19 10:15             ` Andrew Burgess
@ 2022-07-25 11:17               ` Willgerodt, Felix
  0 siblings, 0 replies; 13+ messages in thread
From: Willgerodt, Felix @ 2022-07-25 11:17 UTC (permalink / raw)
  To: Andrew Burgess, Bruno Larsen, gdb-patches

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Dienstag, 19. Juli 2022 12:16
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; Bruno Larsen
> <blarsen@redhat.com>; gdb-patches@sourceware.org
> Subject: RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
> new clang warnings.
> 
> "Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org> writes:
> 
> >> -----Original Message-----
> >> From: Andrew Burgess <aburgess@redhat.com>
> >> Sent: Dienstag, 19. Juli 2022 11:14
> >> To: Willgerodt, Felix <felix.willgerodt@intel.com>; Bruno Larsen
> >> <blarsen@redhat.com>; gdb-patches@sourceware.org
> >> Subject: RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for
> >> new clang warnings.
> >>
> >> "Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org>
> writes:
> >>
> >> >> -----Original Message-----
> >> >> From: Andrew Burgess <aburgess@redhat.com>
> >> >> Sent: Montag, 18. Juli 2022 15:37
> >> >> To: Willgerodt, Felix <felix.willgerodt@intel.com>; Bruno Larsen
> >> >> <blarsen@redhat.com>; gdb-patches@sourceware.org
> >> >> Subject: RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp
> for
> >> >> new clang warnings.
> >> >>
> >> >> "Willgerodt, Felix via Gdb-patches" <gdb-patches@sourceware.org>
> >> writes:
> >> >>
> >> >> >> -----Original Message-----
> >> >> >> From: Bruno Larsen <blarsen@redhat.com>
> >> >> >> Sent: Mittwoch, 8. Juni 2022 14:59
> >> >> >> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> >> >> >> patches@sourceware.org
> >> >> >> Subject: Re: [PATCH 1/1] gdb, testsuite: Adapt
> gdb.base/callfuncs.exp
> >> for
> >> >> >> new clang warnings.
> >> >> >>
> >> >> >>
> >> >> >> On 6/8/22 04:17, Felix Willgerodt via Gdb-patches wrote:
> >> >> >> > This patch fixes two issues with callfuncs.exp, which are both
> related
> >> >> >> > to new Clang warnings:
> >> >> >>
> >> >> >> Hi Felix!
> >> >> >>
> >> >> >> Thanks for this!
> >> >> >>
> >> >> >> >
> >> >> >> > 1) Clang 15.0.0 added a new warning for deprecated non-
> prototype
> >> >> >> functions:
> >> >> >> > https://reviews.llvm.org/D122895
> >> >> >> > Callfuncs.exp is impacted and won't run due to new warnings:
> >> >> >> >
> >> >> >> > callfuncs.c:339:5: warning: a function declaration without a
> prototype
> >> is
> >> >> >> > deprecated in all versions of C and is not supported in C2x
> >> >> >> > [-Wdeprecated-non-prototype]
> >> >> >> > int t_float_values (float_arg1, float_arg2)
> >> >> >> >
> >> >> >> > This patch disables those warnings with -Wno-deprecated-non-
> >> >> prototype.
> >> >> >> > Removing the test for deprecated syntax would also be an option.
> >> But I
> >> >> will
> >> >> >> > leave that for others to decide.
> >> >> >>
> >> >> >> I like your solution with -Wno-deprecated-non-prototype.
> >> >> >>
> >> >> >> I think it is important (at least for now) to keep this test, since we
> have
> >> to
> >> >> >> support very old setups.
> >> >> >>
> >> >> >> >
> >> >> >> > 2) The other new warnings are about comparing a define with
> floats
> >> and
> >> >> >> doubles:
> >> >> >> >
> >> >> >> > callfuncs.c:518:1: warning: floating-point comparison is always
> true;
> >> >> >> constant
> >> >> >> > cannot be represented exactly in type 'float' [-Wliteral-range]
> >> >> >> > DEF_FUNC_VALUES_3(fc, float, crealf, cimagf)
> >> >> >> >
> >> >> >> > This can be fixed by making the define a float.
> >> >> >>
> >> >> >> Genuine question, would this not cause a problem for the times
> where
> >> >> the
> >> >> >> parameters are doubles and DELTA is a float? If it isn't a problem,
> I'm
> >> fine
> >> >> >> with this.
> >> >> >>
> >> >> >
> >> >> > I was wondering about that as well, but it still passes with GCC, clang
> and
> >> >> Intel
> >> >> > compilers and there are no compiler warnings about it.
> >> >> > I couldn't really figure it out if it would actually a problem
> somewhere. I
> >> >> could only
> >> >> > test on linux x86 though. My best guess was, as it is the "smaller
> >> precision"
> >> >> the
> >> >> > compiler will do the right thing.
> >> >>
> >> >> Indeed, my understanding of type promotion is that the compiler will
> >> >> promote the float argument to double or long double as needed.
> >> >>
> >> >> But then, prior to this patch, when DELTA was just (0.001), and would
> be
> >> >> considered a double, I would have expected, in any comparison
> between
> >> a
> >> >> float and DELTA, for the float to be converted to double, so I don't
> >> >> really understand that part of the original patch.
> >> >>
> >> >> Thanks,
> >> >> Andrew
> >> >
> >> > Thanks for your input.
> >> > What I didn't quite realize when writing the patch, is that float literals
> >> without
> >> > suffix are of type double. I double checked the ISO C11 draft and it is
> >> mentioned
> >> > there as well.
> >> >
> >> > So to me this is likely a compiler bug. Or did I miss anything?
> >>
> >> I did wonder the same, but I don't claim to be an expert on language
> >> details.  I don't have immediate access to clang 15, but it might be
> >> interesting to try and reduce the test program down to just that
> >> error/warning case, and see if the code makes sense.
> >>
> >> Thanks,
> >> Andrew
> >
> > I did this experiment:
> >
> >
> > $ cat float.c
> > #define DELTA 0.001
> >
> > int main() {
> > float a = 0.3;
> >
> > if (a == DELTA)
> >   return 1;
> > else
> >   return 0;
> > }
> > $ clang float.c
> > float.c:7:7: warning: floating-point comparison is always false; constant
> > cannot be represented exactly in type 'float' [-Wliteral-range]
> > if (a == DELTA)
> >     ~ ^  ~~~~~
> > 1 warning generated.
> 
> Sure, but in gdb.base/callfuncs.c we seem to take great care to ensure
> we always check delta like:
> 
>   if ( (a - b) < DELTA )
>
> Is there a macro that actually expands to use DELTA with '==' ?

In the testcase (or my samples) there is nothing in regards to extra
Macros. If I rename DELTA to something else, the behaviour
doesn't change.

I did open a ticket with clang: https://github.com/llvm/llvm-project/issues/56608
It seems like it is represented as a double and the wording of the warning
could still be improved. And it seems that the warning isn't too smart.

That said, I did rebuild the clang I have. With that version the float warnings are
Gone for callfuncs.c. So I will drop the float change and repost this (with the Intel
Compiler enhancements we discussed earlier).

Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-07-25 11:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-08  7:17 [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings Felix Willgerodt
2022-06-08 12:58 ` Bruno Larsen
2022-06-08 15:11   ` Willgerodt, Felix
2022-07-18 13:37     ` Andrew Burgess
2022-07-19  9:06       ` Willgerodt, Felix
2022-07-19  9:14         ` Andrew Burgess
2022-07-19  9:20           ` Willgerodt, Felix
2022-07-19 10:15             ` Andrew Burgess
2022-07-25 11:17               ` Willgerodt, Felix
2022-07-15 18:40 ` Tom Tromey
2022-07-18  9:41   ` Willgerodt, Felix
2022-07-18 13:28     ` Andrew Burgess
2022-07-18 13:42       ` Willgerodt, Felix

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).