Re: Configure warning/error in m4/frexp.m4

2023-12-01 Thread Sam James


Bruno Haible  writes:

> Sam James wrote:
>> > It's good if you have the time to test not-yet-released compiler versions.
>> > I don't have that time, and therefore will prefer to wait until the
>> > clang release is out.
>> 
>> OK. You've appreciated reports in the past (even quite recently), I can
>> avoid making them in future if you'd prefer me to avoid it for
>> gnulib.
>
> Let me say it like this:
>   - If a new compiler feature is still being discussed or likely to be 
> changed,
> I would consider it too early to involve Gnulib.
>   - If a new compiler feature is finalized and it's only a matter of time
> until it becomes official, then you're welcome to involve us, as it will
> allow us to react sooner.
>
> This is for general things. There may be special situations which need
> different handling.
>

Fair enough. Thanks, and sorry for the noise.

> Bruno

sam



Re: Configure warning/error in m4/frexp.m4

2023-12-01 Thread Bruno Haible
Sam James wrote:
> > It's good if you have the time to test not-yet-released compiler versions.
> > I don't have that time, and therefore will prefer to wait until the
> > clang release is out.
> 
> OK. You've appreciated reports in the past (even quite recently), I can
> avoid making them in future if you'd prefer me to avoid it for
> gnulib.

Let me say it like this:
  - If a new compiler feature is still being discussed or likely to be changed,
I would consider it too early to involve Gnulib.
  - If a new compiler feature is finalized and it's only a matter of time
until it becomes official, then you're welcome to involve us, as it will
allow us to react sooner.

This is for general things. There may be special situations which need
different handling.

Bruno






Re: Configure warning/error in m4/frexp.m4

2023-12-01 Thread Sam James


Bruno Haible  writes:

> Sam James wrote:
>> it's still going to be an issue if (as
>> planned) Clang flips '-Wincompatible-pointer-types' to error out by
>> default unless they carve out an exception for qualifiers here.
>> 
>> I'll advocate that they do, of course. I just came across it while
>> checking for configure changes w/ recent GCC changes.
>
> It's good if you have the time to test not-yet-released compiler versions.
> I don't have that time, and therefore will prefer to wait until the
> clang release is out.

OK. You've appreciated reports in the past (even quite recently), I can
avoid making them in future if you'd prefer me to avoid it for
gnulib. This tends to be particularly important for gnulib given it's included
in other projects and changes take time to propagate into releases.

Perhaps I should have been clearer that in this instance, I was asking
for opinions rather than saying this is definitely a problem to be fixed, as 
well.

>
> If they really want to make an implicit assignment from 'const char *'
> or 'volatile char *' to 'char *' an error, it will cause problems in
> many packages, namely where argument arrays for calling exec* are
> constructed: Since
>

Anyway, I'll communicate the desire that Clang be consistent with GCC
and not create busywork as these cases aren't really the problematic
type anyway.

thanks,
sam




Re: Configure warning/error in m4/frexp.m4

2023-12-01 Thread Bruno Haible
Paul Eggert wrote:
> We can conform to standard C without a cast with something like the 
> attached (which I haven't installed).

Looks good to me. Please install it; likewise in m4/frexpf.m4.

> (I assume the 'volatile' is there because of the funky things compilers 
> do with x86 double)

It's there more to block the constant propagation that the compilers would
otherwise do.

Bruno






Re: Configure warning/error in m4/frexp.m4

2023-12-01 Thread Bruno Haible
Sam James wrote:
> it's still going to be an issue if (as
> planned) Clang flips '-Wincompatible-pointer-types' to error out by
> default unless they carve out an exception for qualifiers here.
> 
> I'll advocate that they do, of course. I just came across it while
> checking for configure changes w/ recent GCC changes.

It's good if you have the time to test not-yet-released compiler versions.
I don't have that time, and therefore will prefer to wait until the
clang release is out.

