Re: automatically generating node support functions

Here's some follow-on patches, as I threatened yesterday.

0001 adds some material to nodes/README in hopes of compensating for
a couple of removed comments.

0002 fixes gen_node_support.pl's rather badly broken error reporting.
As it stands, it always says that an error is on line 1 of the respective
input file, because it relies for that on perl's "$." which is only
workable when we are reading the file a line at a time.  The scheme
of sucking in the entire file so that we can suppress multi-line C
comments easily doesn't play well with that.  I concluded that the
best way to fix that was to adjust the C-comment-deletion code to
preserve any newlines within a comment, and then we can easily count
lines manually.  The new C-comment-deletion code is a bit brute-force;
maybe there is a better way?

0003 adds boilerplate header comments to the output files, using
wording pretty similar to those written by genbki.pl.

0004 fixes things so that we don't leave a mess of temporary files
if the script dies partway through.  genbki.pl perhaps could use
this as well, but my experience is that genbki usually reports any
errors before starting to write files.  gen_node_support.pl not
so much --- I had to manually clean up the mess several times while
reviewing/testing.

            regards, tom lane

diff --git a/src/backend/nodes/README b/src/backend/nodes/README
index b3dc9afaf7..d8ae35ce58 100644
--- a/src/backend/nodes/README
+++ b/src/backend/nodes/README
@@ -6,10 +6,30 @@ Node Structures
 Introduction
 ------------
 
+Postgres uses "node" types to organize parse trees, plan trees, and
+executor state trees.  All objects that can appear in such trees must
+be declared as node types.  In addition, a few object types that aren't
+part of parse/plan/execute node trees receive NodeTags anyway for
+identification purposes, usually because they are involved in APIs
+where we want to pass multiple object types through the same pointer.
+
 The node structures are plain old C structures with the first field
 being of type NodeTag.  "Inheritance" is achieved by convention:
 the first field can alternatively be of another node type.
 
+Node types typically have support for being copied by copyObject(),
+compared by equal(), serialized by outNode(), and deserialized by
+nodeRead().  For some classes of Nodes, not all of these support
+functions are required; for example, executor state nodes don't
+presently need any of them.  So far as the system is concerned,
+output and read functions are only needed for node types that can
+appear in parse trees stored in the catalogs.  However, we provide
+output functions for many other node types as well, because they
+are very handy for debugging.
+
+Relevant Files
+--------------
+
 Utility functions for manipulating node structures reside in this
 directory.  Some support functions are automatically generated by the
 gen_node_support.pl script, other functions are maintained manually.
@@ -40,7 +60,7 @@ FILES IN THIS DIRECTORY (src/backend/nodes/)
 
 FILES IN src/include/nodes/
 
-    Node definitions:
+    Node definitions primarily appear in:
     nodes.h        - define node tags (NodeTag) (*)
     primnodes.h    - primitive nodes
     parsenodes.h    - parse tree nodes
diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl
index dca5819f95..6816c36e2b 100644
--- a/src/backend/nodes/gen_node_support.pl
+++ b/src/backend/nodes/gen_node_support.pl
@@ -124,19 +124,31 @@ foreach my $infile (@ARGV)
     my $supertype_field;

     my $node_attrs = '';
+    my $node_attrs_lineno;
     my @my_fields;
     my %my_field_types;
     my %my_field_attrs;

     open my $ifh, '<', $infile or die "could not open \"$infile\": $!";

-    my $file_content = do { local $/; <$ifh> };
+    my $raw_file_content = do { local $/; <$ifh> };

-    # strip C comments
-    $file_content =~ s{/\*.*?\*/}{}gs;
+    # strip C comments, preserving newlines so we can count lines correctly
+    my $file_content = '';
+    while ($raw_file_content =~ m{^(.*?)(/\*.*?\*/)(.*)$}s)
+    {
+        $file_content .= $1;
+        my $comment = $2;
+        $raw_file_content = $3;
+        $comment =~ tr/\n//cd;
+        $file_content .= $comment;
+    }
+    $file_content .= $raw_file_content;

