Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

2018-09-11 Thread Andres Freund
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)

2018-09-11 Thread Andrew Dunstan




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)

2018-08-24 Thread Andrew Dunstan




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)

2018-08-24 Thread Andrew Dunstan




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)

2018-08-24 Thread Tom Lane
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)

2018-08-24 Thread Tom Lane
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)

2018-08-24 Thread Andres Freund
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)

2018-08-24 Thread Andrew Dunstan




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)

2018-08-24 Thread Andres Freund
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)

2018-08-24 Thread Tom Lane
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)

2018-08-24 Thread Andres Freund
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)

2018-08-23 Thread Thomas Munro
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)

2018-08-23 Thread Andres Freund
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)

2018-08-23 Thread Peter Eisentraut
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)

2018-08-22 Thread Andres Freund
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)

2018-08-22 Thread Andres Freund
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)

2018-08-22 Thread Tom Lane
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)

2018-08-22 Thread Andres Freund
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)

2018-08-22 Thread David Steele
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)

2018-08-22 Thread Andres Freund
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)

2018-08-22 Thread Peter Eisentraut
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)

2018-08-22 Thread Sandeep Thakkar
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)

2018-08-22 Thread Andres Freund
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)

2018-08-22 Thread Sandeep Thakkar
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)

2018-08-22 Thread Amit Kapila
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)

2018-08-21 Thread Andres Freund
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)

2018-08-21 Thread Andrew Dunstan




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)

2018-08-21 Thread Andres Freund
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)

2018-08-21 Thread Andres Freund
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)

2018-08-21 Thread Joshua D. Drake

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)

2018-08-21 Thread Andrew Dunstan




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)

2018-08-21 Thread Chapman Flack
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)

2018-08-21 Thread Andres Freund
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)

2018-08-21 Thread Tom Lane
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)

2018-08-21 Thread Tom Lane
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)

2018-08-21 Thread Peter Eisentraut
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;
}