r184504 - When building a module, keep *all* declared methods in the global method pool.

[Douglas Gregor]

On Jun 21, 2013, at 8:16 AM, jahanian <fjahanian at apple.com> wrote:

> With multiple identical selectors in global pool now, how do you guarantee to lookup 
> the one or the other you are looking for? Or maybe the logic is already in there.

Sema::LookupMethodInGlobalPool() already has the logic to check whether the set of methods found is consistent; if it is, it just returns the first one (which is fine for this case). We’re going to be exercising that logic more with this change, but it’s been tested fairly well already because distinct modules would still introduce methods with the same selector and signature even before this change.

	- Doug

> - Fariborz
> 
> On Jun 20, 2013, at 5:20 PM, Douglas Gregor <dgregor at apple.com> wrote:
> 
>> Author: dgregor
>> Date: Thu Jun 20 19:20:25 2013
>> New Revision: 184504
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=184504&view=rev
>> Log:
>> When building a module, keep *all* declared methods in the global method pool.
>> 
>> As an optimization, we only kept declared methods with distinct
>> signatures in the global method pool, to keep the method lists
>> small. Under modules, however, one could have two different methods
>> with the same signature that occur in different (sub)modules. If only
>> the later submodule is important, message sends to 'id' with that
>> selector would fail because the first method (the only one that got
>> into the method pool) was hidden. When building a module, keep *all*
>> of the declared methods.
>> 
>> I did a quick check of both module build time and uses of modules, and
>> found no performance regression despite this causing us to keep more
>> methods in the global method pool. Fixes <rdar://problem/14148896>.
>> 
>> Modified:
>>   cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>>   cfe/trunk/test/Modules/Inputs/MethodPoolBSub.h
>>   cfe/trunk/test/Modules/Inputs/module.map
>>   cfe/trunk/test/Modules/method_pool.m
>> 
>> Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=184504&r1=184503&r2=184504&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Thu Jun 20 19:20:25 2013
>> @@ -2130,6 +2130,10 @@ void Sema::addMethodToGlobalList(ObjCMet
>>  // signature.
>>  ObjCMethodList *Previous = List;
>>  for (; List; Previous = List, List = List->getNext()) {
>> +    // If we are building a module, keep all of the methods.
>> +    if (getLangOpts().Modules && !getLangOpts().CurrentModule.empty())
>> +      continue;
>> +
>>    if (!MatchTwoMethodDeclarations(Method, List->Method))
>>      continue;
>> 
>> 
>> Modified: cfe/trunk/test/Modules/Inputs/MethodPoolBSub.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/MethodPoolBSub.h?rev=184504&r1=184503&r2=184504&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/MethodPoolBSub.h (original)
>> +++ cfe/trunk/test/Modules/Inputs/MethodPoolBSub.h Thu Jun 20 19:20:25 2013
>> @@ -1,4 +1,5 @@
>> @interface B (Sub)
>> - (char *)method3;
>> - (char*)method4;
>> +- (id)method6;
>> @end
>> 
>> Modified: cfe/trunk/test/Modules/Inputs/module.map
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module.map?rev=184504&r1=184503&r2=184504&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/module.map (original)
>> +++ cfe/trunk/test/Modules/Inputs/module.map Thu Jun 20 19:20:25 2013
>> @@ -131,6 +131,10 @@ module MethodPoolA {
>> module MethodPoolB {
>>  header "MethodPoolB.h"
>> 
>> +  explicit module Sub2 {
>> +    header "MethodPoolBSub2.h"
>> +  }
>> +
>>  explicit module Sub {
>>    header "MethodPoolBSub.h"
>>  }
>> 
>> Modified: cfe/trunk/test/Modules/method_pool.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/method_pool.m?rev=184504&r1=184503&r2=184504&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/Modules/method_pool.m (original)
>> +++ cfe/trunk/test/Modules/method_pool.m Thu Jun 20 19:20:25 2013
>> @@ -47,6 +47,10 @@ void testMethod3Again(id object) {
>>  char *str = [object method3]; // okay: only found in MethodPoolB.Sub
>> }
>> 
>> +void testMethod6(id object) {
>> +  [object method6];
>> +}
>> +
>> @import MethodPoolA.Sub;
>> 
>> void testMethod3AgainAgain(id object) {
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130621/61406b54/attachment.html>