Combine headerscheck and cpluspluscheck scripts

Started by Peter Eisentrautalmost 2 years ago6 messages
#1Peter Eisentraut
peter@eisentraut.org
1 attachment(s)

headerscheck started in 55ea1091884 (2019) essentially as an adjusted
copy of cpluspluscheck. Since then two scripts have not drifted far
apart. But there are occasionally mistakes keeping the two exclude
lists updated together. I figure we can just combine the two scripts
into one, so it's easier to keep updated.

The attached patch adds an option --cplusplus to headerscheck, with
which it does the same thing as cpluspluscheck, and cpluspluscheck is
removed. The top-level make targets stay the same.

Attachments:

0001-Combine-headerscheck-and-cpluspluscheck-scripts.patchtext/plain; charset=UTF-8; name=0001-Combine-headerscheck-and-cpluspluscheck-scripts.patchDownload
From 097e135bc421af98dc6298510d4cb4d8d778e3fe Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 7 Feb 2024 16:22:07 +0100
Subject: [PATCH] Combine headerscheck and cpluspluscheck scripts

They are mostly the same, and it is tedious to maintain two copies of
essentially the same exclude list.  headerscheck now has a new option
--cplusplus to select the cpluspluscheck functionality.  The top-level
make targets are still the same.
---
 GNUmakefile.in                     |   2 +-
 src/tools/pginclude/cpluspluscheck | 227 -----------------------------
 src/tools/pginclude/headerscheck   |  57 +++++++-
 3 files changed, 54 insertions(+), 232 deletions(-)
 delete mode 100755 src/tools/pginclude/cpluspluscheck

diff --git a/GNUmakefile.in b/GNUmakefile.in
index eba569e930e..4d8fc794bbb 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -133,6 +133,6 @@ headerscheck: submake-generated-headers
 	$(top_srcdir)/src/tools/pginclude/headerscheck $(top_srcdir) $(abs_top_builddir)
 
 cpluspluscheck: submake-generated-headers
-	$(top_srcdir)/src/tools/pginclude/cpluspluscheck $(top_srcdir) $(abs_top_builddir)
+	$(top_srcdir)/src/tools/pginclude/headerscheck --cplusplus $(top_srcdir) $(abs_top_builddir)
 
 .PHONY: dist distdir distcheck docs install-docs world check-world install-world installcheck-world headerscheck cpluspluscheck