+    my $lineno = 0;
     foreach my $line (split /\n/, $file_content)
     {
+        $lineno++;
         chomp $line;
         $line =~ s/\s*$//;
         next if $line eq '';
@@ -153,13 +165,14 @@ foreach my $infile (@ARGV)
                 $is_node_struct = 0;
                 $supertype      = undef;
                 next if $line eq '{';
-                die "$infile:$.: expected opening brace\n";
+                die "$infile:$lineno: expected opening brace\n";
             }
             # second line could be node attributes
             elsif ($subline == 2
                 && $line =~ /^\s*pg_node_attr\(([\w(), ]*)\)$/)
             {
-                $node_attrs = $1;
+                $node_attrs        = $1;
+                $node_attrs_lineno = $lineno;
                 # hack: don't count the line
                 $subline--;
                 next;
@@ -236,7 +249,7 @@ foreach my $infile (@ARGV)
                         else
                         {
                             die
-                              "$infile:$.: unrecognized attribute \"$attr\"\n";
+                              "$infile:$node_attrs_lineno: unrecognized attribute \"$attr\"\n";
                         }
                     }

@@ -330,7 +343,9 @@ foreach my $infile (@ARGV)
                     # strip space between type and "*" (pointer) */
                     $type =~ s/\s+\*$/*/;

-                    die if $type eq '';
+                    die
+                      "$infile:$lineno: cannot parse data type in \"$line\"\n"
+                      if $type eq '';

                     my @attrs;
                     if ($attrs)
@@ -347,7 +362,7 @@ foreach my $infile (@ARGV)
                               )
                             {
                                 die
-                                  "$infile:$.: unrecognized attribute \"$attr\"\n";
+                                  "$infile:$lineno: unrecognized attribute \"$attr\"\n";
                             }
                         }
                     }
@@ -362,7 +377,7 @@ foreach my $infile (@ARGV)
             {
                 if ($is_node_struct)
                 {
-                    #warn "$infile:$.: could not parse \"$line\"\n";
+                    #warn "$infile:$lineno: could not parse \"$line\"\n";
                 }
             }
         }
@@ -552,7 +567,7 @@ _equal${n}(const $n *a, const $n *b)
             my $tt = $1;
             if (!defined $array_size_field)
             {
-                die "no array size defined for $n.$f of type $t";
+                die "no array size defined for $n.$f of type $t\n";
             }
             if ($node_type_info{$n}->{field_types}{$array_size_field} eq
                 'List*')
@@ -597,7 +612,8 @@ _equal${n}(const $n *a, const $n *b)
         }
         else
         {
-            die "could not handle type \"$t\" in struct \"$n\" field \"$f\"";
+            die
+              "could not handle type \"$t\" in struct \"$n\" field \"$f\"\n";
         }
     }

@@ -814,7 +830,7 @@ _read${n}(void)
             }
             if (!defined $array_size_field)
             {
-                die "no array size defined for $n.$f of type $t";
+                die "no array size defined for $n.$f of type $t\n";
             }
             if ($node_type_info{$n}->{field_types}{$array_size_field} eq
                 'List*')
@@ -886,7 +902,8 @@ _read${n}(void)
         }
         else
         {
-            die "could not handle type \"$t\" in struct \"$n\" field \"$f\"";
+            die
+              "could not handle type \"$t\" in struct \"$n\" field \"$f\"\n";
         }

         # for read_as() without read_write_ignore, we have to read the value
diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl
index 6816c36e2b..41d824870b 100644
--- a/src/backend/nodes/gen_node_support.pl
+++ b/src/backend/nodes/gen_node_support.pl
@@ -426,10 +426,34 @@ foreach my $infile (@ARGV)

 my $tmpext = ".tmp$$";

+# opening boilerplate for output files
+my $header_comment =
+  '/*-------------------------------------------------------------------------
+ *
+ * %s
+ *    Generated node infrastructure code
+ *
+ * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * NOTES
+ *  ******************************
+ *  *** DO NOT EDIT THIS FILE! ***
+ *  ******************************
+ *
+ *  It has been GENERATED by src/backend/nodes/gen_node_support.pl
+ *
+ *-------------------------------------------------------------------------
+ */
+';
+
+
 # nodetags.h

 open my $nt, '>', 'nodetags.h' . $tmpext or die $!;

