[PATCH 1/2] Support C++-specific selftests

[PATCH 1/2] Support C++-specific selftests

[David Malcolm]

Currently the "make selftest" target run during each stage of the
build just runs the selftests within cc1.

As part of the fix for PR c++/81167 I want to be able to write
C++-specific selftests (to exercise the C++ implementation of
the pp's format_decoder).

Hence this patch generalizes the existing selftest logic in
Makefile.in so that selftests are run for both cc1 and cc1plus.
It also adds convenience methods for running them under
gdb and under valgrind.

It also reorganizes the existing lang-specific selftests, splitting
them into ones shared by c-family, and those that are specific
to individual members of the c family.  There aren't any of the
latter yet, but the followup adds some for C++.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu in
combination with the followup patch; the number of reported passes
from -fself-test for cc1 at each stage was unaffected by the kit:
  -fself-test: 39233 pass(es),
and cc1plus gained a similar line of output.

OK for trunk?

gcc/ChangeLog:
	* Makefile.in (SELFTEST_FLAGS): Drop "-x c", moving it to...
	(C_SELFTEST_FLAGS): New.
	(CPP_SELFTEST_FLAGS): New.
	(SELFTEST_DEPS): New, from deps of s-selftest.
	(C_SELFTEST_DEPS): New, from deps of s-selftest.
	(CPP_SELFTEST_DEPS): New.
	(selftest): Add dependency on s-selftest-c++.
	(s-selftest): Rename to...
	(s-selftest-c): ...this, moving deps to SELFTEST_DEPS
	and C_SELFTEST_DEPS, and using C_SELFTEST_FLAGS rather
	than SELFTEST_FLAGS.
	(selftest-gdb): Rename to...
	(selftest-c-gdb): ...this, using C_SELFTEST_DEPS and
	C_SELFTEST_FLAGS.
	(selftest-gdb): Reintroduce as an alias for selftest-c-gdb.
	(selftest-valgrind): Rename to...
	(selftest-c-valgrind): ...this, using C_SELFTEST_DEPS and
	C_SELFTEST_FLAGS.
	(selftest-valgrind): Reintroduce as an alias for
	selftest-c-valgrind.
	(s-selftest-c++): New.
	(selftest-c++-gdb): New.
	(selftest-c++-valgrind): New.

gcc/c-family/ChangeLog:
	* c-common.c (selftest::c_family_tests): New.
	* c-common.h (selftest::run_c_tests): Move decl to c/c-lang.h.
	(selftest::c_family_tests): New decl.

gcc/c/ChangeLog:
	* c-lang.c (selftest::run_c_tests): Move body to c_family_tests,
	and call that instead.
	* c-tree.h (selftest::run_c_tests): New decl.

gcc/cp/ChangeLog:
	* cp-lang.c (LANG_HOOKS_RUN_LANG_SELFTESTS): Define as
	selftest::run_cp_tests.
	(selftest::run_cp_tests): New function.
	* cp-tree.h (selftest::run_cp_tests): New decl.
---
 gcc/Makefile.in         | 62 +++++++++++++++++++++++++++++++++++++------------
 gcc/c-family/c-common.c | 16 +++++++++++++
 gcc/c-family/c-common.h |  6 ++++-
 gcc/c/c-lang.c          |  5 +++-
 gcc/c/c-tree.h          |  7 ++++++
 gcc/cp/cp-lang.c        | 24 +++++++++++++++++++
 gcc/cp/cp-tree.h        |  6 +++++
 7 files changed, 109 insertions(+), 17 deletions(-)

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 67d69c1..23c1e03 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1903,30 +1903,62 @@ rest.cross: specs
 # "nul.s" on Windows. Because on Windows "nul" is a reserved file name.
 # Specify the path to gcc/testsuite/selftests within the srcdir
 # as an argument to -fself-test.
-SELFTEST_FLAGS = -nostdinc -x c /dev/null -S -o /dev/null \
+SELFTEST_FLAGS = -nostdinc /dev/null -S -o /dev/null \
 	-fself-test=$(srcdir)/testsuite/selftests
 
-# Run the selftests during the build once we have a driver and a cc1,
+C_SELFTEST_FLAGS = -xc $(SELFTEST_FLAGS)
+CPP_SELFTEST_FLAGS = -xc++ $(SELFTEST_FLAGS)
+
+SELFTEST_DEPS = $(GCC_PASSES) stmp-int-hdrs $(srcdir)/testsuite/selftests
+
+C_SELFTEST_DEPS = cc1$(exeext) $(SELFTEST_DEPS)
+CPP_SELFTEST_DEPS = cc1plus$(exeext) $(SELFTEST_DEPS)
+
+# Run the selftests during the build once we have a driver and the frontend,
 # so that self-test failures are caught as early as possible.
-# Use "s-selftest" to ensure that we only run the selftests if the
-# driver, cc1, or selftest data change.
+# Use "s-selftest-FE" to ensure that we only run the selftests if the
+# driver, frontend, or selftest data change.
 .PHONY: selftest
-selftest: s-selftest
-s-selftest: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs \
-  $(srcdir)/testsuite/selftests
-	$(GCC_FOR_TARGET) $(SELFTEST_FLAGS)
+selftest: s-selftest-c s-selftest-c++
+
+# C selftests
+s-selftest-c: $(C_SELFTEST_DEPS)
+	$(GCC_FOR_TARGET) $(C_SELFTEST_FLAGS)
 	$(STAMP) $@
 
-# Convenience method for running selftests under gdb:
-.PHONY: selftest-gdb
-selftest-gdb: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
-	$(GCC_FOR_TARGET) $(SELFTEST_FLAGS) \
+# Convenience methods for running C selftests under gdb:
+.PHONY: selftest-c-gdb
+selftest-c-gdb: $(C_SELFTEST_DEPS)
+	$(GCC_FOR_TARGET) $(C_SELFTEST_FLAGS) \
 	  -wrapper gdb,--args
 
-# Convenience method for running selftests under valgrind:
+.PHONY: selftest-gdb
+selftest-gdb: selftest-c-gdb
+
+# Convenience methods for running C selftests under valgrind:
+.PHONY: selftest-c-valgrind
+selftest-c-valgrind: $(C_SELFTEST_DEPS)
+	$(GCC_FOR_TARGET) $(C_SELFTEST_FLAGS) \
+	  -wrapper valgrind,--leak-check=full
+
 .PHONY: selftest-valgrind
-selftest-valgrind: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
-	$(GCC_FOR_TARGET) $(SELFTEST_FLAGS) \
+selftest-valgrind: selftest-c-valgrind
+
+# C++ selftests
+s-selftest-c++: $(CPP_SELFTEST_DEPS)
+	$(GCC_FOR_TARGET) $(CPP_SELFTEST_FLAGS)
+	$(STAMP) $@
+
+# Convenience method for running C++ selftests under gdb:
+.PHONY: selftest-c++-gdb
+selftest-c++-gdb: $(CPP_SELFTEST_DEPS)
+	$(GCC_FOR_TARGET) $(CPP_SELFTEST_FLAGS) \
+	  -wrapper gdb,--args
+
+# Convenience method for running C++ selftests under valgrind:
+.PHONY: selftest-c++-valgrind
+selftest-c++-valgrind: $(CPP_SELFTEST_DEPS)
+	$(GCC_FOR_TARGET) $(CPP_SELFTEST_FLAGS) \
 	  -wrapper valgrind,--leak-check=full
 
 # Recompile all the language-independent object files.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 4395e51..8e68e6a 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -7996,4 +7996,20 @@ c_flt_eval_method (bool maybe_c11_only_p)
     return c_ts18661_flt_eval_method ();
 }
 