diff --git a/src/tools/pginclude/cpluspluscheck b/src/tools/pginclude/cpluspluscheck
deleted file mode 100755
index 7edfc44b49a..00000000000
--- a/src/tools/pginclude/cpluspluscheck
+++ /dev/null
@@ -1,227 +0,0 @@
-#!/bin/sh
-
-# Check (almost) all PostgreSQL include files for C++ compatibility.
-#
-# Argument 1 is the top-level source directory, argument 2 the
-# top-level build directory (they might be the same). If not set, they
-# default to the current directory.
-#
-# Needs to be run after configuring and creating all generated headers.
-# It's advisable to configure --with-perl --with-python, else you're
-# likely to get errors from associated headers.
-#
-# No output if everything is OK, else compiler errors.
-#
-# src/tools/pginclude/cpluspluscheck
-# Copyright (c) 2009-2024, PostgreSQL Global Development Group
-
-if [ -z "$1" ]; then
-    srcdir="."
-else
-    srcdir="$1"
-fi
-
-if [ -z "$2" ]; then
-    builddir="."
-else
-    builddir="$2"
-fi
-
-me=`basename $0`
-
-# These switches are g++ specific, you may override if necessary.
-CXXFLAGS=${CXXFLAGS:- -fsyntax-only -Wall}
-
-# Pull some info from configure's results.
-MGLOB="$builddir/src/Makefile.global"
-CPPFLAGS=`sed -n 's/^CPPFLAGS[ 	]*=[ 	]*//p' "$MGLOB"`
-CXX=`sed -n 's/^CXX[ 	]*=[ 	]*//p' "$MGLOB"`
-perl_includespec=`sed -n 's/^perl_includespec[ 	]*=[ 	]*//p' "$MGLOB"`
-python_includespec=`sed -n 's/^python_includespec[ 	]*=[ 	]*//p' "$MGLOB"`
-
-# Extract any -I and -D switches from CPPFLAGS.
-# (If necessary, user can pass more switches by presetting EXTRAFLAGS.)
-for flag in $CPPFLAGS; do
-  case $flag in
-    -I*|-D*) EXTRAFLAGS="$EXTRAFLAGS $flag";;
-  esac
-done
-
-# Create temp directory.
-tmp=`mktemp -d /tmp/$me.XXXXXX`
-
-trap "ret=$?; rm -rf $tmp; exit $ret" 0 1 2 3 15
-
-exit_status=0
-
-# Scan all of src/ and contrib/ for header files.
-for f in `cd "$srcdir" && find src contrib -name '*.h' -print`
-do
-	# Ignore files that are unportable or intentionally not standalone.
-
-	# These files are platform-specific, and c.h will include the
-	# one that's relevant for our current platform anyway.
-	test "$f" = src/include/port/aix.h && continue
-	test "$f" = src/include/port/cygwin.h && continue
-	test "$f" = src/include/port/darwin.h && continue
-	test "$f" = src/include/port/freebsd.h && continue
-	test "$f" = src/include/port/linux.h && continue
-	test "$f" = src/include/port/netbsd.h && continue
-	test "$f" = src/include/port/openbsd.h && continue
-	test "$f" = src/include/port/solaris.h && continue
-	test "$f" = src/include/port/win32.h && continue
-
-	# Additional Windows-specific headers.
-	test "$f" = src/include/port/win32_port.h && continue
-	test "$f" = src/include/port/win32/netdb.h && continue
-	test "$f" = src/include/port/win32/sys/resource.h && continue
-	test "$f" = src/include/port/win32/sys/socket.h && continue
-	test "$f" = src/include/port/win32_msvc/dirent.h && continue
-	test "$f" = src/include/port/win32_msvc/utime.h && continue
-	test "$f" = src/include/port/win32ntdll.h && continue
-	test "$f" = src/port/pthread-win32.h && continue
-
-	# Likewise, these files are platform-specific, and the one
-	# relevant to our platform will be included by atomics.h.
-	test "$f" = src/include/port/atomics/arch-arm.h && continue
-	test "$f" = src/include/port/atomics/arch-hppa.h && continue
-	test "$f" = src/include/port/atomics/arch-ppc.h && continue
-	test "$f" = src/include/port/atomics/arch-x86.h && continue
-	test "$f" = src/include/port/atomics/fallback.h && continue
-	test "$f" = src/include/port/atomics/generic.h && continue
-	test "$f" = src/include/port/atomics/generic-acc.h && continue
-	test "$f" = src/include/port/atomics/generic-gcc.h && continue
-	test "$f" = src/include/port/atomics/generic-msvc.h && continue
-	test "$f" = src/include/port/atomics/generic-sunpro.h && continue
-
-	# sepgsql.h depends on headers that aren't there on most platforms.
-	test "$f" = contrib/sepgsql/sepgsql.h && continue
-
-	# nodetags.h cannot be included standalone: it's just a code fragment.
-	test "$f" = src/include/nodes/nodetags.h && continue
-	test "$f" = src/backend/nodes/nodetags.h && continue
-
-	# These files are not meant to be included standalone, because
-	# they contain lists that might have multiple use-cases.
-	test "$f" = src/include/access/rmgrlist.h && continue
-	test "$f" = src/include/parser/kwlist.h && continue
-	test "$f" = src/pl/plpgsql/src/pl_reserved_kwlist.h && continue
-	test "$f" = src/pl/plpgsql/src/pl_unreserved_kwlist.h && continue
-	test "$f" = src/interfaces/ecpg/preproc/c_kwlist.h && continue
-	test "$f" = src/interfaces/ecpg/preproc/ecpg_kwlist.h && continue
-	test "$f" = src/include/regex/regerrs.h && continue
-	test "$f" = src/include/tcop/cmdtaglist.h && continue
-	test "$f" = src/pl/plpgsql/src/plerrcodes.h && continue
-	test "$f" = src/pl/plpython/spiexceptions.h && continue
-	test "$f" = src/pl/tcl/pltclerrcodes.h && continue
-
-	# Also not meant to be included standalone.
-	test "$f" = src/include/common/unicode_nonspacing_table.h && continue
-	test "$f" = src/include/common/unicode_east_asian_fw_table.h && continue
-
-	test "$f" = src/backend/catalog/syscache_ids.h && continue
-	test "$f" = src/backend/catalog/syscache_info.h && continue
-	test "$f" = src/include/catalog/syscache_ids.h && continue
-	test "$f" = src/include/catalog/syscache_info.h && continue
-
-	# We can't make these Bison output files compilable standalone
-	# without using "%code require", which old Bison versions lack.
-	# parser/gram.h will be included by parser/gramparse.h anyway.
-	test "$f" = contrib/cube/cubeparse.h && continue
-	test "$f" = contrib/seg/segparse.h && continue
-	test "$f" = src/backend/bootstrap/bootparse.h && continue
-	test "$f" = src/backend/parser/gram.h && continue
-	test "$f" = src/backend/replication/repl_gram.h && continue
-	test "$f" = src/backend/replication/syncrep_gram.h && continue
-	test "$f" = src/backend/utils/adt/jsonpath_gram.h && continue
-	test "$f" = src/bin/pgbench/exprparse.h && continue
-	test "$f" = src/pl/plpgsql/src/pl_gram.h && continue
-	test "$f" = src/interfaces/ecpg/preproc/preproc.h && continue
-	test "$f" = src/test/isolation/specparse.h && continue
-
-	# ppport.h is not under our control, so we can't make it standalone.
-	test "$f" = src/pl/plperl/ppport.h && continue
-
-	# regression.h is not actually C, but ECPG code.
-	test "$f" = src/interfaces/ecpg/test/regression.h && continue
-	# printf_hack.h produces "unused function" warnings.
-	test "$f" = src/interfaces/ecpg/test/printf_hack.h && continue
-
-	# pg_trace.h and utils/probes.h can include sys/sdt.h from SystemTap,
-	# which itself contains C++ code and so won't compile with a C++
-	# compiler under extern "C" linkage.
-	test "$f" = src/include/pg_trace.h && continue
-	test "$f" = src/include/utils/probes.h && continue
-
-	# pg_dump is not C++-clean because it uses "public" and "namespace"
-	# as field names, which is unfortunate but we won't change it now.
-	test "$f" = src/bin/pg_dump/compress_gzip.h && continue
-	test "$f" = src/bin/pg_dump/compress_io.h && continue
-	test "$f" = src/bin/pg_dump/compress_lz4.h && continue
-	test "$f" = src/bin/pg_dump/compress_none.h && continue
-	test "$f" = src/bin/pg_dump/compress_zstd.h && continue
-	test "$f" = src/bin/pg_dump/parallel.h && continue
-	test "$f" = src/bin/pg_dump/pg_backup_archiver.h && continue
-	test "$f" = src/bin/pg_dump/pg_dump.h && continue
-
-	# OK, create .c file to include this .h file.
-	{
-	    echo 'extern "C" {'
-	    # Ideally we'd pre-include only the appropriate one of
-	    # postgres.h, postgres_fe.h, or c.h.  We don't always have enough
-	    # info to guess which, but in some subdirectories there's a
-	    # reasonable choice to make, and otherwise we use postgres.h.
-	    # Also, those three files should compile with no pre-include, as
-	    # should src/interfaces headers meant to be exposed to clients.
-	    case "$f" in
-		src/include/postgres.h) ;;
-		src/include/postgres_fe.h) ;;
-		src/include/c.h) ;;
-		src/interfaces/libpq/libpq-fe.h) ;;
-		src/interfaces/libpq/libpq-events.h) ;;
-		src/interfaces/ecpg/ecpglib/ecpglib_extern.h)
-		    echo '#include "postgres_fe.h"' ;;
-		src/interfaces/ecpg/ecpglib/*) ;;
-		src/interfaces/*)
-		    echo '#include "postgres_fe.h"' ;;
-		src/bin/*)
-		    echo '#include "postgres_fe.h"' ;;
-		src/fe_utils/*)
-		    echo '#include "postgres_fe.h"' ;;
-		src/port/*) ;;
-		src/common/*)
-		    echo '#include "c.h"' ;;
-		*)
-		    echo '#include "postgres.h"' ;;
-	    esac
-	    echo "#include \"$f\""
-	    echo '};'
-	} >$tmp/test.cpp
-
-	# Some subdirectories need extra -I switches.
-	case "$f" in
-	    src/pl/plperl/*)
-		EXTRAINCLUDES="$perl_includespec" ;;
-	    src/pl/plpython/*)
-		EXTRAINCLUDES="$python_includespec" ;;
-	    src/interfaces/ecpg/*)
-		EXTRAINCLUDES="-I $builddir/src/interfaces/ecpg/include -I $srcdir/src/interfaces/ecpg/include" ;;
-	    src/backend/parser/*)
-		EXTRAINCLUDES="-I $builddir/src/backend/parser/" ;;
-	    src/backend/utils/adt/*)
-		EXTRAINCLUDES="-I $builddir/src/backend/utils/adt/" ;;
-	    *)
-		EXTRAINCLUDES="" ;;
-	esac
-
-	# Run the test.
-	if ! ${CXX:-g++} -I $builddir -I $srcdir \
-		-I $builddir/src/include -I $srcdir/src/include \
-		-I $builddir/src/interfaces/libpq -I $srcdir/src/interfaces/libpq \
-		$EXTRAINCLUDES $EXTRAFLAGS $CXXFLAGS -c $tmp/test.cpp
-	then
-		exit_status=1
-	fi
-done
-
-exit $exit_status
diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck
index 84b892b5c51..ccdd689ebe9 100755
--- a/src/tools/pginclude/headerscheck
+++ b/src/tools/pginclude/headerscheck
@@ -15,6 +15,13 @@
 # src/tools/pginclude/headerscheck
 # Copyright (c) 2009-2024, PostgreSQL Global Development Group
 
+if [ "$1" = "--cplusplus" ]; then
+	cplusplus=true
+	shift
+else
+	cplusplus=false
+fi
+
 if [ -z "$1" ]; then
     srcdir="."
 else
@@ -29,11 +36,15 @@ fi
 
 me=`basename $0`
 
+# These switches are g++ specific, you may override if necessary.
+CXXFLAGS=${CXXFLAGS:- -fsyntax-only -Wall}
+
 # Pull some info from configure's results.
 MGLOB="$builddir/src/Makefile.global"
 CPPFLAGS=`sed -n 's/^CPPFLAGS[ 	]*=[ 	]*//p' "$MGLOB"`
 CFLAGS=`sed -n 's/^CFLAGS[ 	]*=[ 	]*//p' "$MGLOB"`
 CC=`sed -n 's/^CC[ 	]*=[ 	]*//p' "$MGLOB"`
