Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cdt-dev] CPPClassType.getVisibility(IBinding)

There is no need to delegate to ICPPMember.getVisibility() since the new way of managing visibility is intended to be used for all member bindings, whether they implement ICPPMember or not. An independent implementation of ICPPMember.getVisibility() may at some point be replaced by delegation to ICPPClassType.getVisibility(IBinding).


On Tue, Oct 1, 2013 at 11:42 AM, Andrew Eidsness <eclipse@xxxxxxxxxx> wrote:
I see.  But what about using an instanceof check as in the current PDOMCPPAnnotation, which looks like:

    if (binding instanceof ICPPMember) {
        ICPPMember member = (ICPPMember) binding;
        modifiers &= mask;
        modifiers |= (member.getVisibility() & VISIBILITY_MASK) << VISIBILITY_OFFSET;
    // ...

Since that method doesn't use the ClassTypeHelper.getVisibility, isn't there a chance for problems if the member's
#getVisibility implementation returns something different from the ClassTypeHelper's implementation?

The code I was expecting to see would have been near the bottom of
ClassTypeHelper.getVisibility(ICPPInternalClassTypeMixinHost, IBinding).  An instanceof check right before throwing the
invalidMember exception, something like:

+       if (member instanceof ICPPMember) {
+               visibility = ((ICPPMember) member).getVisibility();
+               if (visibility >= 0)
+                       return visibility;
+       }

        throw invalidMember(classType, member);


On 13-10-01 01:04 PM, Sergey Prigogin wrote:
> On Tue, Oct 1, 2013 at 5:12 AM, Andrew Eidsness <eclipse@xxxxxxxxxx <mailto:eclipse@xxxxxxxxxx>> wrote:
>     I'm trying to figure out why this method doesn't check if the argument is an ICPPMember and then use
>     ICPPMember#getVisibility.  This seems to lead to, at least the potential for, inconsistent results with cases like
>     PDOMCPPAnnotation#encodeAnnotation (which does check instanceof, etc.).
> A nested type is not a ICPPMember but may be used as an argument of  CPPClassType.getVisibility(IBinding).
> See for more information.
>     To be clear, I haven't traced this to a specific problem in the core CDT, I'm just trying to figure out the motivation
>     behind this approach.
>     It does cause problems in some non-standard extensions which worked in 8.1.  Those extensions used internal types and
>     shouldn't be expected to continue working.  I'm trying to figure out the design goals to reduce the chance of the fix to
>     the non-standard extensions breaking in future releases.
>     Thanks,
>     -Andrew
> -sergey
>     _______________________________________________
>     cdt-dev mailing list
>     cdt-dev@xxxxxxxxxxx <mailto:cdt-dev@xxxxxxxxxxx>
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx

cdt-dev mailing list

Back to the top