Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 0/6] Make SELF_CHECK print operands of comparisons
@ 2026-10-19 16:58 Simon Marchi
  2026-10-19 16:58 ` [PATCH 1/6] gdb/unittests: catch gdb_exception_error in scoped_mmap test Simon Marchi
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Simon Marchi @ 2026-10-19 16:58 UTC (permalink / raw)
  To: gcc-patches, gdb-patches; +Cc: Simon Marchi

Tom mentioned [1] how it would be nice to have a SELF_CHECK variant that
took operands as separate arguments and printed them when the comparison
failed.  Some test frameworks like catch2 are actually able to decompose
some expressions passed to the test macro to achieve this automatically.
This series implements the same concept but for GDB's own SELF_CHECK
macro.

[1] https://inbox.sourceware.org/gdb-patches/20261005052602.1288766-1-simon.marchi@polymtl.ca/T/#mef6ee3be132da2d69d3ea22e6182d259f91503a9

Simon Marchi (6):
  gdb/unittests: catch gdb_exception_error in scoped_mmap test
  gdbsupport/selftest: print expression when selftest fails and
    re-format
  include/diagnostics.h: add DIAGNOSTIC_IGNORE_PARENTHESES
  gdb/gmp-utils: rename gdb_mp{z,q,f}::str to to_string
  gdbsupport/selftest: print operand values when a SELF_CHECK fails
  gdb: remove manual printing of values in some selftests

 gdb/aarch64-tdep.c                            |  38 +-
 gdb/ada-typeprint.c                           |   2 +-
 gdb/arm-tdep.c                                |   4 +-
 gdb/cp-name-parser.c                          |   2 +-
 gdb/disasm-selftests.c                        |  11 +-
 gdb/dwarf2/frame.c                            |   2 +-
 gdb/expprint.c                                |   3 +-
 gdb/gdbtypes.c                                |   2 +-
 gdb/gmp-utils.c                               |  10 +-
 gdb/gmp-utils.h                               |  12 +-
 gdb/memrange.h                                |   7 +
 gdb/producer.c                                |  33 +-
 gdb/regcache.c                                |   6 +-
 gdb/rust-parse.c                              |   2 +-
 gdb/selftest-arch.c                           |   2 +-
 gdb/source-cache.c                            |  12 +-
 gdb/split-name.c                              |  28 -
 gdb/typeprint.c                               |   2 +-
 gdb/unittests/array-view-selftests.c          |   5 +
 gdb/unittests/environ-selftests.c             |  32 +-
 gdb/unittests/frame_info_ptr-selftests.c      |   2 +-
 gdb/unittests/function-view-selftests.c       |   4 +-
 gdb/unittests/remote-arg-selftests.c          |  65 +--
 gdb/unittests/scoped_fd-selftests.c           |   8 +-
 .../scoped_ignore_signal-selftests.c          |   7 +-
 gdb/unittests/scoped_mmap-selftests.c         |  17 +-
 gdb/unittests/style-selftests.c               |  10 +-
 gdb/utils.c                                   |   4 +-
 gdb/valprint.c                                |   2 +-
 gdbserver/server.cc                           |  19 +-
 gdbsupport/common-exceptions.h                |   3 +
 gdbsupport/selftest.cc                        |  88 ++-
 gdbsupport/selftest.h                         | 545 +++++++++++++++++-
 include/diagnostics.h                         |   8 +
 34 files changed, 800 insertions(+), 197 deletions(-)


base-commit: ec9be4b35cf367e6ffa0d5881fb20e60c68e2f89
-- 
2.56.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/6] gdb/unittests: catch gdb_exception_error in scoped_mmap test
  2026-10-19 16:58 [PATCH 0/6] Make SELF_CHECK print operands of comparisons Simon Marchi
@ 2026-10-19 16:58 ` Simon Marchi
  2026-10-19 16:58 ` [PATCH 2/6] gdbsupport/selftest: print expression when selftest fails and re-format Simon Marchi
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2026-10-19 16:58 UTC (permalink / raw)
  To: gcc-patches, gdb-patches; +Cc: Simon Marchi

Most places should generally catch gdb_exception_error and not
gdb_exception, to avoid catching gdb_exception_quit and
gdb_exception_forced_quit.  I think that this function is such a place.  I
happened to see it while working on a following patch.

While at it, format the try/catch block using the correct style.

Change-Id: I80e1d43d3ea2f940e3b75baae77672e267fde446
---
 gdb/unittests/scoped_mmap-selftests.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/gdb/unittests/scoped_mmap-selftests.c b/gdb/unittests/scoped_mmap-selftests.c
index 06026b459cda..ce03d7559f7a 100644
--- a/gdb/unittests/scoped_mmap-selftests.c
+++ b/gdb/unittests/scoped_mmap-selftests.c
@@ -112,11 +112,14 @@ test_invalid_filename ()
 {
   bool threw = false;
 
-  try {
+  try
+    {
       ::scoped_mmap m = ::mmap_file ("/this/file/should/not/exist");
-  } catch (const gdb_exception &e) {
+    }
+  catch (const gdb_exception_error &)
+    {
       threw = true;
-  }
+    }
 
   SELF_CHECK (threw);
 }
-- 
2.56.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/6] gdbsupport/selftest: print expression when selftest fails and re-format
  2026-10-19 16:58 [PATCH 0/6] Make SELF_CHECK print operands of comparisons Simon Marchi
  2026-10-19 16:58 ` [PATCH 1/6] gdb/unittests: catch gdb_exception_error in scoped_mmap test Simon Marchi
@ 2026-10-19 16:58 ` Simon Marchi
  2026-10-19 16:58 ` [PATCH 3/6] include/diagnostics.h: add DIAGNOSTIC_IGNORE_PARENTHESES Simon Marchi
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2026-10-19 16:58 UTC (permalink / raw)
  To: gcc-patches, gdb-patches; +Cc: Simon Marchi

This is a small step towards printing rich error messages when selftests
fail.  Add a selftests::failed_selftest function, called by SELF_CHECK
when the check fails.  It formats the error message, which now includes
the text of the failed expression (passed as a string from the
SELF_CHECK macro), and throws an error with the new error code
SELF_CHECK_FAILED_ERROR.  Change the output to a single line starting
with "FAIL:", followed by the location and the expression.

The SELF_CHECK_FAILED_ERROR error code allows the selftest runner to tell a
failed SELF_CHECK apart from other errors thrown during a test, and report
them differently.

This changes the output when there are failed selftests from:

    $ ./gdb -nx -q --data-directory=data-directory -ex "maint selftest string_" -batch
    Running selftest string_appendf.
    Running selftest string_printf.
    Self test failed: self-test failed at /home/smarchi/src/binutils-gdb/gdb/unittests/common-utils-selftests.c:35
    Running selftest string_vappendf.
    Running selftest string_vprintf.
    Self test failed: self-test failed at /home/smarchi/src/binutils-gdb/gdb/unittests/common-utils-selftests.c:35

    Failures:
      string_printf
      string_vprintf

    Ran 4 unit tests, 2 failed

to:

    $ ./gdb -nx -q --data-directory=data-directory -ex "maint selftest string_" -batch
    Running selftest string_appendf.
    Running selftest string_printf.
    FAIL: /home/smarchi/src/binutils-gdb/gdb/unittests/common-utils-selftests.c:35: func ("%s", "test") == "test boo"
    Running selftest string_vappendf.
    Running selftest string_vprintf.
    FAIL: /home/smarchi/src/binutils-gdb/gdb/unittests/common-utils-selftests.c:35: func ("%s", "test") == "test boo"

    Failures:
      string_printf
      string_vprintf

    Ran 4 unit tests, 2 failed

Other errors thrown during a selftest, which used to be printed as:

    Self test failed: oh no

are now printed as:

    FAIL: unexpected exception: oh no

The output looks similar with gdbserver (the formatting code is shared,
so it's expected).

I wish we could add some colors, but since this code is in gdbsupport,
it's not straightforward.  Maybe we can add that later.

Change-Id: Ice0c930add2dc0d2395806cc17f9d2dc247db419
---
 gdbsupport/common-exceptions.h |  3 +++
 gdbsupport/selftest.cc         | 15 ++++++++++++++-
 gdbsupport/selftest.h          | 17 +++++++++++++----
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/gdbsupport/common-exceptions.h b/gdbsupport/common-exceptions.h
index 058260b5f270..1b3de6a2f955 100644
--- a/gdbsupport/common-exceptions.h
+++ b/gdbsupport/common-exceptions.h
@@ -112,6 +112,9 @@ enum errors {
      "_ERROR" is appended to the name.  */
   MAX_COMPLETIONS_REACHED_ERROR,
 
+  /* A SELF_CHECK failed during a selftest.  */
+  SELF_CHECK_FAILED_ERROR,
+
   /* Add more errors here.  */
   NR_ERRORS
 };
diff --git a/gdbsupport/selftest.cc b/gdbsupport/selftest.cc
index 611bb990a0e2..85eee59e3c4b 100644
--- a/gdbsupport/selftest.cc
+++ b/gdbsupport/selftest.cc
@@ -101,7 +101,11 @@ run_tests (gdb::array_view<const char *const> filters, bool verbose)
 	}
       catch (const gdb_exception_error &ex)
 	{
-	  debug_printf ("Self test failed: %s\n", ex.what ());
+	  if (ex.error == SELF_CHECK_FAILED_ERROR)
+	    debug_printf ("FAIL: %s\n", ex.what ());
+	  else
+	    debug_printf ("FAIL: unexpected exception: %s\n", ex.what ());
+
 	  failed.push_back (test.name.c_str ());
 	}
 
@@ -138,4 +142,13 @@ all_selftests ()
   return selftests_range (tests.cbegin (), tests.cend ());
 }
 
+/* See selftest.h.  */
+
+void
+failed_selftest (const char *file, int line, const char *expression)
+{
+  throw_error (SELF_CHECK_FAILED_ERROR, "%s:%d: %s", file, line,
+	       expression);
+}
+
 } /* namespace selftests */
diff --git a/gdbsupport/selftest.h b/gdbsupport/selftest.h
index 2a1539827940..ddf925dd3524 100644
--- a/gdbsupport/selftest.h
+++ b/gdbsupport/selftest.h
@@ -83,14 +83,23 @@ extern void run_tests (gdb::array_view<const char *const> filters,
 
 /* Reset GDB or GDBserver's internal state.  */
 extern void reset ();
+
+/* Function called by macro SELF_CHECK when a selftest fails.  Throw an
+   error with code SELF_CHECK_FAILED_ERROR describing the failure.  */
+
+[[noreturn]] void failed_selftest (const char *file, int line,
+				   const char *expression);
+
 }
 
 /* Check that VALUE is true, and, if not, throw an exception.  */
 
 #define SELF_CHECK(VALUE)						\