+CXX=`sed -n 's/^CXX[ 	]*=[ 	]*//p' "$MGLOB"`
 PG_SYSROOT=`sed -n 's/^PG_SYSROOT[ 	]*=[ 	]*//p' "$MGLOB"`
 perl_includespec=`sed -n 's/^perl_includespec[ 	]*=[ 	]*//p' "$MGLOB"`
 python_includespec=`sed -n 's/^python_includespec[ 	]*=[ 	]*//p' "$MGLOB"`
@@ -43,6 +54,22 @@ CPPFLAGS=`echo "$CPPFLAGS" | sed "s|\\\$(PG_SYSROOT)|$PG_SYSROOT|g"`
 
 # (EXTRAFLAGS is not set here, but user can pass it in if need be.)
 
+if $cplusplus; then
+	ext=cpp
+	COMPILER=${CXX:-g++}
+	# Extract any -I and -D switches from CPPFLAGS.
+	for flag in $CPPFLAGS; do
+	  case $flag in
+	    -I*|-D*) CXXPPFLAGS="$CXXPPFLAGS $flag";;
+	  esac
+	done
+	COMPILER_FLAGS="$CXXPPFLAGS $CXXFLAGS"
+else
+	ext=c
+	COMPILER=${CC:-gcc}
+	COMPILER_FLAGS="$CPPFLAGS $CFLAGS"
+fi
+
 # Create temp directory.
 tmp=`mktemp -d /tmp/$me.XXXXXX`
 
