Improve Assert output
This kind of output is usually not helpful:
TRAP: BadArgument("((context) != ((void *)0) && (((((const
Node*)((context)))->type) == T_AllocSetContext) || ((((const
Node*)((context)))->type) == T_SlabContext) || ((((const
Node*)((context)))->type) == T_GenerationContext)))", File:
"../../../../src/include/utils/memutils.h", Line: 129)
What we probably want is something like:
TRAP: BadArgument("MemoryContextIsValid(context)", File:
"../../../../src/include/utils/memutils.h", Line: 129)
The problem is that the way the Assert macros are written they
macro-expand the arguments before stringifying them. The attached patch
fixes that. This requires both replacing CppAsString by plain "#" and
not redirecting Assert() to Trap().
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Improve-Assert-output.patchtext/plain; charset=UTF-8; name=0001-Improve-Assert-output.patch; x-mac-creator=0; x-mac-type=0Download
From 7db60e612e1d26c7a1859948594c28dc3b54353e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 14 Aug 2019 22:23:34 +0200
Subject: [PATCH] Improve Assert output
If an assertion expression contained a macro, the failed assertion
message would print the expanded macro, which is usually unhelpful and
confusing. Restructure the Assert macros to not expand any macros
when constructing the failure message.
---
src/include/c.h | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/src/include/c.h b/src/include/c.h
index 2a082afab1..f461628a24 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -755,7 +755,7 @@ typedef NameData *Name;
#define Trap(condition, errorType) \
do { \
if (condition) \
- ExceptionalCondition(CppAsString(condition), (errorType), \
+ ExceptionalCondition(#condition, (errorType), \
__FILE__, __LINE__); \
} while (0)
@@ -768,20 +768,34 @@ typedef NameData *Name;
*/
#define TrapMacro(condition, errorType) \
((bool) (! (condition) || \
- (ExceptionalCondition(CppAsString(condition), (errorType), \
+ (ExceptionalCondition(#condition, (errorType), \
__FILE__, __LINE__), 0)))
#define Assert(condition) \
- Trap(!(condition), "FailedAssertion")
+ do { \
+ if (!(condition)) \
+ ExceptionalCondition(#condition, "FailedAssertion", \
+ __FILE__, __LINE__); \
+ } while (0)
#define AssertMacro(condition) \
- ((void) TrapMacro(!(condition), "FailedAssertion"))
+ ((void) ((condition) || \
+ (ExceptionalCondition(#condition, "FailedAssertion", \
+ __FILE__, __LINE__), 0)))
#define AssertArg(condition) \
- Trap(!(condition), "BadArgument")
+ do { \
+ if (!(condition)) \
+ ExceptionalCondition(#condition, "BadArgument", \
+ __FILE__, __LINE__); \
+ } while (0)
#define AssertState(condition) \
- Trap(!(condition), "BadState")
+ do { \
+ if (!(condition)) \
+ ExceptionalCondition(#condition, "BadState", \
+ __FILE__, __LINE__); \
+ } while (0)
/*
* Check that `ptr' is `bndr' aligned.
--
2.22.0
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
This kind of output is usually not helpful:
TRAP: BadArgument("((context) != ((void *)0) && (((((const
Node*)((context)))->type) == T_AllocSetContext) || ((((const
Node*)((context)))->type) == T_SlabContext) || ((((const
Node*)((context)))->type) == T_GenerationContext)))", File:
"../../../../src/include/utils/memutils.h", Line: 129)
What we probably want is something like:
TRAP: BadArgument("MemoryContextIsValid(context)", File:
"../../../../src/include/utils/memutils.h", Line: 129)
+1, that would be a big improvement. The other thing that this
is fixing is that the existing output for Assert et al shows
the *inverted* condition, which I for one always found confusing.
I didn't try to test the patch, but it passes eyeball examination.
regards, tom lane