+#if CHECKING_P
+
+namespace selftest {
+
+/* Run all of the tests within c-family.  */
+
+void
+c_family_tests (void)
+{
+  c_format_c_tests ();
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 1748c19..21f71c3 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1556,8 +1556,12 @@ extern void add_no_sanitize_value (tree node, unsigned int flags);
 
 #if CHECKING_P
 namespace selftest {
+  /* Declarations for specific families of tests within c-family,
+     by source file, in alphabetical order.  */
   extern void c_format_c_tests (void);
-  extern void run_c_tests (void);
+
+  /* The entrypoint for running all of the above tests.  */
+  extern void c_family_tests (void);
 } // namespace selftest
 #endif /* #if CHECKING_P */
 
diff --git a/gcc/c/c-lang.c b/gcc/c/c-lang.c
index 510b7e7..e05741d 100644
--- a/gcc/c/c-lang.c
+++ b/gcc/c/c-lang.c
@@ -58,7 +58,10 @@ namespace selftest {
 void
 run_c_tests (void)
 {
-  c_format_c_tests ();
+  /* Run selftests shared within the C family.  */
+  c_family_tests ();
+
+  /* Additional C-specific tests.  */
 }
 
 } // namespace selftest
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index ce25fae..a8197eb 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -764,4 +764,11 @@ extern tree decl_constant_value_for_optimization (tree);
 
 extern vec<tree> incomplete_record_decls;
 
+#if CHECKING_P
+namespace selftest {
+  extern void run_c_tests (void);
+} // namespace selftest
+#endif /* #if CHECKING_P */
+
+
 #endif /* ! GCC_C_TREE_H */
diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
index defcbdc..805319a 100644
--- a/gcc/cp/cp-lang.c
+++ b/gcc/cp/cp-lang.c
@@ -79,6 +79,11 @@ static tree cxx_enum_underlying_base_type (const_tree);
 #undef LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE
 #define LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE cxx_enum_underlying_base_type
 
+#if CHECKING_P
+#undef LANG_HOOKS_RUN_LANG_SELFTESTS
+#define LANG_HOOKS_RUN_LANG_SELFTESTS selftest::run_cp_tests
+#endif /* #if CHECKING_P */
+
 /* Each front end provides its own lang hook initializer.  */
 struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
 
@@ -229,6 +234,25 @@ tree cxx_enum_underlying_base_type (const_tree type)
   return underlying_type;
 }
 
+#if CHECKING_P
+
+namespace selftest {
+
+/* Implementation of LANG_HOOKS_RUN_LANG_SELFTESTS for the C++ frontend.  */
+
+void
+run_cp_tests (void)
+{
+  /* Run selftests shared within the C family.  */
+  c_family_tests ();
+
+  /* Additional C++-specific tests.  */
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
+
 
 #include "gt-cp-cp-lang.h"
 #include "gtype-cp.h"
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index e33bda6..126c24a 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7334,6 +7334,12 @@ extern tree cp_ubsan_maybe_instrument_downcast	(location_t, tree, tree, tree);
 extern tree cp_ubsan_maybe_instrument_cast_to_vbase (location_t, tree, tree);
 extern void cp_ubsan_maybe_initialize_vtbl_ptrs (tree);
 
+#if CHECKING_P
+namespace selftest {
+  extern void run_cp_tests (void);
+} // namespace selftest
+#endif /* #if CHECKING_P */
+
 /* Inline bodies.  */
 
 inline tree
-- 
1.8.5.3

[PATCH 2/2] C++: bulletproof the %H and %I format codes (PR c++/81167)

[David Malcolm]

The %T format code in the C++ frontend gracefully handles being passed
a NULL type, printing nothing (and hence '' for %qT).

In r248698 (template type diff printing) I converted many uses of pairs
of %qT in the C++ FE to %qH and %qI.

PR c++/81167 reports a case where a NULL is passed to one of these %qH,
and it turns out that we now ICE for this case (with a gcc_assert)
whereas previously we printed a '' for the type.

This patch slightly reworks the %H and %I-handling code so that it
gracefully handles NULL, fixing the ICE in that PR.
Whilst I was at it, I also fixed things so that if only one of the
%H/%I codes is present, we do the right thing (i.e. fall back to
the %T behavior).

Much of the patch is work to generalize the helper code for the
selftests in pretty-print.c so that we can write selftests for
cp/error.c to directly exercise the various cases.

Presumably passing NULL to any of %T, %H, or %I is a bug; this
patch is merely defensive coding against that so that we don't crash;
the patch doesn't address the underlying bug (see the PR for more
details).

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu in
combination with the previous patch.

OK for trunk?

gcc/cp/ChangeLog:
	* cp-lang.c (selftest::run_cp_tests): Call
	selftest::run_cp_error_c_tests.
	* cp-tree.h (selftestrun_cp_error_c_tests): New decl.
	* error.c: Include "selftest.h".
	(struct deferred_printed_type): Remove assertion that "type"
	is non-NULL.
	(comparable_template_types_p): Handle NULL types.
	(cxx_format_postprocessor::handle): Likewise.  Also, handle
	one of the %H or %I being missing.
	(selftest::cp_pretty_printer_test): New class.
	(selftest::test_type_printing): New function.
	(selftest::test_cp_printer): New function.
	(selftest::run_cp_error_c_tests): New function.

gcc/ChangeLog:
	* pretty-print.c
	(selftest::pretty_printer_test::pretty_printer_test): New ctor.
	(selftest::pretty_printer_test::~pretty_printer_test): New dtor.
	(selftest::assert_pp_format): Convert to...
	(selftest::pretty_printer_test::assert_pp_format): ...method.
	(selftest::assert_pp_format_colored): Convert to...
	(selftest::pretty_printer_test::assert_pp_format_colored): ...method.
	(selftest::assert_pp_format_va): Convert to...
	(selftest::pretty_printer_test::assert_pp_format_va): ...method,
	and call set_up_printer.
	(ASSERT_PP_FORMAT_1, ASSERT_PP_FORMAT_2, ASSERT_PP_FORMAT_3): Move to
	selftest.h, adding "test." to handle conversion from functions to
	methods.
	(selftest::test_pp_format): Add pretty_printer_test instance,
	moving save/restore of open_quote/close_quote to
	pretty_printer_test's ctor and dtor.  Update for conversion of
	assertions from functions to methods.
	* selftest.h (selftest::pretty_printer_test): New class.
	(ASSERT_PP_FORMAT_1, ASSERT_PP_FORMAT_2, ASSERT_PP_FORMAT_3): Move
	here from pretty-print.c, adding "test." to handle conversion from
	functions to methods.
---
 gcc/cp/cp-lang.c   |   1 +
 gcc/cp/cp-tree.h   |   4 ++
 gcc/cp/error.c     | 147 +++++++++++++++++++++++++++++++++++++++++++++++++----
 gcc/pretty-print.c | 129 +++++++++++++++++++++++-----------------------
 gcc/selftest.h     |  67 ++++++++++++++++++++++++
 5 files changed, 272 insertions(+), 76 deletions(-)

diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
index 805319a..bc7ca63 100644
--- a/gcc/cp/cp-lang.c
+++ b/gcc/cp/cp-lang.c
@@ -247,6 +247,7 @@ run_cp_tests (void)
   c_family_tests ();
 
   /* Additional C++-specific tests.  */
+  run_cp_error_c_tests ();
 }
 
 } // namespace selftest
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 126c24a..8dbdbeb 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7336,6 +7336,10 @@ extern void cp_ubsan_maybe_initialize_vtbl_ptrs (tree);
 
 #if CHECKING_P
 namespace selftest {
+  /* Declarations for specific families of tests in cp, by source file, in
+     alphabetical order.  */
+  extern void run_cp_error_c_tests (void);
+
   extern void run_cp_tests (void);
 } // namespace selftest
 #endif /* #if CHECKING_P */
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index e53afa7..9553f4e 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -30,6 +30,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/c-objc.h"
 #include "ubsan.h"
 #include "internal-fn.h"
