[PATCH] D110123: [Proof of concept] Serialize fewer transitive methods in `METHOD_POOL`.

[Volodymyr Sapsai via Phabricator via cfe-commits]

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

Simplify the test and make it less sensitive to what "method" clang selects to 
use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110123

Files:
  clang/lib/Serialization/ASTWriter.cpp
  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/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -3130,15 +3130,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/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/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -3130,15 +3130,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;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits