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 +@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]; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
