Re: typedef struct LogicalDecodingContext

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: typedef struct LogicalDecodingContext
Date: 2023-03-02 01:15:34
Message-ID: 1789787.1677719734@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Peter Smith <smithpb2250(at)gmail(dot)com> writes:
> On Thu, Mar 2, 2023 at 10:04 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Sadly, this is almost certainly going to cause bitching on the part of
>> some compilers, because depending on the order of header inclusions
>> they are going to see multiple typedefs for the same name. Redundant
>> "struct foo" declarations are portable C, but redundant "typedef foo"
>> not so much.

> So does your reply mean there is no way really to be sure if such
> changes are OK or not, other than to push them and then revert them
> if/when one of the BF animals complains?

We know which compilers don't like that, I believe, but you'd have
to dig in the commit log or mail archives to find out.

[ ... pokes around ... ] The commit log entries I could find about
this suggest that (at least) older gcc versions complain. Maybe
those are all gone from the buildfarm now, but I wouldn't bet on it.
We were fixing this sort of thing as recently as aa3ac6453.

>> I also wonder if this passes headerscheck and cpluspluscheck.

> FWIW, both those tests passed OK. What does "pass" even mean -- does
> it confirm this patch doesn't suffer the multiple typedef problem you
> anticipated after all?

No, those have nothing to do with duplicate typedefs. headerscheck is
about whether anything is dependent on inclusion order, which I wondered
about for this patch. cpluspluscheck is about whether C++ compilers will
spit up on any of our headers (due to, eg, identifiers that are C++
keywords); we try to keep them clean for the benefit of people who write
extensions in C++. I wouldn't have expected cpluspluscheck to show
anything new with this patch, but people tend to always run these tools
together.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2023-03-02 01:20:58 Re: We shouldn't signal process groups with SIGQUIT
Previous Message Michael Paquier 2023-03-02 01:03:59 Re: add PROCESS_MAIN to VACUUM