Re: [C++ PATCH] [PR13927] Dump alias_decl and fix duplicated error message (regression 3.4/mainline)

["Giovanni Bajo" <giovannibajo at libero dot it>]

Gabriel Dos Reis wrote:

> You should be considering a fix per a patch.

Yes this is exactly what I am willing to do, but I need to understand better
how I must do it.

> Please submit a separate patch for the rest.

I am not refusing to do it at all, I'm just trying to understand how to do it,
since my attempt was wrong.

> In particular, supplement_binding as the comment says should not be
> messing with diagnostics.  If you want to violate that then you need
> to rework the entire stuff (I mean duplicate_decls).
> Supplement_binding assumes that when it is called it is valid to
> supplement the binding, and that the checking logic is done somewhere
> else.

You told me this. My observations, rephrased:

1) What do you mean when you say "supplement_binding assumes that when it is
called it is valid to supplement the binding"? Do you mean that, when we call
it, the supplement binding should be always succesfull? What do you mean with
"successfull"? supplement_binding has a return value: "Returns nonzero if the
new binding was successful". Does your sentence mean that this should be
removed (in the longer run)?

2) The comment at the top of duplicate_decls says: "If NEWDECL is a
redeclaration of OLDDECL, merge the declarations. If the redeclaration is
invalid, a diagnostic is issued, and the error_mark_node is returned.
Otherwise, OLDDECL is returned. If NEWDECL is not a redeclaration of OLDDECL,
NULL_TREE is returned." When I read this, I understand that if newdecl is *not*
a redecleration of olddecl, a diagnostic should not be emitted. In fact,
duplicate_decls is very inconsistent about when to emit a diagnostic.

3) Which is the correct place for this diagnostic "redeclared as different kind
of symbol"? You say supplement_binding is not, fine. Do you want it inside
duplicate_decls? But there is a problem, because if duplicate_decls returns
NULL_TREE on it (and it has to, since they are different symbols), then
push_[local/class/namespace]_binding *will* try to call supplement_binding on
it. After all, it's what it's for. Would you prefer a common error function to
emit a diagnostic for an invalid redeclaration whenever supplement_binding
returns false? This would make it something like: if (!supplement_binding())
error(), which is not totally different from emitting the diagnostic within
supplement_binding itself, but anyway.

4) You say that "if you want to emit a diagnostic in supplement_binding, you
need to rework the entire duplicate_decls". Would you please elaborate on this?
It's not clear to me why duplicate_decls would have to be heavily changed. An
example would help.

Thank you,
Giovanni Bajo