Re: [PATCH] C++'ization of cp/parser.c/h

[Gabriel Dos Reis <gdr at integrable-solutions dot net>]

On Wed, Aug 29, 2012 at 8:01 PM, Aaron Gray <aaronngray.lists@gmail.com> wrote:
> Patch to SVN HEAD that initially C++'izes cp/parser.h and cp/parser.c
> by class'izing the cp_lexer and cp_parser group of functions.
>
> For C programmers and for context all method calls are preceded by
> 'this->' and static method calls by 'cp_parser::' or 'cp_lexer::'.
>
> I have made minimal non orthogonal changes to the code on purpose at
> this stage. This is still a work in progress and I am not sure about
> how to go about preparing a change log for this patch.
>
> There are a number of loose ends :-
>
>   - struct's are used rather than classes for now as the whole file
> gives encapsulation for now.
>   - const's need to be applied
>   - cp_parser_context_free_list is still static and not a member of
> cp_parser yet. This also needs a gengtype change to support
> GTY((deletable)) as a node and not just on gcroots.
>   - cp_token functions have not been class'ized yet.
>   - cp_debug functions are still in global space
>   - cp_unevaluated_opreand is still in global space
>   - cp_lexer::get_preprocessor_token() needs rationalizing
>   - there are still #define's associated with VEC operations that
> should be moved to inline methods
>   - constructors and new methods are still functions as PCH call
> ordering conflicts with them, this also allows keeping code changes
> parallel and recording incremental changes in the code.
>   - no_parameters has been left in but is not used

I think this is heading in the wrong direction.  A class with lot of
member functions is a manifestation of a poor C++ design.  Some
call that a "fat interface."  Ideally a good class design should have very
few observer (and mutation) functions.  Those should form the computational
basis of the class, out of which all other functions should be
implemented -- as
non-member functions.

Have a look at

     http://liz.axiomatics.org/trac/browser/trunk/src/Parser.C

The parser there is defined from a very limited set of computation basis:

      http://liz.axiomatics.org/trac/browser/trunk/src/Parser.H#L79

As a matter of fact, I prefer the non-member functions defined as
static function
(i.e. with internal linkage) so that we get an unambiguous message from the
compiler when a function definition becomes dead code.  Do we need to have a
separate parser.h file that contains code previously defined in parser.c?  Why?

-- Gaby