public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/3] selftest: show values when ASSERT_STREQ fails
  2016-06-09 18:15 [PATCH 0/3] selftest improvements David Malcolm
  2016-06-09 18:15 ` [PATCH 3/3] pretty-print.c: skip color selftests if GCC_COLORS is set David Malcolm
@ 2016-06-09 18:15 ` David Malcolm
  2016-06-13 18:20   ` Jeff Law
  2016-06-09 18:15 ` [PATCH 2/3] selftests: improve reported failure locations David Malcolm
  2 siblings, 1 reply; 7+ messages in thread
From: David Malcolm @ 2016-06-09 18:15 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

Rework ASSERT_STREQ so that it prints the actual and expected values
to stderr when it fails (by moving it to a helper function).

gcc/ChangeLog:
	* selftest.c (selftest::fail_formatted): New function.
	(selftest::assert_streq): New function.
	* selftest.h (selftests::fail_formatted): New decl.
	(selftest::assert_streq): New decl.
	(ASSERT_STREQ): Reimplement in terms of selftest::assert_streq.
---
 gcc/selftest.c | 32 ++++++++++++++++++++++++++++++++
 gcc/selftest.h | 24 +++++++++++++++---------
 2 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/gcc/selftest.c b/gcc/selftest.c
index de804df..e5332db 100644
--- a/gcc/selftest.c
+++ b/gcc/selftest.c
@@ -44,4 +44,36 @@ selftest::fail (const char *file, int line, const char *msg)
   abort ();
 }
 
+/* As "fail", but using printf-style formatted output.  */
+
+void
+selftest::fail_formatted (const char *file, int line, const char *fmt, ...)
+{
+  va_list ap;
+
+  fprintf (stderr, "%s:%i: FAIL: ", file, line);
+  /* TODO: add calling function name as well?  */
+  va_start (ap, fmt);
+  vfprintf (stderr, fmt, ap);
+  va_end (ap);
+  fprintf (stderr, "\n");
+  abort ();
+}
+
+/* Implementation detail of ASSERT_STREQ.  */
+
+void
+selftest::assert_streq (const char *file, int line,
+			const char *desc_expected, const char *desc_actual,
+			const char *val_expected, const char *val_actual)
+{
+  if (0 == strcmp (val_expected, val_actual))
+    ::selftest::pass (file, line, "ASSERT_STREQ");
+  else
+    ::selftest::fail_formatted
+	(file, line, "ASSERT_STREQ (%s, %s) expected=\"%s\" actual=\"%s\"",
+	 desc_expected, desc_actual, val_expected, val_actual);
+}
+
+
 #endif /* #if CHECKING_P */
diff --git a/gcc/selftest.h b/gcc/selftest.h
index d1f8acc..6759734 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -39,6 +39,17 @@ extern void pass (const char *file, int line, const char *msg);
 
 extern void fail (const char *file, int line, const char *msg);
 
+/* As "fail", but using printf-style formatted output.  */
+
+extern void fail_formatted (const char *file, int line, const char *fmt, ...)
+ ATTRIBUTE_PRINTF_3;
+
+/* Implementation detail of ASSERT_STREQ.  */
+
+extern void assert_streq (const char *file, int line,
+			  const char *desc_expected, const char *desc_actual,
+			  const char *val_expected, const char *val_actual);
+
 /* Declarations for specific families of tests (by source file), in
    alphabetical order.  */
 extern void bitmap_c_tests ();
@@ -123,15 +134,10 @@ extern int num_passes;
    ::selftest::pass if they are equal,
    ::selftest::fail if they are non-equal.  */
 
