Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Hi, On 2018-09-11 16:13:15 -0400, Andrew Dunstan wrote: > I've pushed support for the latest MSVC compilers back to all live branches. Thanks! Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 08/24/2018 03:42 PM, Andrew Dunstan wrote: On 08/24/2018 02:38 PM, Tom Lane wrote: Andres Freund writes: On 2018-08-24 14:09:09 -0400, Andrew Dunstan wrote: However, we only support VS2017 down to 9.6 and Vs2015 down to 9.5. Perhaps we should consider backpatching support for those down to 9.3. Hm, I have no strong objections to that. I don't think it's strictly necessary, given 2013 is supported across the board, but for the non MSVC world, we do fix compiler issues in older branches. There's not that much code for the newer versions afaict? +1 for taking a look at how big a patch it would be. But I kind of thought we'd intentionally rejected back-patching some of those changes to begin with, so I'm not sure the end decision will change. The VS2017 patch applies cleanly to 9.5, so that seems easy. The VS2015 patch from 9.5 needs a very small amount of adjustment by the look of it for 9.3 and 9.4, after which I hope the VS2017 patch would again apply cleanly. I'll try to put this together. The trouble with not back patching support to all live branches as new versions come in is that it acts as a significant discouragement to buildfarm owners to use the latest Visual Studio versions. I've never argued stringly on this point before, but I think i'm goiung to ber inclined to in future. Meanwhile, I will turn bowerbird back on but just for >= 9.6 for now. I've pushed support for the latest MSVC compilers back to all live branches. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 08/24/2018 02:38 PM, Tom Lane wrote: Andres Freund writes: On 2018-08-24 14:09:09 -0400, Andrew Dunstan wrote: However, we only support VS2017 down to 9.6 and Vs2015 down to 9.5. Perhaps we should consider backpatching support for those down to 9.3. Hm, I have no strong objections to that. I don't think it's strictly necessary, given 2013 is supported across the board, but for the non MSVC world, we do fix compiler issues in older branches. There's not that much code for the newer versions afaict? +1 for taking a look at how big a patch it would be. But I kind of thought we'd intentionally rejected back-patching some of those changes to begin with, so I'm not sure the end decision will change. The VS2017 patch applies cleanly to 9.5, so that seems easy. The VS2015 patch from 9.5 needs a very small amount of adjustment by the look of it for 9.3 and 9.4, after which I hope the VS2017 patch would again apply cleanly. I'll try to put this together. The trouble with not back patching support to all live branches as new versions come in is that it acts as a significant discouragement to buildfarm owners to use the latest Visual Studio versions. I've never argued stringly on this point before, but I think i'm goiung to ber inclined to in future. Meanwhile, I will turn bowerbird back on but just for >= 9.6 for now. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 08/24/2018 02:36 PM, Tom Lane wrote: Andrew Dunstan writes: I saw Tom's answer, and it will work as far as it goes. But maybe we should look at doing that in configure instead of putting the onus on all buildfarm owners? It already knows if it's using a GNU compiler, not sure how ubiquitous the -ansi and -std=c99 flags are. No, the only reason either of us are doing that is to force use of a flag that's different from what configure would select by default (which evidently is -std=gnu99 for gcc). Most buildfarm owners have no need to do anything. Ah. Ok. Then your answer is good. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Andres Freund writes: > On 2018-08-24 14:09:09 -0400, Andrew Dunstan wrote: >> However, we only support VS2017 down to 9.6 and Vs2015 down to 9.5. Perhaps >> we should consider backpatching support for those down to 9.3. > Hm, I have no strong objections to that. I don't think it's strictly > necessary, given 2013 is supported across the board, but for the non MSVC > world, we do fix compiler issues in older branches. There's not that > much code for the newer versions afaict? +1 for taking a look at how big a patch it would be. But I kind of thought we'd intentionally rejected back-patching some of those changes to begin with, so I'm not sure the end decision will change. regards, tom lane
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Andrew Dunstan writes: > I saw Tom's answer, and it will work as far as it goes. But maybe we > should look at doing that in configure instead of putting the onus on > all buildfarm owners? It already knows if it's using a GNU compiler, not > sure how ubiquitous the -ansi and -std=c99 flags are. No, the only reason either of us are doing that is to force use of a flag that's different from what configure would select by default (which evidently is -std=gnu99 for gcc). Most buildfarm owners have no need to do anything. regards, tom lane
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Hi, On 2018-08-24 14:09:09 -0400, Andrew Dunstan wrote: > I have installed VS2017 on bowerbird and a test is currently running. It's > got past the make phase so I assume everything is kosher. Cool, thanks. > However, we only support VS2017 down to 9.6 and Vs2015 down to 9.5. Perhaps > we should consider backpatching support for those down to 9.3. Hm, I have no strong objections to that. I don't think it's strictly necessary, given 2013 is supported across the board, but for the non MSVC world, we do fix compiler issues in older branches. There's not that much code for the newer versions afaict? Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 08/24/2018 11:46 AM, Andres Freund wrote: Hi, On 2018-08-23 18:44:34 -0700, Andres Freund wrote: Pushed the first two. Seems to have worked like expected. I'll send the presumably affected buildfarm owners an email, asking them whether they want to update. Did that. I have installed VS2017 on bowerbird and a test is currently running. It's got past the make phase so I assume everything is kosher. However, we only support VS2017 down to 9.6 and Vs2015 down to 9.5. Perhaps we should consider backpatching support for those down to 9.3. If not, I will just restrict bowerbird to building 9.6 and up. That's fine by me, we'll still have coverage from, say, currawong, but it's a pity we'll only be able to support the older compilers on the old branches. Andrew, as expected my buildfarm animal mylodon, which uses compiler flags to enforce C89 compliance, failed due to this commit: https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=mylodon=HEAD I'd like to change it so it doesn't enforce C89 compliance across the board, but instead enforces the relevant standard. For that I'd need to change CFLAGS per-branch in the buildfarm. Is that possible already? Do I need two different config files? I saw Tom's answer, and it will work as far as it goes. But maybe we should look at doing that in configure instead of putting the onus on all buildfarm owners? It already knows if it's using a GNU compiler, not sure how ubiquitous the -ansi and -std=c99 flags are. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 2018-08-24 12:10:34 -0400, Tom Lane wrote: > Andres Freund writes: > > I'd like to change it so it doesn't enforce C89 compliance across the > > board, but instead enforces the relevant standard. For that I'd need to > > change CFLAGS per-branch in the buildfarm. Is that possible already? Do > > I need two different config files? > > I just did that on dromedary, with a stanza like this at the bottom: > > if ($branch eq 'HEAD' or $branch ge 'REL_12') > { > $conf{config_env}->{CC} = 'ccache gcc -std=c99'; > } > else > { > $conf{config_env}->{CC} = 'ccache gcc -ansi'; > } Thanks, did something similar, mylodon should become green soon. I kinda was hoping that CFLAGS would directly accept version specific like some other vars directly... Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Andres Freund writes: > I'd like to change it so it doesn't enforce C89 compliance across the > board, but instead enforces the relevant standard. For that I'd need to > change CFLAGS per-branch in the buildfarm. Is that possible already? Do > I need two different config files? I just did that on dromedary, with a stanza like this at the bottom: if ($branch eq 'HEAD' or $branch ge 'REL_12') { $conf{config_env}->{CC} = 'ccache gcc -std=c99'; } else { $conf{config_env}->{CC} = 'ccache gcc -ansi'; } regards, tom lane
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Hi, On 2018-08-23 18:44:34 -0700, Andres Freund wrote: > Pushed the first two. Seems to have worked like expected. > I'll send the presumably affected buildfarm owners an email, asking > them whether they want to update. Did that. Andrew, as expected my buildfarm animal mylodon, which uses compiler flags to enforce C89 compliance, failed due to this commit: https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=mylodon=HEAD I'd like to change it so it doesn't enforce C89 compliance across the board, but instead enforces the relevant standard. For that I'd need to change CFLAGS per-branch in the buildfarm. Is that possible already? Do I need two different config files? Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On Fri, Aug 24, 2018 at 1:44 PM, Andres Freund wrote: > On 2018-08-23 22:02:26 +0200, Peter Eisentraut wrote: >> On 23/08/2018 03:31, Andres Freund wrote: >> > On 2018-08-22 17:09:05 -0700, Andres Freund wrote: >> >> Attached is a version doing so. >> > >> > Mildly updated version attached. This adds an explanatory commit >> > message, removes one stray docs C89 reference, and fixes a mis-squashing >> > of a patch. >> >> Let's do it! > > Pushed the first two. I'll send the presumably affected buildfarm > owners an email, asking them whether they want to update. Note that designated initialisers are not in C++ yet (though IIUC they have been accepted in P0329R4[1] for the draft that will become C++20 whatever it turns out to be). [1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4691.html -- Thomas Munro http://www.enterprisedb.com
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 2018-08-23 22:02:26 +0200, Peter Eisentraut wrote: > On 23/08/2018 03:31, Andres Freund wrote: > > On 2018-08-22 17:09:05 -0700, Andres Freund wrote: > >> Attached is a version doing so. > > > > Mildly updated version attached. This adds an explanatory commit > > message, removes one stray docs C89 reference, and fixes a mis-squashing > > of a patch. > > Let's do it! Pushed the first two. I'll send the presumably affected buildfarm owners an email, asking them whether they want to update. Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 23/08/2018 03:31, Andres Freund wrote: > On 2018-08-22 17:09:05 -0700, Andres Freund wrote: >> Attached is a version doing so. > > Mildly updated version attached. This adds an explanatory commit > message, removes one stray docs C89 reference, and fixes a mis-squashing > of a patch. Let's do it! -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 2018-08-22 17:09:05 -0700, Andres Freund wrote: > Attached is a version doing so. Mildly updated version attached. This adds an explanatory commit message, removes one stray docs C89 reference, and fixes a mis-squashing of a patch. Greetings, Andres Freund >From 4b63f3307180a778018439ad3dee9a89aa4f4352 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 22 Aug 2018 15:10:23 -0700 Subject: [PATCH v2 1/4] Require C99 (and thus MSCV 2013 upwards). In 86d78ef50e01 I enabled configure to check for C99 support, with the goal of checking which platforms support C99. While there are a few machines without C99 support, de-supporting them for v12 was deemed acceptable. While not tested in aforementioned commit, the biggest increase in minimum version comes from MSVC, which gained C99 support fairly late. The subset in MSVC 2013 is sufficient for our needs, at this point. While that is a significant increase in minimum version, the existing windows binaries are already built with a new enough version. Make configure error out if C99 support could not be detected. For MSVC builds, increase the minimum version to 2013. The increase to MSVC 2013 allows us to get rid of VCBuildProject.pm, as that was only required for MSVC 2005/2008. Author: Andres Freund Discussion: https://postgr.es/m/97d4b165-192d-3605-749c-f614a0c4e...@2ndquadrant.com --- configure | 49 + configure.in | 10 + doc/src/sgml/install-windows.sgml | 26 +-- doc/src/sgml/installation.sgml| 2 +- doc/src/sgml/sources.sgml | 33 ++-- src/tools/msvc/MSBuildProject.pm | 78 +--- src/tools/msvc/README | 18 +- src/tools/msvc/Solution.pm| 102 -- src/tools/msvc/VCBuildProject.pm | 309 -- src/tools/msvc/VSObjectFactory.pm | 37 +--- src/tools/msvc/build.pl | 6 +- 11 files changed, 112 insertions(+), 558 deletions(-) delete mode 100644 src/tools/msvc/VCBuildProject.pm diff --git a/configure b/configure index 836d68dad37..dd439ddd2f6 100755 --- a/configure +++ b/configure @@ -4602,6 +4602,13 @@ if test "x$ac_cv_prog_cc_c99" != xno; then : fi + +# Error out if the compiler does not support C99, as the codebase +# relies on that. +if test "$ac_cv_prog_cc_c99" = no; then +as_fn_error $? "C compiler \"$CC\" does not support C99" "$LINENO" 5 +fi + ac_ext=cpp ac_cpp='$CXXCPP $CPPFLAGS' ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' @@ -5361,6 +5368,48 @@ fi # -Wdeclaration-after-statement isn't applicable for C++ + # Really don't want VLAs to be used in our dialect of C + +{ $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; } +if ${pgac_cv_prog_CC_cflags__Werror_vla+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_save_CFLAGS=$CFLAGS +pgac_save_CC=$CC +CC=${CC} +CFLAGS="${CFLAGS} -Werror=vla" +ac_save_c_werror_flag=$ac_c_werror_flag +ac_c_werror_flag=yes +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + pgac_cv_prog_CC_cflags__Werror_vla=yes +else + pgac_cv_prog_CC_cflags__Werror_vla=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +ac_c_werror_flag=$ac_save_c_werror_flag +CFLAGS="$pgac_save_CFLAGS" +CC="$pgac_save_CC" +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__Werror_vla" >&5 +$as_echo "$pgac_cv_prog_CC_cflags__Werror_vla" >&6; } +if test x"$pgac_cv_prog_CC_cflags__Werror_vla" = x"yes"; then + CFLAGS="${CFLAGS} -Werror=vla" +fi + + + # -Wvla is not applicable for C++ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wendif-labels, for CFLAGS" >&5 $as_echo_n "checking whether ${CC} supports -Wendif-labels, for CFLAGS... " >&6; } diff --git a/configure.in b/configure.in index 6e141064e5c..5869ab7c5bc 100644 --- a/configure.in +++ b/configure.in @@ -359,6 +359,13 @@ esac AC_PROG_CC([$pgac_cc_list]) AC_PROG_CC_C99() + +# Error out if the compiler does not support C99, as the codebase +# relies on that. +if test "$ac_cv_prog_cc_c99" = no; then +AC_MSG_ERROR([C compiler "$CC" does not support C99]) +fi + AC_PROG_CXX([$pgac_cxx_list]) # Check if it's Intel's compiler, which (usually) pretends to be gcc, @@ -477,6 +484,9 @@ if test "$GCC" = yes -a "$ICC" = no; then # These work in some but not all gcc versions PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement]) # -Wdeclaration-after-statement isn't applicable for C++ + # Really don't want VLAs to be used in our dialect of C + PGAC_PROG_CC_CFLAGS_OPT([-Werror=vla]) + # -Wvla is not applicable for C++ PGAC_PROG_CC_CFLAGS_OPT([-Wendif-labels]) PGAC_PROG_CXX_CFLAGS_OPT([-Wendif-labels]) PGAC_PROG_CC_CFLAGS_OPT([-Wmissing-format-attribute]) diff --git
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Hi, On 2018-08-22 20:16:24 -0400, Tom Lane wrote: > Andres Freund writes: > > There's a few further potential cleanups due to relying on c99: > > - Use __func__ unconditionally, rather than having configure test for it > > - Use inline unconditionally, rather than having configure test for it > > - Remove tests for AC_TYPE_INTPTR_T, AC_TYPE_UINTPTR_T, > > AC_TYPE_LONG_LONG_INT, we can rely on them being present. > > - probably more in that vein > > I wouldn't be in too much of a hurry to do that, particularly not the > third item. You are confusing "compiler is c99" with "system headers > are c99". Moreover, I don't see that we're buying much with such > changes. Yea, I am not in much of a hurry on any of them. I think the only argument for them is that it'd buy us a littlebit of a reduction in configure runtime... Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Andres Freund writes: > There's a few further potential cleanups due to relying on c99: > - Use __func__ unconditionally, rather than having configure test for it > - Use inline unconditionally, rather than having configure test for it > - Remove tests for AC_TYPE_INTPTR_T, AC_TYPE_UINTPTR_T, > AC_TYPE_LONG_LONG_INT, we can rely on them being present. > - probably more in that vein I wouldn't be in too much of a hurry to do that, particularly not the third item. You are confusing "compiler is c99" with "system headers are c99". Moreover, I don't see that we're buying much with such changes. regards, tom lane
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Hi, On 2018-08-22 05:02:11 -0700, Andres Freund wrote: > If we agree on that, I'm going to propose a patch that includes: > - relevant cleanups to configure > - adapts sources.sgml to refer to C99 instead of C89 > - add some trivial conversions to for(int i;;) and struct initializers, > so the relevant old animals fail > - adds a configure check to enable errors with vla usage (-Werror=vla) > > Questions: > > - do we want to make declarations at arbitrary points errors? It's > already a warning currently. > - other new restrictions that we want to introduce at the same time? Attached is a version doing so. Turns out ripping < msvc 2010 support allows us to get rid of a fair bit of code. Using appveyor I tested that I didn't bungle things too badly. But I don't really know what I'm doing in that area, Andrew or Craig, your input would be appreciated. I tried to update sources.sgml to reflect my understanding of the subset of C99 we're going to use. I'd rather start more restrictive and then argue about relaxing things separately. There's a few further potential cleanups due to relying on c99: - Use __func__ unconditionally, rather than having configure test for it - Use inline unconditionally, rather than having configure test for it - Remove tests for AC_TYPE_INTPTR_T, AC_TYPE_UINTPTR_T, AC_TYPE_LONG_LONG_INT, we can rely on them being present. - probably more in that vein I'd rather do these separately lateron, in case one of them causes trouble on some animals. There's some potential ones I think we should *not* do: - remove AC_C_FLEXIBLE_ARRAY_MEMBER, and rely on it unconditionally. I'm disinclined to do this, because C++ IIRC doesn't support it in any version, and I don't want to make Peter's life unnecessarily hard. - remove AC_C_RESTRICT check, rely on it unconditionally: MSVC still spells this differently. Greetings, Andres Freund >From c579be2ae7a020d6005d84a5b6f24872ba8b1b05 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 22 Aug 2018 15:10:23 -0700 Subject: [PATCH v1 1/4] Require C99 (and thus MSCV 2013 upwards). Author: Andres Freund Discussion: https://postgr.es/m/97d4b165-192d-3605-749c-f614a0c4e...@2ndquadrant.com --- configure| 49 configure.in | 10 +++ doc/src/sgml/sources.sgml| 33 ++--- src/tools/msvc/MSBuildProject.pm | 2 +- src/tools/msvc/README| 6 ++-- src/tools/msvc/build.pl | 6 +--- 6 files changed, 86 insertions(+), 20 deletions(-) diff --git a/configure b/configure index 836d68dad37..dd439ddd2f6 100755 --- a/configure +++ b/configure @@ -4602,6 +4602,13 @@ if test "x$ac_cv_prog_cc_c99" != xno; then : fi + +# Error out if the compiler does not support C99, as the codebase +# relies on that. +if test "$ac_cv_prog_cc_c99" = no; then +as_fn_error $? "C compiler \"$CC\" does not support C99" "$LINENO" 5 +fi + ac_ext=cpp ac_cpp='$CXXCPP $CPPFLAGS' ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' @@ -5361,6 +5368,48 @@ fi # -Wdeclaration-after-statement isn't applicable for C++ + # Really don't want VLAs to be used in our dialect of C + +{ $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; } +if ${pgac_cv_prog_CC_cflags__Werror_vla+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_save_CFLAGS=$CFLAGS +pgac_save_CC=$CC +CC=${CC} +CFLAGS="${CFLAGS} -Werror=vla" +ac_save_c_werror_flag=$ac_c_werror_flag +ac_c_werror_flag=yes +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + pgac_cv_prog_CC_cflags__Werror_vla=yes +else + pgac_cv_prog_CC_cflags__Werror_vla=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +ac_c_werror_flag=$ac_save_c_werror_flag +CFLAGS="$pgac_save_CFLAGS" +CC="$pgac_save_CC" +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__Werror_vla" >&5 +$as_echo "$pgac_cv_prog_CC_cflags__Werror_vla" >&6; } +if test x"$pgac_cv_prog_CC_cflags__Werror_vla" = x"yes"; then + CFLAGS="${CFLAGS} -Werror=vla" +fi + + + # -Wvla is not applicable for C++ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wendif-labels, for CFLAGS" >&5 $as_echo_n "checking whether ${CC} supports -Wendif-labels, for CFLAGS... " >&6; } diff --git a/configure.in b/configure.in index 6e141064e5c..5869ab7c5bc 100644 --- a/configure.in +++ b/configure.in @@ -359,6 +359,13 @@ esac AC_PROG_CC([$pgac_cc_list]) AC_PROG_CC_C99() + +# Error out if the compiler does not support C99, as the codebase +# relies on that. +if test "$ac_cv_prog_cc_c99" = no; then +AC_MSG_ERROR([C compiler "$CC" does not support C99]) +fi + AC_PROG_CXX([$pgac_cxx_list]) # Check if it's Intel's compiler, which
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 8/22/18 10:56 AM, Peter Eisentraut wrote: > On 22/08/2018 14:02, Andres Freund wrote: >> If we agree on that, I'm going to propose a patch that includes: >> - relevant cleanups to configure >> - adapts sources.sgml to refer to C99 instead of C89 >> - add some trivial conversions to for(int i;;) and struct initializers, >> so the relevant old animals fail >> - adds a configure check to enable errors with vla usage (-Werror=vla) > > sounds good Sounds good to me. > >> - do we want to make declarations at arbitrary points errors? It's >> already a warning currently. > > While there are legitimate criticisms, it's a standard feature in C, > C++, and many other languages, so I don't see what we'd gain by fighting it. +1.= -- -David da...@pgmasters.net
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Hi, On 2018-08-22 16:56:15 +0200, Peter Eisentraut wrote: > On 22/08/2018 14:02, Andres Freund wrote: > > - do we want to make declarations at arbitrary points errors? It's > > already a warning currently. > > While there are legitimate criticisms, it's a standard feature in C, > C++, and many other languages, so I don't see what we'd gain by fighting it. I personally don't really care - for C there's really not much of a difference (contrast to C++ with RAII). But Robert and Tom both said this would be an issue with moving to C99 for them. I don't want to hold up making progress here by fighting over this issue. Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 22/08/2018 14:02, Andres Freund wrote: > If we agree on that, I'm going to propose a patch that includes: > - relevant cleanups to configure > - adapts sources.sgml to refer to C99 instead of C89 > - add some trivial conversions to for(int i;;) and struct initializers, > so the relevant old animals fail > - adds a configure check to enable errors with vla usage (-Werror=vla) sounds good > - do we want to make declarations at arbitrary points errors? It's > already a warning currently. While there are legitimate criticisms, it's a standard feature in C, C++, and many other languages, so I don't see what we'd gain by fighting it. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On Wed, Aug 22, 2018 at 5:32 PM, Andres Freund wrote: > Hi, > > On 2018-08-22 17:17:27 +0530, Sandeep Thakkar wrote: > > > We build windows binaries (>=9.3) on Windows 7 and Windows Server 2012 > R2. > > For 9.3, the Visual Studio version is 2010 and for 9.4 and v10, we use > > 2013. For v11, we use 2017. > > Sndeep: Thanks for the information. Did you ever encounter problems (at > build or during runtime) with using those binaries on older platforms? > > IIRC when the binaries were built with VC++ 2013 on 9.4, we had problems running them on XP and hence we had used "/p:PlatformToolset=v120_xp" option to msbuild during build time. From v10, we stopped using that toolset and instead used the default one i.e v120 > Everyone: Given the fact that all the people building windows packages > currently use a new enough stack by a fair margin, I think we should > conclude that there's no obstacle on the windows side of things. > > > If we agree on that, I'm going to propose a patch that includes: > - relevant cleanups to configure > - adapts sources.sgml to refer to C99 instead of C89 > - add some trivial conversions to for(int i;;) and struct initializers, > so the relevant old animals fail > - adds a configure check to enable errors with vla usage (-Werror=vla) > > Questions: > > - do we want to make declarations at arbitrary points errors? It's > already a warning currently. > - other new restrictions that we want to introduce at the same time? > > Greetings, > > Andres Freund > -- Sandeep Thakkar
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Hi, On 2018-08-22 17:17:27 +0530, Sandeep Thakkar wrote: > > We build windows binaries (>=9.3) on Windows 7 and Windows Server 2012 R2. > For 9.3, the Visual Studio version is 2010 and for 9.4 and v10, we use > 2013. For v11, we use 2017. Sndeep: Thanks for the information. Did you ever encounter problems (at build or during runtime) with using those binaries on older platforms? Everyone: Given the fact that all the people building windows packages currently use a new enough stack by a fair margin, I think we should conclude that there's no obstacle on the windows side of things. If we agree on that, I'm going to propose a patch that includes: - relevant cleanups to configure - adapts sources.sgml to refer to C99 instead of C89 - add some trivial conversions to for(int i;;) and struct initializers, so the relevant old animals fail - adds a configure check to enable errors with vla usage (-Werror=vla) Questions: - do we want to make declarations at arbitrary points errors? It's already a warning currently. - other new restrictions that we want to introduce at the same time? Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On Wed, Aug 22, 2018 at 4:59 AM, Andres Freund wrote: > On 2018-08-21 17:58:00 -0400, Andrew Dunstan wrote: > > > > > > On 08/21/2018 04:49 PM, Andres Freund wrote: > > > On 2018-08-21 11:09:15 -0700, Joshua D. Drake wrote: > > > > On 08/21/2018 11:06 AM, Andrew Dunstan wrote: > > > > > > > > > > > > > > > XP at least is essentially a dead platform for us. My animals are > not > > > > > able to build anything after release 10. > > > > I wouldn't think XP should even be on our list anymore. Microsoft > hasn't > > > > supported it in 4 years. > > > XP isn't the only thing relevant here, vista and 2008 R1 are in the > same > > > class. > > > > > > > > > I do have a machine in my laptop graveyard with Vista. The only WS2008 > > instace I have available is R2 and AWS doesn't seem to have any AMIs for > R1. > > > > Honestly, I don't think these matter terribly much. Anyone building now > is > > not likely to be targeting them. > > I agree, I think we should just decree that the minimum is MSVC 2013 and > that people building 12 need to deal with that. I would personally > *additionally* would say that we officially don't support *running* (not > compiling) on XP, 2003, 2008R1 and Vista (all unsupported by MS) - but > that's a somewhat orthogonal decision. > > We build windows binaries (>=9.3) on Windows 7 and Windows Server 2012 R2. For 9.3, the Visual Studio version is 2010 and for 9.4 and v10, we use 2013. For v11, we use 2017. > Greetings, > > Andres Freund > -- Sandeep Thakkar
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On Tue, Aug 21, 2018 at 11:16 PM, Tom Lane wrote: > Andres Freund writes: >> On 2018-08-21 13:29:20 -0400, Tom Lane wrote: >>> We've got a buildfarm handy that could answer the question. >>> Let's just stick a test function in there for a day and see >>> which animals fail. > >> I think we pretty much know the answer already, anything before 2013 >> will fail. > > Do we know that for sure? I thought it was theoretical. > I have MSVC 2010 on my Windows 7 VM where I have tried the C99 constructs provided by Peter E. and below are my findings: Tried compiling below function in one of the .c files in the src/backend --- int bar() { int x; x = 1; int y; y = 2; for (int i = 0; i < 5; i++) ; return 0; } error C2143: syntax error : missing ';' before 'type' error C2065: 'y' : undeclared identifier error C2143: syntax error : missing ';' before 'type' error C2143: syntax error : missing ';' before 'type' error C2143: syntax error : missing ')' before 'type' error C2143: syntax error : missing ';' before 'type' error C2065: 'i' : undeclared identifier warning C4552: '<' : operator has no effect; expected operator with side-effect error C2065: 'i' : undeclared identifier error C2059: syntax error : ')' Tried compiling below struct in one of the .c files in the src/backend --- struct foo { int a; int b; }; struct foo f = { .a = 1, .b = 2 }; error C2059: syntax error : '.' I have also tried compiling above in standalone console application project in MSVC 2010 and able to compile the function successfully, but struct is giving the same error as above. So, it seems to me that it won't work on <= MSVC 2010. I am personally okay with upgrading my Windows VM. I have found a couple of links which suggest that MSVC 2015 has a good amount of conformance with C99 [1]. It seems MSVC 2013 also has decent support for C99 [2], but I am not able to verify if the constructs shared by Peter E can be compiled on it. [1] https://social.msdn.microsoft.com/Forums/en-US/fa17bcdd-7165-4645-a676-ef3797b95918/details-on-c99-support-in-msvc?forum=vcgeneral [2] https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/ -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 2018-08-21 17:58:00 -0400, Andrew Dunstan wrote: > > > On 08/21/2018 04:49 PM, Andres Freund wrote: > > On 2018-08-21 11:09:15 -0700, Joshua D. Drake wrote: > > > On 08/21/2018 11:06 AM, Andrew Dunstan wrote: > > > > > > > > > > > > XP at least is essentially a dead platform for us. My animals are not > > > > able to build anything after release 10. > > > I wouldn't think XP should even be on our list anymore. Microsoft hasn't > > > supported it in 4 years. > > XP isn't the only thing relevant here, vista and 2008 R1 are in the same > > class. > > > > > I do have a machine in my laptop graveyard with Vista. The only WS2008 > instace I have available is R2 and AWS doesn't seem to have any AMIs for R1. > > Honestly, I don't think these matter terribly much. Anyone building now is > not likely to be targeting them. I agree, I think we should just decree that the minimum is MSVC 2013 and that people building 12 need to deal with that. I would personally *additionally* would say that we officially don't support *running* (not compiling) on XP, 2003, 2008R1 and Vista (all unsupported by MS) - but that's a somewhat orthogonal decision. Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 08/21/2018 04:49 PM, Andres Freund wrote: On 2018-08-21 11:09:15 -0700, Joshua D. Drake wrote: On 08/21/2018 11:06 AM, Andrew Dunstan wrote: XP at least is essentially a dead platform for us. My animals are not able to build anything after release 10. I wouldn't think XP should even be on our list anymore. Microsoft hasn't supported it in 4 years. XP isn't the only thing relevant here, vista and 2008 R1 are in the same class. I do have a machine in my laptop graveyard with Vista. The only WS2008 instace I have available is R2 and AWS doesn't seem to have any AMIs for R1. Honestly, I don't think these matter terribly much. Anyone building now is not likely to be targeting them. Of the buildfarm animals, dory looks OK, hamerkop, bowerbird and thrips look not ok, and whelk and woodlouse look borderline. I'm perfectly prepared to upgrade bowerbird if necessary. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 2018-08-21 11:09:15 -0700, Joshua D. Drake wrote: > On 08/21/2018 11:06 AM, Andrew Dunstan wrote: > > > > > > > > XP at least is essentially a dead platform for us. My animals are not > > able to build anything after release 10. > > I wouldn't think XP should even be on our list anymore. Microsoft hasn't > supported it in 4 years. XP isn't the only thing relevant here, vista and 2008 R1 are in the same class. Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 2018-08-21 14:06:18 -0400, Andrew Dunstan wrote: > > > On 08/21/2018 01:31 PM, Andres Freund wrote: > > Hi, > > > > On 2018-08-21 13:29:20 -0400, Tom Lane wrote: > > > Peter Eisentraut writes: > > > > So, does anyone with Windows build experience want to comment on this? > > > > The proposal is to desupport anything older than (probably) MSVC 2013, > > > > or alternatively anything that cannot compile the attached test file. > > > We've got a buildfarm handy that could answer the question. > > > Let's just stick a test function in there for a day and see > > > which animals fail. > > I think we pretty much know the answer already, anything before 2013 > > will fail. The question is more whether that's problematic for the > > people building on windows. My theory, quoted by Peter upthread, is > > that it shouldn't be problematic because 2013 can build binaries that > > run on XP etc. > XP at least is essentially a dead platform for us. My animals are not able > to build anything after release 10. I'm perfectly fine with that, FWIW. It's out of extended support for several years now. But my point was that you can newer versions of MSVC to build things that run on XP (and more relevantly 2008, vista, 7 etc). No idea if we'd need to change anything in our build infra for that. Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 08/21/2018 11:06 AM, Andrew Dunstan wrote: XP at least is essentially a dead platform for us. My animals are not able to build anything after release 10. I wouldn't think XP should even be on our list anymore. Microsoft hasn't supported it in 4 years. JD -- Command Prompt, Inc. || http://the.postgres.company/ || @cmdpromptinc *** A fault and talent of mine is to tell it exactly how it is. *** PostgreSQL centered full stack support, consulting and development. Advocate: @amplifypostgres || Learn: https://postgresconf.org * Unless otherwise stated, opinions are my own. *
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 08/21/2018 01:31 PM, Andres Freund wrote: Hi, On 2018-08-21 13:29:20 -0400, Tom Lane wrote: Peter Eisentraut writes: So, does anyone with Windows build experience want to comment on this? The proposal is to desupport anything older than (probably) MSVC 2013, or alternatively anything that cannot compile the attached test file. We've got a buildfarm handy that could answer the question. Let's just stick a test function in there for a day and see which animals fail. I think we pretty much know the answer already, anything before 2013 will fail. The question is more whether that's problematic for the people building on windows. My theory, quoted by Peter upthread, is that it shouldn't be problematic because 2013 can build binaries that run on XP etc. XP at least is essentially a dead platform for us. My animals are not able to build anything after release 10. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 08/21/2018 01:46 PM, Tom Lane wrote: > Andres Freund writes: >> On 2018-08-21 13:29:20 -0400, Tom Lane wrote: >>> We've got a buildfarm handy that could answer the question. >>> Let's just stick a test function in there for a day and see >>> which animals fail. > >> I think we pretty much know the answer already, anything before 2013 >> will fail. > > Do we know that for sure? I thought it was theoretical. I thought I remembered a message where it had been looked up in docs, but I think the one I was remembering was Peter's "According to my research (completely untested in practice), you need 2010 for mixed code and declarations and 2013 for named initialization of structs." [1] which didn't quite actually say it was documented. -Chap [1] https://www.postgresql.org/message-id/ef986aa7-c7ca-ec34-19d9-fef38716b109%402ndquadrant.com
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Hi, On 2018-08-21 13:46:10 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2018-08-21 13:29:20 -0400, Tom Lane wrote: > >> We've got a buildfarm handy that could answer the question. > >> Let's just stick a test function in there for a day and see > >> which animals fail. > > > I think we pretty much know the answer already, anything before 2013 > > will fail. > > Do we know that for sure? I thought it was theoretical. Pretty much. I'm on mobile data so I don't want to search too much, but I've previously looked it up, and designated initializer support was introduced in 2013. See e.g. the graph in https://blogs.msdn.microsoft.com/somasegar/2013/06/28/c-conformance-roadmap/ Greetings, Andres Freund
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Andres Freund writes: > On 2018-08-21 13:29:20 -0400, Tom Lane wrote: >> We've got a buildfarm handy that could answer the question. >> Let's just stick a test function in there for a day and see >> which animals fail. > I think we pretty much know the answer already, anything before 2013 > will fail. Do we know that for sure? I thought it was theoretical. regards, tom lane
Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
Peter Eisentraut writes: > So, does anyone with Windows build experience want to comment on this? > The proposal is to desupport anything older than (probably) MSVC 2013, > or alternatively anything that cannot compile the attached test file. We've got a buildfarm handy that could answer the question. Let's just stick a test function in there for a day and see which animals fail. regards, tom lane
Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)
On 16/08/2018 15:00, Andres Freund wrote: >> According to my research (completely untested in practice), you need >> 2010 for mixed code and declarations and 2013 for named initialization >> of structs. >> >> I wonder what raising the msvc requirement would imply for supporting >> older Windows versions. > One relevant tidbit is that afaict 2013 still allows *targeting* older > versions of windows, down to XP and 2003, while requiring a newer > platforms to run. See: > https://docs.microsoft.com/en-us/visualstudio/productinfo/vs2013-compatibility-vs > I don't know if that's hard to do, but I strongly suspect that the > existing installers already do that (otherwise supporting newer versions > would likely require separate builds). So, does anyone with Windows build experience want to comment on this? The proposal is to desupport anything older than (probably) MSVC 2013, or alternatively anything that cannot compile the attached test file. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services struct foo { int a; int b; }; struct foo f = { .a = 1, .b = 2, }; int bar() { int x; x = 1; int y; y = 2; for (int i = 0; i < 5; i++) ; return 0; }