Message-ID: <69d9824f-00ea-49b6-b68d-96f0806ec454@iki.fi>
Date: Fri, 10 May 2024 12:53:10 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: consider -Wmissing-variable-declarations
To: Peter Eisentraut <peter@eisentraut.org>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/69d9824f-00ea-49b6-b68d-96f0806ec454%40iki.fi>
Precedence: bulk

On 09/05/2024 12:23, Peter Eisentraut wrote:
> In [0] I had noticed that we have no automated verification that global
> variables are declared in header files.  (For global functions, we have
> this through -Wmissing-prototypes.)  As I mentioned there, I discovered
> the Clang compiler option -Wmissing-variable-declarations, which does
> exactly that.  Clang has supported this for quite some time, and GCC 14,
> which was released a few days ago, now also supports it.  I went and
> installed this option into the standard build flags and cleaned up the
> warnings it found, which revealed a number of interesting things.

Nice! More checks like this is good in general.

> Attached are patches organized by sub-topic.  The most dubious stuff is
> in patches 0006 and 0007.  A bunch of GUC-related variables are not in
> header files but are pulled in via ad-hoc extern declarations.  I can't
> recognize an intentional scheme there, probably just done for
> convenience or copied from previous practice.  These should be organized
> into appropriate header files.

+1 for moving all these to header files. Also all the "other stuff" in 
patch 0007.

-- 
Heikki Linnakangas
Neon (https://neon.tech)