Simplify our Assert infrastructure a little.

tglsfdc · tglsfdc · commit 235eb4db9879 · 2022-10-10T15:16:56.000-04:00
Remove the Trap and TrapMacro macros, which were nearly unused and confusingly had the opposite condition polarity from the otherwise-functionally-equivalent Assert macros. Having done that, it's very hard to justify carrying the errorType argument of ExceptionalCondition, so drop that too, and just let it assume everything's an Assert. This saves about 64K of code space as of current HEAD. Discussion: https://postgr.es/m/3928703.1665345117@sss.pgh.pa.us
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
@@ -1319,7 +1319,7 @@ check_tuple_attribute(HeapCheckContext *ctx)
 	 */
 
 	/*
-	 * Check that VARTAG_SIZE won't hit a TrapMacro on a corrupt va_tag before
+	 * Check that VARTAG_SIZE won't hit an Assert on a corrupt va_tag before
 	 * risking a call into att_addlength_pointer
 	 */
 	if (VARATT_IS_EXTERNAL(tp + ctx->offset))
diff --git a/src/backend/utils/error/assert.c b/src/backend/utils/error/assert.c
@@ -28,20 +28,17 @@
  */
 void
 ExceptionalCondition(const char *conditionName,
-					 const char *errorType,
 					 const char *fileName,
 					 int lineNumber)
 {
 	/* Report the failure on stderr (or local equivalent) */
 	if (!PointerIsValid(conditionName)
-		|| !PointerIsValid(fileName)
-		|| !PointerIsValid(errorType))
+		|| !PointerIsValid(fileName))
 		write_stderr("TRAP: ExceptionalCondition: bad arguments in PID %d\n",
 					 (int) getpid());
 	else
-		write_stderr("TRAP: %s(\"%s\", File: \"%s\", Line: %d, PID: %d)\n",
-					 errorType, conditionName,
-					 fileName, lineNumber, (int) getpid());
+		write_stderr("TRAP: failed Assert(\"%s\"), File: \"%s\", Line: %d, PID: %d\n",
+					 conditionName, fileName, lineNumber, (int) getpid());
 
 	/* Usually this shouldn't be needed, but make sure the msg went out */
 	fflush(stderr);
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
@@ -1832,8 +1832,7 @@ pg_re_throw(void)
 	}
 
 	/* Doesn't return ... */
-	ExceptionalCondition("pg_re_throw tried to return", "FailedAssertion",
-						 __FILE__, __LINE__);
+	ExceptionalCondition("pg_re_throw tried to return", __FILE__, __LINE__);
 }
 
 
diff --git a/src/include/c.h b/src/include/c.h
@@ -796,8 +796,6 @@ typedef NameData *Name;
 #define AssertArg(condition)	((void)true)
 #define AssertState(condition)	((void)true)
 #define AssertPointerAlignment(ptr, bndr)	((void)true)
-#define Trap(condition, errorType)	((void)true)
-#define TrapMacro(condition, errorType) (true)
 
 #elif defined(FRONTEND)
 
@@ -811,60 +809,38 @@ typedef NameData *Name;
 #else							/* USE_ASSERT_CHECKING && !FRONTEND */
 
 /*
- * Trap
- *		Generates an exception if the given condition is true.
+ * Assert
+ *		Generates a fatal exception if the given condition is false.
  */
-#define Trap(condition, errorType) \
+#define Assert(condition) \
 	do { \
-		if (condition) \
-			ExceptionalCondition(#condition, (errorType), \
-								 __FILE__, __LINE__); \
+		if (!(condition)) \
+			ExceptionalCondition(#condition, __FILE__, __LINE__); \
 	} while (0)
 
 /*
- *	TrapMacro is the same as Trap but it's intended for use in macros:
+ * AssertMacro is the same as Assert but it's suitable for use in
+ * expression-like macros, for example:
  *
  *		#define foo(x) (AssertMacro(x != 0), bar(x))
- *
- *	Isn't CPP fun?
  */
-#define TrapMacro(condition, errorType) \
-	((bool) (! (condition) || \
-			 (ExceptionalCondition(#condition, (errorType), \
-								   __FILE__, __LINE__), 0)))
-
-#define Assert(condition) \
-	do { \
-		if (!(condition)) \
-			ExceptionalCondition(#condition, "FailedAssertion", \
-								 __FILE__, __LINE__); \
-	} while (0)
-
 #define AssertMacro(condition) \
 	((void) ((condition) || \
-			 (ExceptionalCondition(#condition, "FailedAssertion", \
-								   __FILE__, __LINE__), 0)))
+			 (ExceptionalCondition(#condition, __FILE__, __LINE__), 0)))
 
-#define AssertArg(condition) \
-	do { \
-		if (!(condition)) \
-			ExceptionalCondition(#condition, "BadArgument", \
-								 __FILE__, __LINE__); \
-	} while (0)
-
-#define AssertState(condition) \
-	do { \
-		if (!(condition)) \
-			ExceptionalCondition(#condition, "BadState", \
-								 __FILE__, __LINE__); \
-	} while (0)
+/*
+ * AssertArg and AssertState are identical to Assert.  Some places use them
+ * to indicate that the complaint is specifically about a bad argument or
+ * unexpected state, but this usage is largely obsolescent.
+ */
+#define AssertArg(condition) Assert(condition)
+#define AssertState(condition) Assert(condition)
 
 /*
  * Check that `ptr' is `bndr' aligned.
  */
 #define AssertPointerAlignment(ptr, bndr) \
-	Trap(TYPEALIGN(bndr, (uintptr_t)(ptr)) != (uintptr_t)(ptr), \
-		 "UnalignedPointer")
+	Assert(TYPEALIGN(bndr, (uintptr_t)(ptr)) == (uintptr_t)(ptr))
 
 #endif							/* USE_ASSERT_CHECKING && !FRONTEND */
 
@@ -876,7 +852,6 @@ typedef NameData *Name;
  */
 #ifndef FRONTEND
 extern void ExceptionalCondition(const char *conditionName,
-								 const char *errorType,
 								 const char *fileName, int lineNumber) pg_attribute_noreturn();
 #endif
 
diff --git a/src/include/postgres.h b/src/include/postgres.h
@@ -135,7 +135,7 @@ typedef enum vartag_external
 	((tag) == VARTAG_INDIRECT ? sizeof(varatt_indirect) : \
 	 VARTAG_IS_EXPANDED(tag) ? sizeof(varatt_expanded) : \
 	 (tag) == VARTAG_ONDISK ? sizeof(varatt_external) : \
-	 TrapMacro(true, "unrecognized TOAST vartag"))
+	 (AssertMacro(false), 0))
 
 /*
  * These structs describe the header of a varlena object that may have been

Original file line number	Diff line number	Diff line change
`@@ -1832,8 +1832,7 @@ pg_re_throw(void)`
`1832`	`1832`	`}`
`1833`	`1833`
`1834`	`1834`	`/* Doesn't return ... */`
`1835`		`- ExceptionalCondition("pg_re_throw tried to return", "FailedAssertion",`
`1836`		`- __FILE__, __LINE__);`
	`1835`	`+ ExceptionalCondition("pg_re_throw tried to return", __FILE__, __LINE__);`
`1837`	`1836`	`}`
`1838`	`1837`
`1839`	`1838`