[cfe-commits] r152321 - in /cfe/trunk: include/clang/AST/Decl.h lib/AST/Decl.cpp

[Benjamin Kramer]

On 08.03.2012, at 21:34, Daniel Dunbar wrote:

> On Thu, Mar 8, 2012 at 11:30 AM, Benjamin Kramer
> <benny.kra at googlemail.com> wrote:
>> 
>> On 08.03.2012, at 19:20, Daniel Dunbar wrote:
>> 
>>> Author: ddunbar
>>> Date: Thu Mar  8 12:20:41 2012
>>> New Revision: 152321
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=152321&view=rev
>>> Log:
>>> [AST] Change NamedDecl::getUnderlyingDecl() to inline the fast (and incredibly common) path.
>>> 
>>> Modified:
>>>    cfe/trunk/include/clang/AST/Decl.h
>>>    cfe/trunk/lib/AST/Decl.cpp
>>> 
>>> Modified: cfe/trunk/include/clang/AST/Decl.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=152321&r1=152320&r2=152321&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/AST/Decl.h (original)
>>> +++ cfe/trunk/include/clang/AST/Decl.h Thu Mar  8 12:20:41 2012
>>> @@ -323,7 +323,13 @@
>>> 
>>>   /// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
>>>   /// the underlying named decl.
>>> -  NamedDecl *getUnderlyingDecl();
>>> +  NamedDecl *getUnderlyingDecl() {
>>> +    if (!(this->getKind() == UsingShadow) &&
>>> +        !(this->getKind() == ObjCCompatibleAlias))
>> 
>> x != y would be a little easier on the eyes than !(x == y)
> 
> Indeed. Thanks.
> 
>> 
>>> +      return this;
>>> +    return getUnderlyingDeclImpl();
>>> +  }
>>> +  NamedDecl *getUnderlyingDeclImpl();
>> 
>> The Impl method should be private …
> 
> Also thanks.
> 
>> 
>>>   const NamedDecl *getUnderlyingDecl() const {
>>>     return const_cast<NamedDecl*>(this)->getUnderlyingDecl();
>>>   }
>>> 
>>> Modified: cfe/trunk/lib/AST/Decl.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=152321&r1=152320&r2=152321&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/AST/Decl.cpp (original)
>>> +++ cfe/trunk/lib/AST/Decl.cpp Thu Mar  8 12:20:41 2012
>>> @@ -983,7 +983,7 @@
>>>   return getLinkage() != NoLinkage;
>>> }
>>> 
>>> -NamedDecl *NamedDecl::getUnderlyingDecl() {
>>> +NamedDecl *NamedDecl::getUnderlyingDeclImpl() {
>>>   NamedDecl *ND = this;
>>>   while (true) {
>>>     if (UsingShadowDecl *UD = dyn_cast<UsingShadowDecl>(ND))
>> 
>> … and can assume that ND is a UsingShadowDecl or ObjCCompatibleAliasDecl now, simplifying the code a bit.
> 
> Meh, I thought about this, but I think it just makes the function more
> verbose for no gain. You still have to figure out which one it is to
> traverse down properly. Feel free to adjust if you see a clean change
> I am missing.

I see, the new assumption doesn't really help here. I committed a minor cleanup in r152339.

- Ben

> 
> Fixed previous two issues in r152332.
> 
> - Daniel
> 
>> 
>> - Ben
>> 
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>