Skip to main content

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

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;
	int mask = ~(VISIBILITY_MASK << VISIBILITY_OFFSET);
	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);

-Andrew

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 https://bugs.eclipse.org/bugs/show_bug.cgi?id=402878 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>
>     https://dev.eclipse.org/mailman/listinfo/cdt-dev
> 
> 
> 
> 
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/cdt-dev
> 



Back to the top