+#include "selftest.h"
 
 #define pp_separate_with_comma(PP) pp_cxx_separate_with (PP, ',')
 #define pp_separate_with_semicolon(PP) pp_cxx_separate_with (PP, ';')
@@ -115,7 +116,8 @@ struct deferred_printed_type
   : m_tree (type), m_buffer_ptr (buffer_ptr), m_verbose (verbose),
     m_quote (quote)
   {
-    gcc_assert (type);
+    /* TYPE (and thus m_tree) can be NULL, or, at least, we should
+       gracefully handle this case.  */
     gcc_assert (buffer_ptr);
   }
 
@@ -3611,6 +3613,11 @@ maybe_print_constexpr_context (diagnostic_context *context)
 static bool
 comparable_template_types_p (tree type_a, tree type_b)
 {
+  /* If either is NULL, then treat them as incomparable, so that
+     we can gracefully fall back to type_to_string for them.  */
+  if (type_a == NULL || type_b == NULL)
+    return false;
+
   if (!CLASS_TYPE_P (type_a))
     return false;
   if (!CLASS_TYPE_P (type_b))
@@ -3892,8 +3899,9 @@ void
 cxx_format_postprocessor::handle (pretty_printer *pp)
 {
   /* If we have one of %H and %I, the other should have
-     been present.  */
-  if (m_type_a.m_tree || m_type_b.m_tree)
+     been present.  Gracefully handle failure by falling
+     back to type_to_string if only one was present.  */
+  if (m_type_a.m_buffer_ptr || m_type_b.m_buffer_ptr)
     {
       /* Avoid reentrancy issues by working with a copy of
 	 m_type_a and m_type_b, resetting them now.  */
@@ -3902,9 +3910,6 @@ cxx_format_postprocessor::handle (pretty_printer *pp)
       m_type_a = deferred_printed_type ();
       m_type_b = deferred_printed_type ();
 
-      gcc_assert (type_a.m_buffer_ptr);
-      gcc_assert (type_b.m_buffer_ptr);
-
       bool show_color = pp_show_color (pp);
 
       const char *type_a_text;
@@ -3930,19 +3935,22 @@ cxx_format_postprocessor::handle (pretty_printer *pp)
 	}
       else
 	{
-	  /* If the types were not comparable, they are printed normally,
-	     and no difference tree is printed.  */
+	  /* If the types were not comparable (or if only one of %H/%I was
+	     provided), they are printed normally, and no difference tree
+	     is printed.  */
 	  type_a_text = type_to_string (type_a.m_tree, type_a.m_verbose);
 	  type_b_text = type_to_string (type_b.m_tree, type_b.m_verbose);
 	}
 
       if (type_a.m_quote)
 	type_a_text = add_quotes (type_a_text, show_color);
-      *type_a.m_buffer_ptr = type_a_text;
+      if (type_a.m_buffer_ptr)
+	*type_a.m_buffer_ptr = type_a_text;
 
        if (type_b.m_quote)
 	type_b_text = add_quotes (type_b_text, show_color);
-      *type_b.m_buffer_ptr = type_b_text;
+      if (type_b.m_buffer_ptr)
+	*type_b.m_buffer_ptr = type_b_text;
    }
 }
 
