Re: [PATCH 0/7] Suppression parsing - preparatory work

[Dodji Seketeli <dodji@seketeli.org>]

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,

-- 
		Dodji