[cfe-commits] r75808 - in /cfe/trunk: include/clang/AST/ASTContext.h include/clang/AST/Type.h include/clang/Basic/DiagnosticSemaKinds.td include/clang/Frontend/PCHBitCodes.h lib/AST/ASTContext.cpp lib/AST/Type.cpp lib/CodeGen/CodeGenTypes.cpp lib/CodeGen/Mangle.cpp lib/Frontend/PCHReader.cpp lib/Frontend/PCHWriter.cpp lib/Sema/Sema.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaOverload.cpp test/SemaObjC/comptypes-3.m test/SemaObjC/conditional-expr-3.m

[Fariborz Jahanian]

On Jul 15, 2009, at 2:22 PM, Steve Naroff wrote:

>
> On Jul 15, 2009, at 4:45 PM, Fariborz Jahanian wrote:
>
>> Nice. Few comments below.
>>
>
> Sure...
>
>> - Fariborz
>>
>> On Jul 15, 2009, at 11:40 AM, Steve Naroff wrote:
>>
>>> Author: snaroff
>>> Date: Wed Jul 15 13:40:39 2009
>>> New Revision: 75808
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=75808&view=rev
>>> Log:
>>> Implement the ObjC pseudo built-in types as clang "BuiltinType's".  
>>> I say pseudo built-in types, since Sema still injects a typedef  
>>> for recognition (i.e. they aren't truly built-ins from a parser  
>>> perspective).
>>>
>>> This removes the static data/methods on ObjCObjectPointerType  
>>> while preserving the nice API (no need to fiddle with ASTContext:-).
>>>
>>> This patch also adds Type::isObjCBuiltinType().
>>>
>>> This should be the last fairly large patch related to recrafting  
>>> the ObjC type system. The follow-on patches should be fairly small.
>>>
>
> <snip>
>
>>> bool isObjCIdType(QualType T) const {
>>> -    return T == ObjCIdType;
>>> +    return T == ObjCIdTypedefType;
>>> }
>>> bool isObjCClassType(QualType T) const {
>>> -    return T == ObjCClassType;
>>> +    return T == ObjCClassTypedefType;
>>
>> I am not sure about this and isObjCIdType(). You are comparing  
>> against a typedef AST node 'id' and typedef AST node for 'Class'.
>> what if 'T' is the builtin type for 'id' or 'Class'? This predicate  
>> returns false.
>>
>
> All I changed is the name (to avoid confusion with ObjCIdTypedefType/ 
> ObjCClassTypedefType).
>
> Nevertheless, you are correct. From a source code perspective, the  
> only real 'id' or 'Class' type is the Typedef type (injected by  
> Sema). This has always been the case I believe.
>
> The Builtin type is simply a clever mechanism to preserve the nice  
> API without resorting to static data. More specifically,  
> ASTContext::ObjCBuiltinIdTy and ASTContext::ObjCBuiltinClassTy will  
> *only* be referred to by ObjCIdTypedefType/ObjCClassTypedefType.  
> Without it, I would have had to move many Type predicates to  
> ASTContext (or fiddle with some weird union in  
> ObjCObjectPointerType). This seemed like an elegant solution to the  
> problem...especially since id/Class are in fact builtin now. The  
> more I played with this solution, the more I liked it.
>

Very nice. OK.

>
>>
>>>
>>> }
>>> bool isObjCSelType(QualType T) const {
>>>   assert(SelStructType && "isObjCSelType used before 'SEL' type is  
>>> built");
>>>
>>> Modified: cfe/trunk/include/clang/AST/Type.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=75808&r1=75807&r2=75808&view=diff
>>>
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- cfe/trunk/include/clang/AST/Type.h (original)
>>> +++ cfe/trunk/include/clang/AST/Type.h Wed Jul 15 13:40:39 2009
>>> @@ -404,6 +404,7 @@
>>> bool isObjCQualifiedIdType() const;           // id<foo>
>>> bool isObjCIdType() const;                    // id
>>> bool isObjCClassType() const;                 // Class
>>> +  bool isObjCBuiltinType() const;               // 'id' or 'Class'
>>> bool isTemplateTypeParmType() const;          // C++ template type  
>>> parameter
>>> bool isNullPtrType() const;                   // C++0x nullptr_t
>>>
>>> @@ -592,8 +593,10 @@
>>>   Overload,  // This represents the type of an overloaded function  
>>> declaration.
>>>   Dependent, // This represents the type of a type-dependent  
>>> expression.
>>>
>>> -    UndeducedAuto  // In C++0x, this represents the type of an  
>>> auto variable
>>> +    UndeducedAuto, // In C++0x, this represents the type of an  
>>> auto variable
>>>                  // that has not been deduced yet.
>>> +    ObjCId,    // This represents the ObjC 'id' type.
>>> +    ObjCClass  // This represents the ObjC 'Class' type.
>>> };
>>> private:
>>> Kind TypeKind;
>>> @@ -1899,7 +1902,7 @@
>>> /// Duplicate protocols are removed and protocol list is  
>>> canonicalized to be in
>>> /// alphabetical order.
>>> class ObjCObjectPointerType : public Type, public  
>>> llvm::FoldingSetNode {
>>> -  QualType PointeeType; // will always point to an interface type.
>>> +  QualType PointeeType; // A builin or interface type.
>>>
>>> // List of protocols for this protocol conforming object type
>>> // List is sorted on protocol name. No protocol is entered more  
>>> than once.
>>> @@ -1909,20 +1912,10 @@
>>>   Type(ObjCObjectPointer, QualType(), /*Dependent=*/false),
>>>   PointeeType(T), Protocols(Protos, Protos+NumP) { }
>>> friend class ASTContext;  // ASTContext creates these.
>>> -  friend class ObjCInterfaceType; // To enable 'id' and 'Class'  
>>> predicates.
>>>
>>> -  static ObjCInterfaceType *IdInterfaceT;
>>> -  static ObjCInterfaceType *ClassInterfaceT;
>>> -  static void setIdInterface(QualType T) {
>>> -    IdInterfaceT =  dyn_cast<ObjCInterfaceType>(T.getTypePtr());
>>> -  }
>>> -  static void setClassInterface(QualType T) {
>>> -    ClassInterfaceT =  dyn_cast<ObjCInterfaceType>(T.getTypePtr());
>>> -  }
>>> -  static ObjCInterfaceType *getIdInterface() { return  
>>> IdInterfaceT; }
>>> -  static ObjCInterfaceType *getClassInterface() { return  
>>> ClassInterfaceT; }
>>> public:
>>> -  // Get the pointee type. Pointee is required to always be an  
>>> interface type.
>>> +  // Get the pointee type. Pointee will either be a built-in type  
>>> (for 'id' and
>>> +  // 'Class') or will be an interface type (for user-defined  
>>> types).
>>> // Note: Pointee can be a TypedefType whose canonical type is an  
>>> interface.
>>> // Example: typedef NSObject T; T *var;
>>> QualType getPointeeType() const { return PointeeType; }
>> Name Pointee and comment above is confusing to me. To me Pointee is  
>> what a pointer is pointing at.
>>
>> Here you mean several things as I understand it.
>> 1) For 'id' and 'Class' it holds the builtin type for 'id' and  
>> 'Class' when their container is ObjCObjectPointerType
>> for the two typedefs 'id' and 'Class'.
>
> Yes.
>
>>
>>
>> 2) For an interface pointer type; such as, "NSObject *" is Pointee  
>> for "NSObject" (as the name implies)?
>>
>
> Yes.
>
>> 3) For the example  in the comment ( typedef NSObject T; T *var)  
>> Pointee referes to "NSObject" right?
>>
>
> Nope. Pointee would be a TypedefType whose canonical type is an  
> interface. I say this in the "Note:".
>
> Do you think I need to rewrite the comment?

Nope. Pointee would be for 'T' in your example. You may want to add  
this for clarification, I guess.

>

You missed this addition comments (there are actually in two places in  
ASTContext.cpp).

> +      for (ObjCObjectPointerType::qual_iterator J = RHSOPT-
>> qual_begin(),
> +           E = RHSOPT->qual_end(); J != E; ++J) {
> +        if ((*J)->lookupProtocolNamed((*I)->getIdentifier()))
> +          RHSImplementsProtocol = true;
> +      }

After setting RHSImplementsProtocol = true; you want to break out of
the loop.

- Thanks, Fariborz

>
> snaroff
>