vsapsai updated this revision to Diff 384606.
vsapsai added a comment.

Rebase. Hopefully MLIR build is fixed and I'll be able to see relevant 
pre-merge check results.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110123/new/

https://reviews.llvm.org/D110123

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/lookup.m
  clang/test/Modules/method_pool_transitive.m

Index: clang/test/Modules/method_pool_transitive.m
===================================================================
--- /dev/null
+++ 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 <Indirect/Indirect.h>
+@interface Immediate : NSObject
+- (void)method;
+@end
+
+//--- Frameworks/Immediate.framework/Modules/module.modulemap
+framework module Immediate {
+  header "Immediate.h"
+  export *
+}
+
+//--- test.m
+#import <Immediate/Immediate.h>
+
+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}}
+}
Index: clang/test/Modules/lookup.m
===================================================================
--- clang/test/Modules/lookup.m
+++ 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;
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ 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<uint16_t>(FullFactoryBits);
     for (const ObjCMethodList *Method = &Methods.Instance; Method;
          Method = Method->getNext())
-      if (Method->getMethod())
+      if (ShouldWriteMethodListNode(Method))
         LE.write<uint32_t>(Writer.getDeclID(Method->getMethod()));
     for (const ObjCMethodList *Method = &Methods.Factory; Method;
          Method = Method->getNext())
-      if (Method->getMethod())
+      if (ShouldWriteMethodListNode(Method))
         LE.write<uint32_t>(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;
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ 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<ObjCMethodDecl *> 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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to