Re: [PATCH v2 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib

[Enze Li <enze.li@hotmail.com>]

Hi Tsukasa,

Thanks for doing this.  I tested this patch on macOS and found one nit.
Please see below.

On Thu, Sep 22 2022 at 08:25:46 AM +0000, Tsukasa OI wrote:

> Gnulib generates a warning if the system version of certain functions
> are used (to redirect the developer to use Gnulib version).  It caused a
> compiler error when...
>
> -   Compiled with Clang
> -   -Werror is specified (by default)
> -   C++ standard used by Clang is before C++17 (by default as of 15.0.0)
>     when this unit test is activated.
>
> This issue is raised as PR28413.
>
> However, previous proposal to fix this issue (a "fix" to Gnulib):
> <https://lists.gnu.org/archive/html/bug-gnulib/2021-10/msg00003.html>
> was rejected because it ruins the intent of Gnulib warnings.
>
> So, we need a Binutils/GDB-side solution.
>
> This commit tries to deal with this issue on the GDB side.  We have
> "include/diagnostics.h" to disable certain warnings only when necessary.
>
> This commit suppresses the Gnulib warnings by adding
> DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS before including "defs.h".
>
> gdb/ChangeLog:
>
> 	pr 28413
> 	* unittests/string_view-selftests.c: Suppress Gnulib-generated
> 	warnings when "defs.h" is included.
> ---
>  gdb/unittests/string_view-selftests.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gdb/unittests/string_view-selftests.c b/gdb/unittests/string_view-selftests.c
> index 2d7261d18d3..c1f7799d94c 100644
> --- a/gdb/unittests/string_view-selftests.c
> +++ b/gdb/unittests/string_view-selftests.c
> @@ -23,7 +23,12 @@
>  
>  #define GNULIB_NAMESPACE gnulib
>  
> +#include "diagnostics.h"
> +
> +DIAGNOSTIC_PUSH
> +DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
>  #include "defs.h"
> +DIAGNOSTIC_POP
>  #include "gdbsupport/selftest.h"
>  #include "gdbsupport/gdb_string_view.h"

With your patch applied on macOS 15.10.7, it still says,

==========================================================================================
  CXX    unittests/string_view-selftests.o
In file included from unittests/string_view-selftests.c:39:
/Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/fstream:579:17: error: The symbol
      ::fdopen refers to the system function. Use gnulib::fdopen instead.
      [-Werror,-Wuser-defined-warnings]
      __file_ = fdopen(__fd, __mdstr);
                ^
./../gnulib/import/stdio.h:826:1: note: from 'diagnose_if' attribute on 'fdopen':
_GL_CXXALIASWARN (fdopen);
^~~~~~~~~~~~~~~~~~~~~~~~~
./../gnulib/import/stdio.h:461:4: note: expanded from macro '_GL_CXXALIASWARN'
   _GL_CXXALIASWARN_1 (func, GNULIB_NAMESPACE)
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./../gnulib/import/stdio.h:463:4: note: expanded from macro '_GL_CXXALIASWARN_1'
   _GL_CXXALIASWARN_2 (func, namespace)
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./../gnulib/import/stdio.h:468:5: note: expanded from macro '_GL_CXXALIASWARN_2'
    _GL_WARN_ON_USE (func, \
    ^~~~~~~~~~~~~~~~~~~~~~~~
./../gnulib/import/stdio.h:632:19: note: expanded from macro '_GL_WARN_ON_USE'
  __attribute__ ((__diagnose_if__ (1, message, "warning")))
                  ^                ~
1 error generated.
make[2]: *** [unittests/string_view-selftests.o] Error 1
==========================================================================================

I think this can be solved by just adding
DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS before including <fstream>
according to your method.

WDYT?
==========================================================================================
diff --git a/gdb/unittests/string_view-selftests.c b/gdb/unittests/string_view-selftests.c
index 2d7261d18d3..b779b9b765a 100644
--- a/gdb/unittests/string_view-selftests.c
+++ b/gdb/unittests/string_view-selftests.c
@@ -23,7 +23,12 @@

 #define GNULIB_NAMESPACE gnulib

+#include "diagnostics.h"
+
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
 #include "defs.h"
+DIAGNOSTIC_POP
 #include "gdbsupport/selftest.h"
 #include "gdbsupport/gdb_string_view.h"

@@ -31,7 +36,10 @@
    included test files are wrapped in a namespace.  */
 #include <string>
 #include <sstream>
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
 #include <fstream>
+DIAGNOSTIC_POP
 #include <iostream>

 /* libstdc++'s testsuite uses VERIFY.  */
==========================================================================================

Other than that, looks good to me.

Reviewed-by: Enze Li <enze.li@hotmail.com>

Thanks,
Enze