If they really want to make an implicit assignment from 'const char *'
or 'volatile char *' to 'char *' an error, it will cause problems in
many packages, namely where argument arrays for calling exec* are
constructed: Since

  const char *argv[] = { "sh", "-c", "--", "pwd" };
  execv (argv[0], (char * const *) argv);

is dangerous (it violates the strict-aliasing rule), people write

  char *argv[] = { "sh", "-c", "--", "pwd" };
  execv (argv[0], argv);

And we don't want to write it as

  char *argv[] = { (char *) "sh", (char *) "-c", (char *) "--", (char *) "pwd" 
};
  execv (argv[0], argv);

because that is just silly.

Bruno






Re: Configure warning/error in m4/frexp.m4

2023-12-01 Thread Paul Eggert

On 2023-12-01 21:52, Bruno Haible wrote:

There is nothing wrong with this code (m4/frexp.m4:159, m4/frexpf.m4:83).


Isn't there? I thought that the C standard doesn't let you access a 
'double volatile' object via a 'void *' or 'void const *' pointer. It'd 
need to be 'void volatile *' or 'void const volatile *'.




We don't want to add a cast here, because — as Paul argues — casts can make
code more difficult to maintain in the long run.


We can conform to standard C without a cast with something like the 
attached (which I haven't installed).


(I assume the 'volatile' is there because of the funky things compilers 
do with x86 double)From 006cf2337a8ca2e3a0b41c488fbe3d95eda201f3 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 1 Dec 2023 22:19:22 -0800
Subject: [PATCH] frexp: pacify clang re address-of-volatile
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Sam James in:
https://lists.gnu.org/r/bug-gnulib/2023-12/msg00013.html
* m4/frexp.m4 (gl_FUNC_FREXP_WORKS): Don’t try to convert
‘double volatile *’ to ‘void const *’ as the C standard
doesn’t allow accessing volatile variables through
pointer-to-nonvolatile.
---
 ChangeLog   | 8 
 m4/frexp.m4 | 5 +++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c713de7b42..f30430d3de 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2023-12-01  Paul Eggert  
 
+	frexp: pacify clang re address-of-volatile
+	Problem reported by Sam James in:
+	https://lists.gnu.org/r/bug-gnulib/2023-12/msg00013.html
+	* m4/frexp.m4 (gl_FUNC_FREXP_WORKS): Don’t try to convert
+	‘double volatile *’ to ‘void const *’ as the C standard
+	doesn’t allow accessing volatile variables through
+	pointer-to-nonvolatile.
+
 	Update portability doc for CHERI, C23
 	* doc/gnulib-readme.texi:
 	Prefer “null pointer” to “@code{NULL}” since C23 has nullptr.
diff --git a/m4/frexp.m4 b/m4/frexp.m4
index 0293765c59..b23aa470c3 100644
--- a/m4/frexp.m4
+++ b/m4/frexp.m4
@@ -1,4 +1,4 @@
-# frexp.m4 serial 18
+# frexp.m4 serial 19
 dnl Copyright (C) 2007-2023 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -156,7 +156,8 @@ int main()
   {
 int exp;
 double y = frexp (x, &exp);
-if (memcmp (&y, &x, sizeof x))
+double x1 = x;
+if (memcmp (&y, &x1, sizeof x1))
   result |= 4;
   }
   return result;
-- 
2.40.1



Re: Configure warning/error in m4/frexp.m4

2023-12-01 Thread Sam James


Sam James  writes:

> Bruno Haible  writes:
>
>> Hi Sam,
>
> Hi Bruno,
>
>>
>>> The configure test in gl_FUNC_FREXP_WORKS within m4/frexp.m4 triggers
>>> a Clang warning/error when Clang is passed
>>> -Werror=incompatible-pointer-types.
>>
>> Gnulib does not support CC or CPPFLAGS or CFLAGS values with -Werror at
>> configure time. Neither with gcc nor with clang.
>
> Yes, I understand that. But Clang is likely to make this change to its
> default and -Werror=... was just a way of emulating that. gnulib _does_
> have to cater to the default strictness of compilers.
>
>>
>> The reason is that [1]
>>   "Many GCC warning options usually don’t point to mistakes in the code;
>>these warnings enforce a certain programming style. It is a project
>>management decision whether you want your code to follow any of these
>>styles. Note that some of these programming styles are conflicting.
>>You cannot have them all; you have to choose among them."
>>
>> For example, there is a warning option that attempts to enforce
>> explicit casts (no implicit conversions), and there is a warning option
>> that attempts to enforce no explicit casts. When you use such a warning
>> option together with -Werror, you are attempting to enforce a certain
>> programming style on the configure tests. Obviously the configure test
>> snippets cannot obey different, conflicting programming styles.
>
> Yep, completely on board with that & I get it. The only reason I
> reported this is because there's a reasonable chance this will be Clang's
> new behaviour by default (see below).
>
>>
>>> /tmp/foo.c: In function ‘main’:
>>> /tmp/foo.c:59:21: warning: passing argument 2 of ‘memcmp’ discards 
>>> ‘volatile’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>>>59 | if (memcmp (&y, &x, sizeof x))
>>>   | ^~
>>
>> There is nothing wrong with this code (m4/frexp.m4:159, m4/frexpf.m4:83).
>>
>> We don't want to add a cast here, because — as Paul argues — casts can make
>> code more difficult to maintain in the long run.
>
> Yes, I agree it's pedantic, but it's still going to be an issue if (as
> planned) Clang flips '-Wincompatible-pointer-types' to error out by
> default unless they carve out an exception for qualifiers here.
>
> I'll advocate that they do, of course. I just came across it while
> checking for configure changes w/ recent GCC changes.

I should add: if your position is, reasonably, "clang really shouldn't
do that unless they carve out an exception for this kind of case",
then that's fine. But the report was not about me just doing -Werror=x
for the sake of it and misunderstands the motivation here.

>
>>
>> Bruno
>>
>> [1] https://www.gnu.org/software/gnulib/manual/html_node/manywarnings.html




Re: Configure warning/error in m4/frexp.m4

2023-12-01 Thread Sam James


Bruno Haible  writes:

> Hi Sam,

Hi Bruno,

>
>> The configure test in gl_FUNC_FREXP_WORKS within m4/frexp.m4 triggers
>> a Clang warning/error when Clang is passed
>> -Werror=incompatible-pointer-types.
>
> Gnulib does not support CC or CPPFLAGS or CFLAGS values with -Werror at
> configure time. Neither with gcc nor with clang.

Yes, I understand that. But Clang is likely to make this change to its
default and -Werror=... was just a way of emulating that. gnulib _does_
have to cater to the default strictness of compilers.

>
> The reason is that [1]
>   "Many GCC warning options usually don’t point to mistakes in the code;
>these warnings enforce a certain programming style. It is a project
>management decision whether you want your code to follow any of these
>styles. Note that some of these programming styles are conflicting.
>You cannot have them all; you have to choose among them."
>
> For example, there is a warning option that attempts to enforce
> explicit casts (no implicit conversions), and there is a warning option
> that attempts to enforce no explicit casts. When you use such a warning
> option together with -Werror, you are attempting to enforce a certain
> programming style on the configure tests. Obviously the configure test
> snippets cannot obey different, conflicting programming styles.

Yep, completely on board with that & I get it. The only reason I
reported this is because there's a reasonable chance this will be Clang's
new behaviour by default (see below).

>
>> /tmp/foo.c: In function ‘main’:
>> /tmp/foo.c:59:21: warning: passing argument 2 of ‘memcmp’ discards 
>> ‘volatile’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>>59 | if (memcmp (&y, &x, sizeof x))
>>   | ^~
>
> There is nothing wrong with this code (m4/frexp.m4:159, m4/frexpf.m4:83).
>
> We don't want to add a cast here, because — as Paul argues — casts can make
> code more difficult to maintain in the long run.

Yes, I agree it's pedantic, but it's still going to be an issue if (as
planned) Clang flips '-Wincompatible-pointer-types' to error out by
default unless they carve out an exception for qualifiers here.

I'll advocate that they do, of course. I just came across it while
checking for configure changes w/ recent GCC changes.

>
> Bruno
>
> [1] https://www.gnu.org/software/gnulib/manual/html_node/manywarnings.html




Re: Configure warning/error in m4/frexp.m4

2023-12-01 Thread Bruno Haible
Hi Sam,

> The configure test in gl_FUNC_FREXP_WORKS within m4/frexp.m4 triggers
> a Clang warning/error when Clang is passed
> -Werror=incompatible-pointer-types.

Gnulib does not support CC or CPPFLAGS or CFLAGS values with -Werror at
configure time. Neither with gcc nor with clang.

The reason is that [1]
  "Many GCC warning options usually don’t point to mistakes in the code;
   these warnings enforce a certain programming style. It is a project
   management decision whether you want your code to follow any of these
   styles. Note that some of these programming styles are conflicting.
   You cannot have them all; you have to choose among them."

For example, there is a warning option that attempts to enforce
explicit casts (no implicit conversions), and there is a warning option
that attempts to enforce no explicit casts. When you use such a warning
option together with -Werror, you are attempting to enforce a certain
programming style on the configure tests. Obviously the configure test
snippets cannot obey different, conflicting programming styles.

> /tmp/foo.c: In function ‘main’:
> /tmp/foo.c:59:21: warning: passing argument 2 of ‘memcmp’ discards ‘volatile’ 
> qualifier from pointer target type [-Wdiscarded-qualifiers]
>59 | if (memcmp (&y, &x, sizeof x))
>   | ^~

There is nothing wrong with this code (m4/frexp.m4:159, m4/frexpf.m4:83).

We don't want to add a cast here, because — as Paul argues — casts can make
code more difficult to maintain in the long run.

Bruno

[1] https://www.gnu.org/software/gnulib/manual/html_node/manywarnings.html






Configure warning/error in m4/frexp.m4

2023-12-01 Thread Sam James
Hi,

The configure test in gl_FUNC_FREXP_WORKS within m4/frexp.m4 triggers
a Clang warning/error when Clang is passed
-Werror=incompatible-pointer-types.

This _isn't_ an issue with GCC 14, as it doesn't consider the lost
const to be a problem, like so:

/tmp/foo.c: In function ‘main’:
/tmp/foo.c:59:21: warning: passing argument 2 of ‘memcmp’ discards ‘volatile’ 
qualifier from pointer target type [-Wdiscarded-qualifiers]
   59 | if (memcmp (&y, &x, sizeof x))
  | ^~
In file included from /tmp/foo.c:3:
/usr/include/string.h:64:50: note: expected ‘const void *’ but argument is of 
type ‘volatile double *’
   64 | extern int memcmp (const void *__s1, const void *__s2, size_t __n)
  |  ^~~~

But Clang on the other hand, when directed to treat incompatible
ptr. types as errors (which they may well do given GCC is) gives:

/tmp/foo.c:59:21: error: passing 'volatile double *' to parameter of type 
'const void *' discards qualifiers 
[-Werror,-Wincompatible-pointer-types-discards-qualifiers]
   59 | if (memcmp (&y, &x, sizeof x))
  | ^~
/usr/include/string.h:64:50: note: passing argument to parameter '__s2' here
   64 | extern int memcmp (const void *__s1, const void *__s2, size_t __n)
  |  ^
1 error generated.

I'm naturally testing with GCC 14 at the moment but this was left over
in some of the older testing we did with Clang so I'm working through
the backlog there as well.

thanks,
sam