[clang] 0314ba3 - [modules] Fix marking `ObjCMethodDecl::isOverriding` when there are no overrides.

[Volodymyr Sapsai via cfe-commits]

Author: Volodymyr Sapsai
Date: 2022-11-24T14:26:02-08:00
New Revision: 0314ba3acbabd8dc6d39b6d49798d22b6dc33802

URL: https://github.com/llvm/llvm-project/commit/0314ba3acbabd8dc6d39b6d49798d22b6dc33802
DIFF: https://github.com/llvm/llvm-project/commit/0314ba3acbabd8dc6d39b6d49798d22b6dc33802.diff

LOG: [modules] Fix marking `ObjCMethodDecl::isOverriding` when there are no overrides.

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

Differential Revision: https://reviews.llvm.org/D138630

Added: 
    clang/test/Modules/override.m

Modified: 
    clang/lib/Sema/SemaDeclObjC.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index 97dff49862bb5..0cd764552b932 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -4438,6 +4438,11 @@ void Sema::CheckObjCMethodOverrides(ObjCMethodDecl *ObjCMethod,
                                     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 @@ void Sema::CheckObjCMethodOverrides(ObjCMethodDecl *ObjCMethod,
   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 @@ void Sema::CheckObjCMethodOverrides(ObjCMethodDecl *ObjCMethod,
               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);

diff  --git a/clang/test/Modules/override.m b/clang/test/Modules/override.m
new file mode 100644
index 0000000000000..247c2be76ee14
--- /dev/null
+++ b/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 
diff erent module, it's not an
+// override as it is the same method and it has the same DeclContext but a
+// 
diff erent object in the memory.
+
+
+//--- include/CheckOverride.h
+ at interface NSObject
+ at end
+
+ at interface CheckOverrideInterfaceOnly: NSObject
+- (void)potentialOverrideInterfaceOnly;
+ at end
+
+ at interface CheckOverrideCategoryOnly: NSObject
+ at end
+ at interface CheckOverrideCategoryOnly(CategoryOnly)
+- (void)potentialOverrideCategoryOnly;
+ at end
+
+ at interface CheckOverrideImplementationOfInterface: NSObject
+- (void)potentialOverrideImplementationOfInterface;
+ at end
+
+ at interface CheckOverrideImplementationOfCategory: NSObject
+ at end
+ at interface CheckOverrideImplementationOfCategory(CategoryImpl)
+- (void)potentialOverrideImplementationOfCategory;
+ at 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>
+
+ at implementation CheckOverrideImplementationOfInterface
+- (void)potentialOverrideImplementationOfInterface {}
+ at end
+
+ at implementation CheckOverrideImplementationOfCategory
+- (void)potentialOverrideImplementationOfCategory {}
+ at end
+
+void triggerOverrideCheck(CheckOverrideInterfaceOnly *intfOnly,
+                          CheckOverrideCategoryOnly *catOnly,
+                          CheckOverrideImplementationOfInterface *intfImpl,
+                          CheckOverrideImplementationOfCategory *catImpl) {
+  [intfOnly potentialOverrideInterfaceOnly];
+  [catOnly potentialOverrideCategoryOnly];
+  [intfImpl potentialOverrideImplementationOfInterface];
+  [catImpl potentialOverrideImplementationOfCategory];
+}