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

[Daniel Dunbar]

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.

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
>