vsapsai created this revision.
vsapsai added reviewers: egorzhdan, akyrtzi.
Herald added a subscriber: ributzka.
Herald added a project: All.
vsapsai requested review of this revision.
Herald added a project: clang.
Incorrect `isOverriding` flag triggers the assertion
`!Overridden.empty()` in `ObjCMethodDecl::getOverriddenMethods` when a
method is marked as overriding but we cannot find any overrides.
When a method is declared in a category and defined in implementation,
we don't treat it as an override because it is the same method with
a separate declaration and a definition. But with modules we can find
a method declaration both in a modular category and a non-modular category
with different memory addresses. Thus we erroneously conclude the method
is overriding. Fix by comparing canonical declarations that are the same
for equal entities coming from different modules.
rdar://92845511
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D138630
Files:
clang/lib/Sema/SemaDeclObjC.cpp
clang/test/Modules/override.m
Index: clang/test/Modules/override.m
===================================================================
--- /dev/null
+++ clang/test/Modules/override.m
@@ -0,0 +1,69 @@
+// UNSUPPORTED: -aix
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -I%t/include %t/test.m \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=CheckOverride
+
+// Test that if we have the same method in a different module, it's not an
+// override as it is the same method and it has the same DeclContext but a
+// different object in the memory.
+
+
+//--- include/CheckOverride.h
+@interface NSObject
+@end
+
+@interface CheckOverrideInterfaceOnly: NSObject
+- (void)potentialOverrideInterfaceOnly;
+@end
+
+@interface CheckOverrideCategoryOnly: NSObject
+@end
+@interface CheckOverrideCategoryOnly(CategoryOnly)
+- (void)potentialOverrideCategoryOnly;
+@end
+
+@interface CheckOverrideImplementationOfInterface: NSObject
+- (void)potentialOverrideImplementationOfInterface;
+@end
+
+@interface CheckOverrideImplementationOfCategory: NSObject
+@end
+@interface CheckOverrideImplementationOfCategory(CategoryImpl)
+- (void)potentialOverrideImplementationOfCategory;
+@end
+
+//--- include/Redirect.h
+// Ensure CheckOverride is imported as the module despite all `-fmodule-name` flags.
+#import <CheckOverride.h>
+
+//--- include/module.modulemap
+module CheckOverride {
+ header "CheckOverride.h"
+}
+module Redirect {
+ header "Redirect.h"
+ export *
+}
+
+//--- test.m
+#import <CheckOverride.h>
+#import <Redirect.h>
+
+@implementation CheckOverrideImplementationOfInterface
+- (void)potentialOverrideImplementationOfInterface {}
+@end
+
+@implementation CheckOverrideImplementationOfCategory
+- (void)potentialOverrideImplementationOfCategory {}
+@end
+
+void triggerOverrideCheck(CheckOverrideInterfaceOnly *intfOnly,
+ CheckOverrideCategoryOnly *catOnly,
+ CheckOverrideImplementationOfInterface *intfImpl,
+ CheckOverrideImplementationOfCategory *catImpl) {
+ [intfOnly potentialOverrideInterfaceOnly];
+ [catOnly potentialOverrideCategoryOnly];
+ [intfImpl potentialOverrideImplementationOfInterface];
+ [catImpl potentialOverrideImplementationOfCategory];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===================================================================
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4438,6 +4438,11 @@
ResultTypeCompatibilityKind RTC) {
if (!ObjCMethod)
return;
+ auto IsMethodInCurrentClass = [CurrentClass](const ObjCMethodDecl *M) {
+ // Checking canonical decl works across modules.
+ return M->getClassInterface()->getCanonicalDecl() ==
+ CurrentClass->getCanonicalDecl();
+ };
// Search for overridden methods and merge information down from them.
OverrideSearch overrides(*this, ObjCMethod);
// Keep track if the method overrides any method in the class's base classes,
@@ -4449,8 +4454,7 @@
for (ObjCMethodDecl *overridden : overrides) {
if (!hasOverriddenMethodsInBaseOrProtocol) {
if (isa<ObjCProtocolDecl>(overridden->getDeclContext()) ||
- CurrentClass != overridden->getClassInterface() ||
- overridden->isOverriding()) {
+ !IsMethodInCurrentClass(overridden) || overridden->isOverriding()) {
CheckObjCMethodDirectOverrides(ObjCMethod, overridden);
hasOverriddenMethodsInBaseOrProtocol = true;
} else if (isa<ObjCImplDecl>(ObjCMethod->getDeclContext())) {
@@ -4475,7 +4479,7 @@
OverrideSearch overrides(*this, overridden);
for (ObjCMethodDecl *SuperOverridden : overrides) {
if (isa<ObjCProtocolDecl>(SuperOverridden->getDeclContext()) ||
- CurrentClass != SuperOverridden->getClassInterface()) {
+ !IsMethodInCurrentClass(SuperOverridden)) {
CheckObjCMethodDirectOverrides(ObjCMethod, SuperOverridden);
hasOverriddenMethodsInBaseOrProtocol = true;
overridden->setOverriding(true);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