@@ -70,6 +97,7 @@ do
 	# Additional Windows-specific headers.
 	test "$f" = src/include/port/win32_port.h && continue
 	test "$f" = src/include/port/win32/netdb.h && continue
+	$cplusplus && test "$f" = src/include/port/win32/sys/resource.h && continue
 	test "$f" = src/include/port/win32/sys/socket.h && continue
 	test "$f" = src/include/port/win32_msvc/dirent.h && continue
 	test "$f" = src/include/port/win32_msvc/utime.h && continue
@@ -135,7 +163,7 @@ do
 	test "$f" = src/test/isolation/specparse.h && continue
 
 	# This produces a "no previous prototype" warning.
-	test "$f" = src/include/storage/checksum_impl.h && continue
+	! $cplusplus && test "$f" = src/include/storage/checksum_impl.h && continue
 
 	# ppport.h is not under our control, so we can't make it standalone.
 	test "$f" = src/pl/plperl/ppport.h && continue
@@ -145,8 +173,28 @@ do
 	# printf_hack.h produces "unused function" warnings.
 	test "$f" = src/interfaces/ecpg/test/printf_hack.h && continue
 
+	if $cplusplus; then
+		# pg_trace.h and utils/probes.h can include sys/sdt.h from SystemTap,
+		# which itself contains C++ code and so won't compile with a C++
+		# compiler under extern "C" linkage.
+		test "$f" = src/include/pg_trace.h && continue
+		test "$f" = src/include/utils/probes.h && continue
+
+		# pg_dump is not C++-clean because it uses "public" and "namespace"
+		# as field names, which is unfortunate but we won't change it now.
+		test "$f" = src/bin/pg_dump/compress_gzip.h && continue
+		test "$f" = src/bin/pg_dump/compress_io.h && continue
+		test "$f" = src/bin/pg_dump/compress_lz4.h && continue
+		test "$f" = src/bin/pg_dump/compress_none.h && continue
+		test "$f" = src/bin/pg_dump/compress_zstd.h && continue
+		test "$f" = src/bin/pg_dump/parallel.h && continue
+		test "$f" = src/bin/pg_dump/pg_backup_archiver.h && continue
+		test "$f" = src/bin/pg_dump/pg_dump.h && continue
+	fi
+
 	# OK, create .c file to include this .h file.
 	{
+	    $cplusplus && echo 'extern "C" {'
 	    # Ideally we'd pre-include only the appropriate one of
 	    # postgres.h, postgres_fe.h, or c.h.  We don't always have enough
 	    # info to guess which, but in some subdirectories there's a
@@ -175,7 +223,8 @@ do
 		    echo '#include "postgres.h"' ;;
 	    esac
 	    echo "#include \"$f\""
-	} >$tmp/test.c
+	    $cplusplus && echo '};'
+	} >$tmp/test.$ext
 
 	# Some subdirectories need extra -I switches.
 	case "$f" in