@@ -4238,3 +4246,122 @@ qualified_name_lookup_error (tree scope, tree name,
       suggest_alternatives_for (location, name, true);
     }
 }
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Subclass of pretty_printer_test for testing cp_printer.  */
+
+class cp_pretty_printer_test : public pretty_printer_test
+{
+ public:
+  void set_up_printer (pretty_printer &pp) FINAL OVERRIDE
+  {
+    pp.format_decoder = cp_printer;
+    pp.m_format_postprocessor = new cxx_format_postprocessor ();
+  }
+};
+
+/* Verify one of the type-printing codes: %T, %H and %I (CODE),
+   reusing the fixture TEST. */
+
+static void
+test_type_printing (cp_pretty_printer_test &test, char code)
+{
+  /* Generate "%T %x" and %qT %x", where 'T' == code.  */
+  char *unquoted = xasprintf ("%%%c %%x", code);
+  char *quoted = xasprintf ("%%q%c %%x", code);
+
+  /* Ensure they can print a type.  */
+  ASSERT_PP_FORMAT_2 ("int 12345678", unquoted, integer_type_node, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("`int' 12345678", quoted, integer_type_node, 0x12345678);
+
+  /* Ensure they gracefully handle NULL.  */
+  ASSERT_PP_FORMAT_2 (" 12345678", unquoted, NULL, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("`' 12345678", quoted, NULL, 0x12345678);
+
+  free (unquoted);
+  free (quoted);
+}
+
+/* Verify the behavior of cp_printer.  */
+
+static void
+test_cp_printer (void)
+{
+  cp_pretty_printer_test test;
+
+  /* Verify various individual format codes, in the order listed in the
+     comment for cp_printer.  For each code, we append a second
+     argument with a known bit pattern (0x12345678), to ensure that we
+     are consuming arguments correctly.  */
+
+  // TODO: %A   function argument-list.
+  // TODO: %C	tree code.
+  // TODO: %D   declaration.
+  // TODO: %E   expression.
+  // TODO: %F   function declaration.
+  // TODO: %L	language as used in extern "lang".
+  // TODO: %O	binary operator.
+  // TODO: %P   function parameter whose position is indicated by an integer.
+  // TODO: %Q	assignment operator.
+  // TODO: %S   substitution (template + args)
+
+  /* Verify %T and %qT.  */
+  test_type_printing (test, 'T');
+
+  // TODO: %V   cv-qualifier.
+  // TODO: %X   exception-specification.
+
+  /* Verify %H and %I type difference (from/to).  */
+  /* Test of %H/%I by themselves (they should act like %T).  */
+  test_type_printing (test, 'H');
+  test_type_printing (test, 'I');
+  /* Test of combinations of %H and %I: unquoted and quoted, in either
+     order, with non-NULL/NULL types.  */
+  ASSERT_PP_FORMAT_3 ("int int 12345678", "%H %I %x",
+		      integer_type_node, integer_type_node, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("`int' `int' 12345678", "%qH %qI %x",
+		      integer_type_node, integer_type_node, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("int int 12345678", "%I %H %x",
+		      integer_type_node, integer_type_node, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("`int' `int' 12345678", "%qI %qH %x",
+		      integer_type_node, integer_type_node, 0x12345678);
+  ASSERT_PP_FORMAT_3 (" int 12345678", "%H %I %x",
+		      NULL, integer_type_node, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("`' `int' 12345678", "%qH %qI %x",
+		      NULL, integer_type_node, 0x12345678);
+  ASSERT_PP_FORMAT_3 (" int 12345678", "%I %H %x",
+		      NULL, integer_type_node, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("`' `int' 12345678", "%qI %qH %x",
+		      NULL, integer_type_node, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("int  12345678", "%H %I %x",
+		      integer_type_node, NULL, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("`int' `' 12345678", "%qH %qI %x",
+		      integer_type_node, NULL, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("int  12345678", "%I %H %x",
+		      integer_type_node, NULL, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("`int' `' 12345678", "%qI %qH %x",
+		      integer_type_node, NULL, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("  12345678", "%H %I %x",
+		      NULL, NULL, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("`' `' 12345678", "%qH %qI %x",
+		      NULL, NULL, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("  12345678", "%I %H %x",
+		      NULL, NULL, 0x12345678);
+  ASSERT_PP_FORMAT_3 ("`' `' 12345678", "%qI %qH %x",
+		      NULL, NULL, 0x12345678);
+}
+
+/* Run all of the selftests within this file.  */
+
+void
+run_cp_error_c_tests (void)
+{
+  test_cp_printer ();
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 570dec7..89c5cd1 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -1302,36 +1302,34 @@ test_basic_printing ()
   ASSERT_STREQ ("hello world", pp_formatted_text (&pp));
 }
 
-/* Helper function for testing pp_format.
-   Verify that pp_format (FMT, ...) followed by pp_output_formatted_text
-   prints EXPECTED, assuming that pp_show_color is SHOW_COLOR.  */
+/* class selftest::pretty_printer_test */
 
-static void
-assert_pp_format_va (const location &loc, const char *expected,
-		     bool show_color, const char *fmt, va_list *ap)
+/* Save the current values of open_quote and close_quote, and set them to
+   locale-independent values.  */
+
+pretty_printer_test::pretty_printer_test ()
+: old_open_quote (open_quote),
+  old_close_quote (close_quote)
 {
-  pretty_printer pp;
-  text_info ti;
-  rich_location rich_loc (line_table, UNKNOWN_LOCATION);
+  open_quote = "`";
+  close_quote = "'";
+}
 
-  ti.format_spec = fmt;
-  ti.args_ptr = ap;
-  ti.err_no = 0;
-  ti.x_data = NULL;
-  ti.m_richloc = &rich_loc;
+/* Restore the saved values of open_quote and close_quote.  */
 
-  pp_show_color (&pp) = show_color;
-  pp_format (&pp, &ti);
-  pp_output_formatted_text (&pp);
-  ASSERT_STREQ_AT (loc, expected, pp_formatted_text (&pp));
+pretty_printer_test::~pretty_printer_test ()
+{
+  open_quote = old_open_quote;
+  close_quote = old_close_quote;
 }
 
 /* Verify that pp_format (FMT, ...) followed by pp_output_formatted_text
    prints EXPECTED, with show_color disabled.  */
 
-static void
-assert_pp_format (const location &loc, const char *expected,
-		  const char *fmt, ...)
+void
+pretty_printer_test::assert_pp_format (const location &loc,
+				       const char *expected,
+				       const char *fmt, ...)
 {
   va_list ap;
 
@@ -1342,9 +1340,10 @@ assert_pp_format (const location &loc, const char *expected,
 
 /* As above, but with colorization enabled.  */
 
-static void
-assert_pp_format_colored (const location &loc, const char *expected,
-			  const char *fmt, ...)
+void
+pretty_printer_test::assert_pp_format_colored (const location &loc,
+					       const char *expected,
+					       const char *fmt, ...)
 {
   /* The tests of colorization assume the default color scheme.
      If GCC_COLORS is set, then the colors have potentially been
@@ -1359,28 +1358,33 @@ assert_pp_format_colored (const location &loc, const char *expected,
   va_end (ap);
 }
 
-/* Helper function for calling testing pp_format,
-   by calling assert_pp_format with various numbers of arguments.
-   These exist mostly to avoid having to write SELFTEST_LOCATION
-   throughout test_pp_format.  */
-
-#define ASSERT_PP_FORMAT_1(EXPECTED, FMT, ARG1)		      \
-  SELFTEST_BEGIN_STMT					      \
-    assert_pp_format ((SELFTEST_LOCATION), (EXPECTED), (FMT), \
-		      (ARG1));				      \
-  SELFTEST_END_STMT
-
-#define ASSERT_PP_FORMAT_2(EXPECTED, FMT, ARG1, ARG2)	      \
-  SELFTEST_BEGIN_STMT					      \
-    assert_pp_format ((SELFTEST_LOCATION), (EXPECTED), (FMT), \
-		      (ARG1), (ARG2));			      \
-  SELFTEST_END_STMT
-
-#define ASSERT_PP_FORMAT_3(EXPECTED, FMT, ARG1, ARG2, ARG3)   \
-  SELFTEST_BEGIN_STMT					      \
-    assert_pp_format ((SELFTEST_LOCATION), (EXPECTED), (FMT), \
-                      (ARG1), (ARG2), (ARG3));		      \
-  SELFTEST_END_STMT
+/* Helper function for testing pp_format.
+   Verify that pp_format (FMT, ...) followed by pp_output_formatted_text
+   prints EXPECTED, assuming that pp_show_color is SHOW_COLOR.  */
+
+void
+pretty_printer_test::assert_pp_format_va (const location &loc,
+					  const char *expected,
+					  bool show_color, const char *fmt,
+					  va_list *ap)
+{
+  pretty_printer pp;
+
+  text_info ti;
+  rich_location rich_loc (line_table, UNKNOWN_LOCATION);
+
+  ti.format_spec = fmt;
+  ti.args_ptr = ap;
+  ti.err_no = 0;
+  ti.x_data = NULL;
+  ti.m_richloc = &rich_loc;
+
+  set_up_printer (pp);
+  pp_show_color (&pp) = show_color;
+  pp_format (&pp, &ti);
+  pp_output_formatted_text (&pp);
+  ASSERT_STREQ_AT (loc, expected, pp_formatted_text (&pp));
+}
 
 /* Verify that pp_format works, for various format codes.  */
 
@@ -1389,13 +1393,10 @@ test_pp_format ()
 {
   /* Avoid introducing locale-specific differences in the results
      by hardcoding open_quote and close_quote.  */
-  const char *old_open_quote = open_quote;
-  const char *old_close_quote = close_quote;
-  open_quote = "`";
-  close_quote = "'";
+  pretty_printer_test test;
 
   /* Verify that plain text is passed through unchanged.  */
-  assert_pp_format (SELFTEST_LOCATION, "unformatted", "unformatted");
+  test.assert_pp_format (SELFTEST_LOCATION, "unformatted", "unformatted");
 
   /* Verify various individual format codes, in the order listed in the
      comment for pp_format above.  For each code, we append a second
@@ -1433,7 +1434,7 @@ test_pp_format ()
   ASSERT_PP_FORMAT_2 ("normal colored normal 12345678",
 		      "normal %rcolored%R normal %x",
 		      "error", 0x12345678);
-  assert_pp_format_colored
+  test.assert_pp_format_colored
     (SELFTEST_LOCATION,
      "normal \33[01;31m\33[Kcolored\33[m\33[K normal 12345678",
      "normal %rcolored%R normal %x", "error", 0x12345678);
@@ -1448,9 +1449,9 @@ test_pp_format ()
 
   /* Verify flag 'q'.  */
   ASSERT_PP_FORMAT_2 ("`foo' 12345678", "%qs %x", "foo", 0x12345678);
-  assert_pp_format_colored (SELFTEST_LOCATION,
-			    "`\33[01m\33[Kfoo\33[m\33[K' 12345678", "%qs %x",
-			    "foo", 0x12345678);
+  test.assert_pp_format_colored (SELFTEST_LOCATION,
+				 "`\33[01m\33[Kfoo\33[m\33[K' 12345678", "%qs %x",
+				 "foo", 0x12345678);
 
   /* Verify %Z.  */
   int v[] = { 1, 2, 3 }; 
@@ -1460,17 +1461,13 @@ test_pp_format ()
   ASSERT_PP_FORMAT_3 ("0 12345678", "%Z %x", v2, 1, 0x12345678);
 
   /* Verify that combinations work, along with unformatted text.  */
-  assert_pp_format (SELFTEST_LOCATION,
-		    "the quick brown fox jumps over the lazy dog",
-		    "the %s %s %s jumps over the %s %s",
-		    "quick", "brown", "fox", "lazy", "dog");
-  assert_pp_format (SELFTEST_LOCATION, "item 3 of 7", "item %i of %i", 3, 7);
-  assert_pp_format (SELFTEST_LOCATION, "problem with `bar' at line 10",
-		    "problem with %qs at line %i", "bar", 10);
-
-  /* Restore old values of open_quote and close_quote.  */
-  open_quote = old_open_quote;
-  close_quote = old_close_quote;
+  test.assert_pp_format (SELFTEST_LOCATION,
+			 "the quick brown fox jumps over the lazy dog",
+			 "the %s %s %s jumps over the %s %s",
+			 "quick", "brown", "fox", "lazy", "dog");
+  ASSERT_PP_FORMAT_2 ("item 3 of 7", "item %i of %i", 3, 7);
+  ASSERT_PP_FORMAT_2 ("problem with `bar' at line 10",
+		      "problem with %qs at line %i", "bar", 10);
 }
 
 /* Run all of the selftests within this file.  */
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 0572fef..96eccac 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -322,6 +322,73 @@ extern int num_passes;
 #define SELFTEST_BEGIN_STMT do {
 #define SELFTEST_END_STMT   } while (0)
 
+
+/* pretty-print.c.  */
+
+namespace selftest {
+
+/* Fixture for testing pretty-printing.
+   Avoids locale-specific differences in the results of prettyprinting tests
+   by temporarily overriding open_quote and close_quote (via ctor/dtor).  */
+
+class pretty_printer_test
+{
+ public:
+  pretty_printer_test ();
+  ~pretty_printer_test ();
+
+  /* Hook for allowing subclasses to manipulate the underlying printer.  */
+  virtual void set_up_printer (pretty_printer &) {}
+
+  /* Verify that pp_format (FMT, ...) followed by pp_output_formatted_text
+     prints EXPECTED, with show_color disabled.  */
+
+  void assert_pp_format (const location &loc, const char *expected,
+			 const char *fmt, ...);
+
+  /* As above, but with colorization enabled.  */
+
+  void assert_pp_format_colored (const location &loc, const char *expected,
+				 const char *fmt, ...);
+
+  /* Helper function for the above.  */
+
+  void assert_pp_format_va (const location &loc, const char *expected,
+			    bool show_color, const char *fmt,
+			    va_list *ap);
+
+ private:
+  const char *old_open_quote;
+  const char *old_close_quote;
+};
+
+} // namespace selftest
+
+/* Helper function for calling testing pp_format, assuming an instance
+   of pretty_printer_test named "test",  by calling test.assert_pp_format
+   with various numbers of arguments.
+   These exist mostly to avoid having to write SELFTEST_LOCATION
+   and "test" throughout test cases.  */
+
+#define ASSERT_PP_FORMAT_1(EXPECTED, FMT, ARG1)		      \
+  SELFTEST_BEGIN_STMT					      \
+    test.assert_pp_format ((SELFTEST_LOCATION), (EXPECTED), (FMT), \
+			   (ARG1));				   \
+  SELFTEST_END_STMT
+
+#define ASSERT_PP_FORMAT_2(EXPECTED, FMT, ARG1, ARG2)	      \
+  SELFTEST_BEGIN_STMT					      \
+    test.assert_pp_format ((SELFTEST_LOCATION), (EXPECTED), (FMT), \
+			   (ARG1), (ARG2));			   \
+  SELFTEST_END_STMT
+
+#define ASSERT_PP_FORMAT_3(EXPECTED, FMT, ARG1, ARG2, ARG3)   \
+  SELFTEST_BEGIN_STMT					      \
+    test.assert_pp_format ((SELFTEST_LOCATION), (EXPECTED), (FMT), \
+			   (ARG1), (ARG2), (ARG3));		   \
+  SELFTEST_END_STMT
+
+
 #endif /* #if CHECKING_P */
 
 #endif /* GCC_SELFTEST_H */
-- 
1.8.5.3

Re: [PATCH 2/2] C++: bulletproof the %H and %I format codes (PR c++/81167)

[Nathan Sidwell]

On 06/22/2017 08:20 PM, David Malcolm wrote:

> PR c++/81167 reports a case where a NULL is passed to one of these %qH,
> and it turns out that we now ICE for this case (with a gcc_assert)
> whereas previously we printed a '' for the type >
> This patch slightly reworks the %H and %I-handling code so that it
> gracefully handles NULL, fixing the ICE in that PR.
> Whilst I was at it, I also fixed things so that if only one of the
> %H/%I codes is present, we do the right thing (i.e. fall back to
> the %T behavior).

I dislike this kind of silently accepting bogus use of an interface.  I 
much prefer the compiler to explode.

At least make it barf with a checking build, even if a production build 
permits the abuse.

nathan

-- 
Nathan Sidwell

Re: [PATCH 1/2] Support C++-specific selftests

[Jeff Law]

On 06/22/2017 06:20 PM, David Malcolm wrote:
> Currently the "make selftest" target run during each stage of the
> build just runs the selftests within cc1.
> 
> As part of the fix for PR c++/81167 I want to be able to write
> C++-specific selftests (to exercise the C++ implementation of
> the pp's format_decoder).
> 
> Hence this patch generalizes the existing selftest logic in
> Makefile.in so that selftests are run for both cc1 and cc1plus.
> It also adds convenience methods for running them under
> gdb and under valgrind.
> 
> It also reorganizes the existing lang-specific selftests, splitting
> them into ones shared by c-family, and those that are specific
> to individual members of the c family.  There aren't any of the
> latter yet, but the followup adds some for C++.
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu in
> combination with the followup patch; the number of reported passes
> from -fself-test for cc1 at each stage was unaffected by the kit:
>   -fself-test: 39233 pass(es),
> and cc1plus gained a similar line of output.
> 
> OK for trunk?
> 
> gcc/ChangeLog:
> 	* Makefile.in (SELFTEST_FLAGS): Drop "-x c", moving it to...
> 	(C_SELFTEST_FLAGS): New.
> 	(CPP_SELFTEST_FLAGS): New.
> 	(SELFTEST_DEPS): New, from deps of s-selftest.
> 	(C_SELFTEST_DEPS): New, from deps of s-selftest.
> 	(CPP_SELFTEST_DEPS): New.
> 	(selftest): Add dependency on s-selftest-c++.
> 	(s-selftest): Rename to...
> 	(s-selftest-c): ...this, moving deps to SELFTEST_DEPS
> 	and C_SELFTEST_DEPS, and using C_SELFTEST_FLAGS rather
> 	than SELFTEST_FLAGS.
> 	(selftest-gdb): Rename to...
> 	(selftest-c-gdb): ...this, using C_SELFTEST_DEPS and
> 	C_SELFTEST_FLAGS.
> 	(selftest-gdb): Reintroduce as an alias for selftest-c-gdb.
> 	(selftest-valgrind): Rename to...
> 	(selftest-c-valgrind): ...this, using C_SELFTEST_DEPS and
> 	C_SELFTEST_FLAGS.
> 	(selftest-valgrind): Reintroduce as an alias for
> 	selftest-c-valgrind.
> 	(s-selftest-c++): New.
> 	(selftest-c++-gdb): New.
> 	(selftest-c++-valgrind): New.
> 
> gcc/c-family/ChangeLog:
> 	* c-common.c (selftest::c_family_tests): New.
> 	* c-common.h (selftest::run_c_tests): Move decl to c/c-lang.h.
> 	(selftest::c_family_tests): New decl.
> 
> gcc/c/ChangeLog:
> 	* c-lang.c (selftest::run_c_tests): Move body to c_family_tests,
> 	and call that instead.
> 	* c-tree.h (selftest::run_c_tests): New decl.
> 
> gcc/cp/ChangeLog:
> 	* cp-lang.c (LANG_HOOKS_RUN_LANG_SELFTESTS): Define as
> 	selftest::run_cp_tests.
> 	(selftest::run_cp_tests): New function.
> 	* cp-tree.h (selftest::run_cp_tests): New decl.
OK.
jeff