RE: [PATCH 1/1] gdb, testsuite: Adapt gdb.base/callfuncs.exp for new clang warnings.

["Willgerodt, Felix" <felix.willgerodt@intel.com>]

> -----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