-#define ASSERT_STREQ(EXPECTED, ACTUAL)			       \
-  SELFTEST_BEGIN_STMT					       \
-  const char *desc = "ASSERT_STREQ (" #EXPECTED ", " #ACTUAL ")"; \
-  const char *expected_ = (EXPECTED);				  \
-  const char *actual_ = (ACTUAL);				  \
-  if (0 == strcmp (expected_, actual_))				  \
-    ::selftest::pass (__FILE__, __LINE__, desc);			       \
-  else							       \
-    ::selftest::fail (__FILE__, __LINE__, desc);			       \
+#define ASSERT_STREQ(EXPECTED, ACTUAL)				    \
+  SELFTEST_BEGIN_STMT						    \
+  ::selftest::assert_streq (__FILE__, __LINE__, #EXPECTED, #ACTUAL, \
+			    (EXPECTED), (ACTUAL));		    \
   SELFTEST_END_STMT
 
 /* Evaluate PRED1 (VAL1), calling ::selftest::pass if it is true,
-- 
1.8.5.3

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

* [PATCH 3/3] pretty-print.c: skip color selftests if GCC_COLORS is set
  2016-06-09 18:15 [PATCH 0/3] selftest improvements David Malcolm
@ 2016-06-09 18:15 ` David Malcolm
  2016-06-13 18:20   ` Jeff Law
  2016-06-09 18:15 ` [PATCH 1/3] selftest: show values when ASSERT_STREQ fails David Malcolm
  2016-06-09 18:15 ` [PATCH 2/3] selftests: improve reported failure locations David Malcolm
  2 siblings, 1 reply; 7+ messages in thread
From: David Malcolm @ 2016-06-09 18:15 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

gcc/ChangeLog:
	* pretty-print.c (assert_pp_format_colored): Skip the test if
	GCC_COLORS is set.
	(test_pp_format): Remove comment about GCC_COLORS.
---
 gcc/pretty-print.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 86ae3a5..3713953 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -1266,6 +1266,12 @@ static void
 assert_pp_format_colored (const location &loc, const char *expected,
 			  const char *fmt, ...)
 {
+  /* The tests of colorization assume that the default color scheme.
+     If GCC_COLORS is set, then the colors have potentially been
+     overridden; skip the test.  */
+  if (getenv ("GCC_COLORS"))
+    return;
+
   va_list ap;
 
   va_start (ap, fmt);
@@ -1347,7 +1353,6 @@ test_pp_format ()
   ASSERT_PP_FORMAT_2 ("normal colored normal 12345678",
 		      "normal %rcolored%R normal %x",
 		      "error", 0x12345678);