-  do {									\
-    if (!(VALUE))							\
-      error (_("self-test failed at %s:%d"), __FILE__, __LINE__);	\
-  } while (0)
+  do									\
+    {									\
+      if (!(VALUE))							\
+	selftests::failed_selftest (__FILE__, __LINE__, #VALUE);	\
+    }									\
+  while (0)
 
 #endif /* GDBSUPPORT_SELFTEST_H */
-- 
2.56.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/6] include/diagnostics.h: add DIAGNOSTIC_IGNORE_PARENTHESES
  2026-10-19 16:58 [PATCH 0/6] Make SELF_CHECK print operands of comparisons Simon Marchi
  2026-10-19 16:58 ` [PATCH 1/6] gdb/unittests: catch gdb_exception_error in scoped_mmap test Simon Marchi
  2026-10-19 16:58 ` [PATCH 2/6] gdbsupport/selftest: print expression when selftest fails and re-format Simon Marchi
@ 2026-10-19 16:58 ` Simon Marchi
  2026-10-19 16:58 ` [PATCH 4/6] gdb/gmp-utils: rename gdb_mp{z,q,f}::str to to_string Simon Marchi
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2026-10-19 16:58 UTC (permalink / raw)
  To: gcc-patches, gdb-patches; +Cc: Simon Marchi, binutils

This is used by a following patch.

Change-Id: I38609e84b9b092170d2da5589fb243f4acd8a2b4
Cc: binutils@sourceware.org
---
 include/diagnostics.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/diagnostics.h b/include/diagnostics.h
index 364789d9af7d..8cae8ac7de86 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -63,6 +63,8 @@
 # define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \
   DIAGNOSTIC_IGNORE ("-Wformat-nonliteral")
 
+# define DIAGNOSTIC_IGNORE_PARENTHESES DIAGNOSTIC_IGNORE ("-Wparentheses")
+
 # if __has_warning ("-Wuser-defined-warnings")
 #  define DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS \
    DIAGNOSTIC_IGNORE ("-Wuser-defined-warnings")
@@ -96,6 +98,8 @@
 # define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \
   DIAGNOSTIC_IGNORE ("-Wformat-nonliteral")
 
+# define DIAGNOSTIC_IGNORE_PARENTHESES DIAGNOSTIC_IGNORE ("-Wparentheses")
+
 # if __GNUC__ >= 5
 #  define DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE \
    DIAGNOSTIC_IGNORE ("-Wunused-but-set-variable")
@@ -142,6 +146,10 @@
 # define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
 #endif
 
+#ifndef DIAGNOSTIC_IGNORE_PARENTHESES
+# define DIAGNOSTIC_IGNORE_PARENTHESES
+#endif
+
 #ifndef DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
 # define DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
 #endif
-- 
2.56.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/6] gdb/gmp-utils: rename gdb_mp{z,q,f}::str to to_string
  2026-10-19 16:58 [PATCH 0/6] Make SELF_CHECK print operands of comparisons Simon Marchi
                   ` (2 preceding siblings ...)
  2026-10-19 16:58 ` [PATCH 3/6] include/diagnostics.h: add DIAGNOSTIC_IGNORE_PARENTHESES Simon Marchi
@ 2026-10-19 16:58 ` Simon Marchi
  2026-10-19 16:58 ` [PATCH 5/6] gdbsupport/selftest: print operand values when a SELF_CHECK fails Simon Marchi
  2026-10-19 16:58 ` [PATCH 6/6] gdb: remove manual printing of values in some selftests Simon Marchi
  5 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2026-10-19 16:58 UTC (permalink / raw)
  To: gcc-patches, gdb-patches; +Cc: Simon Marchi

This makes the following patch a bit easier, allowing those objects to
be printed as part of a SELF_CHECK expression.  Rename hex to
to_hex_string, for symmetry.

Change-Id: I65333129d7074a167f19abae63b3b82da7c1b6a7
---
 gdb/ada-typeprint.c  |  2 +-
 gdb/cp-name-parser.c |  2 +-
 gdb/expprint.c       |  3 ++-
 gdb/gdbtypes.c       |  2 +-
 gdb/gmp-utils.c      | 10 +++++-----
 gdb/gmp-utils.h      | 12 +++++++-----
 gdb/typeprint.c      |  2 +-
 gdb/valprint.c       |  2 +-
 8 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
index 15045a02f356..eaceac9cc3a6 100644
--- a/gdb/ada-typeprint.c
+++ b/gdb/ada-typeprint.c
@@ -1050,7 +1050,7 @@ ada_print_type (struct type *type0, const char *varstring,
 	      }
 	    else
 	      modulus = 0;
-	    gdb_printf (stream, "mod %s", modulus.str ().c_str ());
+	    gdb_printf (stream, "mod %s", modulus.to_string ().c_str ());
 	  }
 	else
 	  {
diff --git a/gdb/cp-name-parser.c b/gdb/cp-name-parser.c
index fe2ffd61a6c8..0c34942ff678 100644
--- a/gdb/cp-name-parser.c
+++ b/gdb/cp-name-parser.c
@@ -342,7 +342,7 @@ cpname_state::parse_number (const char *p, int len, int parsed_float,
       value += dig;
     }
 
-  std::string printed = value.str ();
+  std::string printed = value.to_string ();
   const char *copy = obstack_strdup (&demangle_info->obstack, printed);
 
   if (long_p == 0)
diff --git a/gdb/expprint.c b/gdb/expprint.c
index 4859d48f6a4e..26f54eed0d78 100644
--- a/gdb/expprint.c
+++ b/gdb/expprint.c
@@ -102,7 +102,8 @@ dump_for_expression (struct ui_file *stream, int depth, CORE_ADDR addr)
 void
 dump_for_expression (struct ui_file *stream, int depth, const gdb_mpz &val)
 {
-  gdb_printf (stream, _("%*sConstant: %s\n"), depth, "", val.str ().c_str ());
+  gdb_printf (stream, _("%*sConstant: %s\n"), depth, "",
+	      val.to_string ().c_str ());
 }
 
 void
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index e0c25c58d83b..2db3f7ebe733 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -4881,7 +4881,7 @@ static void
 print_fixed_point_type_info (struct type *type, int spaces)
 {
   gdb_printf ("%*sscaling factor: %s\n", spaces + 2, "",
-	      type->fixed_point_scaling_factor ().str ().c_str ());
+	      type->fixed_point_scaling_factor ().to_string ().c_str ());
 }
 
 static struct obstack dont_print_type_obstack;
diff --git a/gdb/gmp-utils.c b/gdb/gmp-utils.c
index 6438a4615d42..fb9548d2b6ed 100644
--- a/gdb/gmp-utils.c
+++ b/gdb/gmp-utils.c
@@ -47,8 +47,8 @@ extern void ATTRIBUTE_USED debug (const gdb_mpz &);
 void ATTRIBUTE_USED
 debug (const gdb_mpz &val)
 {
-  std::string dec = val.str ();
-  std::string hex = val.hex ();
+  std::string dec = val.to_string ();
+  std::string hex = val.to_hex_string ();
   gdb_printf (gdb_stdlog, "%s (%s)\n", dec.c_str (), hex.c_str ());
   gdb_flush (gdb_stdlog);
 }
@@ -123,11 +123,11 @@ gdb_mpz::export_bits (gdb::array_view<gdb_byte> buf, int endian, bool unsigned_p
       if (mpz_cmp (m_val, lo.m_val) < 0 || mpz_cmp (m_val, hi.m_val) > 0)
 	error (_("Cannot export value %s as %zu-bits %s integer"
 		 " (must be between %s and %s)"),
-	       this->str ().c_str (),
+	       this->to_string ().c_str (),
 	       max_usable_bits,
 	       unsigned_p ? _("unsigned") : _("signed"),
-	       lo.str ().c_str (),
-	       hi.str ().c_str ());
+	       lo.to_string ().c_str (),
+	       hi.to_string ().c_str ());
     }
 
   gdb_mpz truncated = *this;
diff --git a/gdb/gmp-utils.h b/gdb/gmp-utils.h
index 04f8d0c83dc4..92a21457895f 100644
--- a/gdb/gmp-utils.h
+++ b/gdb/gmp-utils.h
@@ -159,10 +159,12 @@ struct gdb_mpz
   }
 
   /* Return a string containing VAL.  */
-  std::string str () const { return gmp_string_printf ("%Zd", m_val); }
+  std::string to_string () const
+  { return gmp_string_printf ("%Zd", m_val); }
 
-  /* As str, but in hex.  */
-  std::string hex () const { return gmp_string_printf ("%#Zx", m_val); }
+  /* As to_string, but in hex.  */
+  std::string to_hex_string () const
+  { return gmp_string_printf ("%#Zx", m_val); }
 
   /* The destructor.  */
   ~gdb_mpz () { mpz_clear (m_val); }
@@ -540,7 +542,7 @@ struct gdb_mpq
   }
 
   /* Return a string representing VAL as "<numerator> / <denominator>".  */
-  std::string str () const { return gmp_string_printf ("%Qd", m_val); }
+  std::string to_string () const { return gmp_string_printf ("%Qd", m_val); }
 
   /* Return VAL rounded to the nearest integer.  */
   gdb_mpz get_rounded () const;
@@ -621,7 +623,7 @@ struct gdb_mpf
 
   /* Convert this value to a string.  FMT is the format to use, and
      should have a single '%' substitution.  */
-  std::string str (const char *fmt) const
+  std::string to_string (const char *fmt) const
   { return gmp_string_printf (fmt, m_val); }
 
   /* The destructor.  */
diff --git a/gdb/typeprint.c b/gdb/typeprint.c
index a23b5b54c0ab..2fd023bce60e 100644
--- a/gdb/typeprint.c
+++ b/gdb/typeprint.c
@@ -619,7 +619,7 @@ print_type_scalar (struct type *type, LONGEST val, struct ui_file *stream)
 void
 print_type_fixed_point (struct type *type, struct ui_file *stream)
 {
-  std::string small_img = type->fixed_point_scaling_factor ().str ();
+  std::string small_img = type->fixed_point_scaling_factor ().to_string ();
 
   gdb_printf (stream, "%s-byte fixed point (small = %s)",
 	      pulongest (type->length ()), small_img.c_str ());
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 7e41943f66fa..affddc387de9 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -876,7 +876,7 @@ generic_val_print_fixed_point (struct value *val, struct ui_file *stream,
 			  type->fixed_point_scaling_factor ());
 
       const char *fmt = type->length () < 4 ? "%.11Fg" : "%.17Fg";
-      std::string str = f.str (fmt);
+      std::string str = f.to_string (fmt);
       gdb_printf (stream, "%s", str.c_str ());
     }
 }
-- 
2.56.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/6] gdbsupport/selftest: print operand values when a SELF_CHECK fails
  2026-10-19 16:58 [PATCH 0/6] Make SELF_CHECK print operands of comparisons Simon Marchi
                   ` (3 preceding siblings ...)
  2026-10-19 16:58 ` [PATCH 4/6] gdb/gmp-utils: rename gdb_mp{z,q,f}::str to to_string Simon Marchi
@ 2026-10-19 16:58 ` Simon Marchi
  2026-10-27 14:13   ` Tom Tromey
  2026-10-19 16:58 ` [PATCH 6/6] gdb: remove manual printing of values in some selftests Simon Marchi
  5 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2026-10-19 16:58 UTC (permalink / raw)
  To: gcc-patches, gdb-patches; +Cc: Simon Marchi

