Thread: pgsql: Assume that we have signed integral types and flexible array mem
Assume that we have signed integral types and flexible array members. These compiler features are required by C99, so remove the configure probes for them. This is part of a series of commits to get rid of no-longer-relevant configure checks and dead src/port/ code. I'm committing them separately to make it easier to back out individual changes if they prove less portable than I expect. Discussion: https://postgr.es/m/15379.1582221614@sss.pgh.pa.us Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/f4d59369d2ddf0ad7850112752ec42fd115825d4 Modified Files -------------- config/c-compiler.m4 | 15 ---------- configure | 72 ---------------------------------------------- configure.in | 2 -- src/include/c.h | 10 +++++++ src/include/pg_config.h.in | 12 -------- src/tools/msvc/Solution.pm | 2 -- 6 files changed, 10 insertions(+), 103 deletions(-)
Re: pgsql: Assume that we have signed integral types and flexiblearray mem
From
Peter Geoghegan
Date:
On Fri, Feb 21, 2020 at 11:31 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Assume that we have signed integral types and flexible array members. I see this change to c.h: +/* + * We require C99, hence the compiler should understand flexible array + * members. However, for documentation purposes we still consider it to be + * project style to write "field[FLEXIBLE_ARRAY_MEMBER]" not just "field[]". + * When computing the size of such an object, use "offsetof(struct s, f)" + * for portability. Don't use "offsetof(struct s, f[0])", as this doesn't + * work with MSVC and with C++ compilers. + */ +#define FLEXIBLE_ARRAY_MEMBER /* empty */ Why not just get rid of the FLEXIBLE_ARRAY_MEMBER hack altogether? I don't think that we need it as a way of drawing attention to the fact that "offsetof(struct s, f[0])" should not be used. That's not idiomatic style anyway. If somebody makes this mistake, then I believe that their code will reliably fail to compile once it hits CF Tester or the buildfarm. -- Peter Geoghegan
Peter Geoghegan <pg@bowt.ie> writes:
> I see this change to c.h:
> +/*
> + * We require C99, hence the compiler should understand flexible array
> + * members. However, for documentation purposes we still consider it to be
> + * project style to write "field[FLEXIBLE_ARRAY_MEMBER]" not just "field[]".
> + * When computing the size of such an object, use "offsetof(struct s, f)"
> + * for portability. Don't use "offsetof(struct s, f[0])", as this doesn't
> + * work with MSVC and with C++ compilers.
> + */
> +#define FLEXIBLE_ARRAY_MEMBER /* empty */
> Why not just get rid of the FLEXIBLE_ARRAY_MEMBER hack altogether?
As I said in the comment, I think it's good style.
Even if you disagree, we shouldn't remove the macro, because that will
just gratuitously break third-party code.
> I don't think that we need it as a way of drawing attention to the
> fact that "offsetof(struct s, f[0])" should not be used. That's not
> idiomatic style anyway. If somebody makes this mistake, then I believe
> that their code will reliably fail to compile once it hits CF Tester
> or the buildfarm.
I'm not 100% sure that aspect of the comment is still correct anyway.
I just copied that advice from the Autoconf output --- but it might well
be referring to the behavior of pre-C99 MSVC versions. However, if it
is correct, why are you sure that violating the advice will lead to
a compile error and not to silently-wrong size calculations?
regards, tom lane
Re: pgsql: Assume that we have signed integral types and flexiblearray mem
From
Peter Geoghegan
Date:
On Fri, Feb 21, 2020 at 2:52 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Why not just get rid of the FLEXIBLE_ARRAY_MEMBER hack altogether? > > As I said in the comment, I think it's good style. I disagree. It doesn't seem important enough to make a fuss over, though. > Even if you disagree, we shouldn't remove the macro, because that will > just gratuitously break third-party code. I'm not sure that it would be all that gratuitous myself. But I'd be satisfied if we stopped using FLEXIBLE_ARRAY_MEMBER, while leaving the symbol behind. > I'm not 100% sure that aspect of the comment is still correct anyway. > I just copied that advice from the Autoconf output --- but it might well > be referring to the behavior of pre-C99 MSVC versions. However, if it > is correct, why are you sure that violating the advice will lead to > a compile error and not to silently-wrong size calculations? I'm not sure of that, and should have been more careful in how I worded my remarks. Still, blithely failing there would be an incredibly user hostile thing for MSVC to do. There is no reason to think that this "offsetof(struct s, f[0])" issue is something that the FLEXIBLE_ARRAY_MEMBER hack was ever particularly concerned with. At least, I have been aware of FLEXIBLE_ARRAY_MEMBER but not aware of the possibly portability issue for years. FLEXIBLE_ARRAY_MEMBER was introduced in 2011 to enable the use of C99 flexible arrays on compilers that supported it at the time. -- Peter Geoghegan