From: Giuliano Procida <gprocida@google.com>
To: Dodji Seketeli <dodji@seketeli.org>
Cc: libabigail@sourceware.org, kernel-team@android.com
Subject: Re: [PATCH 0/7] Suppression parsing - preparatory work
Date: Tue, 6 Oct 2020 21:19:34 +0100 [thread overview]
Message-ID: <CAGvU0HnuS9VR-p+mQGX7akEeY72G2t6Xs4eGFjVykXbHJ3aVuQ@mail.gmail.com> (raw)
In-Reply-To: <87k0w9ozrz.fsf@seketeli.org>
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
>
prev parent reply other threads:[~2020-10-06 20:20 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-17 9:38 Giuliano Procida
2020-08-17 9:38 ` [PATCH 1/7] Add missing newlines to end of test files Giuliano Procida
2020-10-01 14:36 ` Dodji Seketeli
2020-08-17 9:38 ` [PATCH 2/7] Fix two wrongs in test suppression regex Giuliano Procida
2020-10-01 14:42 ` Dodji Seketeli
2020-08-17 9:38 ` [PATCH 3/7] Better suppression section parsing delegation Giuliano Procida
2020-10-01 15:56 ` Dodji Seketeli
2020-10-02 7:20 ` Giuliano Procida
2020-08-17 9:38 ` [PATCH 4/7] Add read_*_suppression success/failure plumbing Giuliano Procida
2020-08-17 9:38 ` [PATCH 5/7] Add error handling to read_suppressions Giuliano Procida
2020-08-17 9:38 ` [PATCH 6/7] Default construct suppression types Giuliano Procida
2020-08-17 9:38 ` [PATCH 7/7] Refresh getter/setter comments Giuliano Procida
2020-10-02 7:25 ` [PATCH 0/7] Suppression parsing - preparatory work Dodji Seketeli
2020-10-06 20:19 ` Giuliano Procida [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAGvU0HnuS9VR-p+mQGX7akEeY72G2t6Xs4eGFjVykXbHJ3aVuQ@mail.gmail.com \
--to=gprocida@google.com \
--cc=dodji@seketeli.org \
--cc=kernel-team@android.com \
--cc=libabigail@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).