On Wed, 5 Oct 2022 at 21:05, David Rowley <[email protected]> wrote:
> 5 warnings remain. 4 of these are for PG_TRY() and co.
I've attached a draft patch for a method I was considering to fix the
warnings we're getting from the nested PG_TRY() statement in
utility.c.
The C preprocessor does not allow name overloading in macros, but of
course, it does allow variable argument marcos with ... so I just
used that and added ##__VA_ARGS__ to each variable. I think that
should work ok providing callers only supply 0 or 1 arguments to the
macro, and of course, make that parameter value the same for each set
of macros used in the PG_TRY() statement.
The good thing about the optional argument is that we don't need to
touch any existing users of PG_TRY(). The attached just modifies the
inner-most PG_TRY() in the only nested PG_TRY() we have in the tree in
utility.c.
The only warning remaining after applying the attached is the "now"
warning in pgbench.c:7509. I'd considered changing this to "thenow"
which translates to "right now" in the part of Scotland that I'm from.
I also considered "nownow", which is used in South Africa [1].
Anyway, I'm not really being serious, but I didn't come up with
anything better than "now2". It's just I didn't like that as it sort
of implies there are multiple definitions of "now" and I struggle with
that... maybe I'm just thinking too much in terms of Newtonian
Relativity...
David
[1] https://www.goodthingsguy.com/fun/now-now-just-now/
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index aa00815787..247d0816ad 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1678,16 +1678,16 @@ ProcessUtilitySlow(ParseState *pstate,
* command itself is queued, which is enough.
*/
EventTriggerInhibitCommandCollection();
- PG_TRY();
+ PG_TRY(2);
{
address =
ExecRefreshMatView((RefreshMatViewStmt *) parsetree,
queryString, params, qc);
}
- PG_FINALLY();
+ PG_FINALLY(2);
{
EventTriggerUndoInhibitCommandCollection();
}
- PG_END_TRY();
+ PG_END_TRY(2);
break;
case T_CreateTrigStmt:
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 4dd9658a3c..c79d6cfba3 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -310,39 +310,48 @@ extern PGDLLIMPORT ErrorContextCallback
*error_context_stack;
* pedantry; we have seen bugs from compilers improperly optimizing code
* away when such a variable was not marked. Beware that gcc's -Wclobbered
* warnings are just about entirely useless for catching such oversights.
+ *
+ * Each of these macros accepts an optional argument which can be specified
+ * to apply a suffix to the variables declared within the macros. This suffix
+ * can be used to avoid the compiler emitting warnings about shadowed
+ * variables when compiling with -Wshadow in situations where nested PG_TRY()
+ * statements are required. The optional suffix may contain any character
+ * that's allowed in a variable name. The suffix, if specified must be the
+ * same within each set of PG_TRY() / PG_CATCH() / PG_FINALLY() / PG_END_TRY()
+ * statements.
*----------
*/
-#define PG_TRY() \
+#define PG_TRY(...) \
do { \
- sigjmp_buf *_save_exception_stack = PG_exception_stack; \
- ErrorContextCallback *_save_context_stack =
error_context_stack; \
- sigjmp_buf _local_sigjmp_buf; \
- bool _do_rethrow = false; \
- if (sigsetjmp(_local_sigjmp_buf, 0) == 0) \
+ sigjmp_buf *_save_exception_stack##__VA_ARGS__ =
PG_exception_stack; \
+ ErrorContextCallback *_save_context_stack##__VA_ARGS__ =
error_context_stack; \
+ sigjmp_buf _local_sigjmp_buf##__VA_ARGS__; \
+ bool _do_rethrow##__VA_ARGS__ = false; \
+ if (sigsetjmp(_local_sigjmp_buf##__VA_ARGS__, 0) == 0) \
{ \
- PG_exception_stack = &_local_sigjmp_buf
+ PG_exception_stack = &_local_sigjmp_buf##__VA_ARGS__
-#define PG_CATCH() \
+#define PG_CATCH(...) \
} \
else \
{ \
- PG_exception_stack = _save_exception_stack; \
- error_context_stack = _save_context_stack
+ PG_exception_stack =
_save_exception_stack##__VA_ARGS__; \
+ error_context_stack = _save_context_stack##__VA_ARGS__
-#define PG_FINALLY() \
+#define PG_FINALLY(...) \
} \
else \
- _do_rethrow = true; \
+ _do_rethrow##__VA_ARGS__ = true; \
{ \
- PG_exception_stack = _save_exception_stack; \
- error_context_stack = _save_context_stack
+ PG_exception_stack =
_save_exception_stack##__VA_ARGS__; \
+ error_context_stack = _save_context_stack##__VA_ARGS__
-#define PG_END_TRY() \
+#define PG_END_TRY(...) \
} \
- if (_do_rethrow) \
+ if (_do_rethrow##__VA_ARGS__) \
PG_RE_THROW(); \
- PG_exception_stack = _save_exception_stack; \
- error_context_stack = _save_context_stack; \
+ PG_exception_stack = _save_exception_stack##__VA_ARGS__; \
+ error_context_stack = _save_context_stack##__VA_ARGS__; \
} while (0)
/*
