[PATCH] D110123: [Proof of concept] Serialize fewer transitive methods in `METHOD_POOL`.
vsapsai updated this revision to Diff 382477.
vsapsai added a comment.
Rebase and update the commit message.
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
+@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}}
+}
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(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;
+
[PATCH] D110123: [Proof of concept] Serialize fewer transitive methods in `METHOD_POOL`.
vsapsai updated this revision to Diff 379516.
vsapsai added a comment.
Try to preserve the order of methods when populating the global method pool.
This attempt is more successful at keeping the existing code working though I
still have 1 unexpected warning that requires further investigation.
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
+@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}}
+}
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
@@ -3017,11 +3017,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);
}
@@ -3052,13 +3052,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();
@@ -3079,15 +3079,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
@@ -3130,15 +3135,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 &&
[PATCH] D110123: [Proof of concept] Serialize fewer transitive methods in `METHOD_POOL`.
vsapsai updated this revision to Diff 376084.
vsapsai added a comment.
Visit all required modules and don't serialize methods from other modules.
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/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
+@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/Immediate.framework/Headers/Immediate.h:3{{using}}
+ // expected-note@Frameworks/Indirect.framework/Headers/Indirect.h:5{{also found}}
+}
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -3017,11 +3017,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);
}
@@ -3052,13 +3052,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();
@@ -3079,15 +3079,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
@@ -3130,15 +3135,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
=
[PATCH] D110123: [Proof of concept] Serialize fewer transitive methods in `METHOD_POOL`.
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
+@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}}
+}
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
+@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}}
+}
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.In
[PATCH] D110123: [Proof of concept] Serialize fewer transitive methods in `METHOD_POOL`.
vsapsai updated this revision to Diff 374016.
vsapsai added a comment.
Add a [failing] test case that checks handling methods from transitive modules.
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,46 @@
+// 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
+
+int foo(id obj) {
+ return [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}}
+}
+
+void bar(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,46 @@
+// 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
+
+int foo(id obj) {
+ return [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}}
+}
+
+void bar(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/S
[PATCH] D110123: [Proof of concept] Serialize fewer transitive methods in `METHOD_POOL`.
vsapsai created this revision.
Herald added a subscriber: ributzka.
vsapsai requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
We are already making an effort to skip selectors that have methods only
from other modules. But in edge cases we keep all the methods which
leads to massive duplication. Try to cut down on the duplication in
ASTWriter and see if it helps with the synthetic test case.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D110123
Files:
clang/lib/Serialization/ASTWriter.cpp
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/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