+printf $nt $header_comment, 'nodetags.h';
+
 my $i = 1;
 foreach my $n (@node_types, @extra_tags)
 {
@@ -457,6 +481,11 @@ open my $eff, '>', 'equalfuncs.funcs.c' . $tmpext  or die $!;
 open my $cfs, '>', 'copyfuncs.switch.c' . $tmpext  or die $!;
 open my $efs, '>', 'equalfuncs.switch.c' . $tmpext or die $!;

+printf $cff $header_comment, 'copyfuncs.funcs.c';
+printf $eff $header_comment, 'equalfuncs.funcs.c';
+printf $cfs $header_comment, 'copyfuncs.switch.c';
+printf $efs $header_comment, 'equalfuncs.switch.c';
+
 # add required #include lines to each file set
 print $cff $node_includes;
 print $eff $node_includes;
@@ -640,6 +669,11 @@ open my $rff, '>', 'readfuncs.funcs.c' . $tmpext  or die $!;
 open my $ofs, '>', 'outfuncs.switch.c' . $tmpext  or die $!;
 open my $rfs, '>', 'readfuncs.switch.c' . $tmpext or die $!;

+printf $off $header_comment, 'outfuncs.funcs.c';
+printf $rff $header_comment, 'readfuncs.funcs.c';
+printf $ofs $header_comment, 'outfuncs.switch.c';
+printf $rfs $header_comment, 'readfuncs.switch.c';
+
 print $off $node_includes;
 print $rff $node_includes;

diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl
index 41d824870b..4a7902e6bf 100644
--- a/src/backend/nodes/gen_node_support.pl
+++ b/src/backend/nodes/gen_node_support.pl
@@ -34,6 +34,8 @@ sub elem
     return grep { $_ eq $x } @_;
 }

+# output file names
+my @output_files;

 # collect node names
 my @node_types = qw(Node);
@@ -450,6 +452,7 @@ my $header_comment =

 # nodetags.h

+push @output_files, 'nodetags.h';
 open my $nt, '>', 'nodetags.h' . $tmpext or die $!;

 printf $nt $header_comment, 'nodetags.h';
@@ -476,9 +479,13 @@ foreach my $infile (sort @ARGV)

 # copyfuncs.c, equalfuncs.c

-open my $cff, '>', 'copyfuncs.funcs.c' . $tmpext   or die $!;
-open my $eff, '>', 'equalfuncs.funcs.c' . $tmpext  or die $!;
-open my $cfs, '>', 'copyfuncs.switch.c' . $tmpext  or die $!;
+push @output_files, 'copyfuncs.funcs.c';
+open my $cff, '>', 'copyfuncs.funcs.c' . $tmpext or die $!;
+push @output_files, 'equalfuncs.funcs.c';
+open my $eff, '>', 'equalfuncs.funcs.c' . $tmpext or die $!;
+push @output_files, 'copyfuncs.switch.c';
+open my $cfs, '>', 'copyfuncs.switch.c' . $tmpext or die $!;
+push @output_files, 'equalfuncs.switch.c';
 open my $efs, '>', 'equalfuncs.switch.c' . $tmpext or die $!;

 printf $cff $header_comment, 'copyfuncs.funcs.c';
@@ -664,9 +671,13 @@ close $efs;

 # outfuncs.c, readfuncs.c

-open my $off, '>', 'outfuncs.funcs.c' . $tmpext   or die $!;
-open my $rff, '>', 'readfuncs.funcs.c' . $tmpext  or die $!;
-open my $ofs, '>', 'outfuncs.switch.c' . $tmpext  or die $!;
+push @output_files, 'outfuncs.funcs.c';
+open my $off, '>', 'outfuncs.funcs.c' . $tmpext or die $!;
+push @output_files, 'readfuncs.funcs.c';
+open my $rff, '>', 'readfuncs.funcs.c' . $tmpext or die $!;
+push @output_files, 'outfuncs.switch.c';
+open my $ofs, '>', 'outfuncs.switch.c' . $tmpext or die $!;
+push @output_files, 'readfuncs.switch.c';
 open my $rfs, '>', 'readfuncs.switch.c' . $tmpext or die $!;

 printf $off $header_comment, 'outfuncs.funcs.c';
@@ -962,10 +973,26 @@ close $ofs;
 close $rfs;


-# now rename the temporary files to their final name
-foreach my $file (
-    qw(nodetags.h copyfuncs.funcs.c copyfuncs.switch.c equalfuncs.funcs.c equalfuncs.switch.c outfuncs.funcs.c
outfuncs.switch.creadfuncs.funcs.c readfuncs.switch.c) 
-  )
+# now rename the temporary files to their final names
+foreach my $file (@output_files)
 {
     Catalog::RenameTempFile($file, $tmpext);
 }
+
+
+# Automatically clean up any temp files if the script fails.
+END
+{
+    # take care not to change the script's exit value
+    my $exit_code = $?;
+
+    if ($exit_code != 0)
+    {
+        foreach my $file (@output_files)
+        {
+            unlink($file . $tmpext);
+        }
+    }
+
+    $? = $exit_code;
+}