The catch2 test framework (and likely others) has a neat trick to print
the operands of failing test assertions.  For instance:

    /home/simark/src/binutils-gdb/catch2/sample.cpp:17: FAILED:
      CHECK( reverse_string ("hello") == "hello" )
    with expansion:
      "olleh" == "hello"

This patch implements something like that for GDB's SELF_CHECK macro.

As I did not instinctively know how to do this, I used Claude to guide
me through the implementation, but all the code was written by me.

The general idea of the trick is that the SELF_CHECK macro:

    SELF_CHECK (len == 4)

now expands to approximately:

    check (decomposer () <= len == 4, ...)

The decomposer object has a templated operator<= that records a
reference to whatever comes after it (in this case the `len` variable)
into a lhs_holder instance:

     template<typename LhsT>
     lhs_holder<LhsT> operator<= (const LhsT &lhs) const
     {
       return lhs_holder<LhsT> {lhs};
     }

The lhs_holder then implements all the comparison operators, such as
operator==, which does the requested comparison with the following
operand:

  template<typename RhsT>
  binary_expr_holder<LhsT, RhsT> operator== (const RhsT &rhs) const
  {
    return binary_expr_holder<LhsT, RhsT> { this->lhs == rhs, "==", this->lhs,
					    rhs };
  }

The binary_expr_holder object holds references to both operands as well
as the result of the comparison.  That binary_expr_holder object is
passed to the check function, which is then able to print the values of
the operands as needed.

It's actually required to go through a separate check function (instead
of doing it all in the SELF_CHECK macro directly), to ensure that any
temporaries that binary_expr_holder binds to are still alive when we
print them.

There is a second check function that takes a lhs_holder<bool>, for
expressions like:

    SELF_CHECK (func_that_returns_bool ());

A test failure of a binary expression now looks like:

    FAIL: /home/smarchi/src/binutils-gdb/gdb/unittests/common-utils-selftests.c:35: func ("%s", "test") == "test boo": "test" == "test boo"

Some previously used constructs are now rejected at compile time, and
require a few changes in the tests:

 - A top-level && or ||.

   Overloading these operators would lose short-circuit evaluation, and
   only the leftmost operand could be decomposed anyway (because
   comparison operators take precedence).  Options to work around this
   are to split the expressions (in the case of &&) or to add an extra
   pair of parentheses around the whole expression.  For &&, I would
   argue it's a good thing to split expressions, so you get a more
   fine-grained message on failure.

 - A lone non-bool value, like SELF_CHECK (ptr).

   The fix for those is to use an explicit comparison.  The second check
   function could accept a templated lhs_holder, instead of a
   lhs_holder<bool>, but since our coding style requires using explicit
   comparisons, I don't see why it should.

   An odd case of this is a use of the frame_id::code_addr_p field.
   While it is spiritually a boolean, it is in fact a 1-bit unsigned int
   bitfield.  So it now requires an explicit `!= 0`.

The expression resulting from the macro expansion triggers a
-Wparentheses warning, so disable it in the SELF_CHECK macro.

Operands are printed using a value_to_string function that is a long
series of "if constexpr" to print various types of operands.  It ends
with an always-false static_assert, such that trying to print an
unsupported type will cause a build failure, and will require telling
the function how to print said type.  For some types like iterators,
there is no sensible value to print in a generic way, so we just print
"{iterator}".

Add a to_string method to mem_range, which is used by value_to_string.

Finally, as a way to exercise this machinery without always having to
purposely make a test fail, I made it so that running the selftests with
-verbose prints all passing tests, including their operands.  For
instance:

    PASS: /home/simark/src/binutils-gdb/gdb/unittests/int-array-iterator.h:75: m_cur_idx < m_size: 2 < 7

Change-Id: Ic81da3024811d8fc9dd6335c50a0adcf75755d5d
---
 gdb/aarch64-tdep.c                            |  38 +-
 gdb/arm-tdep.c                                |   4 +-
 gdb/disasm-selftests.c                        |   2 +-
 gdb/dwarf2/frame.c                            |   2 +-
 gdb/memrange.h                                |   7 +
 gdb/producer.c                                |  33 +-
 gdb/regcache.c                                |   6 +-
 gdb/rust-parse.c                              |   2 +-
 gdb/selftest-arch.c                           |   2 +-
 gdb/source-cache.c                            |  12 +-
 gdb/unittests/array-view-selftests.c          |   5 +
 gdb/unittests/environ-selftests.c             |  32 +-
 gdb/unittests/frame_info_ptr-selftests.c      |   2 +-
 gdb/unittests/function-view-selftests.c       |   4 +-
 gdb/unittests/scoped_fd-selftests.c           |   8 +-
 .../scoped_ignore_signal-selftests.c          |   7 +-
 gdb/unittests/scoped_mmap-selftests.c         |   8 +-
 gdb/unittests/style-selftests.c               |  10 +-
 gdb/utils.c                                   |   4 +-
 gdbserver/server.cc                           |  19 +-
 gdbsupport/selftest.cc                        |  81 ++-
 gdbsupport/selftest.h                         | 540 +++++++++++++++++-
 22 files changed, 741 insertions(+), 87 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 109fca6bf110..733ee5c5279c 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -692,7 +692,7 @@ aarch64_analyze_prologue_test (void)
   info.bfd_arch_info = bfd_scan_arch ("aarch64");
 
   struct gdbarch *gdbarch = gdbarch_find_by_info (info);
-  SELF_CHECK (gdbarch != NULL);
+  SELF_CHECK (gdbarch != nullptr);
 
   struct aarch64_prologue_cache cache;
   cache.saved_regs = trad_frame_alloc_saved_regs (gdbarch);
@@ -721,8 +721,10 @@ aarch64_analyze_prologue_test (void)
 	else if (i == AARCH64_LR_REGNUM)
 	  SELF_CHECK (cache.saved_regs[i].addr () == -264);
 	else
-	  SELF_CHECK (cache.saved_regs[i].is_realreg ()
-		      && cache.saved_regs[i].realreg () == i);
+	  {
+	    SELF_CHECK (cache.saved_regs[i].is_realreg ());
+	    SELF_CHECK (cache.saved_regs[i].realreg () == i);
+	  }
       }
 
     for (int i = 0; i < AARCH64_D_REGISTER_COUNT; i++)
@@ -730,8 +732,8 @@ aarch64_analyze_prologue_test (void)
 	int num_regs = gdbarch_num_regs (gdbarch);
 	int regnum = i + num_regs + AARCH64_D0_REGNUM;
 
-	SELF_CHECK (cache.saved_regs[regnum].is_realreg ()
-		    && cache.saved_regs[regnum].realreg () == regnum);
+	SELF_CHECK (cache.saved_regs[regnum].is_realreg ());
+	SELF_CHECK (cache.saved_regs[regnum].realreg () == regnum);
       }
   }
 
@@ -763,8 +765,10 @@ aarch64_analyze_prologue_test (void)
 	else if (i == 19)
 	  SELF_CHECK (cache.saved_regs[i].addr () == -48);
 	else
-	  SELF_CHECK (cache.saved_regs[i].is_realreg ()
-		      && cache.saved_regs[i].realreg () == i);
+	  {
+	    SELF_CHECK (cache.saved_regs[i].is_realreg ());
+	    SELF_CHECK (cache.saved_regs[i].realreg () == i);
+	  }
       }
 
     for (int i = 0; i < AARCH64_D_REGISTER_COUNT; i++)
@@ -776,8 +780,10 @@ aarch64_analyze_prologue_test (void)
 	if (i == 0)
 	  SELF_CHECK (cache.saved_regs[regnum].addr () == -24);
 	else
-	  SELF_CHECK (cache.saved_regs[regnum].is_realreg ()
-		      && cache.saved_regs[regnum].realreg () == regnum);
+	  {
+	    SELF_CHECK (cache.saved_regs[regnum].is_realreg ());
+	    SELF_CHECK (cache.saved_regs[regnum].realreg () == regnum);
+	  }
       }
   }
 
@@ -921,8 +927,10 @@ aarch64_analyze_prologue_test (void)
 	  else if (i == AARCH64_LR_REGNUM)
 	    SELF_CHECK (cache.saved_regs[i].addr () == -40);
 	  else
-	    SELF_CHECK (cache.saved_regs[i].is_realreg ()
-			&& cache.saved_regs[i].realreg () == i);
+	    {
+	      SELF_CHECK (cache.saved_regs[i].is_realreg ());
+	      SELF_CHECK (cache.saved_regs[i].realreg () == i);
+	    }
 	}
 
       if (tdep->has_pauth ())
@@ -960,8 +968,10 @@ aarch64_analyze_prologue_test (void)
 	  else if (i == AARCH64_LR_REGNUM)
 	    SELF_CHECK (cache.saved_regs[i].addr () == -40);
 	  else
-	    SELF_CHECK (cache.saved_regs[i].is_realreg ()
-			&& cache.saved_regs[i].realreg () == i);
+	    {
+	      SELF_CHECK (cache.saved_regs[i].is_realreg ());
+	      SELF_CHECK (cache.saved_regs[i].realreg () == i);
+	    }
 	}
     }
 }
@@ -6520,7 +6530,7 @@ aarch64_process_record_test (void)
   info.bfd_arch_info = bfd_scan_arch ("aarch64");
 
   struct gdbarch *gdbarch = gdbarch_find_by_info (info);
-  SELF_CHECK (gdbarch != NULL);
+  SELF_CHECK (gdbarch != nullptr);
 
   aarch64_insn_decode_record aarch64_record;
 
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index bffd5d221997..a963c8c3f803 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -14673,7 +14673,7 @@ arm_record_test (void)
 
   struct gdbarch *gdbarch = gdbarch_find_by_info (info);
 
-  SELF_CHECK (gdbarch != NULL);
+  SELF_CHECK (gdbarch != nullptr);
 
   /* 16-bit Thumb instructions.  */
   {
@@ -14789,7 +14789,7 @@ arm_analyze_prologue_test ()
 
       struct gdbarch *gdbarch = gdbarch_find_by_info (info);
 
-      SELF_CHECK (gdbarch != NULL);
+      SELF_CHECK (gdbarch != nullptr);
 
       /* The "sub" instruction contains an immediate value rotate count of 0,
 	 which resulted in a 32-bit shift of a 32-bit value, caught by
diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
index 485c1245118b..1aa399a57d4a 100644
--- a/gdb/disasm-selftests.c
+++ b/gdb/disasm-selftests.c
@@ -158,7 +158,7 @@ get_test_insn (struct gdbarch *gdbarch, size_t *len)
 	      }
 
 	    gdbarch = gdbarch_find_by_info (info);
-	    SELF_CHECK (gdbarch != NULL);
+	    SELF_CHECK (gdbarch != nullptr);
 
 	    try
 	      {
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 95e9f1e7c34d..eec1b77c2708 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -568,7 +568,7 @@ execute_cfa_program_test (struct gdbarch *gdbarch)
   SELF_CHECK (fs.regs.cfa_offset == 4);
   SELF_CHECK (fs.regs.cfa_how == CFA_REG_OFFSET);
   SELF_CHECK (fs.regs.cfa_exp.empty ());
-  SELF_CHECK (fs.regs.prev == NULL);
+  SELF_CHECK (fs.regs.prev == nullptr);
 }
 
 } /* namespace selftests */
diff --git a/gdb/memrange.h b/gdb/memrange.h
index 18d53a40024c..9b8111dccbcf 100644
--- a/gdb/memrange.h
+++ b/gdb/memrange.h
@@ -44,6 +44,13 @@ struct mem_range
   /* Returns true if ADDR is in this mem_range.  */
   bool contains (CORE_ADDR addr) const;
 
+  /* Return a string representation of this mem_range.  */
+  std::string to_string () const
+  {
+    return string_printf ("{start = %s, length = %d}",
+			  hex_string (this->start), this->length);
+  }
+
   /* Lowest address in the range.  */
   CORE_ADDR start;
 
diff --git a/gdb/producer.c b/gdb/producer.c
index 9e0da6f4eacd..1ece4237fa94 100644
--- a/gdb/producer.c
+++ b/gdb/producer.c
@@ -242,8 +242,9 @@ producer_parsing_tests ()
 	 " Intel(R) 64,	Version 14.0.1.074 Build 20130716");
 
     int major = 0, minor = 0;
