diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -8151,13 +8151,16 @@ if (Reader.DeserializationListener) Reader.DeserializationListener->SelectorRead(Data.ID, Sel); - InstanceMethods.append(Data.Instance.begin(), Data.Instance.end()); - FactoryMethods.append(Data.Factory.begin(), Data.Factory.end()); + // Append methods in the reverse order, so that later we can process them + // in the order they appear in the source code by iterating through + // the vector in the reverse order. + InstanceMethods.append(Data.Instance.rbegin(), Data.Instance.rend()); + FactoryMethods.append(Data.Factory.rbegin(), Data.Factory.rend()); InstanceBits = Data.InstanceBits; FactoryBits = Data.FactoryBits; InstanceHasMoreThanOneDecl = Data.InstanceHasMoreThanOneDecl; FactoryHasMoreThanOneDecl = Data.FactoryHasMoreThanOneDecl; - return true; + return false; } /// Retrieve the instance methods found by this visitor. @@ -8186,9 +8189,8 @@ /// Add the given set of methods to the method list. static void addMethodsToPool(Sema &S, ArrayRef Methods, ObjCMethodList &List) { - for (unsigned I = 0, N = Methods.size(); I != N; ++I) { - S.addMethodToGlobalList(&List, Methods[I]); - } + for (auto I = Methods.rbegin(), E = Methods.rend(); I != E; ++I) + S.addMethodToGlobalList(&List, *I); } void ASTReader::ReadMethodPool(Selector Sel) { diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -3045,11 +3045,11 @@ unsigned DataLen = 4 + 2 + 2; // 2 bytes for each of the method counts for (const ObjCMethodList *Method = &Methods.Instance; Method; Method = Method->getNext()) - if (Method->getMethod()) + if (ShouldWriteMethodListNode(Method)) DataLen += 4; for (const ObjCMethodList *Method = &Methods.Factory; Method; Method = Method->getNext()) - if (Method->getMethod()) + if (ShouldWriteMethodListNode(Method)) DataLen += 4; return emitULEBKeyDataLength(KeyLen, DataLen, Out); } @@ -3080,13 +3080,13 @@ unsigned NumInstanceMethods = 0; for (const ObjCMethodList *Method = &Methods.Instance; Method; Method = Method->getNext()) - if (Method->getMethod()) + if (ShouldWriteMethodListNode(Method)) ++NumInstanceMethods; unsigned NumFactoryMethods = 0; for (const ObjCMethodList *Method = &Methods.Factory; Method; Method = Method->getNext()) - if (Method->getMethod()) + if (ShouldWriteMethodListNode(Method)) ++NumFactoryMethods; unsigned InstanceBits = Methods.Instance.getBits(); @@ -3107,15 +3107,20 @@ LE.write(FullFactoryBits); for (const ObjCMethodList *Method = &Methods.Instance; Method; Method = Method->getNext()) - if (Method->getMethod()) + if (ShouldWriteMethodListNode(Method)) LE.write(Writer.getDeclID(Method->getMethod())); for (const ObjCMethodList *Method = &Methods.Factory; Method; Method = Method->getNext()) - if (Method->getMethod()) + if (ShouldWriteMethodListNode(Method)) LE.write(Writer.getDeclID(Method->getMethod())); assert(Out.tell() - Start == DataLen && "Data length is wrong"); } + +private: + static bool ShouldWriteMethodListNode(const ObjCMethodList *Node) { + return (Node->getMethod() && !Node->getMethod()->isFromASTFile()); + } }; } // namespace @@ -3158,15 +3163,21 @@ if (Chain && ID < FirstSelectorID) { // Selector already exists. Did it change? bool changed = false; - for (ObjCMethodList *M = &Data.Instance; - !changed && M && M->getMethod(); M = M->getNext()) { - if (!M->getMethod()->isFromASTFile()) + for (ObjCMethodList *M = &Data.Instance; M && M->getMethod(); + M = M->getNext()) { + if (!M->getMethod()->isFromASTFile()) { changed = true; + Data.Instance = *M; + break; + } } - for (ObjCMethodList *M = &Data.Factory; !changed && M && M->getMethod(); + for (ObjCMethodList *M = &Data.Factory; M && M->getMethod(); M = M->getNext()) { - if (!M->getMethod()->isFromASTFile()) + if (!M->getMethod()->isFromASTFile()) { changed = true; + Data.Factory = *M; + break; + } } if (!changed) continue; diff --git a/clang/test/Modules/lookup.m b/clang/test/Modules/lookup.m --- a/clang/test/Modules/lookup.m +++ b/clang/test/Modules/lookup.m @@ -10,8 +10,8 @@ void test(id x) { [x method]; // expected-warning@-1{{multiple methods named 'method' found}} -// expected-note@Inputs/lookup_left.h:2{{using}} -// expected-note@Inputs/lookup_right.h:3{{also found}} +// expected-note@Inputs/lookup_right.h:3{{using}} +// expected-note@Inputs/lookup_left.h:2{{also found}} } // CHECK-PRINT: - (int)method; diff --git a/clang/test/Modules/method_pool_transitive.m b/clang/test/Modules/method_pool_transitive.m new file mode 100644 --- /dev/null +++ b/clang/test/Modules/method_pool_transitive.m @@ -0,0 +1,40 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: %clang_cc1 -Wobjc-multiple-method-names -fsyntax-only -fmodules-cache-path=%t/modules.cache -fmodules -fimplicit-module-maps -F %t/Frameworks %t/test.m -verify + +// Verify we are handling methods from transitive modules, not just from immediate ones. + +//--- Frameworks/Indirect.framework/Headers/Indirect.h +@interface NSObject +@end + +@interface Indirect : NSObject +- (int)method; +@end + +//--- Frameworks/Indirect.framework/Modules/module.modulemap +framework module Indirect { + header "Indirect.h" + export * +} + +//--- Frameworks/Immediate.framework/Headers/Immediate.h +#import +@interface Immediate : NSObject +- (void)method; +@end + +//--- Frameworks/Immediate.framework/Modules/module.modulemap +framework module Immediate { + header "Immediate.h" + export * +} + +//--- test.m +#import + +void test(id obj) { + [obj method]; // expected-warning{{multiple methods named 'method' found}} + // expected-note@Frameworks/Indirect.framework/Headers/Indirect.h:5{{using}} + // expected-note@Frameworks/Immediate.framework/Headers/Immediate.h:3{{also found}} +}