Re: src/tools/pginclude considered harmful (was Re: - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: src/tools/pginclude considered harmful (was Re:
Date
Msg-id 200607171943.k6HJhEr01850@momjian.us
Whole thread Raw
In response to Re: src/tools/pginclude considered harmful (was Re: [PATCHES]  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: src/tools/pginclude considered harmful (was Re:  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
FYI, I updated pginclude/README to explain the complexity of
removing includes from include files:    pgfixinclude    sort include references    run multiple times:
pgcompinclude       pgrminclude /src/include    pgrminclude /    pgcheckdefinesThere is a complexity when modifying
/src/include. If include file 1includes file 2, and file 2 includes file 3, then when file 1 isprocessed, it needs only
file2, not file 3.  However, if later, includefile 2 is processed, and file 3 is not needed by file 2 and is
removed,file1 might then need to include file 3.  For this reason, thepgcompinclude and pgrminclude /src/include steps
mustbe run severaltimes until all includes compile cleanly.
 

I followed this process, but didn't full understand why multiple include
runs were necessary until I thought about it.

FYI, 527 include were removed from non-header C files in this run.  That
is not something that can be easily done manually.

---------------------------------------------------------------------------

Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > I agree with reverting. The tool looks pretty broken anyway, with 
> > hardcoded paths and all sorts of stuff quite apart from logic problems.
> 
> Well, it's only intended to work on Bruce's system, so until someone
> else takes over the position of chief gruntwork-doer I don't think the
> portability issues are much of a factor.  What I'm worried about at the
> moment is the correctness issue.
> 
> After some reflection it seems that there is only one case where removal
> of a needed include file would not lead to a compiler error or warning,
> assuming gcc with ordinary -W settings (notably -Wmissing-prototypes).
> That case is exactly what Kris found: removal of a #define that is
> tested via #ifdef or #if defined().  (Can anyone think of other cases?)
> 
> It's not hard to imagine getting burnt by this through ordinary manual
> rearrangement or cleanup of inclusions, so I'm thinking that blaming
> pgrminclude may be too hasty.  It'd be worth our time to make a tool to
> check for it.  I'm going to work on a Perl script that sucks up all the
> #defines in all our .h files and looks for "#ifdef foo" or "defined foo"
> where foo is defined in a file not included by the referencing file
> (gcc -H looks like the right tool for checking that).  Another thing
> such a script could easily do is check for inclusion of system headers
> before c.h.
> 
>             regards, tom lane

--  Bruce Momjian   bruce@momjian.us EnterpriseDB    http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +


pgsql-hackers by date:

Previous
From: Josh Berkus
Date:
Subject: Re: plPHP and plRuby
Next
From: Zdenek Kotala
Date:
Subject: Re: TODO: Mark change-on-restart-only values in