Re: Add PGDLLEXPORT to PG_FUNCTION_INFO_V1

Tom Lane wrote:
> Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> writes:
>> Currently, PG_FUNCTION_INFO_V1 is defined as
[...]
>
>> Is there a good reason why the "funcname" declaration is not decorated
>> with PGDLLEXPORT?
>
> The lack of complaints about this suggest that it's not actually necessary
> to do so. So what this makes me wonder is whether we can't drop the
> DLLEXPORT on the finfo function too. I'd rather reduce the number of
> Microsoft-isms in the code, not increase it.

I understand the sentiment.

But it seems to actually cause a problem on Windows, at least there was a
complaint here: http://stackoverflow.com/q/39964233

Adding PGDLLEXPORT solved the problem there.

I guess that there are not more complaints about that because
few people build C functions on Windows themselves (lack of PGXS)
and those who do are probably knowledgeable enough that they can
fix it themselves by sticking an extra declaration with PGDLLEXPORT
into their source file.

PostgreSQL itself seems to use export files that explicitly declare the
exported symbols, so it gets away without these decorations.

>> BTW, I admit I don't understand the comment.
>
> It seems like an obvious typo. Or, possibly, sloppy thinking that
> contributed to failure to recognize that the keyword isn't needed.

Looking through the history, it seems to be as follows:
- In 5fde8613, the comment was added (mistakenly) together with a DLLIMPORT decoration.
- In 77e50a61, the decoration was changed to PGDLLEXPORT, but the comment forgotten.
- In e7128e8d, the function prototype was added to the macro, but
the PGDLLEXPORT decoration was forgotten.

The dlltool mentioned is MinGW, so it doesn't apply to people building
with MSVC.

Maybe the comment should be like this:

/*
* Macro to build an info function associated with the given function name.
* Win32 loadable functions usually link with 'dlltool --export-all' or use
* a .DEF file, but it doesn't hurt to add PGDLLEXPORT in case they don't.
*/

Yours,
Laurenz Albe