The newNode() macro can be turned into a static inline function, which
makes it a lot simpler. See attached. This was not possible when the
macro was originally written, as we didn't require compiler to have
static inline support, but nowadays we do.
This was last discussed in 2008, see discussion at
https://www.postgresql.org/message-id/26133.1220037409%40sss.pgh.pa.us.
In those tests, Tom observed that gcc refused to inline the static
inline function. That was weird, the function is very small and doesn't
do anything special. Whatever the problem was, I think we can dismiss it
with modern compilers. It does get inlined on gcc 12 and clang 14 that I
have installed.
--
Heikki Linnakangas
Neon (https://neon.tech)
From 6ad4c4cf49ef5b3f7ed22acc258a868f1a13f6f4 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Thu, 14 Dec 2023 01:08:16 +0100
Subject: [PATCH 1/1] Convert newNode() into a static inline function
Because it's simpler. We didn't require static inline support from
compiler when this was originally written, but now we do.
One complication is that the static inline function doesn't work in
the frontend, because it calls palloc0fast() which isn't included
frontend programs. That's OK, the old macro also didn't work in
frontend programs if you actually tried to call it, but we now need to
explicitly put it in an #ifndef FRONTEND block to keep the compiler
happy.
---
src/backend/nodes/Makefile | 1 -
src/backend/nodes/meson.build | 1 -
src/backend/nodes/nodes.c | 31 -------------------------
src/include/nodes/nodes.h | 43 +++++++++++------------------------
4 files changed, 13 insertions(+), 63 deletions(-)
delete mode 100644 src/backend/nodes/nodes.c
diff --git a/src/backend/nodes/Makefile b/src/backend/nodes/Makefile
index ebbe9052cb7..66bbad8e6e0 100644
--- a/src/backend/nodes/Makefile
+++ b/src/backend/nodes/Makefile
@@ -23,7 +23,6 @@ OBJS = \
makefuncs.o \
multibitmapset.o \
nodeFuncs.o \
- nodes.o \
outfuncs.o \
params.o \
print.o \
diff --git a/src/backend/nodes/meson.build b/src/backend/nodes/meson.build
index 31467a12d3b..1efbf2c11ca 100644
--- a/src/backend/nodes/meson.build
+++ b/src/backend/nodes/meson.build
@@ -7,7 +7,6 @@ backend_sources += files(
'makefuncs.c',
'multibitmapset.c',
'nodeFuncs.c',
- 'nodes.c',
'params.c',
'print.c',
'read.c',
diff --git a/src/backend/nodes/nodes.c b/src/backend/nodes/nodes.c
deleted file mode 100644
index 1913a4bdf7d..00000000000
--- a/src/backend/nodes/nodes.c
+++ /dev/null
@@ -1,31 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * nodes.c
- * support code for nodes (now that we have removed the home-brew
- * inheritance system, our support code for nodes is much simpler)
- *
- * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- *
- * IDENTIFICATION
- * src/backend/nodes/nodes.c
- *
- * HISTORY
- * Andrew Yu Oct 20, 1994 file creation
- *
- *-------------------------------------------------------------------------
- */
-#include "postgres.h"
-
-#include "nodes/nodes.h"
-
-/*
- * Support for newNode() macro
- *
- * In a GCC build there is no need for the global variable newNodeMacroHolder.
- * However, we create it anyway, to support the case of a non-GCC-built
- * loadable module being loaded into a GCC-built backend.
- */
-
-Node *newNodeMacroHolder;
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 4c32682e4ce..dbb8eed122c 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -132,6 +132,8 @@ typedef struct Node
#define nodeTag(nodeptr) (((const Node*)(nodeptr))->type)
+#ifndef FRONTEND
+
/*
* newNode -
* create a new node of the specified size and tag the node with the
@@ -139,43 +141,24 @@ typedef struct Node
*
* !WARNING!: Avoid using newNode directly. You should be using the
* macro makeNode. eg. to create a Query node, use makeNode(Query)
- *
- * Note: the size argument should always be a compile-time constant, so the
- * apparent risk of multiple evaluation doesn't matter in practice.
- */
-#ifdef __GNUC__
-
-/* With GCC, we can use a compound statement within an expression */
-#define newNode(size, tag) \
-({ Node *_result; \
- AssertMacro((size) >= sizeof(Node)); /* need the tag, at least */ \
- _result = (Node *) palloc0fast(size); \
- _result->type = (tag); \
- _result; \
-})
-#else
-
-/*
- * There is no way to dereference the palloc'ed pointer to assign the
- * tag, and also return the pointer itself, so we need a holder variable.
- * Fortunately, this macro isn't recursive so we just define
- * a global variable for this purpose.
*/
-extern PGDLLIMPORT Node *newNodeMacroHolder;
+static inline Node *
+newNode(size_t size, NodeTag tag)
+{
+ Node *result;
-#define newNode(size, tag) \
-( \
- AssertMacro((size) >= sizeof(Node)), /* need the tag, at least */ \
- newNodeMacroHolder = (Node *) palloc0fast(size), \
- newNodeMacroHolder->type = (tag), \
- newNodeMacroHolder \
-)
-#endif /* __GNUC__ */
+ Assert(size >= sizeof(Node)); /* need the tag, at least */
+ result = (Node *) palloc0fast(size);
+ result->type = tag;
+ return result;
+}
#define makeNode(_type_) ((_type_ *) newNode(sizeof(_type_),T_##_type_))
#define NodeSetTag(nodeptr,t) (((Node*)(nodeptr))->type = (t))
+#endif /* FRONTEND */
+
#define IsA(nodeptr,_type_) (nodeTag(nodeptr) == T_##_type_)
/*
--
2.39.2
