Re: manywarnings for C++

2017-08-07 Thread Reuben Thomas
On 7 August 2017 at 21:09, Bruno Haible  wrote:

> Reuben Thomas wrote:
> > ​Thanks, that seems to fix it.​
>
> Good! So, I've pushed it.
>

​Thanks very much, that means I'm down to only one gnulib diff in Enchant,
which adds extra project-specific instructions to INSTALL.

-- 
https://rrt.sc3d.org 


Re: manywarnings for C++

2017-08-07 Thread Bruno Haible
Reuben Thomas wrote:
> ​Thanks, that seems to fix it.​

Good! So, I've pushed it.

Bruno




Re: manywarnings for C++

2017-08-07 Thread Reuben Thomas
On 7 August 2017 at 18:12, Bruno Haible  wrote:

> Reuben Thomas wrote:
> > ​OK, now that some of the code is in a new file, I get:
> >
> > configure.ac:90: error: gl_MANYWARN_ALL_GCC: unknown language: C++
> > m4/manywarnings.m4:38: gl_MANYWARN_ALL_GCC is expanded from...
> > configure.ac:90: the top level
>
> Ah, 'aclocal' does not grok that it needs to include manywarnings-c++.m4 in
> aclocal.m4. Does it work with this change?
>

​Thanks, that seems to fix it.​


Re: manywarnings for C++

2017-08-07 Thread Bruno Haible
Reuben Thomas wrote:
> ​OK, now that some of the code is in a new file, I get:
> 
> configure.ac:90: error: gl_MANYWARN_ALL_GCC: unknown language: C++
> m4/manywarnings.m4:38: gl_MANYWARN_ALL_GCC is expanded from...
> configure.ac:90: the top level

Ah, 'aclocal' does not grok that it needs to include manywarnings-c++.m4 in
aclocal.m4. Does it work with this change?


diff --git a/m4/manywarnings-c++.m4 b/m4/manywarnings-c++.m4
index cb1548a..c023f88 100644
--- a/m4/manywarnings-c++.m4
+++ b/m4/manywarnings-c++.m4
@@ -4,9 +4,9 @@ dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
 dnl with or without modifications, as long as this notice is preserved.
 
