[cfe-commits] r84435 - /cfe/trunk/include/clang/AST/DeclObjC.h

[steve naroff]

On Oct 18, 2009, at 7:13 PM, Daniel Dunbar wrote:

> On Sun, Oct 18, 2009 at 2:49 PM, steve naroff <snaroff at apple.com>  
> wrote:
>>
>> On Oct 18, 2009, at 4:25 PM, Daniel Dunbar wrote:
>>
>>> Author: ddunbar
>>> Date: Sun Oct 18 15:25:46 2009
>>> New Revision: 84435
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=84435&view=rev
>>> Log:
>>> Add ObjCCategoryImplDecl::getName() -> StringRef, and a FIXME.
>>>
>>> Modified:
>>>   cfe/trunk/include/clang/AST/DeclObjC.h
>>>
>>> Modified: cfe/trunk/include/clang/AST/DeclObjC.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=84435&r1=84434&r2=84435&view=diff
>>>
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> = 
>>> = 
>>> ====================================================================
>>> --- cfe/trunk/include/clang/AST/DeclObjC.h (original)
>>> +++ cfe/trunk/include/clang/AST/DeclObjC.h Sun Oct 18 15:25:46 2009
>>> @@ -944,20 +944,29 @@
>>>
>>>  ObjCCategoryDecl *getCategoryClass() const;
>>>
>>> +  /// getName - Get the name of identifier for the class interface
>>> associated
>>> +  /// with this implementation as a StringRef.
>>> +  //
>>> +  // FIXME: This is a bad API, we are overriding the  
>>> NamedDecl::getName,
>>> to mean
>>> +  // something different.
>>
>> Why is getName() a "bad API"?
>
> The problem is that NamedDecl also has a getName() function, which
> this is overriding, but the two mean something different.
>
> This one should probably be called getCategoryName, although
> alternately it might make more sense for the categorie's name to be
> the actual category name, and clients that want the interface name can
> get it by getting the interface.
>

That would make more sense to me. I think the API is fine...the  
implementation might be incorrect.

Let's take another example....you have the same FIXME: comment in  
ObjCImplementationDecl and I don't see the problem. Seems like a  
totally valid use of polymorphism (i.e. it give you the name of the  
implementation). The fact that it derives the name from the  
ObjCInterfaceDecl seems like an implementation detail...

snaroff

> - Daniel