[cfe-commits] r42311 - in /cfe/trunk: AST/Decl.cpp Parse/MinimalAction.cpp Parse/ParseObjc.cpp Sema/Sema.h Sema/SemaDecl.cpp clang.xcodeproj/project.pbxproj include/clang/AST/ASTContext.h include/clang/AST/Decl.h include/clang/Basic/DiagnosticKinds.def include/clang/Parse/Action.h test/Sema/class-def-test-1.m test/Sema/class-impl-1.m

[Fariborz Jahanian]

On Oct 3, 2007, at 10:06 PM, Chris Lattner wrote:

> On Sep 25, 2007, at 11:38 AM, Fariborz Jahanian wrote:
>> Author: fjahanian
>> Date: Tue Sep 25 13:38:09 2007
>> New Revision: 42311
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=42311&view=rev
>> Log:
>> This patch introduces a new class to keep track of class  
>> implementation info. It also adds more
>> semantic checks for class and protocol declarations. Test cases are  
>> good indications of kind of
>> checking being done in this patch.
>
> Very nice Fariborz,
>
> Here's a question for you and Steve:
>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
>> +++ cfe/trunk/include/clang/AST/ASTContext.h Tue Sep 25 13:38:09 2007
>> @@ -39,6 +39,7 @@
>>   llvm::DenseMap<const RecordDecl*, const RecordLayout*>  
>> RecordLayoutInfo;
>>   llvm::DenseMap<const IdentifierInfo*, ObjcInterfaceDecl*>  
>> ClassNameInfo;
>>   llvm::DenseMap<const IdentifierInfo*, ObjcProtocolDecl*>  
>> ProtocolNameInfo;
>> +  llvm::SmallVector<ObjcImplementationDecl*, 8>  
>> ImplementationClassInfo;
>>   RecordDecl *CFConstantStringTypeDecl;
>> public:
>>
>> @@ -172,6 +173,16 @@
>>                             ObjcProtocolDecl* ProtocolDecl)
>>   { ProtocolNameInfo[ProtocolName] = ProtocolDecl; }
>>
>> +  ObjcImplementationDecl* getObjcImplementationClass(unsigned ix) {
>> +    return ImplementationClassInfo[ix];
>> +  }
>> +  void setObjcImplementationClass(ObjcImplementationDecl*  
>> ImplDecl) {
>> +    ImplementationClassInfo.push_back(ImplDecl);
>> +  }
>> +  unsigned sizeObjcImplementationClass() const {
>> +    return ImplementationClassInfo.size();
>> +  }
>> +
>
> This is a bit strange to me.  It has no analog in C.  My  
> understanding is that this:
>
>> +Sema::DeclTy *Sema::ObjcStartClassImplementation(Scope *S,
>> +                      SourceLocation AtClassImplLoc,
>> +                      IdentifierInfo *ClassName, SourceLocation  
>> ClassLoc,
>> +                      IdentifierInfo *SuperClassname,
>> +                      SourceLocation SuperClassLoc) {
>> +  ObjcInterfaceDecl* IDecl = 0;
>> +  // Check for another declaration kind with the same name.
>> +  ScopedDecl *PrevDecl = LookupScopedDecl(ClassName,  
>> Decl::IDNS_Ordinary,
>> +                                          ClassLoc, S);
>
> Looks up the forward declaration for the implementation, which is  
> checked and then:
>
>> +  if (PrevDecl && !isa<ObjcInterfaceDecl>(PrevDecl)) {
>> +    Diag(ClassLoc, diag::err_redefinition_different_kind,
>> +         ClassName->getName());
>> +    Diag(PrevDecl->getLocation(), diag::err_previous_definition);
>> +  }
>> +  else {
>> +    // Is there an interface declaration of this class; if not,  
>> warn!
>> +    IDecl = Context.getObjCInterfaceDecl(ClassName);
>> +    if (!IDecl)
>> +      Diag(ClassLoc, diag::warn_undef_interface, ClassName- 
>> >getName());
>> +  }
>> +
> ...
>> +
>> +  ObjcImplementationDecl* IMPDecl =
>> +    new ObjcImplementationDecl(AtClassImplLoc, ClassName, SDecl);
>
> an implementation decl object is made.
>
> Would it be feasible to handle this like we do for struct forward  
> definitions, where the definition of a struct goes back and fills in  
> the declaration with the implementation info?  It seems strange to  
> me that objc code like this:
>
> @interface OBJECT @end
> @interface INTF  : OBJECT
> @end
> @implementation INTF @end
>
> Creates an 'ObjcInterfaceDecl' for INTF, then a separate  
> 'ObjcImplementationDecl' later, which isn't added to the scope  
> info.  If it were, you could drop this linear time lookup code:

ObjcInterfaceDecl and ObjcImplementationDecl are not same as 'struct  
S'. ObjcInterfaceDecl is part of the type system.  
ObjcImplementationDecl is a node which encapsulates information about  
methods which have implementations (definitions) in current TU. List  
of such objects are later on used in meta-data generation.

There are far more @interface declaration than  @implementation (which  
is typically a few per TU). It is not feasible to add fields to  
ObjcInterfaceDecl needed by @implementation.

- Fariborz

>
>
>> +  // Check that there is no duplicate implementation of this class.
>> +  bool err = false;
>> +  for (unsigned i = 0; i != Context.sizeObjcImplementationClass();  
>> i++) {
>> +    if (Context.getObjcImplementationClass(i)->getIdentifier() ==  
>> ClassName) {
>> +      Diag(ClassLoc, diag::err_dup_implementation_class, ClassName- 
>> >getName());
>> +      err = true;
>> +      break;
>> +    }
>> +  }
>> +  if (!err)
>> +    Context.setObjcImplementationClass(IMPDecl);
>
> and just turn this into a simple query, like we have for  
> redefinition of structs.
>
> Is this craziness?
>
> -Chris