-# Specialization of gl_MANYWARN_ALL_GCC for _AC_LANG = C++.
-# This macro can be AC_REQUIREd.
-AC_DEFUN([gl_MANYWARN_ALL_GCC(C++)],
+# Implementation of the specialization of gl_MANYWARN_ALL_GCC
+# for _AC_LANG = C++.
+AC_DEFUN([gl_MANYWARN_ALL_GCC_CXX_IMPL],
 [
   AC_LANG_PUSH([C++])
 
diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4
index 3124b1e..a3d255a 100644
--- a/m4/manywarnings.m4
+++ b/m4/manywarnings.m4
@@ -38,7 +38,7 @@ AC_DEFUN([gl_MANYWARN_COMPLEMENT],
 AC_DEFUN([gl_MANYWARN_ALL_GCC],
 [_AC_LANG_DISPATCH([$0], _AC_LANG, $@)])
 
-# Specialization for _AC_LANG = C. This macro can be AC_REQUIREd.
+# Specialization for _AC_LANG = C.
 AC_DEFUN([gl_MANYWARN_ALL_GCC(C)],
 [
   AC_LANG_PUSH([C])
@@ -314,3 +314,9 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC(C)],
 
   AC_LANG_POP([C])
 ])
+
+# Specialization for _AC_LANG = C++.
+AC_DEFUN([gl_MANYWARN_ALL_GCC(C++)],
+[
+  gl_MANYWARN_ALL_GCC_CXX_IMPL([$1])
+])




Re: manywarnings for C++

2017-08-07 Thread Reuben Thomas
On 7 August 2017 at 17:21, Reuben Thomas  wrote:

> On 7 August 2017 at 17:16, Reuben Thomas  wrote:
>
>> On 7 August 2017 at 17:11, Reuben Thomas  wrote:
>>
>>> On 7 August 2017 at 16:42, Bruno Haible  wrote:
>>>

 > Currently in my configure.ac, I have:
 >
 >   AC_LANG_PUSH([C++])
 >   gl_MANYWARN_ALL_GCC([cxx_warnings])
 >
 >   dnl Enable all G++ warnings not in this list.
 >   gl_MANYWARN_COMPLEMENT([cxx_warnings], [$cxx_warnings], [$nw])
 >   for w in $cxx_warnings; do
 > gl_WARN_ADD([$w])
 >   done
 >   AC_LANG_POP
 >
 > which seems to work.

 Yes, this is how the multi-language facilities are supposed to be used.

>>>
>>> ​OK, now that some of the code is in a new file, I get:
>>>
>>> configure.ac:90: error: gl_MANYWARN_ALL_GCC: unknown language: C++
>>> m4/manywarnings.m4:38: gl_MANYWARN_ALL_GCC is expanded from...
>>> configure.ac:90: the top level
>>>
>>> Line 90 is the call of gl_MANYWARN_ALL_GCC in the code extract above.​
>>>
>>
>> ​many​warnings-c++.m4 is not referenced in aclocal.m4, though it has been
>> symlinked into the project's m4 directory by bootstrap.
>>
> ​
> Is this because aclocal finds gl_MANYWARN_ALL_GCC defined in
> manywarnings.m4, so doesn't bother looking in manywarnings-c++.m4
> ​.
>

​So as a workaround, I tried replacing the line

gl_MANYWARN_ALL_GCC([cxx_warnings])

by

m4_indir([gl_MANYWARN_ALL_GCC(C++)], [cxx_warnings])

​but now I get a straightforward error from aclocal:​

/usr/bin/m4:configure.ac:90: undefined macro `gl_MANYWARN_ALL_GCC(C++)'

​I guess because aclocal doesn't scan for macros called via m4_indir??​

-- 
https://rrt.sc3d.org 


Re: manywarnings for C++

2017-08-07 Thread Reuben Thomas
On 7 August 2017 at 17:16, Reuben Thomas  wrote:

> On 7 August 2017 at 17:11, Reuben Thomas  wrote:
>
>> On 7 August 2017 at 16:42, Bruno Haible  wrote:
>>
>>>
>>> > Currently in my configure.ac, I have:
>>> >
>>> >   AC_LANG_PUSH([C++])
>>> >   gl_MANYWARN_ALL_GCC([cxx_warnings])
>>> >
>>> >   dnl Enable all G++ warnings not in this list.
>>> >   gl_MANYWARN_COMPLEMENT([cxx_warnings], [$cxx_warnings], [$nw])
>>> >   for w in $cxx_warnings; do
>>> > gl_WARN_ADD([$w])
>>> >   done
>>> >   AC_LANG_POP
>>> >
>>> > which seems to work.
>>>
>>> Yes, this is how the multi-language facilities are supposed to be used.
>>>
>>
>> ​OK, now that some of the code is in a new file, I get:
>>
>> configure.ac:90: error: gl_MANYWARN_ALL_GCC: unknown language: C++
>> m4/manywarnings.m4:38: gl_MANYWARN_ALL_GCC is expanded from...
>> configure.ac:90: the top level
>>
>> Line 90 is the call of gl_MANYWARN_ALL_GCC in the code extract above.​
>>
>
> ​many​warnings-c++.m4 is not referenced in aclocal.m4, though it has been
> symlinked into the project's m4 directory by bootstrap.
>
​
Is this because aclocal finds gl_MANYWARN_ALL_GCC defined in
manywarnings.m4, so doesn't bother looking in manywarnings-c++.m4?

-- 
https://rrt.sc3d.org 


Re: manywarnings for C++

2017-08-07 Thread Reuben Thomas
On 7 August 2017 at 17:11, Reuben Thomas  wrote:

> On 7 August 2017 at 16:42, Bruno Haible  wrote:
>
>>
>> > Currently in my configure.ac, I have:
>> >
>> >   AC_LANG_PUSH([C++])
>> >   gl_MANYWARN_ALL_GCC([cxx_warnings])
>> >
>> >   dnl Enable all G++ warnings not in this list.
>> >   gl_MANYWARN_COMPLEMENT([cxx_warnings], [$cxx_warnings], [$nw])
>> >   for w in $cxx_warnings; do
>> > gl_WARN_ADD([$w])
>> >   done
>> >   AC_LANG_POP
>> >
>> > which seems to work.
>>
>> Yes, this is how the multi-language facilities are supposed to be used.
>>
>
> ​OK, now that some of the code is in a new file, I get:
>
> configure.ac:90: error: gl_MANYWARN_ALL_GCC: unknown language: C++
> m4/manywarnings.m4:38: gl_MANYWARN_ALL_GCC is expanded from...
> configure.ac:90: the top level
>
> Line 90 is the call of gl_MANYWARN_ALL_GCC in the code extract above.​
>

​many​warnings-c++.m4 is not referenced in aclocal.m4, though it has been
symlinked into the project's m4 directory by bootstrap.

-- 
https://rrt.sc3d.org 


Re: manywarnings for C++

2017-08-07 Thread Reuben Thomas
On 7 August 2017 at 16:42, Bruno Haible  wrote:

>
> > Currently in my configure.ac, I have:
> >
> >   AC_LANG_PUSH([C++])
> >   gl_MANYWARN_ALL_GCC([cxx_warnings])
> >
> >   dnl Enable all G++ warnings not in this list.
> >   gl_MANYWARN_COMPLEMENT([cxx_warnings], [$cxx_warnings], [$nw])
> >   for w in $cxx_warnings; do
> > gl_WARN_ADD([$w])
> >   done
> >   AC_LANG_POP
> >
> > which seems to work.
>
> Yes, this is how the multi-language facilities are supposed to be used.
>

​OK, now that some of the code is in a new file, I get:

configure.ac:90: error: gl_MANYWARN_ALL_GCC: unknown language: C++
m4/manywarnings.m4:38: gl_MANYWARN_ALL_GCC is expanded from...
configure.ac:90: the top level

Line 90 is the call of gl_MANYWARN_ALL_GCC in the code extract above.​

The main reason why the AC_LANG_PUSH/POP pairs are still there is that the
> macros can be AC_REQUIREd.


​Ah, I see, thanks.

-- 
https://rrt.sc3d.org 


Re: manywarnings for C++

2017-08-07 Thread Bruno Haible
Hi Reuben,

> ​I simply went through the GCC manual and gcc-warning.spec and added in C++
> warnings and removed those which were C only. I'm working with C++ in
> Enchant at present, but I can't say I'm well up on modern C++ (or indeed
> any other flavour).

OK, then if we find that some warnings are counterproductive, we can disable
them afterwards, on a case-by-case basis.

> There's one issue left that I'm aware of: the recipe in the comments of the
> C and C++ flavors of gl_MANYWARN_ALL_GCC for comparing the *.spec list of
> warnings with one's own compiler no longer works, since it will grep all
> the warnings for all languages defined in manywarnings.m4.

Ah, good point. I fixed this by moving the new code to a separate file
manywarnings-c++.m4. This makes it also easier to 'diff' the two cases.


2017-08-06  Reuben Thomas  

manywarnings: Add support for C++.
* build-aux/g++-warning.spec: New file.
* m4/manywarnings-c++.m4: New file.
* modules/manywarnings (Files): Add it.


> Currently in my configure.ac, I have:
> 
>   AC_LANG_PUSH([C++])
>   gl_MANYWARN_ALL_GCC([cxx_warnings])
> 
>   dnl Enable all G++ warnings not in this list.
>   gl_MANYWARN_COMPLEMENT([cxx_warnings], [$cxx_warnings], [$nw])
>   for w in $cxx_warnings; do
> gl_WARN_ADD([$w])
>   done
>   AC_LANG_POP
> 
> which seems to work.

Yes, this is how the multi-language facilities are supposed to be used.

> However, it seems I should be able to call
> gl_MANYWARN_ALL_GCC(C++) directly

How about this?
  m4_indir([gl_MANYWARN_ALL_GCC(C++)], [my_cxx_warning_options])
See [1]. Yuck, that's where the GNU Build System gets ugly.

> I guess this explains why gl_MANYWARN_ALL_GCC({C,C++}) still contain
> AC_LANG_PUSH/POP pairs, as they can in fact be called directly?

The main reason why the AC_LANG_PUSH/POP pairs are still there is that the
macros can be AC_REQUIREd. If you write

   AC_DEFUN([MY_FOO], [
 AC_LANG_PUSH([C++])
 AC_REQUIRE([gl_MANYWARN_ALL_GCC(C++)])
 ...
   ])

the expansion of gl_MANYWARN_ALL_GCC(C++) will end up _before_ the
AC_LANG_PUSH([C++]).

Bruno

[1] https://www.gnu.org/software/m4/manual/m4-1.4.15/html_node/Indir.html




Re: manywarnings for C++

2017-08-07 Thread Eric Blake
On 08/07/2017 10:14 AM, Reuben Thomas wrote:
> On 7 August 2017 at 16:12, Bruno Haible  wrote:
> 
>> Reuben Thomas wrote:
>>> the ability to AC_DEFUN a
>>> macro with a particular argument, as in:​
>>>
>>> ​AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS(C)],​
>>>
>>> so that it is possible to AC_REQUIRE such a macro.
>>
>> The '(C)' part is not an argument to the function. It's part of the
>> function name. By convention, such a function name is used to indicate the
>> implementation of an autoconf macro for a particular _AC_LANG value.
>>
> 
> ​So how does one supply an argument? Is it
> 
> gl_UNKNOWN_WARNINGS_ARE_ERRORS(C)([variable])?​

Such a function can only be called indirectly (and in fact, AC_REQUIRE
is probably the best such way to call it indirectly).  Autoconf's
AC_LANG magic already knows how to indirectly call several macros
defined with different language-specific mechanisms, and it looks like
this patch is adding another similar use.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: manywarnings for C++

2017-08-07 Thread Bruno Haible
Reuben Thomas wrote:
> the ability to AC_DEFUN a
> macro with a particular argument, as in:​
> 
> ​AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS(C)],​
> 
> so that it is possible to AC_REQUIRE such a macro.

The '(C)' part is not an argument to the function. It's part of the
function name. By convention, such a function name is used to indicate the
implementation of an autoconf macro for a particular _AC_LANG value.

Bruno




Re: manywarnings for C++

2017-08-07 Thread Reuben Thomas
On 7 August 2017 at 16:12, Bruno Haible  wrote:

> Reuben Thomas wrote:
> > the ability to AC_DEFUN a
> > macro with a particular argument, as in:​
> >
> > ​AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS(C)],​
> >
> > so that it is possible to AC_REQUIRE such a macro.
>
> The '(C)' part is not an argument to the function. It's part of the
> function name. By convention, such a function name is used to indicate the
> implementation of an autoconf macro for a particular _AC_LANG value.
>

​So how does one supply an argument? Is it

gl_UNKNOWN_WARNINGS_ARE_ERRORS(C)([variable])?​

-- 
https://rrt.sc3d.org 


Re: manywarnings for C++

2017-08-07 Thread Reuben Thomas
On 6 August 2017 at 23:50, Bruno Haible  wrote:

>   - g++-warning.spec: Looks OK to me. I trust that your C++ experience is
> fresher than mine (I learned C++ in 1997).
>

​I simply went through the GCC manual and gcc-warning.spec and added in C++
warnings and removed those which were C only. I'm working with C++ in
Enchant at present, but I can't say I'm well up on modern C++ (or indeed
any other flavour).

I attach an updated patch against manywarnings.m4, and, for simplicity, the
same g++-warning.spec.

There's one issue left that I'm aware of: the recipe in the comments of the
C and C++ flavors of gl_MANYWARN_ALL_GCC for comparing the *.spec list of
warnings with one's own compiler no longer works, since it will grep all
the warnings for all languages defined in manywarnings.m4. I'm unsure how
best to fix this.

Currently in my configure.ac, I have:

  AC_LANG_PUSH([C++])
  gl_MANYWARN_ALL_GCC([cxx_warnings])

  dnl Enable all G++ warnings not in this list.
  gl_MANYWARN_COMPLEMENT([cxx_warnings], [$cxx_warnings], [$nw])
  for w in $cxx_warnings; do
gl_WARN_ADD([$w])
  done
  AC_LANG_POP

which seems to work. However, it seems I should be able to call
gl_MANYWARN_ALL_GCC(C++) directly; but where then do I put the argument
giving the variable in which to put the warnings? I'm confused by the
apparent argument in the name of the macro!

I guess this explains why gl_MANYWARN_ALL_GCC({C,C++}) still contain
AC_LANG_PUSH/POP pairs, as they can in fact be called directly?

-- 
https://rrt.sc3d.org 
--- gnulib/m4/manywarnings.m4   2017-08-07 13:10:45.183548018 +0100
+++ gl/m4/manywarnings.m4   2017-08-07 13:09:01.319551087 +0100
@@ -314,3 +314,244 @@
 
   AC_LANG_POP([C])
 ])
+
+# Specialization for _AC_LANG = C++. This macro can be AC_REQUIREd.
+AC_DEFUN([gl_MANYWARN_ALL_GCC(C++)],
+[
+  AC_LANG_PUSH([C++])
+
+  dnl First, check for some issues that only occur when combining multiple
+  dnl gcc warning categories.
+  AC_REQUIRE([AC_PROG_CXX])
+  if test -n "$GXX"; then
+
+dnl Check if -W -Werror -Wno-missing-field-initializers is supported
+dnl with the current $CXX $CXXFLAGS $CPPFLAGS.
+AC_MSG_CHECKING([whether -Wno-missing-field-initializers is supported])
+AC_CACHE_VAL([gl_cv_cxx_nomfi_supported], [
+  gl_save_CXXFLAGS="$CXXFLAGS"
+  CXXFLAGS="$CXXFLAGS -W -Werror -Wno-missing-field-initializers"
+  AC_COMPILE_IFELSE(
+[AC_LANG_PROGRAM([[]], [[]])],
+[gl_cv_cxx_nomfi_supported=yes],
+[gl_cv_cxx_nomfi_supported=no])
+  CXXFLAGS="$gl_save_CXXFLAGS"])
+AC_MSG_RESULT([$gl_cv_cxx_nomfi_supported])
+
+if test "$gl_cv_cxx_nomfi_supported" = yes; then
+  dnl Now check whether -Wno-missing-field-initializers is needed
+  dnl for the { 0, } construct.
+  AC_MSG_CHECKING([whether -Wno-missing-field-initializers is needed])
+  AC_CACHE_VAL([gl_cv_cxx_nomfi_needed], [
+gl_save_CXXFLAGS="$CXXFLAGS"
+CXXFLAGS="$CXXFLAGS -W -Werror"
+AC_COMPILE_IFELSE(
+  [AC_LANG_PROGRAM(
+ [[int f (void)
+   {
+ typedef struct { int a; int b; } s_t;
+ s_t s1 = { 0, };
+ return s1.b;
+   }
+ ]],
+ [[]])],
+  [gl_cv_cxx_nomfi_needed=no],
+  [gl_cv_cxx_nomfi_needed=yes])
+CXXFLAGS="$gl_save_CXXFLAGS"
+  ])
+  AC_MSG_RESULT([$gl_cv_cxx_nomfi_needed])
+fi
+
+dnl Next, check if -Werror -Wuninitialized is useful with the
+dnl user's choice of $CXXFLAGS; some versions of gcc warn that it
+dnl has no effect if -O is not also used
+AC_MSG_CHECKING([whether -Wuninitialized is supported])
+AC_CACHE_VAL([gl_cv_cxx_uninitialized_supported], [
+  gl_save_CXXFLAGS="$CXXFLAGS"
+  CXXFLAGS="$CXXFLAGS -Werror -Wuninitialized"
+  AC_COMPILE_IFELSE(
+[AC_LANG_PROGRAM([[]], [[]])],
+[gl_cv_cxx_uninitialized_supported=yes],
+[gl_cv_cxx_uninitialized_supported=no])
+  CXXFLAGS="$gl_save_CXXFLAGS"])
+AC_MSG_RESULT([$gl_cv_cxx_uninitialized_supported])
+
+  fi
+
+  # List all gcc warning categories.
+  # To compare this list to your installed GCC's, run this Bash command:
+  #
+  # comm -3 \
+  #  <(sed -n 's/^  *\(-[^ ]*\) .*/\1/p' manywarnings.m4 | sort) \
+  #  <(gcc --help=warnings | sed -n 's/^  \(-[^ ]*\) .*/\1/p' | sort |
+  #  grep -v -x -f <(
+  # awk '/^[^#]/ {print $1}' ../build-aux/g++-warning.spec))
+
+  gl_manywarn_set=
+  for gl_manywarn_item in \
+-W \
+-Wabi \
+-Wabi-tag \
+-Waddress \
+-Waggressive-loop-optimizations \
+-Wall \
+-Wattributes \
+-Wbool-compare \
+-Wbuiltin-macro-redefined \
+-Wcast-align \
+-Wchar-subscripts \
+-Wchkp \
+-Wclobbered \
+-Wcomment \
+-Wcomments \
+-Wconditionally-supported \
+-Wconversion-null \
+-Wcoverage-mismatch \
+-Wcpp \
+-Wctor-dtor-privacy \
+

Re: manywarnings for C++

2017-08-07 Thread Reuben Thomas
On 6 August 2017 at 23:50, Bruno Haible  wrote:

> Hi Reuben,
>
> > For reference, these versions do not address this request from Bruno:
>
> I've now addressed this issue. I hope you'll find the new code structure
> more systematic.
>

​Thanks very much for this. The main feature of the code which I'd not
anticipated (nor even known it was possible) is the ability to AC_DEFUN a
macro with a particular argument, as in:​

​AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS(C)],​

so that it is possible to AC_REQUIRE such a macro.

-- 
https://rrt.sc3d.org 


Re: manywarnings for C++

2017-08-06 Thread Bruno Haible
Hi Reuben,

> For reference, these versions do not address this request from Bruno:

I've now addressed this issue. I hope you'll find the new code structure
more systematic.

Here's how I see the status / needed tweaks on your patches:
  - warnings.m4: No change needed any more.
  - manywarnings.m4: define gl_MANYWARN_ALL_GCC(C++) instead of 
gl_MANYWARN_ALL_GXX.
  - g++-warning.spec: Looks OK to me. I trust that your C++ experience is
fresher than mine (I learned C++ in 1997).
Do you agree?

Bruno




Re: manywarnings for C++

2017-08-06 Thread Bruno Haible
I wrote on 2017-02-17:
> Please also fix the AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS]) invocation
> in gl_WARN_ADD and gl_CXX_WARN_ADD. Since this macro expands to different
> code after AC_LANG_PUSH([C++]) than after AC_LANG_PUSH([C]), it is wrong
> to just AC_REQUIRE it. Needs to be a bit more intelligent.
> 
> Look at how _AC_LANG_ABBREV and _AC_LANG_PREFIX can be used. Maybe libtool.m4,
> which also plays around with AC_LANG, gives you some hint about this.

To fix this, I prepared a test project (attached), and modified warnings.m4
and manywarnings.m4 until
  $ ./autogen.sh
  $ rm -f config.cache; ./configure -C
produced the config.status and config.cache that I expected.


2017-08-06  Bruno Haible  

warnings, manywarnings: Add support for multiple languages, not just C.
* warnings.m4 (gl_UNKNOWN_WARNINGS_ARE_ERRORS_IMPL): Renamed from
gl_UNKNOWN_WARNINGS_ARE_ERRORS.
(gl_UNKNOWN_WARNINGS_ARE_ERRORS(C)): New macro.
(gl_UNKNOWN_WARNINGS_ARE_ERRORS(C++)): New macro.
(gl_UNKNOWN_WARNINGS_ARE_ERRORS): Dispatch to
gl_UNKNOWN_WARNINGS_ARE_ERRORS(_AC_LANG).
(gl_WARN_ADD): Require the gl_UNKNOWN_WARNINGS_ARE_ERRORS specialization
of the current language. If C++ is the current language, modify
WARN_CXXFLAGS instead of WARN_CFLAGS.
* manywarnings.m4 (gl_MANYWARN_ALL_GCC(C)): New macro, extracted from
gl_MANYWARN_ALL_GCC.
(gl_MANYWARN_ALL_GCC): Dispatch to gl_MANYWARN_ALL_GCC(_AC_LANG).

diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4
index 6a8939b..3124b1e 100644
--- a/m4/manywarnings.m4
+++ b/m4/manywarnings.m4
@@ -1,4 +1,4 @@
-# manywarnings.m4 serial 10
+# manywarnings.m4 serial 11
 dnl Copyright (C) 2008-2017 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -33,8 +33,16 @@ AC_DEFUN([gl_MANYWARN_COMPLEMENT],
 # Add all documented GCC warning parameters to variable VARIABLE.
 # Note that you need to test them using gl_WARN_ADD if you want to
 # make sure your gcc understands it.
+#
+# The effects of this macro depend on the current language (_AC_LANG).
 AC_DEFUN([gl_MANYWARN_ALL_GCC],
+[_AC_LANG_DISPATCH([$0], _AC_LANG, $@)])
+
+# Specialization for _AC_LANG = C. This macro can be AC_REQUIREd.
+AC_DEFUN([gl_MANYWARN_ALL_GCC(C)],
 [
+  AC_LANG_PUSH([C])
+
   dnl First, check for some issues that only occur when combining multiple
   dnl gcc warning categories.
   AC_REQUIRE([AC_PROG_CC])
@@ -303,4 +311,6 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
   fi
 
   $1=$gl_manywarn_set
+
+  AC_LANG_POP([C])
 ])
diff --git a/m4/warnings.m4 b/m4/warnings.m4
index e697174..aa2735b 100644
--- a/m4/warnings.m4
+++ b/m4/warnings.m4
@@ -1,4 +1,4 @@
-# warnings.m4 serial 11
+# warnings.m4 serial 12
 dnl Copyright (C) 2008-2017 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -20,10 +20,12 @@ m4_ifdef([AS_VAR_APPEND],
 # -
 # Check if the compiler supports OPTION when compiling PROGRAM.
 #
-# FIXME: gl_Warn must be used unquoted until we can assume Autoconf
-# 2.64 or newer.
+# The effects of this macro depend on the current language (_AC_LANG).
 AC_DEFUN([gl_COMPILER_OPTION_IF],
-[AS_VAR_PUSHDEF([gl_Warn], [gl_cv_warn_[]_AC_LANG_ABBREV[]_$1])dnl
+[
+dnl FIXME: gl_Warn must be used unquoted until we can assume Autoconf
+dnl 2.64 or newer.
+AS_VAR_PUSHDEF([gl_Warn], [gl_cv_warn_[]_AC_LANG_ABBREV[]_$1])dnl
 AS_VAR_PUSHDEF([gl_Flags], [_AC_LANG_PREFIX[]FLAGS])dnl
 AS_LITERAL_IF([$1],
   [m4_pushdef([gl_Positive], m4_bpatsubst([$1], [^-Wno-], [-W]))],
@@ -51,27 +53,50 @@ AS_VAR_POPDEF([gl_Warn])dnl
 # --
 # Clang doesn't complain about unknown warning options unless one also
 # specifies -Wunknown-warning-option -Werror.  Detect this.
+#
+# The effects of this macro depend on the current language (_AC_LANG).
 AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS],
+[_AC_LANG_DISPATCH([$0], _AC_LANG, $@)])
+
+# Specialization for _AC_LANG = C. This macro can be AC_REQUIREd.
+AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS(C)],
+[
+  AC_LANG_PUSH([C])
+  gl_UNKNOWN_WARNINGS_ARE_ERRORS_IMPL
+  AC_LANG_POP([C])
+])
+
+# Specialization for _AC_LANG = C++. This macro can be AC_REQUIREd.
+AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS(C++)],
+[
+  AC_LANG_PUSH([C++])
+  gl_UNKNOWN_WARNINGS_ARE_ERRORS_IMPL
+  AC_LANG_POP([C++])
+])
+
+AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS_IMPL],
 [gl_COMPILER_OPTION_IF([-Werror -Wunknown-warning-option],
[gl_unknown_warnings_are_errors='-Wunknown-warning-option -Werror'],
[gl_unknown_warnings_are_errors=])])
 
-# gl_WARN_ADD(OPTION, [VARIABLE = WARN_CFLAGS],
+# gl_WARN_ADD(OPTION, [VARIABLE = WARN_CFLAGS/WARN_CXXFLAGS],
 # [PROGRAM = AC_LANG_PROGRAM()])
-# 

Re: manywarnings for C++

2017-08-05 Thread Paul Eggert

Thanks, as I try to avoid C++ I hope Bruno has the time to review that one.



Re: manywarnings for C++

2017-08-05 Thread Reuben Thomas
On 7 March 2017 at 12:20, Reuben Thomas  wrote:

> On 22 February 2017 at 13:55, Reuben Thomas  wrote:
>
>>
>> ​I attach the latest state of play, which consists simply of updated
>> versions of manywarnings.m4 and warnings.m4, in place of the new files I
>> had before. (To test them I have simply placed them in a gnulib patch
>> directory which is applied by bootstrap; I love how simple it is!)
>>
>
> ​Ping!
>

​Ping!! Attached, the latest version of my patches (in fact they've not
changed since last time, but there is an extra file I forgot to include
before), and now I attach patches to existing files as such rather than as
complete files.

For reference, these versions do not address this request from Bruno:

Please also fix the AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS]) invocation in
> gl_WARN_ADD and gl_CXX_WARN_ADD. Since this macro expands to different code
> after AC_LANG_PUSH([C++]) than after AC_LANG_PUSH([C]), it is wrong to
> just AC_REQUIRE it. Needs to be a bit more intelligent.


> Look at how _AC_LANG_ABBREV and _AC_LANG_PREFIX can be used. Maybe
> libtool.m4, which also plays around with AC_LANG, gives you some hint
> about this.


​I previously replied:​

I didn't fully understand this bit. I see that _AC_LANG_ABBREV/_AC_LANG_PREFIX
> are used to inject the name of the language into the names of variables. I
> see that gl_UNKNOWN_WARNINGS_ARE_ERRORS changes depending on the language
> because gl_COMPILER_OPTION_IF changes. (By the way, can we now assume
> autoconf 2.64 (of July 2009) or newer and hence quote gl_Warn, as per the
> comment, there?)
> Since gl_UNKNOWN_WARNINGS_ARE_ERRORS is therefore language-dependent, that
> makes me think it needs to be called, not AC_REQUIREd, by
> gl_{CXX_,}_WARN_ADD (because the language might change each time), but if
> it were that simple, you'd've said so. So I'm not sure what else is needed…


-- 
https://rrt.sc3d.org 
--- gnulib/m4/manywarnings.m4   2017-08-03 18:10:49.139888596 +0100
+++ gl/m4/manywarnings.m4   2017-04-04 22:27:00.204131193 +0100
@@ -293,3 +274,248 @@
 
   $1=$gl_manywarn_set
 ])
+
+# gl_MANYWARN_ALL_GXX(VARIABLE)
+# -
+# Add all documented G++ warning parameters to variable VARIABLE.
+# Note that you need to test them using gl_WARN_ADD if you want to
+# make sure your gcc understands it.
+AC_DEFUN([gl_MANYWARN_ALL_GXX],
+[
+  AC_LANG_PUSH([C++])
+
+  dnl First, check for some issues that only occur when combining multiple
+  dnl gcc warning categories.
+  AC_REQUIRE([AC_PROG_CXX])
+  if test -n "$GXX"; then
+
+dnl Check if -W -Werror -Wno-missing-field-initializers is supported
+dnl with the current $CXX $CXXFLAGS $CPPFLAGS.
+AC_MSG_CHECKING([whether -Wno-missing-field-initializers is supported])
+AC_CACHE_VAL([gl_cv_cxx_nomfi_supported], [
+  gl_save_CXXFLAGS="$CXXFLAGS"
+  CXXFLAGS="$CXXFLAGS -W -Werror -Wno-missing-field-initializers"
+  AC_COMPILE_IFELSE(
+[AC_LANG_PROGRAM([[]], [[]])],
+[gl_cv_cxx_nomfi_supported=yes],
+[gl_cv_cxx_nomfi_supported=no])
+  CXXFLAGS="$gl_save_CXXFLAGS"])
+AC_MSG_RESULT([$gl_cv_cxx_nomfi_supported])
+
+if test "$gl_cv_cxx_nomfi_supported" = yes; then
+  dnl Now check whether -Wno-missing-field-initializers is needed
+  dnl for the { 0, } construct.
+  AC_MSG_CHECKING([whether -Wno-missing-field-initializers is needed])
+  AC_CACHE_VAL([gl_cv_cxx_nomfi_needed], [
+gl_save_CXXFLAGS="$CXXFLAGS"
+CXXFLAGS="$CXXFLAGS -W -Werror"
+AC_COMPILE_IFELSE(
+  [AC_LANG_PROGRAM(
+ [[int f (void)
+   {
+ typedef struct { int a; int b; } s_t;
+ s_t s1 = { 0, };
+ return s1.b;
+   }
+ ]],
+ [[]])],
+  [gl_cv_cxx_nomfi_needed=no],
+  [gl_cv_cxx_nomfi_needed=yes])
+CXXFLAGS="$gl_save_CXXFLAGS"
+  ])
+  AC_MSG_RESULT([$gl_cv_cxx_nomfi_needed])
+fi
+
+dnl Next, check if -Werror -Wuninitialized is useful with the
+dnl user's choice of $CXXFLAGS; some versions of gcc warn that it
+dnl has no effect if -O is not also used
+AC_MSG_CHECKING([whether -Wuninitialized is supported])
+AC_CACHE_VAL([gl_cv_cxx_uninitialized_supported], [
+  gl_save_CXXFLAGS="$CXXFLAGS"
+  CXXFLAGS="$CXXFLAGS -Werror -Wuninitialized"
+  AC_COMPILE_IFELSE(
+[AC_LANG_PROGRAM([[]], [[]])],
+[gl_cv_cxx_uninitialized_supported=yes],
+[gl_cv_cxx_uninitialized_supported=no])
+  CXXFLAGS="$gl_save_CXXFLAGS"])
+AC_MSG_RESULT([$gl_cv_cxx_uninitialized_supported])
+
+  fi
+
+  # List all gcc warning categories.
+  # To compare this list to your installed GCC's, run this Bash command:
+  #
+  # comm -3 \
+  #  <(sed -n 's/^  *\(-[^ ]*\) .*/\1/p' manywarnings.m4 | sort) \
+  #  <(gcc --help=warnings | sed -n 's/^  \(-[^ ]*\) .*/\1/p' | sort |
+  #  grep 

Re: manywarnings for C++

2017-03-07 Thread Reuben Thomas
On 22 February 2017 at 13:55, Reuben Thomas  wrote:

>
> ​I attach the latest state of play, which consists simply of updated
> versions of manywarnings.m4 and warnings.m4, in place of the new files I
> had before. (To test them I have simply placed them in a gnulib patch
> directory which is applied by bootstrap; I love how simple it is!)
>

​Ping!

(By the way, if my emails are still being listed as spam because of
SPF/DKIM interacting badly with the listserv, does anyone have any
suggestion as to the most effective place to ask nicely for the listserv to
be suitably upgraded/configured?)

-- 
http://rrt.sc3d.org


Re: manywarnings for C++

2017-02-22 Thread Reuben Thomas
On 21 February 2017 at 14:58, Bruno Haible  wrote:

> Hi Reuben,
>
> This looks reasonable: Since a project can use both gcc and g++ and since
> the
> desired warning options for gcc and g++ are likely different, it's good to
> have different macros, that set different variables (WARN_CFLAGS vs.
> WARN_CXXFLAGS).
>

​Thanks very much for the detailed guidance.​

Going further, there is no real benefit of having manywarnings-cxx a
> different
> module. Since an m4 macro that is not invoked does not contribute to the
> size
> nor the execution time of the 'configure' script, I would just add the two
> new macros gl_MANYWARN_ALL_GXX and gl_CXX_WARN_ADD - in manywarnings.m4
> and warnings.m4, respectively.
>

​Done.


> However, two things are important:
>   - That the name of the *_cv_* variables are different in the C++ macro
> than in the C macro.
>

​Done.


>   - That you invoke AC_LANG_PUSH([C++]) at the beginning and
> AC_LANG_POP([C++])
> at the end of the macro, so that AC_LANG_SOURCE invocations do the
> right
> thing.
>

​Done, but do I also need to add an AC_LANG_PUSH/POP([C]) at the beginning
and end of gl_MANYWARN_ALL_GCC?​ If not, why not? (Is C assumed as the
default?)

Please also fix the AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS]) invocation
> in gl_WARN_ADD and gl_CXX_WARN_ADD. Since this macro expands to different
> code after AC_LANG_PUSH([C++]) than after AC_LANG_PUSH([C]), it is wrong
> to just AC_REQUIRE it. Needs to be a bit more intelligent.
>
> Look at how _AC_LANG_ABBREV and _AC_LANG_PREFIX can be used. Maybe
> libtool.m4,
> which also plays around with AC_LANG, gives you some hint about this.
>

I didn't fully understand this bit. I see that
_AC_LANG_ABBREV/_AC_LANG_PREFIX are used to inject the name of the language
into the names of variables. I see that gl_UNKNOWN_WARNINGS_ARE_ERRORS
changes depending on the language because gl_COMPILER_OPTION_IF changes.
(By the way, can we now assume autoconf 2.64 (of July 2009) or newer and
hence quote gl_Warn, as per the comment, there?)

Since gl_UNKNOWN_WARNINGS_ARE_ERRORS is therefore language-dependent, that
makes me think it needs to be called, not AC_REQUIREd, by
gl_{CXX_,}_WARN_ADD (because the language might change each time), but if
it were that simple, you'd've said so. So I'm not sure what else is needed…

​I attach the latest state of play, which consists simply of updated
versions of manywarnings.m4 and warnings.m4, in place of the new files I
had before. (To test them I have simply placed them in a gnulib patch
directory which is applied by bootstrap; I love how simple it is!)

-- 
http://rrt.sc3d.org


manywarnings.m4
Description: application/m4


warnings.m4
Description: application/m4


Re: manywarnings for C++

2017-02-21 Thread Reuben Thomas
On 21 February 2017 at 15:05, Bruno Haible  wrote:

> Hi Reuben,
>
> > ​For example, would it be sensible to have a single spec file, with each
> > row as follows:
> >
> > warning language-list comment
> >
> > i.e. the first and third columns as at present, and the second giving the
> > languages for which the flag is used?
>
> In my opinion
>
> 1) How to specify the set of options to avoid is orthogonal to the C vs C++
>consideration.
>

Sorry, I think I confused things, I should have said "the second giving the
list of languages for which the flag is [explicitly] disabled".

2) I find the usage approach from
>https://www.gnu.org/software/gnulib/manual/html_node/manywarnings.html
>to be just as easy to use as a .spec file, and furthermore it is more
>flexible:
>- It allows to use different sets of warnings for clang than for gcc
>  (remember that clang disguises as gcc). You need a line such as
>CC_BRAND=`LC_ALL=C $CC --version | sed -e '2,$d' -e 's/ .*//'` #
> either gcc or clang
>  to distinguish them.
>- It allows multiline descriptions why a warning is avoided.
>

​I agree, I use this approach. Maybe I misunderstood what the specfile is
for: I thought it was a way to generate and check the list of ​warning
flags to go in manywarnings.m4, not something one would directly use in a
project.

-- 
http://rrt.sc3d.org


Re: manywarnings for C++

2017-02-21 Thread Bruno Haible
Hi Reuben,

> ​For example, would it be sensible to have a single spec file, with each
> row as follows:
> 
> warning language-list comment
> 
> i.e. the first and third columns as at present, and the second giving the
> languages for which the flag is used?

In my opinion

1) How to specify the set of options to avoid is orthogonal to the C vs C++
   consideration.

2) I find the usage approach from
   https://www.gnu.org/software/gnulib/manual/html_node/manywarnings.html
   to be just as easy to use as a .spec file, and furthermore it is more
   flexible:
   - It allows to use different sets of warnings for clang than for gcc
 (remember that clang disguises as gcc). You need a line such as
   CC_BRAND=`LC_ALL=C $CC --version | sed -e '2,$d' -e 's/ .*//'` # either 
gcc or clang
 to distinguish them.
   - It allows multiline descriptions why a warning is avoided.

Bruno




Re: manywarnings for C++

2017-02-21 Thread Bruno Haible
Hi Reuben,

This looks reasonable: Since a project can use both gcc and g++ and since the
desired warning options for gcc and g++ are likely different, it's good to
have different macros, that set different variables (WARN_CFLAGS vs.
WARN_CXXFLAGS).

> I'd be happy to work this up into a patch given some guidance on what shape
> it should take.

In the current state, there is overlap between manywarnings.m4 and
manywarnings-cxx.m4 (macro gl_MANYWARN_COMPLEMENT defined twice).
Therefore I would leave gl_MANYWARN_COMPLEMENT defined only in manywarnings.m4,
and change modules/manywarnings-cxx to reference also m4/manywarnings.m4.

Going further, there is no real benefit of having manywarnings-cxx a different
module. Since an m4 macro that is not invoked does not contribute to the size
nor the execution time of the 'configure' script, I would just add the two
new macros gl_MANYWARN_ALL_GXX and gl_CXX_WARN_ADD - in manywarnings.m4
and warnings.m4, respectively.

However, two things are important:
  - That the name of the *_cv_* variables are different in the C++ macro
than in the C macro.
  - That you invoke AC_LANG_PUSH([C++]) at the beginning and AC_LANG_POP([C++])
at the end of the macro, so that AC_LANG_SOURCE invocations do the right
thing.

Please also fix the AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS]) invocation
in gl_WARN_ADD and gl_CXX_WARN_ADD. Since this macro expands to different
code after AC_LANG_PUSH([C++]) than after AC_LANG_PUSH([C]), it is wrong
to just AC_REQUIRE it. Needs to be a bit more intelligent.

Look at how _AC_LANG_ABBREV and _AC_LANG_PREFIX can be used. Maybe libtool.m4,
which also plays around with AC_LANG, gives you some hint about this.

Bruno




Re: manywarnings for C++

2017-02-21 Thread Reuben Thomas
On 21 February 2017 at 12:13, Reuben Thomas  wrote:

> I needed this recently, so I made it. I have not attempted to refactor the
> code, as there seem to be multiple sensible-seeming ways to do this; I
> attach a simple duplication of the manywarnings files, reconfigured for C++.
>
> I'd be happy to work this up into a patch given some guidance on what
> shape it should take.
>

​For example, would it be sensible to have a single spec file, with each
row as follows:

warning language-list comment

i.e. the first and third columns as at present, and the second giving the
languages for which the flag is used?

-- 
http://rrt.sc3d.org