[0/7] Suppression parsing - preparatory work

Hi Dodji.

Quite a while ago I had a series of patches with the aim of improving
libabigail's suppression parsing with the main aims:

* adding error handling and reporting
* refactoring for easier maintenance (both fixes and features)

Early on in the series, I changed the way regexes were parsed and
passed in and out of the suppression specifications. This wasn't
something you were happy with, so I shelved the series.

I've taken a lttle time to remove those changes and rebase the series.
My plan is to feed changes to you in digistible batches.

This batch contains:

* 2 commits that fix issues in an uncontroversial way
* 3 commits to add the outer shell of error handling
* 2 commits to simplify how suppressions are constructed

The error handling commits do not add any error reporting but do add
placeholder TODOs for where this could be added.

The constructor change commits remove the non-default constructors for
the 4 suppression types as they are antithetical to a table-driver
parser where there are a large number of optional fields. If these
constructors need to be preserved for other reasons, that can still be
done. For (human) use, a better interface for building suppressions
might be where setters can be chained (a.k.a. fluent interface) but
that might not be your preference.

Please take a fresh look at these.

If you wanted to look ahead to what the end state could be, please see
https://github.com/myxoid/libabigail/tree/suppression-parsing-revisited
but note that the last few commits are not ready for consumption.

Thank you,
Giuliano.

Giuliano Procida (7):
  Add missing newlines to end of test files.
  Fix two wrongs in test suppression regex
  Better suppression section parsing delegation.
  Add read_*_suppression success/failure plumbing.
  Add error handling to read_suppressions.
  Default construct suppression types.
  Refresh getter/setter comments.

 include/abg-suppression.h                     |  48 +-
 src/abg-suppression-priv.h                    |  28 +-
 src/abg-suppression.cc                        | 618 +++++++-----------
 src/abg-tools-utils.cc                        |   6 +-
 .../test-diff-suppr/test0-type-suppr-2.suppr  |   2 +-
 .../test22-suppr-removed-var-sym-4.suppr      |   2 +-
 .../test23-alias-filter-0.suppr               |   2 +-
 .../test23-alias-filter-4.suppr               |   2 +-
 .../test28-add-aliased-function-1.suppr       |   2 +-
 .../test28-add-aliased-function-2.suppr       |   2 +-
 .../test28-add-aliased-function-3.suppr       |   2 +-
 .../test28-add-aliased-function-4.suppr       |   2 +-
 .../test38-char-class-in-ini.abignore         |   2 +-
 .../test41-enumerator-changes-0.suppr         |   2 +-
 .../test-diff-suppr/test7-var-suppr-7.suppr   |   2 +-
 .../test01-equal-in-property-string.abignore  |   2 +-
 16 files changed, 274 insertions(+), 450 deletions(-)

Hello Giuliano,

Giuliano Procida <gprocida@google.com> a écrit:

> Quite a while ago I had a series of patches with the aim of improving
> libabigail's suppression parsing with the main aims:
>
> * adding error handling and reporting
> * refactoring for easier maintenance (both fixes and features)
>
> Early on in the series, I changed the way regexes were parsed and
> passed in and out of the suppression specifications. This wasn't
> something you were happy with, so I shelved the series.
>
> I've taken a lttle time to remove those changes and rebase the series.
> My plan is to feed changes to you in digistible batches.
>
> This batch contains:
>
> * 2 commits that fix issues in an uncontroversial way

Thanks!  I happily applied these.

> * 3 commits to add the outer shell of error handling
> * 2 commits to simplify how suppressions are constructed
>
> The error handling commits do not add any error reporting but do add
> placeholder TODOs for where this could be added.
>
> The constructor change commits remove the non-default constructors for
> the 4 suppression types as they are antithetical to a table-driver
> parser where there are a large number of optional fields.

I do really prefer that we keep recursive descent parsers in all the
parsers of the project.  Why? Because they are the simplest parsers to
understand for someone who just wants to /debug/ it to fix things in the
way the parser.  Yes, they are more verbose and the grammar handling is
hard coded.  But that's a tradeoff I accept to keep the whole project as
"debuggable" as it can be, given the inherent complexity of the core
subject we are trying to tackle here.

So, yeah, I am really not a fan for introducing a table-driven parser in
this context at this point.

[...]

Cheers,

Hi there.

On Fri, 2 Oct 2020 at 22:53, Dodji Seketeli <dodji@seketeli.org> wrote:

> Hello Giuliano,
>
> Giuliano Procida <gprocida@google.com> a écrit:
>
> > Quite a while ago I had a series of patches with the aim of improving
> > libabigail's suppression parsing with the main aims:
> >
> > * adding error handling and reporting
> > * refactoring for easier maintenance (both fixes and features)
> >
> > Early on in the series, I changed the way regexes were parsed and
> > passed in and out of the suppression specifications. This wasn't
> > something you were happy with, so I shelved the series.
> >
> > I've taken a lttle time to remove those changes and rebase the series.
> > My plan is to feed changes to you in digistible batches.
> >
> > This batch contains:
> >
> > * 2 commits that fix issues in an uncontroversial way
>
> Thanks!  I happily applied these.
>
> > * 3 commits to add the outer shell of error handling
> > * 2 commits to simplify how suppressions are constructed
> >
> > The error handling commits do not add any error reporting but do add
> > placeholder TODOs for where this could be added.
> >
> > The constructor change commits remove the non-default constructors for
> > the 4 suppression types as they are antithetical to a table-driver
> > parser where there are a large number of optional fields.
>
> I do really prefer that we keep recursive descent parsers in all the
> parsers of the project.  Why? Because they are the simplest parsers to
> understand for someone who just wants to /debug/ it to fix things in the
> way the parser.  Yes, they are more verbose and the grammar handling is
> hard coded.  But that's a tradeoff I accept to keep the whole project as
> "debuggable" as it can be, given the inherent complexity of the core
> subject we are trying to tackle here.
>
> So, yeah, I am really not a fan for introducing a table-driven parser in
> this context at this point.
>

Understood.

I've mothballed this line of development (see
https://sourceware.org/bugzilla/show_bug.cgi?id=19608#c3).

If someone wants to try an alternative approach, they will at least get an
idea of the scale of the changes needed.


>
> [...]
>
> Cheers,
>

Regards,
Giuliano.


>
> --
>                 Dodji
>