Re: [COMMITTERS] pgsql: Add configure infrastructure to detect support for C99's restric

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-committers(at)postgresql(dot)org, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Add configure infrastructure to detect support for C99's restric
Date: 2017-10-12 04:59:53
Message-ID: 20171012045953.khz4bjdrar5mzrm7@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On 2017-10-11 17:13:20 -0700, Andres Freund wrote:
> Hi,
>
> On 2017-10-11 23:11:15 +0000, Andres Freund wrote:
> > Add configure infrastructure to detect support for C99's restrict.
> >
> > Will be used in later commits improving performance for a few key
> > routines where information about aliasing allows for significantly
> > better code generation.
> >
> > This allows to use the C99 'restrict' keyword without breaking C89, or
> > for that matter C++, compilers. If not supported it's defined to be
> > empty.
>
> Woodlouse doesn't like this, erroring out with:
> C:\buildfarm\buildenv\HEAD\pgsql.build\src\include\libpq/pqformat.h(47): error C2219: syntax error : type qualifier must be after '*' (src/backend/access/common/printsimple.c) [C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj]
>
> It's MSVC being super peculiar about error checks. I think msvc is just
> confused by the pointer hiding typedef. Using some online MSVC (and
> other compilers) frontend:
> https://godbolt.org/g/TD3nmA
>
> I confirmed that removing the pointer hiding typedef indeed resolves the
> isssue.
>
> I can't quite decide whether msvc just has taste and dislikes pointer
> hiding typedefs as much as I do, or whether it's incredibly stupid ;)
>
> I'll add a comment and use StringInfoData *.

That fixed that problem I think. But unfortunately since then another
problem has been reported by some other animals, all with older msvc
versions afaict (thrips - vs 2012, bowerbird - vs 2012). Those report
that the defining of restrict to __restrict interfers with some system
headers:
C:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\include\stdlib.h(619): error C2485: '__restrict' : unrecognized extended attribute (src/backend/access/brin/brin.c) [c:\prog\bf\root\HEAD\pgsql.build\postgres.vcxproj]

Presumably that is because the headers contain __declspec(restrict) on
some function declarations.

I've temporarily silenced that error by moving the stdlib.h include
before the definition of restrict, but that seems fairly fragile. I
primarily wanted to see whether there's other problems. At least thrips
is is now happy.

I see a number of options to continue:
- only define restrict on msvc 2013+ - for some reason woodlouse didn't
complain about this problem, but I'm very doubtful that that's
actually reliable.
- rename restrict to pg_restrict. That's annoying because we'd have to
copy the autoconf test.
- live with the hack of including stdlib.h early, in pg_config.h.win32.
- $better_idea

Comments?

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Andres Freund 2017-10-12 05:05:13 Re: [HACKERS] Re: pgsql: Add configure infrastructure to detect support for C99's restric
Previous Message Andres Freund 2017-10-12 04:01:42 pgsql: Replace remaining uses of pq_sendint with pq_sendint{8, 16, 32}.

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2017-10-12 05:05:13 Re: [HACKERS] Re: pgsql: Add configure infrastructure to detect support for C99's restric
Previous Message Michael Paquier 2017-10-12 04:42:54 Re: BUG #14849: jsonb_build_object doesn't like VARIADIC calls very much