Horton

Re: ECPG cleanup and fix for clang compile-time problem

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org, Peter Eisentraut <peter_e@gmx.net>, Michael Paquier <michael.paquier@gmail.com>
Date: 2024-04-19T03:03:46Z
Lists: pgsql-hackers 
Hi,

On 2024-04-18 22:18:34 -0400, Tom Lane wrote:
> Here is a patch series that aims to clean up ecpg's preprocessor
> code a little and fix the compile time problems we're seeing with
> late-model clang [1].  I guess whether it's a cleanup is in the eye of
> the beholder, but it definitely succeeds at fixing compile time: for
> me, the time needed to compile preproc.o with clang 16 drops from
> 104 seconds to less than 1 second.  It might be a little faster at
> processing input too, though that wasn't the primary goal.

Nice! I'll look at this more later.


For now I just wanted to point one minor detail:

> (If our code coverage tools worked on bison/flex stuff,
> maybe this'd be less scary ... but they don't.)

For bison coverage seems to work, see e.g.:

https://coverage.postgresql.org/src/interfaces/ecpg/preproc/preproc.y.gcov.html#10638

I think the only reason it doesn't work for flex is that we have
/* LCOV_EXCL_START */
/* LCOV_EXCL_STOP */

around the scanner "body".  Without that I get reasonable-looking, albeit not
very comforting, coverage for pgc.l as well.

                                               |Lines      |Functions|Branches
Filename                                       |Rate    Num|Rate  Num|Rate   Num
src/interfaces/ecpg/preproc/pgc.l              |65.9%   748|87.5%   8|    -    0
src/interfaces/ecpg/preproc/preproc.y          |29.9%  4964|66.7%  15|    -    0


This has been introduced by

commit 421167362242ce1fb46d6d720798787e7cd65aad
Author: Peter Eisentraut <peter_e@gmx.net>
Date:   2017-08-10 23:33:47 -0400

    Exclude flex-generated code from coverage testing

    Flex generates a lot of functions that are not actually used.  In order
    to avoid coverage figures being ruined by that, mark up the part of the
    .l files where the generated code appears by lcov exclusion markers.
    That way, lcov will typically only reported on coverage for the .l file,
    which is under our control, but not for the .c file.

    Reviewed-by: Michael Paquier <michael.paquier@gmail.com>


but I don't think it's working as intended, as it's also preventing coverage
for the actual scanner definition.

Greetings,

Andres Freund

Commits

  1. ecpg: clean up some other assorted memory leaks.

  2. ecpg: fix some memory leakage of data-type-related structures.

  3. ecpg: put all string-valued tokens returned by pgc.l in local storage.

  4. ecpg: fix more minor mishandling of bad input in preprocessor.

  5. ecpg: fix some minor mishandling of bad input in preprocessor.

  6. ecpg: avoid breaking the IDENT precedence level in two.

  7. ecpg: improve preprocessor's memory management.

  8. ecpg: move some functions into a new file ecpg/preproc/util.c.

  9. ecpg: re-implement preprocessor's string management.

  10. ecpg: major cleanup, simplification, and documentation of parse.pl.

  11. ecpg: remove check_rules.pl.

  12. ecpg: clean up documentation of parse.pl, and add more input checking.

  13. Clean up indentation and whitespace inconsistencies in ecpg.

  14. Exclude flex-generated code from coverage testing