From d57d96e3929cd7ef2484e9d3ac95b1c9ba774823 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 6 Mar 2026 17:29:36 +0100 Subject: [PATCH v2 2/3] Change copyObject() to use typeof_unqual Currently, when the argument of copyObject() is const-qualified, the return type is also, because the use of typeof carries over all the qualifiers. This is incorrect, since the point of copyObject() is to make a copy to mutate. But apparently no code ran into it. The new implementation uses typeof_unqual, which drops the qualifiers, making this work correctly. typeof_unqual is standardized in C23, but all recent versions of all the usual compilers support it even in non-C23 mode, at least as __typeof_unqual__. We add a configure/meson test for typeof_unqual and __typeof_unqual__ and use it if it's available, else we use the existing fallback of just returning void *. We test the underscore variant first so that there is a higher chance that clang used for bitcode also supports it, since we don't test that separately. Unlike the typeof test, the typeof_unqual test also tests with a void pointer similar to how copyObject() would use it, because that is not handled by MSVC, so we want the test to fail there. Reviewed-by: David Geier Discussion: https://www.postgresql.org/message-id/flat/92f9750f-c7f6-42d8-9a4a-85a3cbe808f3%40eisentraut.org --- config/c-compiler.m4 | 65 +++++++++++ configure | 108 ++++++++++++++++++ configure.ac | 2 + meson.build | 62 ++++++++++ src/include/c.h | 12 ++ src/include/nodes/nodes.h | 4 +- src/include/pg_config.h.in | 14 +++ .../test_cplusplusext/test_cplusplusext.cpp | 3 +- 8 files changed, 267 insertions(+), 3 deletions(-) diff --git a/config/c-compiler.m4 b/config/c-compiler.m4 index 5fd768b7332..88333ef301d 100644 --- a/config/c-compiler.m4 +++ b/config/c-compiler.m4 @@ -176,6 +176,40 @@ if test "$pgac_cv_c_typeof" != no; then fi])# PGAC_C_TYPEOF +# PGAC_C_TYPEOF_UNQUAL +# -------------------- +# Check if the C compiler understands typeof_unqual or a variant. Define +# HAVE_TYPEOF_UNQUAL if so, and define 'typeof_unqual' to the actual key word. +# +AC_DEFUN([PGAC_C_TYPEOF_UNQUAL], +[AC_CACHE_CHECK(for typeof_unqual, pgac_cv_c_typeof_unqual, +[pgac_cv_c_typeof_unqual=no +# Test the underscore variant first so that there is a higher chance +# that clang used for bitcode also supports it, since we don't test +# that separately. +# +# Test with a void pointer, because MSVC doesn't handle that, and we +# need that for copyObject(). +for pgac_kw in __typeof_unqual__ typeof_unqual; do + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], +[int x = 0; +$pgac_kw(x) y; +const void *a; +void *b; +y = x; +b = ($pgac_kw(*a) *) a; +return y;])], +[pgac_cv_c_typeof_unqual=$pgac_kw]) + test "$pgac_cv_c_typeof_unqual" != no && break +done]) +if test "$pgac_cv_c_typeof_unqual" != no; then + AC_DEFINE(HAVE_TYPEOF_UNQUAL, 1, + [Define to 1 if your compiler understands `typeof_unqual' or something similar.]) + if test "$pgac_cv_c_typeof_unqual" != typeof_unqual; then + AC_DEFINE_UNQUOTED(typeof_unqual, $pgac_cv_c_typeof_unqual, [Define to how the compiler spells `typeof_unqual'.]) + fi +fi])# PGAC_C_TYPEOF_UNQUAL + # PGAC_CXX_TYPEOF # ---------------- @@ -205,6 +239,37 @@ if test "$pgac_cv_cxx_typeof" != no; then fi])# PGAC_CXX_TYPEOF +# PGAC_CXX_TYPEOF_UNQUAL +# ---------------------- +# Check if the C++ compiler understands typeof_unqual or a variant. Define +# HAVE_CXX_TYPEOF_UNQUAL if so, and define 'pg_cxx_typeof_unqual' to the actual key word. +# +AC_DEFUN([PGAC_CXX_TYPEOF_UNQUAL], +[AC_CACHE_CHECK(for C++ typeof_unqual, pgac_cv_cxx_typeof_unqual, +[pgac_cv_cxx_typeof_unqual=no +AC_LANG_PUSH(C++) +for pgac_kw in __typeof_unqual__ typeof_unqual; do + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], +[int x = 0; +$pgac_kw(x) y; +const void *a; +void *b; +y = x; +b = ($pgac_kw(*a) *) a; +return y;])], +[pgac_cv_cxx_typeof_unqual=$pgac_kw]) + test "$pgac_cv_cxx_typeof_unqual" != no && break +done +AC_LANG_POP([])]) +if test "$pgac_cv_cxx_typeof_unqual" != no; then + AC_DEFINE(HAVE_CXX_TYPEOF_UNQUAL, 1, + [Define to 1 if your C++ compiler understands `typeof_unqual' or something similar.]) + if test "$pgac_cv_cxx_typeof_unqual" != typeof_unqual; then + AC_DEFINE_UNQUOTED(pg_cxx_typeof_unqual, $pgac_cv_cxx_typeof_unqual, [Define to how the C++ compiler spells `typeof_unqual'.]) + fi +fi])# PGAC_CXX_TYPEOF_UNQUAL + + # PGAC_C_TYPES_COMPATIBLE # ----------------------- diff --git a/configure b/configure index 4aaaf92ba0a..bef63a8c595 100755 --- a/configure +++ b/configure @@ -15101,6 +15101,114 @@ _ACEOF fi fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for typeof_unqual" >&5 +$as_echo_n "checking for typeof_unqual... " >&6; } +if ${pgac_cv_c_typeof_unqual+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_cv_c_typeof_unqual=no +# Test the underscore variant first so that there is a higher chance +# that clang used for bitcode also supports it, since we don't test +# that separately. +# +# Test with a void pointer, because MSVC doesn't handle that, and we +# need that for copyObject(). +for pgac_kw in __typeof_unqual__ typeof_unqual; do + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ +int x = 0; +$pgac_kw(x) y; +const void *a; +void *b; +y = x; +b = ($pgac_kw(*a) *) a; +return y; + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + pgac_cv_c_typeof_unqual=$pgac_kw +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext + test "$pgac_cv_c_typeof_unqual" != no && break +done +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_c_typeof_unqual" >&5 +$as_echo "$pgac_cv_c_typeof_unqual" >&6; } +if test "$pgac_cv_c_typeof_unqual" != no; then + +$as_echo "#define HAVE_TYPEOF_UNQUAL 1" >>confdefs.h + + if test "$pgac_cv_c_typeof_unqual" != typeof_unqual; then + +cat >>confdefs.h <<_ACEOF +#define typeof_unqual $pgac_cv_c_typeof_unqual +_ACEOF + + fi +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for C++ typeof_unqual" >&5 +$as_echo_n "checking for C++ typeof_unqual... " >&6; } +if ${pgac_cv_cxx_typeof_unqual+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_cv_cxx_typeof_unqual=no +ac_ext=cpp +ac_cpp='$CXXCPP $CPPFLAGS' +ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_cxx_compiler_gnu + +for pgac_kw in __typeof_unqual__ typeof_unqual; do + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ +int x = 0; +$pgac_kw(x) y; +const void *a; +void *b; +y = x; +b = ($pgac_kw(*a) *) a; +return y; + ; + return 0; +} +_ACEOF +if ac_fn_cxx_try_compile "$LINENO"; then : + pgac_cv_cxx_typeof_unqual=$pgac_kw +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext + test "$pgac_cv_cxx_typeof_unqual" != no && break +done +ac_ext=c +ac_cpp='$CPP $CPPFLAGS' +ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_c_compiler_gnu + +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_cxx_typeof_unqual" >&5 +$as_echo "$pgac_cv_cxx_typeof_unqual" >&6; } +if test "$pgac_cv_cxx_typeof_unqual" != no; then + +$as_echo "#define HAVE_CXX_TYPEOF_UNQUAL 1" >>confdefs.h + + if test "$pgac_cv_cxx_typeof_unqual" != typeof_unqual; then + +cat >>confdefs.h <<_ACEOF +#define pg_cxx_typeof_unqual $pgac_cv_cxx_typeof_unqual +_ACEOF + + fi +fi { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_types_compatible_p" >&5 $as_echo_n "checking for __builtin_types_compatible_p... " >&6; } if ${pgac_cv__types_compatible+:} false; then : diff --git a/configure.ac b/configure.ac index 9bc457bac87..4d3c69b1c4b 100644 --- a/configure.ac +++ b/configure.ac @@ -1732,6 +1732,8 @@ PGAC_PRINTF_ARCHETYPE PGAC_CXX_PRINTF_ARCHETYPE PGAC_C_TYPEOF PGAC_CXX_TYPEOF +PGAC_C_TYPEOF_UNQUAL +PGAC_CXX_TYPEOF_UNQUAL PGAC_C_TYPES_COMPATIBLE PGAC_C_BUILTIN_CONSTANT_P PGAC_C_BUILTIN_OP_OVERFLOW diff --git a/meson.build b/meson.build index 2df54409ca6..70dc64c349a 100644 --- a/meson.build +++ b/meson.build @@ -2965,6 +2965,68 @@ int main(void) endforeach endif +# Check if the C compiler understands typeof_unqual or a variant. Define +# HAVE_TYPEOF_UNQUAL if so, and define 'typeof_unqual' to the actual key word. +# +# Test the underscore variant first so that there is a higher chance +# that clang used for bitcode also supports it, since we don't test +# that separately. +# +# Test with a void pointer, because MSVC doesn't handle that, and we +# need that for copyObject(). +foreach kw : ['__typeof_unqual__', 'typeof_unqual'] + if cc.compiles(''' +int main(void) +{ + int x = 0; + @0@(x) y; + const void *a; + void *b; + y = x; + b = (@0@(*a) *) a; + return y; +} +'''.format(kw), + name: kw, + args: test_c_args, include_directories: postgres_inc) + + cdata.set('HAVE_TYPEOF_UNQUAL', 1) + if kw != 'typeof_unqual' + cdata.set('typeof_unqual', kw) + endif + + break + endif +endforeach + +# Check if the C++ compiler understands typeof_unqual or a variant. +if have_cxx + foreach kw : ['__typeof_unqual__', 'typeof_unqual'] + if cxx.compiles(''' +int main(void) +{ + int x = 0; + @0@(x) y; + const void *a; + void *b; + y = x; + b = (@0@(*a) *) a; + return y; +} +'''.format(kw), + name: 'C++ ' + kw, + args: test_c_args, include_directories: postgres_inc) + + cdata.set('HAVE_CXX_TYPEOF_UNQUAL', 1) + if kw != 'typeof_unqual' + cdata.set('pg_cxx_typeof_unqual', kw) + endif + + break + endif + endforeach +endif + # MSVC doesn't cope well with defining restrict to __restrict, the # spelling it understands, because it conflicts with diff --git a/src/include/c.h b/src/include/c.h index 5b678283469..2aab74d8b0e 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -452,7 +452,19 @@ extern "C++" #ifndef HAVE_TYPEOF #define HAVE_TYPEOF 1 #endif +/* + * and analogously for typeof_unqual + */ +#undef typeof_unqual +#ifdef pg_cxx_typeof_unqual +#define typeof_unqual(x) pg_cxx_typeof_unqual(x) +#elif !defined(HAVE_CXX_TYPEOF_UNQUAL) +#define typeof_unqual(x) std::remove_cv_t> +#endif +#ifndef HAVE_TYPEOF_UNQUAL +#define HAVE_TYPEOF_UNQUAL 1 #endif +#endif /* __cplusplus */ /* * CppAsString diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h index 59a7df31aba..a2925ae4946 100644 --- a/src/include/nodes/nodes.h +++ b/src/include/nodes/nodes.h @@ -226,8 +226,8 @@ extern int16 *readAttrNumberCols(int numCols); extern void *copyObjectImpl(const void *from); /* cast result back to argument type, if supported by compiler */ -#ifdef HAVE_TYPEOF -#define copyObject(obj) ((typeof(obj)) copyObjectImpl(obj)) +#ifdef HAVE_TYPEOF_UNQUAL +#define copyObject(obj) ((typeof_unqual(*(obj)) *) copyObjectImpl(obj)) #else #define copyObject(obj) copyObjectImpl(obj) #endif diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index cb0f53fade4..79379a4d125 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -73,6 +73,10 @@ */ #undef HAVE_CXX_TYPEOF +/* Define to 1 if your C++ compiler understands `typeof_unqual' or something + similar. */ +#undef HAVE_CXX_TYPEOF_UNQUAL + /* Define to 1 if you have the declaration of `fdatasync', and to 0 if you don't. */ #undef HAVE_DECL_FDATASYNC @@ -458,6 +462,10 @@ /* Define to 1 if your compiler understands `typeof' or something similar. */ #undef HAVE_TYPEOF +/* Define to 1 if your compiler understands `typeof_unqual' or something + similar. */ +#undef HAVE_TYPEOF_UNQUAL + /* Define to 1 if you have the header file. */ #undef HAVE_UCHAR_H @@ -784,6 +792,9 @@ /* Define to how the C++ compiler spells `typeof'. */ #undef pg_cxx_typeof +/* Define to how the C++ compiler spells `typeof_unqual'. */ +#undef pg_cxx_typeof_unqual + /* Define to keyword to use for C99 restrict support, or to nothing if not supported */ #undef pg_restrict @@ -804,3 +815,6 @@ /* Define to how the compiler spells `typeof'. */ #undef typeof + +/* Define to how the compiler spells `typeof_unqual'. */ +#undef typeof_unqual diff --git a/src/test/modules/test_cplusplusext/test_cplusplusext.cpp b/src/test/modules/test_cplusplusext/test_cplusplusext.cpp index ea04a761184..93cd7dd07f7 100644 --- a/src/test/modules/test_cplusplusext/test_cplusplusext.cpp +++ b/src/test/modules/test_cplusplusext/test_cplusplusext.cpp @@ -37,7 +37,8 @@ test_cplusplus_add(PG_FUNCTION_ARGS) int32 a = PG_GETARG_INT32(0); int32 b = PG_GETARG_INT32(1); RangeTblRef *node = makeNode(RangeTblRef); - RangeTblRef *copy = copyObject(node); + const RangeTblRef *nodec = node; + RangeTblRef *copy = copyObject(nodec); List *list = list_make1(node); foreach_ptr(RangeTblRef, rtr, list) -- 2.53.0