This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: RFC: Variable-length VECTOR_CSTs

From: Richard Sandiford <richard dot sandiford at linaro dot org>
To: David Malcolm <dmalcolm at redhat dot com>
Cc: gcc-patches at gcc dot gnu dot org
Date: Wed, 06 Dec 2017 14:40:01 +0000
Subject: Re: RFC: Variable-length VECTOR_CSTs
Authentication-results: sourceware.org; auth=none
References: <87shcxl2ka.fsf@linaro.org> <1511963747.27881.23.camel@redhat.com>

David Malcolm <dmalcolm@redhat.com> writes:
> On Wed, 2017-11-29 at 11:57 +0000, Richard Sandiford wrote:
>
> [...]
>
> I can't really comment on the representation ideas, but I'm always
> happy to see new selftests...
>
> *************** test_labels ()
>> *** 13954,13959 ****
>> --- 14179,14350 ----
>>     ASSERT_FALSE (FORCED_LABEL (label_decl));
>>   }
>>   
>> + /* Check that VECTOR_CST Y contains the elements in X.  */
>> + 
>> + static void
>> + check_vector_cst (vec<tree> x, tree y)
>> + {
>> +   for (unsigned int i = 0; i < x.length (); ++i)
>> +     ASSERT_EQ (wi::to_wide (x[i]), wi::to_wide (vector_cst_elt (y,
>> i)));
>> + }
>
> ...a couple of nits/suggestions:
>
> (a) How about renaming "x" to "expected"?  Maybe rename "y" to
> "actual"?  (to better document the sense of the test).

Good idea.  I keep getting the ASSERT_* argument order for actual
vs. expected confused, so having more explicit names would help.
Done in the updated patch.

However, I needed the patch below to use those names, since "actual" and
"expected" are also used internally by the macros.  I tried to protect
other "user-level" names too.

Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu.
Bordering on obvious, but just in case: OK to install?

> (b) At first glance, I wondered if this routine should also have
> something like:
>
>   ASSERT_EQ (expected.length (), VECTOR_CST_NELTS (actual));
>
> Though that seems to come from the vector type, and it's always 8 in
> these examples, so I'm not sure.

Guess it can't hurt :-).  Done in the updated patch.

Thanks,
Richard


gcc/
2017-12-05  Richard Sandiford  <richard.sandiford@linaro.org>

	* selftest.h (ASSERT_TRUE_AT, ASSERT_FALSE_AT, ASSERT_EQ_AT)
	(ASSERT_NE, ASSERT_PRED1): Add underscores to local variable names
	* selftest-rtl.h (ASSERT_RTX_EQ, ASSERT_RTX_PTR_EQ): Likewise.

Index: gcc/selftest.h
===================================================================
--- gcc/selftest.h	2017-11-29 11:06:34.324730917 +0000
+++ gcc/selftest.h	2017-12-06 14:34:28.377084919 +0000
@@ -219,12 +219,12 @@ #define ASSERT_TRUE(EXPR)				\
 
 #define ASSERT_TRUE_AT(LOC, EXPR)			\
   SELFTEST_BEGIN_STMT					\
-  const char *desc = "ASSERT_TRUE (" #EXPR ")";		\
-  bool actual = (EXPR);					\
-  if (actual)						\
-    ::selftest::pass ((LOC), desc);			\
+  const char *desc_ = "ASSERT_TRUE (" #EXPR ")";	\
+  bool actual_ = (EXPR);				\
+  if (actual_)						\
+    ::selftest::pass ((LOC), desc_);			\
   else							\