-    SELF_CHECK (producer_is_icc (extern_f_14_0, &major, &minor)
-		&& major == 14 && minor == 0);
+    SELF_CHECK (producer_is_icc (extern_f_14_0, &major, &minor));
+    SELF_CHECK (major == 14);
+    SELF_CHECK (minor == 0);
     SELF_CHECK (!producer_is_gcc (extern_f_14_0, &major, &minor));
   }
 
@@ -253,8 +254,9 @@ producer_parsing_tests ()
 	 " Intel(R) 64, Version 14.0");
 
     int major = 0, minor = 0;
-    SELF_CHECK (producer_is_icc (intern_f_14, &major, &minor)
-		&& major == 14 && minor == 0);
+    SELF_CHECK (producer_is_icc (intern_f_14, &major, &minor));
+    SELF_CHECK (major == 14);
+    SELF_CHECK (minor == 0);
     SELF_CHECK (!producer_is_gcc (intern_f_14, &major, &minor));
   }
 
@@ -263,8 +265,9 @@ producer_parsing_tests ()
       = ("Intel(R) C++ Intel(R) 64 Compiler XE for applications running on"
 	 " Intel(R) 64, Version 14.0");
     int major = 0, minor = 0;
-    SELF_CHECK (producer_is_icc (intern_c_14, &major, &minor)
-		&& major == 14 && minor == 0);
+    SELF_CHECK (producer_is_icc (intern_c_14, &major, &minor));
+    SELF_CHECK (major == 14);
+    SELF_CHECK (minor == 0);
     SELF_CHECK (!producer_is_gcc (intern_c_14, &major, &minor));
   }
 
@@ -273,8 +276,9 @@ producer_parsing_tests ()
       = ("Intel(R) C++ Intel(R) 64 Compiler for applications running on"
 	 " Intel(R) 64, Version 18.0 Beta");
     int major = 0, minor = 0;
-    SELF_CHECK (producer_is_icc (intern_c_18, &major, &minor)
-		&& major == 18 && minor == 0);
+    SELF_CHECK (producer_is_icc (intern_c_18, &major, &minor));
+    SELF_CHECK (major == 18);
+    SELF_CHECK (minor == 0);
   }
 
   {
@@ -282,16 +286,18 @@ producer_parsing_tests ()
     SELF_CHECK (!producer_is_icc (gnu, nullptr, nullptr));
 
     int major = 0, minor = 0;
-    SELF_CHECK (producer_is_gcc (gnu, &major, &minor)
-		&& major == 4 && minor == 7);
+    SELF_CHECK (producer_is_gcc (gnu, &major, &minor));
+    SELF_CHECK (major == 4);
+    SELF_CHECK (minor == 7);
   }
 
   {
     static const char gnu_exp[] = "GNU C++14 5.0.0 20150123 (experimental)";
     int major = 0, minor = 0;
     SELF_CHECK (!producer_is_icc (gnu_exp, nullptr, nullptr));
-    SELF_CHECK (producer_is_gcc (gnu_exp, &major, &minor)
-		&& major == 5 && minor == 0);
+    SELF_CHECK (producer_is_gcc (gnu_exp, &major, &minor));
+    SELF_CHECK (major == 5);
+    SELF_CHECK (minor == 0);
   }
 
   {
@@ -315,7 +321,8 @@ producer_parsing_tests ()
     int major = 0, minor = 0;
     SELF_CHECK (!producer_is_gcc (gas_exp, &major, &minor));
     SELF_CHECK (producer_is_gas (gas_exp, &major, &minor));
-    SELF_CHECK (major == 2 && minor == 39);
+    SELF_CHECK (major == 2);
+    SELF_CHECK (minor == 39);
 
     static const char gas_incomplete_exp[] = "GNU AS ";
     SELF_CHECK (!producer_is_gas (gas_incomplete_exp, &major, &minor));
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 773e8ac37d0a..ed5ea1699840 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1688,7 +1688,7 @@ get_thread_arch_regcache_and_check (inferior *inf_for_target_calls,
   regcache *regcache
     = get_thread_arch_regcache (inf_for_target_calls, ptid, arch);
 
-  SELF_CHECK (regcache != NULL);
+  SELF_CHECK (regcache != nullptr);
   SELF_CHECK (regcache->ptid () == ptid);
   SELF_CHECK (regcache->arch () == arch);
 }
@@ -2030,7 +2030,7 @@ cooked_read_test (struct gdbarch *gdbarch)
 		  || bfd_arch == bfd_arch_m32c
 		  || bfd_arch == bfd_arch_mep
 		  || bfd_arch == bfd_arch_sh)
-		SELF_CHECK (status == REG_VALID || status == REG_UNKNOWN);
+		SELF_CHECK ((status == REG_VALID || status == REG_UNKNOWN));
 	      else if (bfd_arch == bfd_arch_mips
 		       || bfd_arch == bfd_arch_h8300)
 		SELF_CHECK (status == REG_UNKNOWN);
@@ -2144,7 +2144,7 @@ cooked_write_test (struct gdbarch *gdbarch)
 	{
 	  /* If we don't know how to create the expected value for the
 	     this type, make it fail.  */
-	  SELF_CHECK (0);
+	  SELF_CHECK (false);
 	}
 
       readwrite.cooked_write (regnum, expected);
diff --git a/gdb/rust-parse.c b/gdb/rust-parse.c
index 0c4f2a501abc..bef247f71c7a 100644
--- a/gdb/rust-parse.c
+++ b/gdb/rust-parse.c
@@ -2207,7 +2207,7 @@ rust_lex_exception_test (rust_parser *parser, const char *input,
     {
       /* The "kind" doesn't matter.  */
       rust_lex_test_one (parser, input, DECIMAL_INTEGER);
-      SELF_CHECK (0);
+      SELF_CHECK (false);
     }
   catch (const gdb_exception_error &except)
     {
diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
index f8e28f89eca9..9ddad71c6d0e 100644
--- a/gdb/selftest-arch.c
+++ b/gdb/selftest-arch.c
@@ -64,7 +64,7 @@ foreach_arch_test_generator (const std::string &name,
 	= ([=] ()
 	   {
 	     struct gdbarch *gdbarch = gdbarch_find_by_info (info);
-	     SELF_CHECK (gdbarch != NULL);
+	     SELF_CHECK (gdbarch != nullptr);
 	     function (gdbarch);
 	     reset ();
 	   });
diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index 804a6a08a8c3..248805cd534e 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -500,13 +500,13 @@ static void extract_lines_test ()
   std::string input_text = "abc\ndef\nghi\njkl\n";
   std::string result;
 
-  SELF_CHECK (extract_lines (input_text, 1, 1, &result)
-	      && result == "abc\n");
+  SELF_CHECK (extract_lines (input_text, 1, 1, &result));
+  SELF_CHECK (result == "abc\n");
   SELF_CHECK (!extract_lines (input_text, 2, 1, &result));
-  SELF_CHECK (extract_lines (input_text, 1, 2, &result)
-	      && result == "abc\ndef\n");
-  SELF_CHECK (extract_lines ("abc", 1, 1, &result)
-	      && result == "abc");
+  SELF_CHECK (extract_lines (input_text, 1, 2, &result));
+  SELF_CHECK (result == "abc\ndef\n");
+  SELF_CHECK (extract_lines ("abc", 1, 1, &result));
+  SELF_CHECK (result == "abc");
 }
 }
 #endif
diff --git a/gdb/unittests/array-view-selftests.c b/gdb/unittests/array-view-selftests.c
index ff31912ab03a..854704ed50a0 100644
--- a/gdb/unittests/array-view-selftests.c
+++ b/gdb/unittests/array-view-selftests.c
@@ -667,6 +667,11 @@ struct foo
     return this->n == other.n;
   }
 
+  std::string to_string () const
+  {
+    return std::to_string (this->n);
+  }
+
   int n;
 
   /* Number of times the assignment operator has been called.  */
diff --git a/gdb/unittests/environ-selftests.c b/gdb/unittests/environ-selftests.c
index b643876b1440..e47c24371aad 100644
--- a/gdb/unittests/environ-selftests.c
+++ b/gdb/unittests/environ-selftests.c
@@ -41,12 +41,12 @@ test_vector_initialization ()
 
   /* When the vector is initialized, there should always be one NULL
      element in it.  */
-  SELF_CHECK (env.envp ()[0] == NULL);
+  SELF_CHECK (env.envp ()[0] == nullptr);
   SELF_CHECK (env.user_set_env ().size () == 0);
   SELF_CHECK (env.user_unset_env ().size () == 0);
 
   /* Make sure that there is no other element.  */
-  SELF_CHECK (env.get ("PWD") == NULL);
+  SELF_CHECK (env.get ("PWD") == nullptr);
 }
 
 /* Test initialization using the host's environ.  */
@@ -105,7 +105,7 @@ test_set_A_unset_B_unset_A_cannot_find_A_can_find_B ()
   SELF_CHECK (streq (env.get ("GDB_SELFTEST_ENVIRON_2"), "bbb"));
 
   env.unset ("GDB_SELFTEST_ENVIRON_1");
-  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON_1") == NULL);
+  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON_1") == nullptr);
   SELF_CHECK (streq (env.get ("GDB_SELFTEST_ENVIRON_2"), "bbb"));
 
   /* The user-set environ var list must contain only one element
@@ -127,10 +127,10 @@ test_unset_set_empty_vector ()
   SELF_CHECK (set_contains (env.user_set_env (), std::string ("PWD=test")));
   SELF_CHECK (env.user_unset_env ().size () == 0);
   /* The second element must be NULL.  */
-  SELF_CHECK (env.envp ()[1] == NULL);
+  SELF_CHECK (env.envp ()[1] == nullptr);
   SELF_CHECK (env.user_set_env ().size () == 1);
   env.unset ("PWD");
-  SELF_CHECK (env.envp ()[0] == NULL);
+  SELF_CHECK (env.envp ()[0] == nullptr);
   SELF_CHECK (env.user_set_env ().size () == 0);
   SELF_CHECK (env.user_unset_env ().size () == 1);
   SELF_CHECK (set_contains (env.user_unset_env (), std::string ("PWD")));
