Re: Preliminary results for proposed new pgindent implementation

On 2017-06-17 21:55, Tom Lane wrote:
> I spent some time looking into this. I reverted your commits
> 198457848ae5c86bec3336a9437dd5aa30f480c2 (Replace err.h functions with
> standard C equivalents) and fb10acb040b90bdcbad09defd303363db29257d1
> (Remove inclusion of sys/cdefs.h) locally and tried to build without
> those.

I wanted to mirror that move, but forgot to not rebase the repository,
so I removed those two commits instead of committing their negatives.
Sorry about that.

> I've successfully worked around the err.h change by adding
> cut-down versions of FreeBSD 11's err.h and err.c to the fileset
> (see attached).

I thought about something like:
#ifdef __FreeBSD__
#include <err.h>
#define ERR(...) err(__VA_ARGS__)
#define ERRX(...) errx(__VA_ARGS__)
#else
#include "err.h"
#endif

and then call ERR() and ERRX() instead of err() and errx(). But that
requires C99. And I would have a very hard time convincing anyone that
it makes any sense from FreeBSD's perspective, since indent is part of
the base system, where <err.h> is guaranteed to exist.

Perhaps it would be best for everyone if indent was moved out of FreeBSD
base, so that portability arguments would make more sense. But that
would take time and some debate.

> However, it's proving impossible to work around having
> "#include <sys/cdefs.h>" as the first live code in the files. I thought
> maybe we could provide a dummy cdefs.h file, but that breaks things on
> platforms where cdefs.h is a real thing and is relied on by other system
> headers --- which includes both Linux and BSD. It seems we would have
> to have something like #ifdef HAVE_SYS_CDEFS_H, but that is already a
> departure from FreeBSD practice.

I was thinking if I could get away with putting those into #ifdef
__FreeBSD__ ... #endif. I think that it might be feasible unlike the
idea above. I could be wrong.

> So what I'm currently thinking is that we have to diverge from the
> FreeBSD sources to the extent of removing #include <sys/cdefs.h>
> and the __FBSDID() calls, and instead inserting #include "c.h" to
> pick up PG's own portability definitions. The thing that forced me
> into the latter is that there seems no way to avoid compiler warnings
> if we don't decorate the declarations of err() and errx() with noreturn
> and printf-format attributes --- and we need c.h to provide portable
> ways of writing those. But there are probably other portability things
> that we'll need c.h for, anyway, especially if we want to make it work
> on Windows. So I'm thinking this is a small and easily maintainable
> difference from the upstream FreeBSD files.

That works for me.

> When I inserted #include "c.h", I got duplicate-macro-definition warnings
> about "true" and "false", so I would ask you to add this:

Done.