Hi, On Fri, Mar 27, 2026 at 01:15:29PM +0100, Peter Eisentraut wrote: > On 27.03.26 11:07, Bertrand Drouvot wrote: > > That looks ok but I wonder if we should also add '-Werror=vla' to it ( > > and remove it from common_warning_flags) to be in sync with what > > configure.ac > > is doing: > > > > " > > PGAC_PROG_CC_CFLAGS_OPT([-Werror=vla]) > > # -Wvla is not applicable for C++ > > " > > Hmm. This comment is not actually fully correct. With g++, you can write a > VLA and it will work, and also -Wvla will warn about it. So we should > actually add that option to C++ as well. (It is true that VLAs are not in > standard C++, and g++ with -pedantic will also warn about it.)
Oh right, just did a few tests and agree with your comment above. I agree that it should also be added for C++ in autoconf then. What about the attached? That said, your patch LGTM given that the VLA remark was not appropriate. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
>From 4eec8268131d339b970ed1f694f33255fa114f62 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Fri, 27 Mar 2026 12:39:33 +0000 Subject: [PATCH v1] configure: Apply -Werror=vla to C++ as well as C The comment part of d9dd406fe281 mentioned that -Wvla is not applicable for C++. That is not fully correct: it is true that VLAs are not part of the C++ standard, and g++ with -pedantic will also warn about them as a non-standard extension. However, -Wvla itself works fine in C++ and will catch VLA usage just as in C. Fix configure.ac to apply -Werror=vla to C++ as well. There is no need to fix meson.build as it already includes it in common_warning_flags. Author: Bertrand Drouvot <[email protected]> Suggested-by: Peter Eisentraut <[email protected]> Discussion: https://postgr.es/m/7bf60ab1-2b5d-4a77-93ce-815072a0a014%40eisentraut.org --- configure | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++-- configure.ac | 4 ++-- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/configure b/configure index 8e0e7483c1d..e022617cbdc 100755 --- a/configure +++ b/configure @@ -5300,7 +5300,7 @@ fi PERMIT_DECLARATION_AFTER_STATEMENT=-Wno-declaration-after-statement fi - # Really don't want VLAs to be used in our dialect of C + # Really don't want VLAs to be used { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Werror=vla, for CFLAGS" >&5 $as_echo_n "checking whether ${CC} supports -Werror=vla, for CFLAGS... " >&6; } @@ -5341,7 +5341,57 @@ if test x"$pgac_cv_prog_CC_cflags__Werror_vla" = x"yes"; then fi - # -Wvla is not applicable for C++ + { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -Werror=vla, for CXXFLAGS" >&5 +$as_echo_n "checking whether ${CXX} supports -Werror=vla, for CXXFLAGS... " >&6; } +if ${pgac_cv_prog_CXX_cxxflags__Werror_vla+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_save_CXXFLAGS=$CXXFLAGS +pgac_save_CXX=$CXX +CXX=${CXX} +CXXFLAGS="${CXXFLAGS} -Werror=vla" +ac_save_cxx_werror_flag=$ac_cxx_werror_flag +ac_cxx_werror_flag=yes +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 + +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ + + ; + return 0; +} +_ACEOF +if ac_fn_cxx_try_compile "$LINENO"; then : + pgac_cv_prog_CXX_cxxflags__Werror_vla=yes +else + pgac_cv_prog_CXX_cxxflags__Werror_vla=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +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 + +ac_cxx_werror_flag=$ac_save_cxx_werror_flag +CXXFLAGS="$pgac_save_CXXFLAGS" +CXX="$pgac_save_CXX" +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CXX_cxxflags__Werror_vla" >&5 +$as_echo "$pgac_cv_prog_CXX_cxxflags__Werror_vla" >&6; } +if test x"$pgac_cv_prog_CXX_cxxflags__Werror_vla" = x"yes"; then + CXXFLAGS="${CXXFLAGS} -Werror=vla" +fi + + # On macOS, complain about usage of symbols newer than the deployment target { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Werror=unguarded-availability-new, for CFLAGS" >&5 diff --git a/configure.ac b/configure.ac index 2baac5e9da7..4c12ecb1078 100644 --- a/configure.ac +++ b/configure.ac @@ -547,9 +547,9 @@ if test "$GCC" = yes -a "$ICC" = no; then PERMIT_DECLARATION_AFTER_STATEMENT=-Wno-declaration-after-statement fi AC_SUBST(PERMIT_DECLARATION_AFTER_STATEMENT) - # Really don't want VLAs to be used in our dialect of C + # Really don't want VLAs to be used PGAC_PROG_CC_CFLAGS_OPT([-Werror=vla]) - # -Wvla is not applicable for C++ + PGAC_PROG_CXX_CFLAGS_OPT([-Werror=vla]) # On macOS, complain about usage of symbols newer than the deployment target PGAC_PROG_CC_CFLAGS_OPT([-Werror=unguarded-availability-new]) PGAC_PROG_CXX_CFLAGS_OPT([-Werror=unguarded-availability-new]) -- 2.34.1