@@ -148,10 +148,10 @@ test_vector_clear ()
   env.set (gdb_selftest_env_var, "1");
 
   env.clear ();
-  SELF_CHECK (env.envp ()[0] == NULL);
+  SELF_CHECK (env.envp ()[0] == nullptr);
   SELF_CHECK (env.user_set_env ().size () == 0);
   SELF_CHECK (env.user_unset_env ().size () == 0);
-  SELF_CHECK (env.get (gdb_selftest_env_var) == NULL);
+  SELF_CHECK (env.get (gdb_selftest_env_var) == nullptr);
 }
 
 /* Test that after a std::move the moved-from object is left at a
@@ -169,15 +169,15 @@ test_std_move ()
   SELF_CHECK (env.user_set_env ().size () == 1);
 
   env2 = std::move (env);
-  SELF_CHECK (env.envp ()[0] == NULL);
+  SELF_CHECK (env.envp ()[0] == nullptr);
   SELF_CHECK (streq (env2.get ("A"), "1"));
-  SELF_CHECK (env2.envp ()[1] == NULL);
+  SELF_CHECK (env2.envp ()[1] == nullptr);
   SELF_CHECK (env.user_set_env ().size () == 0);
   SELF_CHECK (set_contains (env2.user_set_env (), std::string ("A=1")));
   SELF_CHECK (env2.user_set_env ().size () == 1);
   env.set ("B", "2");
   SELF_CHECK (streq (env.get ("B"), "2"));
-  SELF_CHECK (env.envp ()[1] == NULL);
+  SELF_CHECK (env.envp ()[1] == nullptr);
 }
 
 /* Test that the move constructor leaves everything at a valid
@@ -193,16 +193,16 @@ test_move_constructor ()
   SELF_CHECK (set_contains (env.user_set_env (), std::string ("A=1")));
 
   gdb_environ env2 = std::move (env);
-  SELF_CHECK (env.envp ()[0] == NULL);
+  SELF_CHECK (env.envp ()[0] == nullptr);
   SELF_CHECK (env.user_set_env ().size () == 0);
   SELF_CHECK (streq (env2.get ("A"), "1"));
-  SELF_CHECK (env2.envp ()[1] == NULL);
+  SELF_CHECK (env2.envp ()[1] == nullptr);
   SELF_CHECK (set_contains (env2.user_set_env (), std::string ("A=1")));
   SELF_CHECK (env2.user_set_env ().size () == 1);
 
   env.set ("B", "2");
   SELF_CHECK (streq (env.get ("B"), "2"));
-  SELF_CHECK (env.envp ()[1] == NULL);
+  SELF_CHECK (env.envp ()[1] == nullptr);
   SELF_CHECK (set_contains (env.user_set_env (), std::string ("B=2")));
   SELF_CHECK (env.user_set_env ().size () == 1);
 }
@@ -229,7 +229,7 @@ test_self_move ()
 
   SELF_CHECK (streq (env.get ("A"), "1"));
   SELF_CHECK (streq (env.envp ()[0], "A=1"));
-  SELF_CHECK (env.envp ()[1] == NULL);
+  SELF_CHECK (env.envp ()[1] == nullptr);
   SELF_CHECK (set_contains (env.user_set_env (), std::string ("A=1")));
   SELF_CHECK (env.user_set_env ().size () == 1);
 }
@@ -242,7 +242,7 @@ test_set_unset_reset ()
   gdb_environ env = gdb_environ::from_host_environ ();
 
   /* Our test variable should already be present in the host's environment.  */
-  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") != NULL);
+  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") != nullptr);
 
   /* Set our test variable to another value.  */
   env.set ("GDB_SELFTEST_ENVIRON", "test");
@@ -253,7 +253,7 @@ test_set_unset_reset ()
   /* And unset our test variable.  The variable still exists in the
      host's environment, but doesn't exist in our vector.  */
   env.unset ("GDB_SELFTEST_ENVIRON");
-  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") == NULL);
+  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") == nullptr);
   SELF_CHECK (env.user_set_env ().size () == 0);
   SELF_CHECK (env.user_unset_env ().size () == 1);
   SELF_CHECK (set_contains (env.user_unset_env (),
diff --git a/gdb/unittests/frame_info_ptr-selftests.c b/gdb/unittests/frame_info_ptr-selftests.c
index dc67ebf80f38..f5d58e4c94cc 100644
--- a/gdb/unittests/frame_info_ptr-selftests.c
+++ b/gdb/unittests/frame_info_ptr-selftests.c
@@ -30,7 +30,7 @@ validate_user_created_frame (frame_id id)
 {
   SELF_CHECK (id.stack_status == FID_STACK_VALID);
   SELF_CHECK (id.stack_addr == 0x1234);
-  SELF_CHECK (id.code_addr_p);
+  SELF_CHECK (id.code_addr_p != 0);
   SELF_CHECK (id.code_addr == 0x5678);
 }
 
diff --git a/gdb/unittests/function-view-selftests.c b/gdb/unittests/function-view-selftests.c
index 7a61465d440d..4c702d2d5419 100644
--- a/gdb/unittests/function-view-selftests.c
+++ b/gdb/unittests/function-view-selftests.c
@@ -162,9 +162,9 @@ test_function_view ()
 
   /* Check the nullptr_t op= */
   gdb::function_view<int (int)> check_op_eq_null (plus_one_fn_int);
-  SELF_CHECK (check_op_eq_null);
+  SELF_CHECK (check_op_eq_null != nullptr);
   check_op_eq_null = nullptr;
-  SELF_CHECK (!check_op_eq_null);
+  SELF_CHECK (check_op_eq_null == nullptr);
 }
 
 /* A template function where the function_view type is dependent on a
diff --git a/gdb/unittests/scoped_fd-selftests.c b/gdb/unittests/scoped_fd-selftests.c
index 42cb18f342c0..f1c5667a972d 100644
--- a/gdb/unittests/scoped_fd-selftests.c
+++ b/gdb/unittests/scoped_fd-selftests.c
@@ -42,7 +42,11 @@ test_destroy ()
     SELF_CHECK (sfd.get () == fd);
   }
 
-  SELF_CHECK (close (fd) == -1 && errno == EBADF);
+  int ret = close (fd);
+  int saved_errno = errno;
+
+  SELF_CHECK (ret == -1);
+  SELF_CHECK (saved_errno == EBADF);
 }
 
 /* Test that the file descriptor can be released.  */
@@ -61,7 +65,7 @@ test_release ()
     SELF_CHECK (sfd.release () == fd);
   }
 
-  SELF_CHECK (close (fd) == 0 || errno != EBADF);
+  SELF_CHECK ((close (fd) == 0 || errno != EBADF));
 }
 
 /* Test that the file descriptor can be converted to a FILE *.  */
diff --git a/gdb/unittests/scoped_ignore_signal-selftests.c b/gdb/unittests/scoped_ignore_signal-selftests.c
index 7a0f2c2764fc..b7e67ea84a3a 100644
--- a/gdb/unittests/scoped_ignore_signal-selftests.c
+++ b/gdb/unittests/scoped_ignore_signal-selftests.c
@@ -77,8 +77,11 @@ test_sigpipe ()
     errno = 0;
 
     r = write (fd[1], &c, 1);
-    SELF_CHECK (r == -1 && errno == EPIPE
-		&& got_sigpipe == expect_sig);
+    int saved_errno = errno;
+
+    SELF_CHECK (r == -1);
+    SELF_CHECK (saved_errno == EPIPE);
+    SELF_CHECK (got_sigpipe == expect_sig);
   };
 
   /* Check that without a scoped_ignore_sigpipe in scope we indeed get
diff --git a/gdb/unittests/scoped_mmap-selftests.c b/gdb/unittests/scoped_mmap-selftests.c
index ce03d7559f7a..a2fbc0e1f3e5 100644
--- a/gdb/unittests/scoped_mmap-selftests.c
+++ b/gdb/unittests/scoped_mmap-selftests.c
@@ -47,7 +47,11 @@ test_destroy ()
     SELF_CHECK (mem != nullptr);
   }
 
-  SELF_CHECK (msync (mem, sysconf (_SC_PAGESIZE), 0) == -1 && errno == ENOMEM);
+  int ret = msync (mem, sysconf (_SC_PAGESIZE), 0);
+  int saved_errno = errno;
+
+  SELF_CHECK (ret == -1);
+  SELF_CHECK (saved_errno == ENOMEM);
 }
 
 /* Test that the memory can be released.  */
@@ -65,7 +69,7 @@ test_release ()
     SELF_CHECK (mem != nullptr);
   }
 
-  SELF_CHECK (msync (mem, sysconf (_SC_PAGESIZE), 0) == 0 || errno != ENOMEM);
+  SELF_CHECK ((msync (mem, sysconf (_SC_PAGESIZE), 0) == 0 || errno != ENOMEM));
 
   munmap (mem, sysconf (_SC_PAGESIZE));
 }
diff --git a/gdb/unittests/style-selftests.c b/gdb/unittests/style-selftests.c
index 9035050bd6ce..632a1e42a4e9 100644
--- a/gdb/unittests/style-selftests.c
+++ b/gdb/unittests/style-selftests.c
@@ -23,8 +23,14 @@
 namespace selftests {
 namespace style {
 
-#define CHECK_RGB(R, G, B) \
-  SELF_CHECK (rgb[0] == (R) && rgb[1] == (G) && rgb[2] == (B))
+#define CHECK_RGB(R, G, B)		\
+  do					\
+    {					\
+      SELF_CHECK (rgb[0] == (R));	\
+      SELF_CHECK (rgb[1] == (G));	\
+      SELF_CHECK (rgb[2] == (B));	\
+    }					\
+  while (0)
 
 static void
 run_tests ()
diff --git a/gdb/utils.c b/gdb/utils.c
index 5deecdbba942..0ed943cc4929 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3354,8 +3354,8 @@ gdb_realpath_check_trailer (const char *input, const char *trailer)
   size_t len = strlen (result.get ());
   size_t trail_len = strlen (trailer);
 
-  SELF_CHECK (len >= trail_len
-	      && streq (result.get () + len - trail_len, trailer));
+  SELF_CHECK (len >= trail_len);
+  SELF_CHECK (streq (result.get () + len - trail_len, trailer));
 }
 
 static void
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 74774fcde7ca..9a1a38978b73 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -4075,7 +4075,9 @@ test_memory_tagging_functions (void)
   type = 255;
   strcpy (packet.data (), "qMemTags:0,0:0");
   parse_fetch_memtags_request (packet.data (), &addr, &len, &type);
-  SELF_CHECK (addr == 0 && len == 0 && type == 0);
+  SELF_CHECK (addr == 0);
+  SELF_CHECK (len == 0);
+  SELF_CHECK (type == 0);
 
   /* Valid request, addr, len and type updated.  */
   addr = 0;
@@ -4083,7 +4085,9 @@ test_memory_tagging_functions (void)
   type = 0;
   strcpy (packet.data (), "qMemTags:deadbeef,ff:5");
   parse_fetch_memtags_request (packet.data (), &addr, &len, &type);
-  SELF_CHECK (addr == 0xdeadbeef && len == 255 && type == 5);
+  SELF_CHECK (addr == 0xdeadbeef);
+  SELF_CHECK (len == 255);
+  SELF_CHECK (type == 5);
 
   /* Test creating a qMemTags reply.  */
 
@@ -4107,7 +4111,10 @@ test_memory_tagging_functions (void)
   strcpy (packet.data (), "QMemTags:0,0:0:");
   SELF_CHECK (parse_store_memtags_request (packet.data (),
 					   &addr, &len, tags, &type) == true);
-  SELF_CHECK (addr == 0 && len == 0 && type == 0 && tags.size () == 0);
+  SELF_CHECK (addr == 0);
+  SELF_CHECK (len == 0);
+  SELF_CHECK (type == 0);
+  SELF_CHECK (tags.size () == 0);
 
   /* Valid request and non-empty tag data: addr, len, type
      and tags updated.  */
@@ -4119,8 +4126,10 @@ test_memory_tagging_functions (void)
 	  "QMemTags:deadbeef,ff:5:0001020304");
   SELF_CHECK (parse_store_memtags_request (packet.data (), &addr, &len, tags,
 					   &type) == true);
