Re: [PATCH 1/2] make the c++ pretty printer inherit from the C one instead of include it
On Sun, Aug 4, 2013 at 8:37 PM, Trevor Saunders wrote: > On Wed, Jul 31, 2013 at 10:02:29PM -0500, Gabriel Dos Reis wrote: >> * declare the "pointer to function fields" as virtual functions -- >> that is what I meant >> with the (necessarily poor) emulation through the casts. > > I guess you'll work on this later in the patch series you're sending, > but its worth noting making pretty_print_info::format_decoder a virtual > function is non-trivial, it turns out to be important that some > consumers can leave it null instead of making it what is currently > default_tree_printer. This is because gcov and maybe other things link > against diagnostic.c and pretty-print.c but not all the tree stuff that > would be required for default_tree_printer. Trev -- Thanks for the piece of info. Writing OO programming in C is not fun at all; and undoing OO C programs is even less fun. I originally expected clients to derive from the pretty printers and add their own behavior. But, it turns out that since OO programming in C isn't fun, things have grown barnacles and entangled in ways I did not intend or anticipate. Hopefully, now that we have a better abstraction tool, we can express the intent much more directly. I will send in a patch that adds the inheritance chain from pretty_printer to cxx_pretty_printer. Unfortunately, it does not add the virtual functions. This is because I want to handle 'constructors' in a separate patch. -- Gaby
Re: [PATCH 1/2] make the c++ pretty printer inherit from the C one instead of include it
On Wed, Jul 31, 2013 at 10:02:29PM -0500, Gabriel Dos Reis wrote: > * declare the "pointer to function fields" as virtual functions -- > that is what I meant > with the (necessarily poor) emulation through the casts. I guess you'll work on this later in the patch series you're sending, but its worth noting making pretty_print_info::format_decoder a virtual function is non-trivial, it turns out to be important that some consumers can leave it null instead of making it what is currently default_tree_printer. This is because gcov and maybe other things link against diagnostic.c and pretty-print.c but not all the tree stuff that would be required for default_tree_printer. Trev > * override those that needed to be overridden in cxx_pretty_printer. > * adjust the macros. > * Have the associated constructors do the right thing. > > > > -- Gaby
Re: [PATCH 1/2] make the c++ pretty printer inherit from the C one instead of include it
On Wed, Jul 31, 2013 at 10:16 PM, Trevor Saunders wrote: > On Wed, Jul 31, 2013 at 10:02:29PM -0500, Gabriel Dos Reis wrote: >> On Wed, Jul 31, 2013 at 8:14 PM, Trevor Saunders >> wrote: >> > bootstrapped and same test results on x86_64-linux-gnu against r201084 >> > >> > gcc/cp/ >> > * cxx-pretty-print.h (cxx_pretty_printer): inherit from >> > c_pretty_printer >> > instead of include it. >> > * (cxx_pretty_print.c): adjust accordingly. >> >> This is on my todo stack; thanks for looking into it. >> >> The way to handle this is: >> >> * yes, use inheritance -- that is what I simulated with the C abstractions >> * declare the "pointer to function fields" as virtual functions -- >> that is what I meant >> with the (necessarily poor) emulation through the casts. >> * override those that needed to be overridden in cxx_pretty_printer. >> * adjust the macros. >> * Have the associated constructors do the right thing. > > I agree with all of that, I was just trying to keep patches small but if > you prefer I guess I could do one giant patch that redoes all of this. At the minimum, the inheritance and the virtual functions should go together. -- Gaby
Re: [PATCH 1/2] make the c++ pretty printer inherit from the C one instead of include it
On Wed, Jul 31, 2013 at 10:02:29PM -0500, Gabriel Dos Reis wrote: > On Wed, Jul 31, 2013 at 8:14 PM, Trevor Saunders > wrote: > > bootstrapped and same test results on x86_64-linux-gnu against r201084 > > > > gcc/cp/ > > * cxx-pretty-print.h (cxx_pretty_printer): inherit from > > c_pretty_printer > > instead of include it. > > * (cxx_pretty_print.c): adjust accordingly. > > This is on my todo stack; thanks for looking into it. > > The way to handle this is: > > * yes, use inheritance -- that is what I simulated with the C abstractions > * declare the "pointer to function fields" as virtual functions -- > that is what I meant > with the (necessarily poor) emulation through the casts. > * override those that needed to be overridden in cxx_pretty_printer. > * adjust the macros. > * Have the associated constructors do the right thing. I agree with all of that, I was just trying to keep patches small but if you prefer I guess I could do one giant patch that redoes all of this. Trev > > > > -- Gaby
Re: [PATCH 1/2] make the c++ pretty printer inherit from the C one instead of include it
On Wed, Jul 31, 2013 at 8:14 PM, Trevor Saunders wrote: > bootstrapped and same test results on x86_64-linux-gnu against r201084 > > gcc/cp/ > * cxx-pretty-print.h (cxx_pretty_printer): inherit from > c_pretty_printer > instead of include it. > * (cxx_pretty_print.c): adjust accordingly. This is on my todo stack; thanks for looking into it. The way to handle this is: * yes, use inheritance -- that is what I simulated with the C abstractions * declare the "pointer to function fields" as virtual functions -- that is what I meant with the (necessarily poor) emulation through the casts. * override those that needed to be overridden in cxx_pretty_printer. * adjust the macros. * Have the associated constructors do the right thing. -- Gaby
