[cfe-commits] r49058 - in /cfe/trunk: include/clang/AST/Decl.h include/clang/AST/DeclObjC.h include/clang/Basic/DiagnosticKinds.def lib/AST/Decl.cpp lib/Sema/Sema.h lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclObjC.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaExprObjC.cpp test/Sema/alias-test-1.m

[Chris Lattner]

On Apr 1, 2008, at 4:04 PM, Steve Naroff wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=49058&view=rev
> Log:
> Fairly large "cleaup" related to changing ObjCCompatibleAliasDecl  
> superclass (to inherit from NamedDecl, instead of ScopedDecl).
>
> - Added a DenseMap to associate an IdentifierInfo with the  
> ObjCCompatibleAliasDecl.
> - Renamed LookupScopedDecl->LookupDecl and changed it's return type  
> to Decl. Also added lookup for ObjCCompatibleAliasDecl's.
> - Removed Sema::LookupInterfaceDecl(). Converted clients to used  
> LookupDecl().
> - Some minor indentation changes.
>
> Will deal with ObjCInterfaceDecl and getObjCInterfaceDecl() in a  
> separate commit...

Nice!  Some trivial nits:

>
> +  /// ObjCAliasDecls - Keep track of all class declarations declared
> +  /// with @compatibility_alias, so that we can emit errors on  
> duplicates and
> +  /// find the declarations when needed. This construct is ancient  
> and will
> +  /// likely never be seen. Nevertheless, it is here for  
> compatibility.
> +  typedef llvm::DenseMap<IdentifierInfo*, ObjCCompatibleAliasDecl*>  
> ObjCAliasTy;

If you make the key be "const IdentifierInfo*", then you can change:

> Sema::DeclTy *Sema::isTypeName(const IdentifierInfo &II, Scope *S)  
> const {
>   Decl *IIDecl = II.getFETokenInfo<Decl>();
>   // Find first occurance of none-tagged declaration
> -  while(IIDecl && IIDecl->getIdentifierNamespace() !=  
> Decl::IDNS_Ordinary)
> +  while (IIDecl && IIDecl->getIdentifierNamespace() !=  
> Decl::IDNS_Ordinary)
>     IIDecl = cast<ScopedDecl>(IIDecl)->getNext();
> -  if (!IIDecl)
> +
> +  if (!IIDecl) {
> +    if (getLangOptions().ObjC1) {
> +      // @interface and @compatibility_alias result in new type  
> references.
> +      // Creating a class alias is *extremely* rare.
> +      ObjCAliasTy::const_iterator I =  
> ObjCAliasDecls.find((IdentifierInfo*)&II);

... this to avoid a cast.

Also, can 'isTypeName' just call LookupDecl?

> +/// getObjCInterfaceDecl - Look up a for a class declaration in the  
> scope.
> +/// return 0 if one not found.
> +/// FIXME: removed this when ObjCInterfaceDecl's aren't ScopedDecl's.
> +ObjCInterfaceDecl *Sema::getObjCInterfaceDecl(IdentifierInfo *Id) {
>   ScopedDecl *IDecl;
>   // Scan up the scope chain looking for a decl that matches this  
> identifier
>   // that is in the appropriate namespace.
> +  for (IDecl = Id->getFETokenInfo<ScopedDecl>(); IDecl;
>        IDecl = IDecl->getNext())
>     if (IDecl->getIdentifierNamespace() == Decl::IDNS_Ordinary)
>       break;

It would be nice to change this to call LookupDecl as well, just to  
have a single place that walks the scope chains.

Overall, great cleanup!

-Chris