-  SELF_CHECK (addr == 0xdeadbeef && len == 255 && type == 5
-	      && tags.size () == 5);
+  SELF_CHECK (addr == 0xdeadbeef);
+  SELF_CHECK (len == 255);
+  SELF_CHECK (type == 5);
+  SELF_CHECK (tags.size () == 5);
 }
 
 /* Exercise the behavior of doing a 0-length comparison for a register in a
diff --git a/gdbsupport/selftest.cc b/gdbsupport/selftest.cc
index 85eee59e3c4b..55cf6d51d418 100644
--- a/gdbsupport/selftest.cc
+++ b/gdbsupport/selftest.cc
@@ -144,11 +144,84 @@ all_selftests ()
 
 /* See selftest.h.  */
 
-void
-failed_selftest (const char *file, int line, const char *expression)
+std::string
+detail::quote_string (std::string_view str, char quote)
 {
-  throw_error (SELF_CHECK_FAILED_ERROR, "%s:%d: %s", file, line,
-	       expression);
+  std::string result (1, quote);
+
+  for (char c : str)
+    switch (c)
+      {
+      case '\n':
+	result += "\\n";
+	break;
+
+      case '\t':
+	result += "\\t";
+	break;
+
+      case '\r':
+	result += "\\r";
+	break;
+
+      case '\0':
+	result += "\\0";
+	break;
+
+      case '\\':
+	result += "\\\\";
+	break;
+
+      default:
+	if (c == quote)
+	  {
+	    result += '\\';
+	    result += c;
+	  }
+	else if (c_isprint (c))
+	  result += c;
+	else
+	  result += string_printf ("\\x%02x", static_cast<unsigned char> (c));
+	break;
+      }
+
+  result += quote;
+  return result;
+}
+
+/* Return a string describing the check of EXPRESSION at FILE:LINE, used
+   in the reports of passed and failed checks.  EXPANDS_TO may be
+   nullptr.  */
+
+static std::string
+describe_check (const char *file, int line, const char *expression,
+		const char *expands_to)
+{
+  if (expands_to != nullptr)
+    return string_printf ("%s:%d: %s: %s", file, line, expression,
+			  expands_to);
+
+  return string_printf ("%s:%d: %s", file, line, expression);
+}
+
+/* See selftest.h.  */
+
+void
+passed_selftest (const char *file, int line, const char *expression,
+		 const char *expands_to)
+{
+  debug_printf ("PASS: %s\n",
+		describe_check (file, line, expression, expands_to).c_str ());
+}
+
+/* See selftest.h.  */
+
+void
+failed_selftest (const char *file, int line, const char *expression,
+		 const char *expands_to)
+{
+  throw_error (SELF_CHECK_FAILED_ERROR, "%s",
+	       describe_check (file, line, expression, expands_to).c_str ());
 }
 
 } /* namespace selftests */
diff --git a/gdbsupport/selftest.h b/gdbsupport/selftest.h
index ddf925dd3524..4c5789e089e2 100644
--- a/gdbsupport/selftest.h
+++ b/gdbsupport/selftest.h
@@ -19,12 +19,32 @@
 #ifndef GDBSUPPORT_SELFTEST_H
 #define GDBSUPPORT_SELFTEST_H
 
+#include "diagnostics.h"
 #include "gdbsupport/array-view.h"
+#include "gdbsupport/enumerate.h"
+#include "gdbsupport/enum-flags.h"
 #include "gdbsupport/function-view.h"
 #include "gdbsupport/iterator-range.h"
+#include "gdbsupport/packed.h"
+#include <atomic>
+#include <iterator>
+#include <optional>
 #include <set>
+#include <string_view>
+#include <utility>
 #include <vector>
 
+/* The SELF_CHECK macro expands to an expression that triggers -Wparentheses,
+   which it suppresses locally with DIAGNOSTIC_IGNORE_PARENTHESES.  Before
+   version 10, GCC did not honor diagnostic pragmas coming from macro
+   expansions, so disable the warning for the whole translation unit instead.
+   This can be removed once the minimum supported GCC version is at least
+   10.  */
+
+#if defined(__GNUC__) && !defined(__clang__) && __GNUC__ < 10
+#  pragma GCC diagnostic ignored "-Wparentheses"
+#endif
+
 /* A test is just a function that does some checks and throws an
    exception if something has gone wrong.  */
 
@@ -88,18 +108,524 @@ extern void reset ();
    error with code SELF_CHECK_FAILED_ERROR describing the failure.  */
 
 [[noreturn]] void failed_selftest (const char *file, int line,
-				   const char *expression);
+				   const char *expression,
+				   const char *expands_to);
 