-    ::selftest::fail ((LOC), desc);			\
+    ::selftest::fail ((LOC), desc_);			\
   SELFTEST_END_STMT
 
 /* Evaluate EXPR and coerce to bool, calling
@@ -239,12 +239,12 @@ #define ASSERT_FALSE(EXPR)					\
 
 #define ASSERT_FALSE_AT(LOC, EXPR)				\
   SELFTEST_BEGIN_STMT						\
-  const char *desc = "ASSERT_FALSE (" #EXPR ")";			\
-  bool actual = (EXPR);							\
-  if (actual)								\
-    ::selftest::fail ((LOC), desc);			\
-  else									\
-    ::selftest::pass ((LOC), desc);					\
+  const char *desc_ = "ASSERT_FALSE (" #EXPR ")";		\
+  bool actual_ = (EXPR);					\
+  if (actual_)							\
+    ::selftest::fail ((LOC), desc_);				\
+  else								\
+    ::selftest::pass ((LOC), desc_);				\
   SELFTEST_END_STMT
 
 /* Evaluate EXPECTED and ACTUAL and compare them with ==, calling
@@ -259,11 +259,11 @@ #define ASSERT_EQ(EXPECTED, ACTUAL) \
 
 #define ASSERT_EQ_AT(LOC, EXPECTED, ACTUAL)		       \
   SELFTEST_BEGIN_STMT					       \
-  const char *desc = "ASSERT_EQ (" #EXPECTED ", " #ACTUAL ")"; \
+  const char *desc_ = "ASSERT_EQ (" #EXPECTED ", " #ACTUAL ")"; \
   if ((EXPECTED) == (ACTUAL))				       \
-    ::selftest::pass ((LOC), desc);			       \
+    ::selftest::pass ((LOC), desc_);			       \
   else							       \
-    ::selftest::fail ((LOC), desc);			       \
+    ::selftest::fail ((LOC), desc_);			       \
   SELFTEST_END_STMT
 
 /* Evaluate EXPECTED and ACTUAL and compare them with !=, calling
@@ -272,11 +272,11 @@ #define ASSERT_EQ_AT(LOC, EXPECTED, ACTU
 
 #define ASSERT_NE(EXPECTED, ACTUAL)			       \
   SELFTEST_BEGIN_STMT					       \
-  const char *desc = "ASSERT_NE (" #EXPECTED ", " #ACTUAL ")"; \
+  const char *desc_ = "ASSERT_NE (" #EXPECTED ", " #ACTUAL ")"; \
   if ((EXPECTED) != (ACTUAL))				       \
-    ::selftest::pass (SELFTEST_LOCATION, desc);			       \
+    ::selftest::pass (SELFTEST_LOCATION, desc_);	       \
   else							       \
-    ::selftest::fail (SELFTEST_LOCATION, desc);			       \
+    ::selftest::fail (SELFTEST_LOCATION, desc_);	       \
   SELFTEST_END_STMT
 
 /* Evaluate EXPECTED and ACTUAL and compare them with strcmp, calling
@@ -312,14 +312,14 @@ #define ASSERT_STR_CONTAINS(HAYSTACK, NE
 /* Evaluate PRED1 (VAL1), calling ::selftest::pass if it is true,
    ::selftest::fail if it is false.  */
 
-#define ASSERT_PRED1(PRED1, VAL1)			\
-  SELFTEST_BEGIN_STMT					\
-  const char *desc = "ASSERT_PRED1 (" #PRED1 ", " #VAL1 ")";	\
-  bool actual = (PRED1) (VAL1);				\
-  if (actual)						\
-    ::selftest::pass (SELFTEST_LOCATION, desc);			\
-  else							\
-    ::selftest::fail (SELFTEST_LOCATION, desc);			\
+#define ASSERT_PRED1(PRED1, VAL1)				\
+  SELFTEST_BEGIN_STMT						\
+  const char *desc_ = "ASSERT_PRED1 (" #PRED1 ", " #VAL1 ")";	\
+  bool actual_ = (PRED1) (VAL1);				\
+  if (actual_)							\
+    ::selftest::pass (SELFTEST_LOCATION, desc_);		\
+  else								\
+    ::selftest::fail (SELFTEST_LOCATION, desc_);		\
   SELFTEST_END_STMT
 
 #define SELFTEST_BEGIN_STMT do {
Index: gcc/selftest-rtl.h
===================================================================
--- gcc/selftest-rtl.h	2017-11-01 09:41:40.726615206 +0000
+++ gcc/selftest-rtl.h	2017-12-06 14:34:28.377084919 +0000
@@ -49,8 +49,8 @@ #define ASSERT_RTL_DUMP_EQ_WITH_REUSE(EX
 
 #define ASSERT_RTX_EQ(EXPECTED, ACTUAL) 				\
   SELFTEST_BEGIN_STMT							\
-  const char *desc = "ASSERT_RTX_EQ (" #EXPECTED ", " #ACTUAL ")";	\
-  ::selftest::assert_rtx_eq_at (SELFTEST_LOCATION, desc, (EXPECTED),	\
+  const char *desc_ = "ASSERT_RTX_EQ (" #EXPECTED ", " #ACTUAL ")";	\
+  ::selftest::assert_rtx_eq_at (SELFTEST_LOCATION, desc_, (EXPECTED),	\
 				(ACTUAL));				\
   SELFTEST_END_STMT
 
@@ -62,8 +62,8 @@ extern void assert_rtx_eq_at (const loca
 
 #define ASSERT_RTX_PTR_EQ(EXPECTED, ACTUAL) \
   SELFTEST_BEGIN_STMT							\
-  const char *desc = "ASSERT_RTX_PTR_EQ (" #EXPECTED ", " #ACTUAL ")";  \
-  ::selftest::assert_rtx_ptr_eq_at (SELFTEST_LOCATION, desc, (EXPECTED), \
+  const char *desc_ = "ASSERT_RTX_PTR_EQ (" #EXPECTED ", " #ACTUAL ")";  \
+  ::selftest::assert_rtx_ptr_eq_at (SELFTEST_LOCATION, desc_, (EXPECTED), \
 				    (ACTUAL));				\
   SELFTEST_END_STMT

Follow-Ups:
- ASSERT_* macros (was Re: RFC: Variable-length VECTOR_CSTs)
  - From: David Malcolm

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]