Re: Pretty printers for versioned namespace

[Jonathan Wakely <jwakely at redhat dot com>]

On 19/01/17 22:01 +0100, François Dumont wrote:
> On 10/01/2017 13:39, Jonathan Wakely wrote:
> > I've committed the attached patch, which passes the tests for the
> > default configuration and the versioned namespace configuration.
> >
> > I added another helper function, strip_versioned_namespace, which is
> > more expressive than doing typename.replace(vers_nsp, '') everywhere.
> > I've also renamed vers_nsp to _versioned_namespace (using the naming
> > convention for global variables private to the module). I've added
> > checks so that if that variable is None then the extra printers and
> > special cases for the versioned namespace are skipped. That's not
> > currently used, but it would allow us to optimise things later if
> > needed.
>
> Very nice feature indeed, see below.
>
> > I also needed to update the new SharedPtrMethodsMatcher to add
> > "(__\d+)?" to the regular expression.
> >
> >
> > > @@ -1392,47 +1406,54 @@ def register_type_printers(obj):
> > >    add_one_type_printer(obj, 'discard_block_engine', 'ranlux48')
> > >    add_one_type_printer(obj, 'shuffle_order_engine', 'knuth_b')
> > >
> > > +    # Consider optional versioned namespace
> > > +    opt_nsp = '(' + vers_nsp + ')?'
> > > +
> > >    # Do not show defaulted template arguments in class templates
> > >    add_one_template_type_printer(obj, 'unique_ptr<T>',
> > > -            'unique_ptr<(.*), std::default_delete<\\1 ?> >',
> > > -            'unique_ptr<{1}>')
> > > +        '{0}unique_ptr<(.*), std::{0}default_delete<\\2 ?> 
> > > > '.format(opt_nsp),
> > > +        'unique_ptr<{2}>')
> >
> > This is ugly. Mixing python string formatting with regular expressions
> > makes it harder to read, and is inconsistent with how the versioned
> > namespace is handled elsewhere. In Printer.add_version and
> > add_one_type_printer we just register two names, one using std:: and
> > one using std::__7::. We can do the same for the template type
> > printers.
>
>    Yes, your approach is much nicer even if it results in more type 
> printer registered.
>
>    My plan was to submit the attached patch but this doesn't work as 
> the python module seems to be loaded before libstdc++.so. If you know 
> a way to test for versioned namespace before starting registering 
> printers this patch might still be useful. Otherwise I will just 
> forget it.

See the attached patch, which decides at configure-time whether to
enable the versioned namespace printers or not. This is what I had in
mind.

Attachment: versioned.patch
Description: Text document