@@ -194,10 +243,10 @@ do
 	esac
 
 	# Run the test.
-	if ! ${CC:-gcc} $CPPFLAGS $CFLAGS -I $builddir -I $srcdir \
+	if ! $COMPILER $COMPILER_FLAGS -I $builddir -I $srcdir \
 		-I $builddir/src/include -I $srcdir/src/include \
 		-I $builddir/src/interfaces/libpq -I $srcdir/src/interfaces/libpq \
-		$EXTRAINCLUDES $EXTRAFLAGS -c $tmp/test.c -o $tmp/test.o
+		$EXTRAINCLUDES $EXTRAFLAGS -c $tmp/test.$ext -o $tmp/test.o
 	then
 		exit_status=1
 	fi
-- 
2.43.0

#2Thomas Munro
thomas.munro@gmail.com
In reply to: Peter Eisentraut (#1)
Re: Combine headerscheck and cpluspluscheck scripts

+1

#3Michael Paquier
michael@paquier.xyz
In reply to: Thomas Munro (#2)
Re: Combine headerscheck and cpluspluscheck scripts

On Thu, Mar 07, 2024 at 01:37:36PM +1300, Thomas Munro wrote:

+1

Looking at the patch, nice cleanup.
--
Michael

#4Peter Eisentraut
peter@eisentraut.org
In reply to: Michael Paquier (#3)
Re: Combine headerscheck and cpluspluscheck scripts

On 07.03.24 08:30, Michael Paquier wrote:

On Thu, Mar 07, 2024 at 01:37:36PM +1300, Thomas Munro wrote:

+1

Looking at the patch, nice cleanup.

Committed, thanks.

#5Anton Voloshin
a.voloshin@postgrespro.ru
In reply to: Peter Eisentraut (#4)
1 attachment(s)
Re: Combine headerscheck and cpluspluscheck scripts

Hello, hackers,

On 10/03/2024 12:03, Peter Eisentraut wrote:

Committed, thanks.

This commit (7b8e2ae2f) have turned cpluspluscheck script into a
--cplusplus option for headerscheck. I propose to update the
src/tools/pginclude/README correspondingly, please see the attached patch.

--
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru

Attachments:

0001-Update-src-tools-pginclude-README-to-match-recent-ch.patchtext/x-patch; charset=UTF-8; name=0001-Update-src-tools-pginclude-README-to-match-recent-ch.patchDownload
From a50e58f117341e8a9df5f97fa05630f7b8f4bd86 Mon Sep 17 00:00:00 2001
From: Anton Voloshin <a.voloshin@postgrespro.ru>
Date: Tue, 16 Apr 2024 17:19:28 +0300
Subject: [PATCH] Update src/tools/pginclude/README to match recent changes to
 cpluspluscheck

7b8e2ae2f have turned cpluspluscheck from separate script into
a --cplusplus option for headerscheck. Update README correspondingly.
---
 src/tools/pginclude/README | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/tools/pginclude/README b/src/tools/pginclude/README
index 712eca76fb3..65372057dad 100644
--- a/src/tools/pginclude/README
+++ b/src/tools/pginclude/README
@@ -1,10 +1,10 @@
 src/tools/pginclude/README
 
-NOTE: headerscheck and cpluspluscheck are in current use, and any
-problems they find should generally get fixed.  The other scripts
-in this directory have not been used in some time, and have issues.
-pgrminclude in particular has a history of creating more problems
-than it fixes.  Be very wary of applying their results blindly.
+NOTE: headerscheck and headerscheck --cplusplus are in current use, and any
+problems they find should generally get fixed.  The other scripts in this
+directory have not been used in some time, and have issues.  pgrminclude in
+particular has a history of creating more problems than it fixes.  Be very
+wary of applying their results blindly.
 
 
 pginclude
@@ -84,16 +84,16 @@ prerequisite, even if postgres_fe.h or c.h would be more appropriate.
 Also note that the contents of macros are not checked; this is intentional.
 
 
-cpluspluscheck
-==============
+headerscheck --cplusplus
+========================
 
-This script can be run to verify that all Postgres include files meet
-the project convention that they will compile as C++ code.  Although
-the project's coding language is C, some people write extensions in C++,
-so it's helpful for include files to be C++-clean.
+The headerscheck in --cplusplus mode can be run to verify that all Postgres
+include files meet the project convention that they will compile as C++
+code.  Although the project's coding language is C, some people write
+extensions in C++, so it's helpful for include files to be C++-clean.
 
 A small number of header files are exempted from this requirement,
-and are skipped by the cpluspluscheck script.
+and are skipped by the script in the --cplusplus mode.
 
 The easy way to run the script is to say "make -s cpluspluscheck" in
 the top-level build directory after completing a build.  You should
-- 
2.43.2

#6Peter Eisentraut
peter@eisentraut.org
In reply to: Anton Voloshin (#5)
Re: Combine headerscheck and cpluspluscheck scripts

On 16.04.24 17:17, Anton Voloshin wrote:

On 10/03/2024 12:03, Peter Eisentraut wrote:

Committed, thanks.

This commit (7b8e2ae2f) have turned cpluspluscheck script into a
--cplusplus option for headerscheck.  I propose to update the
src/tools/pginclude/README correspondingly, please see the attached patch.

Fixed, thanks!