Re: [PATCH] libgccjit: Add option to hide stderr logs [PR104073]

[Antoni Boucher <bouanto@zoho.com>]

[-- Attachment #1: Type: text/plain, Size: 1958 bytes --]

Thanks for the review.
Here's the updated patch.

Le mardi 18 janvier 2022 à 18:22 -0500, David Malcolm a écrit :
> On Mon, 2022-01-17 at 21:02 -0500, Antoni Boucher via Gcc-patches
> wrote:
> > Hi.
> > This option will be useful for rustc_codegen_gcc to hide the error
> > about unsupported 128-bit integer types.
> > 
> > David, if you know of a better way to check if these types are
> > supported than creating such a type and checking if it causes an
> > error,
> > I will not need this patch.
> 
> Off the top of my head I don't know of such a way.
> 
> That said, this seems to be vaguely analogous to a test in a
> "configure" script, attempting a compile and seeing if it succeeds.
> 
> This seems like a useful pattern for libgccjit to support, so that
> client code can query the capabilities of the host, so I think the
> idea
> of this patch is sound.
> 
> As for the details of the patch, I don't like adding new members to
> the
> enums in libgccjit.h; I prefer adding new entrypoints, as the latter
> gives a way to tell if client code uses the new entrypoint as part of
> the ELF metadata, so that we can tell directly that client code is
> incompatible with an older libgccjit.so from the symbol metadata in
> the
> built binary.
> 
> So I'd prefer something like:
> 
>   extern void
>   gcc_jit_context_set_bool_print_errors_to_stderr (gcc_jit_context
> *ctxt,
>                                                    int enabled);
> 
> where gcc_jit_context_set_bool_print_errors_to_stderr defaults to
> true,
> but client code can use:
> 
>   gcc_jit_context_set_bool_print_errors_to_stderr (ctxt, false);
> 
> Or maybe have a way to specify the FILE * for errors to be printed
> to,
> defaulting to stderr, but settable to NULL if you want to suppress
> the
> printing?  That might be more flexible.
> 
> Thoughts?
> Dave
> 


[-- Attachment #2: 0001-libgccjit-Add-function-to-hide-stderr-logs-PR104073.patch --]
[-- Type: text/x-patch, Size: 7508 bytes --]

From 1f1b7d2298956268e2678a157a34c8f9ac370f3a Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Sun, 23 Jan 2022 11:37:07 -0500
Subject: [PATCH] libgccjit: Add function to hide stderr logs [PR104073]

2022-01-23  Antoni Boucher <bouanto@zoho.com

gcc/jit/
	PR jit/104073
	* docs/topics/compatibility.rst (LIBGCCJIT_ABI_23): New ABI tag.
	* docs/topics/contexts.rst: Add documentation for the new
	function gcc_jit_context_set_bool_print_errors_to_stderr.
	* jit-common.h: New enum value
	(INNER_BOOL_OPTION_PRINT_ERRORS_TO_STDERR).
	* jit-recording.cc: Handle the new option
	INNER_BOOL_OPTION_PRINT_ERRORS_TO_STDERR.
	* libgccjit.cc: New function
	(gcc_jit_context_set_bool_print_errors_to_stderr).
	* libgccjit.h: New function
	(gcc_jit_context_set_bool_print_errors_to_stderr).
	* libgccjit.map (LIBGCCJIT_ABI_23): New ABI tag.
---
 gcc/jit/docs/topics/compatibility.rst |  9 +++++++++
 gcc/jit/docs/topics/contexts.rst      | 15 +++++++++++++++
 gcc/jit/jit-common.h                  |  1 +
 gcc/jit/jit-recording.cc              | 27 +++++++++++++++++----------
 gcc/jit/libgccjit.cc                  | 17 +++++++++++++++++
 gcc/jit/libgccjit.h                   | 18 ++++++++++++++++++
 gcc/jit/libgccjit.map                 | 14 ++++++++++++++
 7 files changed, 91 insertions(+), 10 deletions(-)

diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 16cebe31a10..d4b0ccd98ff 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -302,3 +302,12 @@ thread-local storage model of a variable:
 section of a variable:
 
   * :func:`gcc_jit_lvalue_set_link_section`
+
+.. _LIBGCCJIT_ABI_23:
+
+``LIBGCCJIT_ABI_23``
+-----------------------
+``LIBGCCJIT_ABI_23`` covers the addition of an API entrypoint to hide stderr
+logs:
+
+  * :func:`gcc_jit_context_set_bool_print_errors_to_stderr`
diff --git a/gcc/jit/docs/topics/contexts.rst b/gcc/jit/docs/topics/contexts.rst
index 68ab7ab1321..00b45262fda 100644
--- a/gcc/jit/docs/topics/contexts.rst
+++ b/gcc/jit/docs/topics/contexts.rst
@@ -489,6 +489,21 @@ Boolean options
 
       #ifdef LIBGCCJIT_HAVE_gcc_jit_context_set_bool_use_external_driver
 
+.. function:: void \
+              gcc_jit_context_set_bool_print_errors_to_stderr (gcc_jit_context *ctxt, \
+                                                                 int enabled)
+
+   By default, libgccjit will print errors to stderr.
+
+   This entrypoint can be used to disable the printing.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_23`; you can test for
+   its presence using
+
+   .. code-block:: c
+
+      #ifdef LIBGCCJIT_HAVE_gcc_jit_context_set_bool_print_errors_to_stderr
+
 Integer options
 ***************
 
diff --git a/gcc/jit/jit-common.h b/gcc/jit/jit-common.h
index 1a8cd8bac7b..c3a0b6765c0 100644
--- a/gcc/jit/jit-common.h
+++ b/gcc/jit/jit-common.h
@@ -198,6 +198,7 @@ enum inner_bool_option
 {
   INNER_BOOL_OPTION_ALLOW_UNREACHABLE_BLOCKS,
   INNER_BOOL_OPTION_USE_EXTERNAL_DRIVER,
+  INNER_BOOL_OPTION_PRINT_ERRORS_TO_STDERR,
 
   NUM_INNER_BOOL_OPTIONS
 };
diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index 1e3fadfacd7..36d962ea391 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -592,6 +592,7 @@ recording::context::context (context *parent_ctxt)
       memset (m_int_options, 0, sizeof (m_int_options));
       memset (m_bool_options, 0, sizeof (m_bool_options));
       memset (m_inner_bool_options, 0, sizeof (m_inner_bool_options));
+      m_inner_bool_options[INNER_BOOL_OPTION_PRINT_ERRORS_TO_STDERR] = true;
     }
 
   memset (m_basic_types, 0, sizeof (m_basic_types));
@@ -1551,15 +1552,20 @@ recording::context::add_error_va (location *loc, const char *fmt, va_list ap)
   if (!ctxt_progname)
     ctxt_progname = "libgccjit.so";
 
-  if (loc)
-    fprintf (stderr, "%s: %s: error: %s\n",
-	     ctxt_progname,
-	     loc->get_debug_string (),
-	     errmsg);
-  else
-    fprintf (stderr, "%s: error: %s\n",
-	     ctxt_progname,
-	     errmsg);
+  bool print_errors_to_stderr =
+      get_inner_bool_option (INNER_BOOL_OPTION_PRINT_ERRORS_TO_STDERR);
+  if (print_errors_to_stderr)
+  {
+    if (loc)
+      fprintf (stderr, "%s: %s: error: %s\n",
+	       ctxt_progname,
+	       loc->get_debug_string (),
+	       errmsg);
+    else
+      fprintf (stderr, "%s: error: %s\n",
+	       ctxt_progname,
+	       errmsg);
+  }
 
   if (!m_error_count)
     {
@@ -1687,7 +1693,8 @@ static const char * const
 static const char * const
  inner_bool_option_reproducer_strings[NUM_INNER_BOOL_OPTIONS] = {
   "gcc_jit_context_set_bool_allow_unreachable_blocks",
-  "gcc_jit_context_set_bool_use_external_driver"
+  "gcc_jit_context_set_bool_use_external_driver",
+  "gcc_jit_context_set_bool_print_errors_to_stderr",
 };
 
 /* Write the current value of all options to the log file (if any).  */
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 4c352e8c93d..778f8e926a2 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -3431,6 +3431,23 @@ gcc_jit_context_set_bool_allow_unreachable_blocks (gcc_jit_context *ctxt,
     bool_value);
 }
 
+/* Public entrypoint.  See description in libgccjit.h.
+
+   After error-checking, the real work is done by the
+   gcc::jit::recording::context::set_inner_bool_option method in
+   jit-recording.cc.  */
+
+void
+gcc_jit_context_set_bool_print_errors_to_stderr (gcc_jit_context *ctxt,
+						 int enabled)
+{
+  RETURN_IF_FAIL (ctxt, NULL, NULL, "NULL context");
+  JIT_LOG_FUNC (ctxt->get_logger ());
+  ctxt->set_inner_bool_option (
+    gcc::jit::INNER_BOOL_OPTION_PRINT_ERRORS_TO_STDERR,
+    enabled);
+}
+
 /* Public entrypoint.  See description in libgccjit.h.
 
    After error-checking, the real work is done by the
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 2a5ffacb1fe..16b8637dd0a 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -293,6 +293,24 @@ gcc_jit_context_set_bool_allow_unreachable_blocks (gcc_jit_context *ctxt,
    tested for with #ifdef.  */
 #define LIBGCCJIT_HAVE_gcc_jit_context_set_bool_allow_unreachable_blocks
 
+/* By default, libgccjit will print errors to stderr.
+
+   This option can be used to disable the printing.
+
+   This entrypoint was added in LIBGCCJIT_ABI_23; you can test for
+   its presence using
+     #ifdef LIBGCCJIT_HAVE_gcc_jit_context_set_bool_print_errors_to_stderr
+*/
+
+extern void
+gcc_jit_context_set_bool_print_errors_to_stderr (gcc_jit_context *ctxt,
+						 int enabled);
+
+/* Pre-canned feature macro to indicate the presence of
+   gcc_jit_context_set_bool_allow_unreachable_blocks.  This can be
+   tested for with #ifdef.  */
+#define LIBGCCJIT_HAVE_gcc_jit_context_set_bool_print_errors_to_stderr
+
 /* Implementation detail:
    libgccjit internally generates assembler, and uses "driver" code
    for converting it to other formats (e.g. shared libraries).
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index f373fd39ac7..aa161b438c8 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -243,3 +243,17 @@ LIBGCCJIT_ABI_19 {
     gcc_jit_context_new_union_constructor;
     gcc_jit_global_set_initializer_rvalue;
 } LIBGCCJIT_ABI_18;
+
+LIBGCCJIT_ABI_20 {
+} LIBGCCJIT_ABI_19;
+
+LIBGCCJIT_ABI_21 {
+} LIBGCCJIT_ABI_20;
+
+LIBGCCJIT_ABI_22 {
+} LIBGCCJIT_ABI_21;
+
+LIBGCCJIT_ABI_23 {
+  global:
+    gcc_jit_context_set_bool_print_errors_to_stderr;
+} LIBGCCJIT_ABI_22;
-- 
2.26.2.7.g19db9cfb68.dirty