Re: [PATCH] Fix PR c++/21312: Access violation diagnostic given twice

[Mark Mitchell <mark at codesourcery dot com>]

Simon Martin wrote:

> 2007-02-03  Simon Martin  <simartin@users.sourceforge.net>
> 
> 	PR c++/21312
> 	* semantics.c (finish_qualified_id_expr): Disable the access checks
> 	because they have already been performed.

I don't think this approach makes sense.  First, it's a bit costly in
that you're adding a layer of {push,pop}_deferring_access_checks.  More
importantly, if you want to solve these kinds of duplicate-diagnostic
problems I think we need to think about an overriding principle for
them, rather than trying to attack them ad-hoc.  I'm not trying to pick
on you; until know we've been ad-hoc, and I see that the same solution
you're using is already in use in finish_id_expression.  But, I think we
need to do better.

Obviously, it's much worse to miss the diagnostic than to issue it
twice, so we have to be careful.  We also have to be careful of
overloaded functions: when we see "X::f", and "f" is not overloaded, we
can check access right away; when it is overloaded, we have to defer the
check until we know which "f" is in use.  Finally, we have to be careful
to check from the template instantiation machinery in the same way that
we do when parsing, since at template-parse time we won't have enough
information to do the checks in all cases.

What that suggests is that we need to clearly specify the places at
which we're going to check accessibility, and then make sure that all
paths go through exactly one of those places.

Since finish_non_static_data_member is called with a FIELD_DECL (rather
than an IDENTIFIER_NODE), it's always possible for the caller to check
access up front.  In fact, two of the three calls to
finish_non_static_data_member outside of the template-instantiation
machinery already use the trick that you're using; you're adding it to
the third call.  So, that suggests that the check in
finish_non_static_data_member is probably unnecessary throughout.  The
only thing we need to check is that we already do the check during
template instantiation, and, if we don't, add the check in there.  Then,
remove the code like that which you added at the other calls to
finish_non_static_data_member.

What do you think of that plan?

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713