[cfe-dev] Fix for PR4701

David Chisnall csdavec at swansea.ac.uk
Tue Aug 18 06:19:23 PDT 2009


Hi Daniel,

On 18 Aug 2009, at 07:16, Daniel Dunbar wrote:

> Hi David,
>
> I'd like to sort out what we are doing with IsaExpr. I still don't see
> a reason to complicate codegen and other clients with this expression.
> This could just be because I don't want to implement support for it
> (*grin*), but I think that if we really care about being future
> looking we would move people to runtime functions; a custom expression
> node for syntax which is identical to other general language syntax is
> pretty gross...

I left the isa stuff for Steve to decide about.  The idea behind it  
(explained in Steve's very long email before the big change last  
month) was that isa should be a language feature, rather than an  
explicit ivar.  This simplifies base classes, where it's relatively  
easy to forget to declare the isa ivar and then have the compiler  
generate broken code, but given how rare writing base classes actually  
is, I'm not fully convinced that it's needed.

> Also, I noticed a regression with this patch, the problem stems from  
> this:
> --
> +    if (BaseType != Context.ObjCIdRedefinitionType) {
> +      BaseType = Context.ObjCIdRedefinitionType;
> --
> is this supposed to be ==? Otherwise it doesn't make sense, since
> Context.ObjCIdRedefitionType gets initialized to the null QualType.

If a definition is supplied for the id type, then  
Context.ObjCIdRedefinitionType will have been filled in by Sema.  If  
not, then BaseType will already be an opaque type and so casting it  
should make no difference (although I can add an explicit test if you  
prefer).  Can you give me an example in which this causes  
regressions?  I think this now means that the ObjCIsaExpr construction  
section is no longer reached, which I can fix if Steve decides we want  
to keep ObjCIsaExpr or remove completely if he doesn't, but ->isa  
should still work in cases where the type is defined (it does for me,  
anyway...).

David



More information about the cfe-dev mailing list