Re: C++ error printer cleanup, phase 1

[Gabriel Dos Reis <gdr at codesourcery dot com>]

Zack Weinberg <zack@codesourcery.com> writes:

| On Mon, Sep 24, 2001 at 02:28:05AM +0200, Gabriel Dos Reis wrote:
| > Zack Weinberg <zack@codesourcery.com> writes:
| > | 
| > | Oh.  That is still a nontrivial unification project.  diagnostic.c has
| > | dependencies on the rest of the back end which are not acceptable for
| > | cpplib.
| > 
| > Recently, I moved RTL bits in a separate file.  What other nontrivial
| > dependencies do you have in mind? (I see use of certain diagnostic
| > flags, but I don't think they are nontrivial issues).
| 
| Well, there are still references to trees; that's not hard to get rid
| of though.

If memory serves me well, the tree thingy is not an issue. 
Neil's planed location data structure will use tree as a way of
packaging tokens with their location information -- Neil, do I recall
wrong?  
Anyway, the tree issue, as you say, is not a nontrivial project.

| The real issue isn't back end dependencies at all, now I've thought
| about it a bit harder.  It's a basic interface incompatibility.

Ah, now the reason is different.  Good.  Eventually, we should reach a
fixed-point.

| cpplib has no global variables - this is a deliberate design
| decision.  As such all its error routines have a signature like

That deliberate design is something I do appreciate, and successive
patches I've cheked in were so as to get rid of reference to global
variables. You might want to check for past patches.

|  void cpp_error (struct cpp_reader *, const char *, ...);
| 
| Therefore diagnostic.c's exported error routines - error, warning, etc

For the record, the move in the C++ land is to have diagnostic
reporting subroutines take a context as an argument -- see the
cp-parser-branch for exemple.  Any changes made to the diagnostic
library should be aware of that.  Within that framework cpp_reader
will just have a hook (or be will a sub-class of) for
diagnostic_context. 

[...]

| Now, I'm not disputing that there is a great deal of conceptual
| similarity between diagnostic.c and cpperror.c.  And I do think that
| with time and effort they can be unified.  I just don't think it is
| a trivial job, or currently practical.

It is not about aesthetics.  It is about pratical engineering:  If
you check the archive for past discussions, you'll notice that there
are real interests in unifying the diagnostic reporting machinery so as
to ease extensions (e.g., make the whole compiler diagnostic reporting
machinery output a specific format for other tools to parse).  If you
insist on duplicating the diagnostic library facilities, you'll force
people to duplicate effort.  

Please keep those data in mind for future patches.

There is no doubt that diagnostic.c should be cleaned up.  Most of the
stuff in that file were practically lifted from various places (where
they do not belong to) as a first iteration of "improving diagnostics
(about template)" project.  I do welcome any patch to make the situation
better, but please don't forget that there other dsign issues we
should keep in mind.

As a curiosity, what goal are you aiming at with your patches?
Knowing that may help me understand better your side.

-- Gaby
CodeSourcery, LLC                       http://www.codesourcery.com