+/* Function called by macro SELF_CHECK when a selftest passes, in verbose
+   mode.  Print a line describing the check.  EXPANDS_TO may be nullptr.  */
+
+extern void passed_selftest (const char *file, int line,
+			     const char *expression, const char *expands_to);
+
+namespace detail
+{
+
+/* Always false, but dependent on T, for use in static_assert.
+
+   Older compilers reject a plain "static_assert (false)" in a template,
+   even if the template is never instantiated.  This was changed by P2593
+   ("Allowing static_assert(false)"), implemented as a defect report (so
+   in C++17 mode too) in GCC 13 and Clang 17.  This can be removed once the
+   minimum supported versions are at least these.  */
+
+template<typename T>
+constexpr bool dependent_false_v = false;
+
+/* The result of decomposing a binary comparison (like "a == b") in a
+   SELF_CHECK.  */
+
+template<typename LhsT, typename RhsT>
+struct binary_expr_holder
+{
+  template<typename Rhs2T>
+  auto operator&& (const Rhs2T &) const
+  {
+    static_assert (dependent_false_v<Rhs2T>,
+		   "SELF_CHECK does not support a top-level &&: "
+		   "split it into separate SELF_CHECKs, or wrap the "
+		   "whole expression in extra parentheses, e.g. "
+		   "SELF_CHECK ((a && b)).");
+  }
+
+  template<typename Rhs2T>
+  auto operator|| (const Rhs2T &) const
+  {
+    static_assert (dependent_false_v<Rhs2T>,
+		   "SELF_CHECK does not support a top-level ||: "
+		   "wrap the whole expression in extra parentheses, "
+		   "e.g. SELF_CHECK ((a || b)).");
+  }
+
+  /* The result of the expression.  */
+  bool result;
+
+  /* The operator as a string, e.g. "!=".  */
+  const char *op;
+
+  /* The operands.  */
+  const LhsT &lhs;
+  const RhsT &rhs;
+};
+
+/* The result of decomposing the left operand of the expression of a
+   SELF_CHECK.  */
+
+template<typename LhsT>
+struct lhs_holder
+{
+  template<typename RhsT>
+  binary_expr_holder<LhsT, RhsT> operator== (const RhsT &rhs) const
+  {
+    return binary_expr_holder<LhsT, RhsT> { this->lhs == rhs, "==", this->lhs,
+					    rhs };
+  }
+
+  template<typename RhsT>
+  binary_expr_holder<LhsT, RhsT> operator!= (const RhsT &rhs) const
+  {
+    return binary_expr_holder<LhsT, RhsT> { this->lhs != rhs, "!=", this->lhs,
+					    rhs };
+  }
+
+  template<typename RhsT>
+  binary_expr_holder<LhsT, RhsT> operator> (const RhsT &rhs) const
+  {
+    return binary_expr_holder<LhsT, RhsT> { this->lhs > rhs, ">", this->lhs,
+					    rhs };
+  }
+
+  template<typename RhsT>
+  binary_expr_holder<LhsT, RhsT> operator< (const RhsT &rhs) const
+  {
+    return binary_expr_holder<LhsT, RhsT> { this->lhs < rhs, "<", this->lhs,
+					    rhs };
+  }
+
+  template<typename RhsT>
+  binary_expr_holder<LhsT, RhsT> operator<= (const RhsT &rhs) const
+  {
+    return binary_expr_holder<LhsT, RhsT> { this->lhs <= rhs, "<=", this->lhs,
+					    rhs };
+  }
+
+  template<typename RhsT>
+  binary_expr_holder<LhsT, RhsT> operator>= (const RhsT &rhs) const
+  {
+    return binary_expr_holder<LhsT, RhsT> { this->lhs >= rhs, ">=", this->lhs,
+					    rhs };
+  }
+
+  template<typename RhsT>
+  auto operator&& (const RhsT &) const
+  {
+    static_assert (dependent_false_v<RhsT>,
+		   "SELF_CHECK does not support a top-level &&: "
+		   "split it into separate SELF_CHECKs, or wrap the "
+		   "whole expression in extra parentheses, e.g. "
+		   "SELF_CHECK ((a && b)).");
+  }
+
+  template<typename RhsT>
+  auto operator|| (const RhsT &) const
+  {
+    static_assert (dependent_false_v<RhsT>,
+		   "SELF_CHECK does not support a top-level ||: "
+		   "wrap the whole expression in extra parentheses, "
+		   "e.g. SELF_CHECK ((a || b)).");
+  }
+
+  /* The left operand.  */
+  const LhsT &lhs;
+};
+
+/* Starting point of the decomposition of the expression of a SELF_CHECK.
+
+   The general idea is:
+
+     - SELF_CHECK (a == b) expands to "decomposer () <= a == b"
+     - "decomposer () <= a" gets evaluated first, yields a lhs_holder
+     - "<lhs_holder> == b" gets evaluated, yields a binary_expr_holder
+     - The resulting binary_expr_holder is passed to the check function
+   */
+
+struct decomposer
+{
+  template<typename LhsT>
+  lhs_holder<LhsT> operator<= (const LhsT &lhs) const
+  {
+    return lhs_holder<LhsT> {lhs};
+  }
+};
+
+/* Return STR between QUOTE characters, with QUOTE, special and
+   non-printable characters escaped.  */
+
+std::string quote_string (std::string_view str, char quote = '"');
+
+/* Trait to detect types that can be iterated on with std::begin and
+   std::end.  */
+
+template<typename T, typename = void>
+struct is_range : std::false_type
+{};
+
+template<typename T>
+struct is_range<T,
+		std::void_t<decltype (std::begin (std::declval<const T &> ())),
+			    decltype (std::end (std::declval<const T &> ()))>>
+  : std::true_type
+{};
+
+template<typename T>
+constexpr bool is_range_v = is_range<T>::value;
+
+/* Trait to detect smart pointer types, that is types with a get method
+   returning a pointer.  */
+
+template<typename T, typename = void>
+struct is_smart_pointer : std::false_type
+{};
+
+template<typename T>
+using get_method_result_t = decltype (std::declval<const T &> ().get ());
+
+template<typename T>
+struct is_smart_pointer
+  <T, std::enable_if_t<std::is_pointer_v<get_method_result_t<T>>>>
+  : std::true_type
+{};
+
+template<typename T>
+constexpr bool is_smart_pointer_v = is_smart_pointer<T>::value;
+
+/* Trait to detect std::optional types.  */
+
+template<typename T>
+struct is_optional : std::false_type
+{};
+
+template<typename T>
+struct is_optional<std::optional<T>> : std::true_type
+{};
+
+template<typename T>
+constexpr bool is_optional_v = is_optional<T>::value;
+
+/* Trait to detect enum_flags types.  */
+
+template<typename T>
+struct is_enum_flags : std::false_type
+{};
+
+template<typename E>
+struct is_enum_flags<enum_flags<E>> : std::true_type
+{};
+
+template<typename T>
+constexpr bool is_enum_flags_v = is_enum_flags<T>::value;
+
+/* Trait to detect std::pair types.  */
+
+template<typename T>
+struct is_pair : std::false_type
+{};
+
+template<typename T1, typename T2>
+struct is_pair<std::pair<T1, T2>> : std::true_type
+{};
+
+template<typename T>
+constexpr bool is_pair_v = is_pair<T>::value;
+
+/* Trait to detect gdb::function_view types.  */
+
+template<typename T>
+struct is_function_view : std::false_type
+{};
+
+template<typename Sig>
+struct is_function_view<gdb::function_view<Sig>> : std::true_type
+{};
+
+template<typename T>
+constexpr bool is_function_view_v = is_function_view<T>::value;
+
+/* Trait to detect packed types.  */
+
+template<typename T>
+struct is_packed : std::false_type
+{};
+
+template<typename T, size_t Bytes>
+struct is_packed<packed<T, Bytes>> : std::true_type
+{
+  using wrapped_type = T;
+};
+
+template<typename T>
+constexpr bool is_packed_v = is_packed<T>::value;
+
+/* Trait to detect iterator types, which define an iterator_category
+   member type.  */
+
+template<typename T, typename = void>
+struct is_iterator : std::false_type
+{};
+
+template<typename T>
+struct is_iterator<T, std::void_t<typename T::iterator_category>>
+  : std::true_type
+{};
+
+template<typename T>
+constexpr bool is_iterator_v = is_iterator<T>::value;
+
+/* Trait to detect std::atomic types.  */
+
+template<typename T>
+struct is_atomic : std::false_type
+{};
+
+template<typename T>
+struct is_atomic<std::atomic<T>> : std::true_type
+{};
+
+template<typename T>
+constexpr bool is_atomic_v = is_atomic<T>::value;
+
+/* Trait to detect types with a to_string method.  */
+
+template<typename T, typename = void>
+struct has_to_string_method : std::false_type
+{};
+
+template<typename T>
+struct has_to_string_method<
+  T, std::void_t<decltype (std::declval<const T &> ().to_string ())>>
+  : std::true_type
+{};
+
+template<typename T>
+constexpr bool has_to_string_method_v = has_to_string_method<T>::value;
+
+/* Return a string representation of VAL, used to print the operands of
+   SELF_CHECKs.  Fails to compile if ValT is a type that it doesn't know
+   how to print.  */
+
+template<typename ValT>
+std::string
+value_to_string (const ValT &val)
+{
+  constexpr auto is_array = std::is_array_v<ValT>;
+  using element_type = std::remove_cv_t<std::remove_extent_t<ValT>>;
+  using pointee_type = std::remove_cv_t<std::remove_pointer_t<ValT>>;
+
+  if constexpr (is_array && std::is_same_v<element_type, char>)
+    {
+      /* Print fixed-size character arrays as strings.  The array is not
+	 necessarily null-terminated, don't read past its end.  */
+      constexpr auto extent = std::extent_v<ValT>;
+      return quote_string (std::string_view (val, strnlen (val, extent)));
+    }
+  else if constexpr (std::is_same_v<ValT, std::nullptr_t>)
+    {
+      /* This must come before the std::string_view case, because nullptr
+	 converts to std::string_view.  */
+      return "nullptr";
+    }
+  else if constexpr (std::is_pointer_v<ValT>
+		     && std::is_same_v<pointee_type, char>)
+    {
+      /* Char pointer, print as string.  This must come before the
+	 std::string_view case, because converting a null pointer to
+	 std::string_view is undefined behavior.  */
+      if (val == nullptr)
+	return "nullptr";
+
+      return quote_string (val);
+    }
+  else if constexpr (std::is_convertible_v<const ValT &, std::string_view>)
+    {
+      /* std::string and friends.  */
+      return quote_string (val);
+    }
+  else if constexpr (std::is_pointer_v<ValT>)
+    {
+      /* Any other pointer.  */
+      if (val == nullptr)
+	return "nullptr";
+
+      return host_address_to_string (val);
+    }
+  else if constexpr (std::is_same_v<ValT, bool>)
+    return val ? "true" : "false";
+  else if constexpr (std::is_same_v<ValT, char>)
+    return quote_string (std::string_view (&val, 1), '\'');
+  else if constexpr (std::is_integral_v<ValT>)
+    return std::to_string (val);
+  else if constexpr (std::is_enum_v<ValT>)
+    {
+      /* Print the numerical value.  Don't go through value_to_string, the
+	 underlying type could be char, and that would print it as a quoted
+	 character.  */
+      return std::to_string (static_cast<std::underlying_type_t<ValT>> (val));
+    }
+  else if constexpr (is_function_view_v<ValT>)
+    {
+      /* There is nothing interesting to print about the callable.  */
+      if (val == nullptr)
+	return "nullptr";
+
+      return "{function_view}";
+    }
+  else if constexpr (is_atomic_v<ValT>)
+    return value_to_string (val.load ());
+  else if constexpr (is_packed_v<ValT>)
+    {
+      using wrapped_type = typename is_packed<ValT>::wrapped_type;
+
+      return value_to_string (static_cast<wrapped_type> (val));
+    }
+  else if constexpr (is_pair_v<ValT>)
+    return ("{" + value_to_string (val.first) + ", "
+	    + value_to_string (val.second) + "}");
+  else if constexpr (is_enum_flags_v<ValT>)
+    {
+      /* These are bit flags, so hex is more readable.  */
+      auto raw = static_cast<typename ValT::underlying_type> (val);
+
+      return string_printf ("0x%llx", static_cast<unsigned long long> (raw));
+    }
+  else if constexpr (has_to_string_method_v<ValT>)
+    {
+      auto str = val.to_string ();
+
+      /* Some to_string methods return a const char *, print it like
+	 any other value of that type.  */
+      if constexpr (std::is_same_v<decltype (str), std::string>)
+	return str;
+      else
+	return value_to_string (str);
+    }
+  else if constexpr (is_optional_v<ValT>)
+    {
+      if (!val.has_value ())
+	return "std::nullopt";
+
+      return value_to_string (*val);
+    }
+  else if constexpr (is_smart_pointer_v<ValT>)
+    return value_to_string (val.get ());
+  else if constexpr (is_range_v<ValT>)
+    {
+      /* Don't print too many elements, to keep the message readable.  */
+      constexpr std::size_t max_elements = 32;
+      std::string result = "{";
+
+      for (auto [i, elem] : gdb::ranges::views::enumerate (val))
+	{
+	  if (i > 0)
+	    result += ", ";
+
+	  if (i == max_elements)
+	    {
+	      result += "...";
+	      break;
+	    }
+
+	  result += value_to_string (elem);
+	}
+
+      result += "}";
+      return result;
+    }
+  else if constexpr (is_iterator_v<ValT>)
+    {
+      /* Iterators can't be printed meaningfully, and dereferencing them is
+	 not always valid.  */
+      return "{iterator}";
+    }
+  else
+    {
+      static_assert (dependent_false_v<ValT>,
+		     "value_to_string: don't know how to print this type, add "
+		     "a to_string method to it or a case in value_to_string");
+      return {};
+    }
+}
+
+/* Report a passed (if running verbose) or failed selftest, based on the
+   result in EXPR.  This function handles the binary expressions like:
+
+     SELF_CHECK (a == b)  */
+
+template<typename LhsT, typename RhsT>
+void
+check (const binary_expr_holder<LhsT, RhsT> &expr, const char *file, int line,
+       const char *expression)
+{
+  auto expands_to = [&] ()
+    {
+      return string_printf ("%s %s %s",
+			    value_to_string (expr.lhs).c_str (), expr.op,
+			    value_to_string (expr.rhs).c_str ());
+    };
+
+  if (!expr.result)
+    failed_selftest (file, line, expression, expands_to ().c_str ());
+  else if (run_verbose ())
+    passed_selftest (file, line, expression, expands_to ().c_str ());
+}
+
+/* Report a passed (if running verbose) or failed selftest, based on the
+   result in EXPR.  This function handles the unary expressions like:
+
+     SELF_CHECK (some_bool_expression)
+
+   This function could be templated and accept anything that converts to bool,
+   but our coding style is to use explicit comparison, so we might as well
+   enforce that.  */
+
+inline void
+check (const lhs_holder<bool> &expr, const char *file, int line,
+       const char *expression)
+{
+  if (!expr.lhs)
+    failed_selftest (file, line, expression, nullptr);
+  else if (run_verbose ())
+    passed_selftest (file, line, expression, nullptr);
+}
+
+/* Reject non-bool values, like:
+
+     SELF_CHECK (ptr)
+
+   See the bool overload above.  The point of this function is to give a clear
+   error message.  */
+
+template<typename LhsT>
+void
+check (const lhs_holder<LhsT> &, const char *, int, const char *)
+{
+  static_assert (dependent_false_v<LhsT>,
+		 "SELF_CHECK on a non-bool value: use an explicit comparison, "
+		 "e.g. SELF_CHECK (ptr != nullptr).");
+}
+
+} /* namespace detail */
 }
 
 /* Check that VALUE is true, and, if not, throw an exception.  */
 