-  /* The following assumes an empty value for GCC_COLORS.  */
   assert_pp_format_colored
     (SELFTEST_LOCATION,
      "normal \33[01;31m\33[Kcolored\33[m\33[K normal 12345678",
-- 
1.8.5.3

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

* [PATCH 0/3] selftest improvements
@ 2016-06-09 18:15 David Malcolm
  2016-06-09 18:15 ` [PATCH 3/3] pretty-print.c: skip color selftests if GCC_COLORS is set David Malcolm
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: David Malcolm @ 2016-06-09 18:15 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

PR bootstrap/71471 highlighted the need for selftests to provide more
information when they fail.  The report contained:
  src/gcc/pretty-print.c:1246: FAIL: ASSERT_STREQ (expected, pp_formatted_text (&pp))

This showed a string-equality failure within the helper function
assert_pp_format_va, but didn't show which actual comparison was
failing.

Patch 1 updates ASSERT_STREQ so that the output includes the actual
and expected strings, which would have allowed us to identify the
failing test by searching the source.

Patch 2 introduces a way to pass location information to helper
functions so that the file/line that's printed for a failure points to
the specific case, rather than to the inside of the helper function,
and uses it within pretty-print.c's selftests.  This would have
allowed us to directly identify the failing test.

Patch 3 fixes a bug that turned out not to be the cause of the PR.

I've tested the combination of these patches:
  * successful bootstrap&regrtest on x86_64-pc-linux-gnu
  * successful build/-fselftest of stage 1 on powerpc-ibm-aix7.1.3.0

OK for trunk?

David Malcolm (3):
  selftest: show values when ASSERT_STREQ fails
  selftests: improve reported failure locations
  pretty-print.c: skip color selftests if GCC_COLORS is set

 gcc/input.c        |   4 +-
 gcc/pretty-print.c | 135 +++++++++++++++++++++++++++++++++--------------------
 gcc/selftest.c     |  40 ++++++++++++++--
 gcc/selftest.h     |  76 +++++++++++++++++++++---------
 4 files changed, 177 insertions(+), 78 deletions(-)

-- 
1.8.5.3

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

* [PATCH 2/3] selftests: improve reported failure locations
  2016-06-09 18:15 [PATCH 0/3] selftest improvements David Malcolm
  2016-06-09 18:15 ` [PATCH 3/3] pretty-print.c: skip color selftests if GCC_COLORS is set David Malcolm
  2016-06-09 18:15 ` [PATCH 1/3] selftest: show values when ASSERT_STREQ fails David Malcolm
@ 2016-06-09 18:15 ` David Malcolm
  2016-06-13 18:21   ` Jeff Law
  2 siblings, 1 reply; 7+ messages in thread
From: David Malcolm @ 2016-06-09 18:15 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

This patch introduce a selftest::location struct to wrap up __FILE__
and __LINE__ information (and __FUNCTION__) throughout the selftests,
allowing location information to be passed around.

It updates the helper functions in pretty-print.c to pass through
the precise location of each test, so that if a failure occurs, the
correct line number is printed, rather than a line within a helper
function.

gcc/ChangeLog:
	* input.c (test_reading_source_line): Use SELFTEST_LOCATION.
	* pretty-print.c (assert_pp_format_va): Add location param and use
	it with ASSERT_STREQ_AT.
	(assert_pp_format): Add location param and pass it to
	assert_pp_format_va.
	(assert_pp_format_colored): Likewise.
	(ASSERT_PP_FORMAT_1): New.
	(ASSERT_PP_FORMAT_2): New.
	(ASSERT_PP_FORMAT_3): New.
	(test_pp_format): Provide SELFTEST_LOCATION throughout, either
	explicitly, or implicitly via the above macros.
	* selftest.c (selftest::pass): Use a selftest::location rather
	than file and line.
	(selftest::fail): Likewise.  Print the function name.
	(selftest::fail_formatted): Likewise.
	(selftest::assert_streq): Use a selftest::location rather than
	file and line.
	* selftest.h (selftest::location): New struct.
	(SELFTEST_LOCATION): New macro.
	(selftest::pass): Accept a const location & rather than file
	and line.
	(selftest::fail): Likewise.
	(selftest::fail_formatted): Likewise.
	(selftest::assert_streq): Likewise.
	(ASSERT_TRUE): Update for above changes, using SELFTEST_LOCATION.
	(ASSERT_FALSE): Likewise.
	(ASSERT_EQ): Likewise.
	(ASSERT_NE): Likewise.
	(ASSERT_STREQ): Likewise.
	(ASSERT_PRED1): Likewise.
	(ASSERT_STREQ_AT): New macro.
---
 gcc/input.c        |   4 +-
 gcc/pretty-print.c | 128 ++++++++++++++++++++++++++++++++---------------------
 gcc/selftest.c     |  20 ++++-----
 gcc/selftest.h     |  60 ++++++++++++++++++-------
 4 files changed, 134 insertions(+), 78 deletions(-)

diff --git a/gcc/input.c b/gcc/input.c
index 0b340a8..704ee75 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -1231,10 +1231,10 @@ test_reading_source_line ()
   ASSERT_EQ (53, line_size);
   if (!strncmp ("     The quick brown fox jumps over the lazy dog.  */",
 	       source_line, line_size))
-    ::selftest::pass (__FILE__, __LINE__,
+    ::selftest::pass (SELFTEST_LOCATION,
 		      "source_line matched expected value");
   else
-    ::selftest::fail (__FILE__, __LINE__,
+    ::selftest::fail (SELFTEST_LOCATION,
 		      "source_line did not match expected value");
 }
 
diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 8febda0..86ae3a5 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -1227,8 +1227,8 @@ test_basic_printing ()
    prints EXPECTED, assuming that pp_show_color is SHOW_COLOR.  */
 
 static void
-assert_pp_format_va (const char *expected, bool show_color, const char *fmt,
-		     va_list *ap)
+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;
@@ -1243,34 +1243,59 @@ assert_pp_format_va (const char *expected, bool show_color, const char *fmt,
   pp_show_color (&pp) = show_color;
   pp_format (&pp, &ti);
   pp_output_formatted_text (&pp);
-  ASSERT_STREQ (expected, pp_formatted_text (&pp));
+  ASSERT_STREQ_AT (loc, expected, pp_formatted_text (&pp));
 }
 
 /* Verify that pp_format (FMT, ...) followed by pp_output_formatted_text
    prints EXPECTED, with show_color disabled.  */
 
 static void
-assert_pp_format (const char *expected, const char *fmt, ...)
+assert_pp_format (const location &loc, const char *expected,
+		  const char *fmt, ...)
 {
   va_list ap;
 
   va_start (ap, fmt);
-  assert_pp_format_va (expected, false, fmt, &ap);
+  assert_pp_format_va (loc, expected, false, fmt, &ap);
   va_end (ap);
 }
 
 /* As above, but with colorization enabled.  */
 
 static void
-assert_pp_format_colored (const char *expected, const char *fmt, ...)
+assert_pp_format_colored (const location &loc, const char *expected,
+			  const char *fmt, ...)
 {
   va_list ap;
 
   va_start (ap, fmt);
-  assert_pp_format_va (expected, true, fmt, &ap);
+  assert_pp_format_va (loc, expected, true, fmt, &ap);
   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
+
 /* Verify that pp_format works, for various format codes.  */
 
 static void
@@ -1284,68 +1309,71 @@ test_pp_format ()
   close_quote = "'";
 
   /* Verify that plain text is passed through unchanged.  */
-  assert_pp_format ("unformatted", "unformatted");
+  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
      argument with a known bit pattern (0x12345678), to ensure that we
      are consuming arguments correctly.  */
-  assert_pp_format ("-27 12345678", "%d %x", -27, 0x12345678);
-  assert_pp_format ("-5 12345678", "%i %x", -5, 0x12345678);
-  assert_pp_format ("10 12345678", "%u %x", 10, 0x12345678);
-  assert_pp_format ("17 12345678", "%o %x", 15, 0x12345678);
-  assert_pp_format ("cafebabe 12345678", "%x %x", 0xcafebabe, 0x12345678);
-  assert_pp_format ("-27 12345678", "%ld %x", (long)-27, 0x12345678);
-  assert_pp_format ("-5 12345678", "%li %x", (long)-5, 0x12345678);
-  assert_pp_format ("10 12345678", "%lu %x", (long)10, 0x12345678);
-  assert_pp_format ("17 12345678", "%lo %x", (long)15, 0x12345678);
-  assert_pp_format ("cafebabe 12345678", "%lx %x", (long)0xcafebabe,
-		    0x12345678);
-  assert_pp_format ("-27 12345678", "%lld %x", (long long)-27, 0x12345678);
-  assert_pp_format ("-5 12345678", "%lli %x", (long long)-5, 0x12345678);
-  assert_pp_format ("10 12345678", "%llu %x", (long long)10, 0x12345678);
-  assert_pp_format ("17 12345678", "%llo %x", (long long)15, 0x12345678);
-  assert_pp_format ("cafebabe 12345678", "%llx %x", (long long)0xcafebabe,
-		    0x12345678);
-  assert_pp_format ("-27 12345678", "%wd %x", (HOST_WIDE_INT)-27, 0x12345678);
-  assert_pp_format ("-5 12345678", "%wi %x", (HOST_WIDE_INT)-5, 0x12345678);
-  assert_pp_format ("10 12345678", "%wu %x", (unsigned HOST_WIDE_INT)10,
-		    0x12345678);
-  assert_pp_format ("17 12345678", "%wo %x", (HOST_WIDE_INT)15, 0x12345678);
-  assert_pp_format ("0xcafebabe 12345678", "%wx %x", (HOST_WIDE_INT)0xcafebabe,
-		    0x12345678);
-  assert_pp_format ("A 12345678", "%c %x", 'A', 0x12345678);
-  assert_pp_format ("hello world 12345678", "%s %x", "hello world",
-		    0x12345678);
+  ASSERT_PP_FORMAT_2 ("-27 12345678", "%d %x", -27, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("-5 12345678", "%i %x", -5, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("10 12345678", "%u %x", 10, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("17 12345678", "%o %x", 15, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("cafebabe 12345678", "%x %x", 0xcafebabe, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("-27 12345678", "%ld %x", (long)-27, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("-5 12345678", "%li %x", (long)-5, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("10 12345678", "%lu %x", (long)10, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("17 12345678", "%lo %x", (long)15, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("cafebabe 12345678", "%lx %x", (long)0xcafebabe,
+		      0x12345678);
+  ASSERT_PP_FORMAT_2 ("-27 12345678", "%lld %x", (long long)-27, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("-5 12345678", "%lli %x", (long long)-5, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("10 12345678", "%llu %x", (long long)10, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("17 12345678", "%llo %x", (long long)15, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("cafebabe 12345678", "%llx %x", (long long)0xcafebabe,
+		      0x12345678);
+  ASSERT_PP_FORMAT_2 ("-27 12345678", "%wd %x", (HOST_WIDE_INT)-27, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("-5 12345678", "%wi %x", (HOST_WIDE_INT)-5, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("10 12345678", "%wu %x", (unsigned HOST_WIDE_INT)10,
+		      0x12345678);
+  ASSERT_PP_FORMAT_2 ("17 12345678", "%wo %x", (HOST_WIDE_INT)15, 0x12345678);
+  ASSERT_PP_FORMAT_2 ("0xcafebabe 12345678", "%wx %x", (HOST_WIDE_INT)0xcafebabe,
+		      0x12345678);
+  ASSERT_PP_FORMAT_2 ("A 12345678", "%c %x", 'A', 0x12345678);
+  ASSERT_PP_FORMAT_2 ("hello world 12345678", "%s %x", "hello world",
+		      0x12345678);
   /* We can't test for %p; the pointer is printed in an implementation-defined
      manner.  */
-  assert_pp_format ("normal colored normal 12345678",
-		    "normal %rcolored%R normal %x",
-		    "error", 0x12345678);
+  ASSERT_PP_FORMAT_2 ("normal colored normal 12345678",
+		      "normal %rcolored%R normal %x",
+		      "error", 0x12345678);
   /* The following assumes an empty value for GCC_COLORS.  */
   assert_pp_format_colored
-    ("normal \33[01;31m\33[Kcolored\33[m\33[K normal 12345678",
+    (SELFTEST_LOCATION,
+     "normal \33[01;31m\33[Kcolored\33[m\33[K normal 12345678",
      "normal %rcolored%R normal %x", "error", 0x12345678);
   /* TODO:
      %m: strerror(text->err_no) - does not consume a value from args_ptr.  */
-  assert_pp_format ("% 12345678", "%% %x", 0x12345678);
-  assert_pp_format ("` 12345678", "%< %x", 0x12345678);
-  assert_pp_format ("' 12345678", "%> %x", 0x12345678);
-  assert_pp_format ("' 12345678", "%' %x", 0x12345678);
-  assert_pp_format ("abc 12345678", "%.*s %x", 3, "abcdef", 0x12345678);
-  assert_pp_format ("abc 12345678", "%.3s %x", "abcdef", 0x12345678);
+  ASSERT_PP_FORMAT_1 ("% 12345678", "%% %x", 0x12345678);
+  ASSERT_PP_FORMAT_1 ("` 12345678", "%< %x", 0x12345678);
+  ASSERT_PP_FORMAT_1 ("' 12345678", "%> %x", 0x12345678);
+  ASSERT_PP_FORMAT_1 ("' 12345678", "%' %x", 0x12345678);
+  ASSERT_PP_FORMAT_3 ("abc 12345678", "%.*s %x", 3, "abcdef", 0x12345678);
+  ASSERT_PP_FORMAT_2 ("abc 12345678", "%.3s %x", "abcdef", 0x12345678);
 
   /* Verify flag 'q'.  */
-  assert_pp_format ("`foo' 12345678", "%qs %x", "foo", 0x12345678);
-  assert_pp_format_colored ("`\33[01m\33[Kfoo\33[m\33[K' 12345678", "%qs %x",
+  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);
 
   /* Verify that combinations work, along with unformatted text.  */
-  assert_pp_format ("the quick brown fox jumps over the lazy dog",
+  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 ("item 3 of 7", "item %i of %i", 3, 7);
-  assert_pp_format ("problem with `bar' at line 10",
+  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.  */
diff --git a/gcc/selftest.c b/gcc/selftest.c
index e5332db..ed6e517 100644
--- a/gcc/selftest.c
+++ b/gcc/selftest.c
@@ -29,7 +29,7 @@ int selftest::num_passes;
 /* Record the successful outcome of some aspect of a test.  */
 
 void
-selftest::pass (const char */*file*/, int /*line*/, const char */*msg*/)
+selftest::pass (const location &/*loc*/, const char */*msg*/)
 {
   num_passes++;
 }
@@ -37,22 +37,22 @@ selftest::pass (const char */*file*/, int /*line*/, const char */*msg*/)
 /* Report the failed outcome of some aspect of a test and abort.  */
 
 void
-selftest::fail (const char *file, int line, const char *msg)
+selftest::fail (const location &loc, const char *msg)
 {
-  fprintf (stderr,"%s:%i: FAIL: %s\n", file, line, msg);
-  /* TODO: add calling function name as well?  */
+  fprintf (stderr,"%s:%i: %s: FAIL: %s\n", loc.m_file, loc.m_line,
+	   loc.m_function, msg);
   abort ();
 }
 
 /* As "fail", but using printf-style formatted output.  */
 
 void
-selftest::fail_formatted (const char *file, int line, const char *fmt, ...)
+selftest::fail_formatted (const location &loc, const char *fmt, ...)
 {
   va_list ap;
 
-  fprintf (stderr, "%s:%i: FAIL: ", file, line);
-  /* TODO: add calling function name as well?  */
+  fprintf (stderr, "%s:%i: %s: FAIL: ", loc.m_file, loc.m_line,
+	   loc.m_function);
   va_start (ap, fmt);
   vfprintf (stderr, fmt, ap);
   va_end (ap);
@@ -63,15 +63,15 @@ selftest::fail_formatted (const char *file, int line, const char *fmt, ...)
 /* Implementation detail of ASSERT_STREQ.  */
 
 void
-selftest::assert_streq (const char *file, int line,
+selftest::assert_streq (const location &loc,
 			const char *desc_expected, const char *desc_actual,
 			const char *val_expected, const char *val_actual)
 {
   if (0 == strcmp (val_expected, val_actual))
-    ::selftest::pass (file, line, "ASSERT_STREQ");
+    ::selftest::pass (loc, "ASSERT_STREQ");
   else
     ::selftest::fail_formatted
-	(file, line, "ASSERT_STREQ (%s, %s) expected=\"%s\" actual=\"%s\"",
+	(loc, "ASSERT_STREQ (%s, %s) expected=\"%s\" actual=\"%s\"",
 	 desc_expected, desc_actual, val_expected, val_actual);
 }
 
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 6759734..e719f5f 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -27,26 +27,45 @@ along with GCC; see the file COPYING3.  If not see
 
 namespace selftest {
 
+/* A struct describing the source-location of a selftest, to make it
+   easier to track down failing tests.  */
+
+struct location
+{
+  location (const char *file, int line, const char *function)
+    : m_file (file), m_line (line), m_function (function) {}
+
+  const char *m_file;
+  int m_line;
+  const char *m_function;
+};
+
+/* A macro for use in selftests and by the ASSERT_ macros below,
+   constructing a selftest::location for the current source location.  */
+
+#define SELFTEST_LOCATION \
+  (::selftest::location (__FILE__, __LINE__, __FUNCTION__))
+
 /* The entrypoint for running all tests.  */
 
 extern void run_tests ();
 
 /* Record the successful outcome of some aspect of the test.  */
 
-extern void pass (const char *file, int line, const char *msg);
+extern void pass (const location &loc, const char *msg);
 
 /* Report the failed outcome of some aspect of the test and abort.  */
 
-extern void fail (const char *file, int line, const char *msg);
+extern void fail (const location &loc, const char *msg);
 
 /* As "fail", but using printf-style formatted output.  */
 
-extern void fail_formatted (const char *file, int line, const char *fmt, ...)
- ATTRIBUTE_PRINTF_3;
+extern void fail_formatted (const location &loc, const char *fmt, ...)
+ ATTRIBUTE_PRINTF_2;
 
 /* Implementation detail of ASSERT_STREQ.  */
 
-extern void assert_streq (const char *file, int line,
+extern void assert_streq (const location &loc,
 			  const char *desc_expected, const char *desc_actual,
 			  const char *val_expected, const char *val_actual);
 
@@ -85,9 +104,9 @@ extern int num_passes;
   const char *desc = "ASSERT_TRUE (" #EXPR ")";		\
   bool actual = (EXPR);					\
   if (actual)						\
-    ::selftest::pass (__FILE__, __LINE__, desc);	\
+    ::selftest::pass (SELFTEST_LOCATION, desc);	\
   else							\
-    ::selftest::fail (__FILE__, __LINE__, desc);		\
+    ::selftest::fail (SELFTEST_LOCATION, desc);		\
   SELFTEST_END_STMT
 
 /* Evaluate EXPR and coerce to bool, calling
@@ -99,9 +118,9 @@ extern int num_passes;
   const char *desc = "ASSERT_FALSE (" #EXPR ")";		\
   bool actual = (EXPR);					\
   if (actual)							\
-    ::selftest::fail (__FILE__, __LINE__, desc);				\
+    ::selftest::fail (SELFTEST_LOCATION, desc);				\
   else								\
-    ::selftest::pass (__FILE__, __LINE__, desc);				\
+    ::selftest::pass (SELFTEST_LOCATION, desc);				\
   SELFTEST_END_STMT
 
 /* Evaluate EXPECTED and ACTUAL and compare them with ==, calling
@@ -112,9 +131,9 @@ extern int num_passes;
   SELFTEST_BEGIN_STMT					       \
   const char *desc = "ASSERT_EQ (" #EXPECTED ", " #ACTUAL ")"; \
   if ((EXPECTED) == (ACTUAL))				       \
-    ::selftest::pass (__FILE__, __LINE__, desc);			       \
+    ::selftest::pass (SELFTEST_LOCATION, desc);			       \
   else							       \
-    ::selftest::fail (__FILE__, __LINE__, desc);			       \
+    ::selftest::fail (SELFTEST_LOCATION, desc);			       \
   SELFTEST_END_STMT
 
 /* Evaluate EXPECTED and ACTUAL and compare them with !=, calling
@@ -125,9 +144,9 @@ extern int num_passes;
   SELFTEST_BEGIN_STMT					       \
   const char *desc = "ASSERT_NE (" #EXPECTED ", " #ACTUAL ")"; \
   if ((EXPECTED) != (ACTUAL))				       \
-    ::selftest::pass (__FILE__, __LINE__, desc);			       \
+    ::selftest::pass (SELFTEST_LOCATION, desc);			       \
   else							       \
-    ::selftest::fail (__FILE__, __LINE__, desc);			       \
+    ::selftest::fail (SELFTEST_LOCATION, desc);			       \
   SELFTEST_END_STMT
 
 /* Evaluate EXPECTED and ACTUAL and compare them with strcmp, calling
@@ -136,7 +155,16 @@ extern int num_passes;
 
 #define ASSERT_STREQ(EXPECTED, ACTUAL)				    \
   SELFTEST_BEGIN_STMT						    \
-  ::selftest::assert_streq (__FILE__, __LINE__, #EXPECTED, #ACTUAL, \
+  ::selftest::assert_streq (SELFTEST_LOCATION, #EXPECTED, #ACTUAL, \
+			    (EXPECTED), (ACTUAL));		    \
+  SELFTEST_END_STMT
+
+/* Like ASSERT_STREQ_AT, but treat LOC as the effective location of the
+   selftest.  */
+
+#define ASSERT_STREQ_AT(LOC, EXPECTED, ACTUAL)			    \
+  SELFTEST_BEGIN_STMT						    \
+  ::selftest::assert_streq ((LOC), #EXPECTED, #ACTUAL,		    \
 			    (EXPECTED), (ACTUAL));		    \
   SELFTEST_END_STMT
 
@@ -148,9 +176,9 @@ extern int num_passes;
   const char *desc = "ASSERT_PRED1 (" #PRED1 ", " #VAL1 ")";	\
   bool actual = (PRED1) (VAL1);				\
   if (actual)						\
-    ::selftest::pass (__FILE__, __LINE__, desc);			\
+    ::selftest::pass (SELFTEST_LOCATION, desc);			\
   else							\
-    ::selftest::fail (__FILE__, __LINE__, desc);			\
+    ::selftest::fail (SELFTEST_LOCATION, desc);			\
   SELFTEST_END_STMT
 
 #define SELFTEST_BEGIN_STMT do {
-- 
1.8.5.3

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

* Re: [PATCH 1/3] selftest: show values when ASSERT_STREQ fails
  2016-06-09 18:15 ` [PATCH 1/3] selftest: show values when ASSERT_STREQ fails David Malcolm
@ 2016-06-13 18:20   ` Jeff Law
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Law @ 2016-06-13 18:20 UTC (permalink / raw)
  To: David Malcolm, gcc-patches

On 06/09/2016 12:42 PM, David Malcolm wrote:
> Rework ASSERT_STREQ so that it prints the actual and expected values
> to stderr when it fails (by moving it to a helper function).
>
> gcc/ChangeLog:
> 	* selftest.c (selftest::fail_formatted): New function.
> 	(selftest::assert_streq): New function.
> 	* selftest.h (selftests::fail_formatted): New decl.
> 	(selftest::assert_streq): New decl.
> 	(ASSERT_STREQ): Reimplement in terms of selftest::assert_streq.
OK.
jeff

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

* Re: [PATCH 3/3] pretty-print.c: skip color selftests if GCC_COLORS is set
  2016-06-09 18:15 ` [PATCH 3/3] pretty-print.c: skip color selftests if GCC_COLORS is set David Malcolm
@ 2016-06-13 18:20   ` Jeff Law
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Law @ 2016-06-13 18:20 UTC (permalink / raw)
  To: David Malcolm, gcc-patches

On 06/09/2016 12:42 PM, David Malcolm wrote:
> gcc/ChangeLog:
> 	* pretty-print.c (assert_pp_format_colored): Skip the test if
> 	GCC_COLORS is set.
> 	(test_pp_format): Remove comment about GCC_COLORS.
OK.
jeff

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

* Re: [PATCH 2/3] selftests: improve reported failure locations
  2016-06-09 18:15 ` [PATCH 2/3] selftests: improve reported failure locations David Malcolm
@ 2016-06-13 18:21   ` Jeff Law
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Law @ 2016-06-13 18:21 UTC (permalink / raw)
  To: David Malcolm, gcc-patches

On 06/09/2016 12:42 PM, David Malcolm wrote:
> This patch introduce a selftest::location struct to wrap up __FILE__
> and __LINE__ information (and __FUNCTION__) throughout the selftests,
> allowing location information to be passed around.
>
> It updates the helper functions in pretty-print.c to pass through
> the precise location of each test, so that if a failure occurs, the
> correct line number is printed, rather than a line within a helper
> function.
>
> gcc/ChangeLog:
> 	* input.c (test_reading_source_line): Use SELFTEST_LOCATION.
> 	* pretty-print.c (assert_pp_format_va): Add location param and use
> 	it with ASSERT_STREQ_AT.
> 	(assert_pp_format): Add location param and pass it to
> 	assert_pp_format_va.
> 	(assert_pp_format_colored): Likewise.
> 	(ASSERT_PP_FORMAT_1): New.
> 	(ASSERT_PP_FORMAT_2): New.
> 	(ASSERT_PP_FORMAT_3): New.
> 	(test_pp_format): Provide SELFTEST_LOCATION throughout, either
> 	explicitly, or implicitly via the above macros.
> 	* selftest.c (selftest::pass): Use a selftest::location rather
> 	than file and line.
> 	(selftest::fail): Likewise.  Print the function name.
> 	(selftest::fail_formatted): Likewise.
> 	(selftest::assert_streq): Use a selftest::location rather than
> 	file and line.
> 	* selftest.h (selftest::location): New struct.
> 	(SELFTEST_LOCATION): New macro.
> 	(selftest::pass): Accept a const location & rather than file
> 	and line.
> 	(selftest::fail): Likewise.
> 	(selftest::fail_formatted): Likewise.
> 	(selftest::assert_streq): Likewise.
> 	(ASSERT_TRUE): Update for above changes, using SELFTEST_LOCATION.
> 	(ASSERT_FALSE): Likewise.
> 	(ASSERT_EQ): Likewise.
> 	(ASSERT_NE): Likewise.
> 	(ASSERT_STREQ): Likewise.
> 	(ASSERT_PRED1): Likewise.
> 	(ASSERT_STREQ_AT): New macro.
OK.
jeff

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

end of thread, other threads:[~2016-06-13 18:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-09 18:15 [PATCH 0/3] selftest improvements David Malcolm
2016-06-09 18:15 ` [PATCH 3/3] pretty-print.c: skip color selftests if GCC_COLORS is set David Malcolm
2016-06-13 18:20   ` Jeff Law
2016-06-09 18:15 ` [PATCH 1/3] selftest: show values when ASSERT_STREQ fails David Malcolm
2016-06-13 18:20   ` Jeff Law
2016-06-09 18:15 ` [PATCH 2/3] selftests: improve reported failure locations David Malcolm
2016-06-13 18:21   ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).