-#define SELF_CHECK(VALUE)						\
-  do									\
-    {									\
-      if (!(VALUE))							\
-	selftests::failed_selftest (__FILE__, __LINE__, #VALUE);	\
-    }									\
+#define SELF_CHECK(VALUE)                                                  \
+  do                                                                       \
+    {                                                                      \
+      DIAGNOSTIC_PUSH                                                      \
+      DIAGNOSTIC_IGNORE_PARENTHESES                                        \
+      selftests::detail::check (selftests::detail::decomposer () <= VALUE, \
+				__FILE__, __LINE__, #VALUE);               \
+      DIAGNOSTIC_POP                                                       \
+    }                                                                      \
   while (0)
 
 #endif /* GDBSUPPORT_SELFTEST_H */
-- 
2.56.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 6/6] gdb: remove manual printing of values in some selftests
  2026-10-19 16:58 [PATCH 0/6] Make SELF_CHECK print operands of comparisons Simon Marchi
                   ` (4 preceding siblings ...)
  2026-10-19 16:58 ` [PATCH 5/6] gdbsupport/selftest: print operand values when a SELF_CHECK fails Simon Marchi
@ 2026-10-19 16:58 ` Simon Marchi
  5 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2026-10-19 16:58 UTC (permalink / raw)
  To: gcc-patches, gdb-patches; +Cc: Simon Marchi

Now that SELF_CHECK prints the values of the operands of a failing
comparison, some selftests no longer need to print them by hand.

 - split-name.c: check_split_name prints the result and expected
   components on failure.  Remove that, and components_to_string, which
   is now unused.

 - disasm-selftests.c: disassemble_insn prints the expected string on
   failure.  Remove that.  Keep printing the instruction bytes, which
   give context.

 - remote-arg-selftests.c: the test counts failures, prints the split
   and joined arguments in verbose mode, and checks the failure count at
   the end.  Replace that with a SELF_CHECK for each comparison.  Keep
   printing the input in verbose mode.  The test now stops at the first
   failing case, like most selftests.

Change-Id: I1e4cc3da7e607c36bb0d67ef9562bb8216a2d42f
---
 gdb/disasm-selftests.c               |  9 +---
 gdb/split-name.c                     | 28 ------------
 gdb/unittests/remote-arg-selftests.c | 65 ++--------------------------
 3 files changed, 6 insertions(+), 96 deletions(-)

diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
index 1aa399a57d4a..9bd9e497b533 100644
--- a/gdb/disasm-selftests.c
+++ b/gdb/disasm-selftests.c
@@ -401,19 +401,14 @@ disassemble_insn (gdbarch *gdbarch, gdb::byte_vector &insn,
   std::string buffer
     = disassemble_one_insn_to_string (gdbarch, insn);
 
-  bool check_ok = buffer == expected;
-
-  if (run_verbose () || !check_ok)
+  if (run_verbose () || buffer != expected)
     {
       for (gdb_byte b : insn)
 	debug_printf ("0x%02x ", b);
       debug_printf ("-> %s\n", buffer.c_str ());
     }
 
-  if (!check_ok)
-    debug_printf ("expected: %s\n", expected.c_str ());
-
-  SELF_CHECK (check_ok);
+  SELF_CHECK (buffer == expected);
 }
 
 } /* namespace selftests */
diff --git a/gdb/split-name.c b/gdb/split-name.c
index b8605c4040f4..ea71e0589d1b 100644
--- a/gdb/split-name.c
+++ b/gdb/split-name.c
@@ -19,7 +19,6 @@
 
 #include "split-name.h"
 #include "cp-support.h"
-#include "gdbsupport/enumerate.h"
 #include "gdbsupport/selftest.h"
 
 /* See split-name.h.  */
@@ -82,27 +81,6 @@ split_name (const char *name, split_style style)
 
 namespace selftests {
 
-/* Return a string representation of COMPONENTS, for debugging.  */
-
-static std::string
-components_to_string (const std::vector<std::string_view> &components)
-{
-  std::string str = "{";
-
-  for (auto [i, component] : gdb::ranges::views::enumerate (components))
-    {
-      if (i > 0)
-	str += ",";
-
-      str += " \"";
-      str += component;
-      str += "\"";
-    }
-
-  str += " }";
-  return str;
-}
-
 /* Check that splitting NAME using STYLE gives EXPECTED.  */
 
 static void
@@ -110,12 +88,6 @@ check_split_name (const char *name, split_style style,
 		  const std::vector<std::string_view> &expected)
 {
   std::vector<std::string_view> result = split_name (name, style);
-
-  if (result != expected)
-    debug_printf ("split_name (\"%s\"): got %s, expected %s\n", name,
-		  components_to_string (result).c_str (),
-		  components_to_string (expected).c_str ());
-
   SELF_CHECK (result == expected);
 }
 
diff --git a/gdb/unittests/remote-arg-selftests.c b/gdb/unittests/remote-arg-selftests.c
index e55a7185c618..a306925c48dd 100644
--- a/gdb/unittests/remote-arg-selftests.c
+++ b/gdb/unittests/remote-arg-selftests.c
@@ -82,7 +82,6 @@ arg_test_desc desc[] = {
 static void
 self_test ()
 {
-  int failure_count = 0;
   for (const auto &d : desc)
     {
       if (run_verbose ())
@@ -94,50 +93,11 @@ self_test ()
 
       /* Split argument string into individual arguments.  */
       std::vector<std::string> split_args = gdb::remote_args::split (d.input);
-
-      if (run_verbose ())
-	{
-	  debug_printf ("Split:\n");
-
-	  size_t len = std::max (split_args.size (), d.split.size ());
-	  for (size_t i = 0; i < len; ++i)
-	    {
-	      const char *got = "N/A";
-	      const char *expected = got;
-
-	      if (i < split_args.size ())
-		got = split_args[i].c_str ();
-
-	      if (i < d.split.size ())
-		expected = d.split[i].c_str ();
-
-	      debug_printf ("  got (%s), expected (%s)\n", got, expected);
-	    }
-	}
-
-      if (split_args != d.split)
-	{
-	  ++failure_count;
-	  if (run_verbose ())
-	    debug_printf ("FAIL\n");
-	  continue;
-	}
+      SELF_CHECK (split_args == d.split);
 
       /* Now join the arguments.  */
-      std::string joined_args
-	= gdb::remote_args::join (split_args);
-
-      if (run_verbose ())
-	debug_printf ("Joined (%s), expected (%s)\n",
-		      joined_args.c_str (), d.joined.c_str ());
-
-      if (joined_args != d.joined)
-	{
-	  ++failure_count;
-	  if (run_verbose ())
-	    debug_printf ("FAIL\n");
-	  continue;
-	}
+      std::string joined_args = gdb::remote_args::join (split_args);
+      SELF_CHECK (joined_args == d.joined);
 
       /* The contents of JOINED_ARGS will not be identical to D.INPUT.
 	 There are multiple ways that an argument can be escaped, and our
@@ -146,25 +106,8 @@ self_test ()
 	 SPLIT_ARGS.  So test that next.  */
       std::vector<std::string> split_args_v2
 	= gdb::remote_args::split (joined_args);
-
-      if (split_args_v2 != split_args)
-	{
-	  ++failure_count;
-	  if (run_verbose ())
-	    {
-	      debug_printf ("Re-split:\n");
-	      for (const auto &a : split_args_v2)
-		debug_printf ("  got (%s)\n", a.c_str ());
-	      debug_printf ("FAIL\n");
-	    }
-	  continue;
-	}
-
-      if (run_verbose ())
-	debug_printf ("PASS\n");
+      SELF_CHECK (split_args_v2 == split_args);
     }
-
-  SELF_CHECK (failure_count == 0);
 }
 
 } /* namespace remote_args_tests */
-- 
2.56.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 5/6] gdbsupport/selftest: print operand values when a SELF_CHECK fails
  2026-10-19 16:58 ` [PATCH 5/6] gdbsupport/selftest: print operand values when a SELF_CHECK fails Simon Marchi
@ 2026-10-27 14:13   ` Tom Tromey
  2026-10-27 17:06     ` Simon Marchi
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2026-10-27 14:13 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gcc-patches, gdb-patches

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> Some previously used constructs are now rejected at compile time, and
Simon> require a few changes in the tests:

Simon>  - A top-level && or ||.

I think this approach is very clever but I'm not really a fan of the result.
It seems completely reasonable to me to want to write || or && in a test.

Simon>  - A lone non-bool value, like SELF_CHECK (ptr).

This isn't really that weird either.

Simon> Operands are printed using a value_to_string function that is a long
Simon> series of "if constexpr" to print various types of operands.  It ends
Simon> with an always-false static_assert, such that trying to print an
Simon> unsupported type will cause a build failure, and will require telling
Simon> the function how to print said type.

Why this way and not overloads?

It seems to me that the drawback of this approach is that now the test
framework has to have visibility on every possible thing that could be
printed.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 5/6] gdbsupport/selftest: print operand values when a SELF_CHECK fails
  2026-10-27 14:13   ` Tom Tromey
@ 2026-10-27 17:06     ` Simon Marchi
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2026-10-27 17:06 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi; +Cc: gcc-patches, gdb-patches

On 10/27/26 10:13 AM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> Some previously used constructs are now rejected at compile time, and
> Simon> require a few changes in the tests:
> 
> Simon>  - A top-level && or ||.
> 
> I think this approach is very clever but I'm not really a fan of the result.
> It seems completely reasonable to me to want to write || or && in a test.

If you really need it (you only really "need" it for ||), you can always
add extra parentheses.

> Simon>  - A lone non-bool value, like SELF_CHECK (ptr).
> 
> This isn't really that weird either.

We could make it work, I just chose not to because of our coding style.

> Simon> Operands are printed using a value_to_string function that is a long
> Simon> series of "if constexpr" to print various types of operands.  It ends
> Simon> with an always-false static_assert, such that trying to print an
> Simon> unsupported type will cause a build failure, and will require telling
> Simon> the function how to print said type.
> 
> Why this way and not overloads?

I guess you mean templates with SFINAE to select the right printer?  If
so: because it's easier to understand than separate templated functions,
especially the order in which rules are applied.  With templates, you
have to know the template deduction rules, which isn't obvious.  With a
single function, the decision tree is right there, easy to follow.

> It seems to me that the drawback of this approach is that now the test
> framework has to have visibility on every possible thing that could be
> printed.

We can abstract it somewhat by having the test framework just call
`val.to_string()` or `to_string(val)`, and then the specific `to_string`
implementations are not known by the test framework.

Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-10-27 17:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-19 16:58 [PATCH 0/6] Make SELF_CHECK print operands of comparisons Simon Marchi
2026-10-19 16:58 ` [PATCH 1/6] gdb/unittests: catch gdb_exception_error in scoped_mmap test Simon Marchi
2026-10-19 16:58 ` [PATCH 2/6] gdbsupport/selftest: print expression when selftest fails and re-format Simon Marchi
2026-10-19 16:58 ` [PATCH 3/6] include/diagnostics.h: add DIAGNOSTIC_IGNORE_PARENTHESES Simon Marchi
2026-10-19 16:58 ` [PATCH 4/6] gdb/gmp-utils: rename gdb_mp{z,q,f}::str to to_string Simon Marchi
2026-10-19 16:58 ` [PATCH 5/6] gdbsupport/selftest: print operand values when a SELF_CHECK fails Simon Marchi
2026-10-27 14:13   ` Tom Tromey
2026-10-27 17:06     ` Simon Marchi
2026-10-19 16:58 ` [PATCH 6/6] gdb: remove manual printing of values in some selftests Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox