[PATCH] D154460: [ODRHash] Stop hashing `ObjCMethodDecl::isPropertyAccessor` as it doesn't capture inherent method quality.
vsapsai created this revision. vsapsai added reviewers: ChuanqiXu, jansvoboda11, Bigcheese. Herald added a subscriber: ributzka. Herald added a project: All. vsapsai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. `isPropertyAccessor` depends on the surrounding code and not on the method itself. That's why it can be different in different modules. And mismatches shouldn't be an error. rdar://109481753 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D154460 Files: clang/lib/AST/ODRHash.cpp clang/test/Modules/compare-objc-nonisolated-methods.m Index: clang/test/Modules/compare-objc-nonisolated-methods.m === --- clang/test/Modules/compare-objc-nonisolated-methods.m +++ clang/test/Modules/compare-objc-nonisolated-methods.m @@ -5,12 +5,17 @@ // is not an error because it depends on the surrounding code and not on the method itself. // RUN: %clang_cc1 -fsyntax-only -verify -I%t/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=Override %t/test-overriding.m +// Test that different values of `ObjCMethodDecl::isPropertyAccessor` in different modules +// is not an error because it depends on the surrounding code and not on the method itself. +// RUN: %clang_cc1 -fsyntax-only -verify -I%t/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=PropertyAccessor %t/test-property_accessor.m + //--- include/Common.h @interface NSObject @end //--- include/Indirection.h #import +#import //--- include/module.modulemap module Common { @@ -25,6 +30,10 @@ header "Override.h" export * } +module PropertyAccessor { + header "PropertyAccessor.h" + export * +} //--- include/Override.h #import @@ -52,3 +61,31 @@ void triggerOverrideCheck(SubClass *sc) { [sc potentialOverride]; } + +//--- include/PropertyAccessor.h +#import +@interface PropertySubClass: NSObject +- (int)potentialProperty; +- (void)setPotentialProperty:(int)p; +@end + +//--- PropertyAccessor_Internal.h +#import +@interface PropertySubClass() +@property int potentialProperty; +@end + +//--- test-property_accessor.m +//expected-no-diagnostics +// Get a version of `PropertySubClass` where `-[PropertySubClass potentialProperty]` +// is a property accessor. +#import "PropertyAccessor_Internal.h" + +// Get a version of `PropertySubClass` where `-[PropertySubClass potentialProperty]` +// is not a property accessor because module "PropertyAccessor" doesn't know about PropertyAccessor_Internal.h. +#import + +void triggerPropertyAccessorCheck(PropertySubClass *x) { + int tmp = [x potentialProperty]; + [x setPotentialProperty: tmp]; +} Index: clang/lib/AST/ODRHash.cpp === --- clang/lib/AST/ODRHash.cpp +++ clang/lib/AST/ODRHash.cpp @@ -373,7 +373,6 @@ void VisitObjCMethodDecl(const ObjCMethodDecl *Method) { ID.AddInteger(Method->getDeclKind()); Hash.AddBoolean(Method->isInstanceMethod()); // false if class method -Hash.AddBoolean(Method->isPropertyAccessor()); Hash.AddBoolean(Method->isVariadic()); Hash.AddBoolean(Method->isSynthesizedAccessorStub()); Hash.AddBoolean(Method->isDefined()); Index: clang/test/Modules/compare-objc-nonisolated-methods.m === --- clang/test/Modules/compare-objc-nonisolated-methods.m +++ clang/test/Modules/compare-objc-nonisolated-methods.m @@ -5,12 +5,17 @@ // is not an error because it depends on the surrounding code and not on the method itself. // RUN: %clang_cc1 -fsyntax-only -verify -I%t/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=Override %t/test-overriding.m +// Test that different values of `ObjCMethodDecl::isPropertyAccessor` in different modules +// is not an error because it depends on the surrounding code and not on the method itself. +// RUN: %clang_cc1 -fsyntax-only -verify -I%t/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=PropertyAccessor %t/test-property_accessor.m + //--- include/Common.h @interface NSObject @end //--- include/Indirection.h #import +#import //--- include/module.modulemap module Common { @@ -25,6 +30,10 @@ header "Override.h" export * } +module PropertyAccessor { + header "PropertyAccessor.h" + export * +} //--- include/Override.h #import @@ -52,3 +61,31 @@ void triggerOverrideCheck(SubClass *sc) { [sc potentialOverride]; } + +//--- include/PropertyAccessor.h +#import +@interface PropertySubClass: NSObject +- (int)potentialProperty; +- (void)setPotentialProperty:(int)p; +@end + +//--- PropertyAccessor_Internal.h +#import +@interface PropertySubClass() +@property int potentialProperty; +@end + +//--- test-property_accessor.m +//expected-no-diagnostics +//
[PATCH] D154460: [ODRHash] Stop hashing `ObjCMethodDecl::isPropertyAccessor` as it doesn't capture inherent method quality.
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf7e0aae7284b: [ODRHash] Stop hashing `ObjCMethodDecl::isPropertyAccessor` as it doesn't… (authored by vsapsai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154460/new/ https://reviews.llvm.org/D154460 Files: clang/lib/AST/ODRHash.cpp clang/test/Modules/compare-objc-nonisolated-methods.m Index: clang/test/Modules/compare-objc-nonisolated-methods.m === --- clang/test/Modules/compare-objc-nonisolated-methods.m +++ clang/test/Modules/compare-objc-nonisolated-methods.m @@ -5,12 +5,17 @@ // is not an error because it depends on the surrounding code and not on the method itself. // RUN: %clang_cc1 -fsyntax-only -verify -I%t/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=Override %t/test-overriding.m +// Test that different values of `ObjCMethodDecl::isPropertyAccessor` in different modules +// is not an error because it depends on the surrounding code and not on the method itself. +// RUN: %clang_cc1 -fsyntax-only -verify -I%t/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=PropertyAccessor %t/test-property_accessor.m + //--- include/Common.h @interface NSObject @end //--- include/Indirection.h #import +#import //--- include/module.modulemap module Common { @@ -25,6 +30,10 @@ header "Override.h" export * } +module PropertyAccessor { + header "PropertyAccessor.h" + export * +} //--- include/Override.h #import @@ -52,3 +61,31 @@ void triggerOverrideCheck(SubClass *sc) { [sc potentialOverride]; } + +//--- include/PropertyAccessor.h +#import +@interface PropertySubClass: NSObject +- (int)potentialProperty; +- (void)setPotentialProperty:(int)p; +@end + +//--- PropertyAccessor_Internal.h +#import +@interface PropertySubClass() +@property int potentialProperty; +@end + +//--- test-property_accessor.m +//expected-no-diagnostics +// Get a version of `PropertySubClass` where `-[PropertySubClass potentialProperty]` +// is a property accessor. +#import "PropertyAccessor_Internal.h" + +// Get a version of `PropertySubClass` where `-[PropertySubClass potentialProperty]` +// is not a property accessor because module "PropertyAccessor" doesn't know about PropertyAccessor_Internal.h. +#import + +void triggerPropertyAccessorCheck(PropertySubClass *x) { + int tmp = [x potentialProperty]; + [x setPotentialProperty: tmp]; +} Index: clang/lib/AST/ODRHash.cpp === --- clang/lib/AST/ODRHash.cpp +++ clang/lib/AST/ODRHash.cpp @@ -373,7 +373,6 @@ void VisitObjCMethodDecl(const ObjCMethodDecl *Method) { ID.AddInteger(Method->getDeclKind()); Hash.AddBoolean(Method->isInstanceMethod()); // false if class method -Hash.AddBoolean(Method->isPropertyAccessor()); Hash.AddBoolean(Method->isVariadic()); Hash.AddBoolean(Method->isSynthesizedAccessorStub()); Hash.AddBoolean(Method->isDefined()); Index: clang/test/Modules/compare-objc-nonisolated-methods.m === --- clang/test/Modules/compare-objc-nonisolated-methods.m +++ clang/test/Modules/compare-objc-nonisolated-methods.m @@ -5,12 +5,17 @@ // is not an error because it depends on the surrounding code and not on the method itself. // RUN: %clang_cc1 -fsyntax-only -verify -I%t/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=Override %t/test-overriding.m +// Test that different values of `ObjCMethodDecl::isPropertyAccessor` in different modules +// is not an error because it depends on the surrounding code and not on the method itself. +// RUN: %clang_cc1 -fsyntax-only -verify -I%t/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=PropertyAccessor %t/test-property_accessor.m + //--- include/Common.h @interface NSObject @end //--- include/Indirection.h #import +#import //--- include/module.modulemap module Common { @@ -25,6 +30,10 @@ header "Override.h" export * } +module PropertyAccessor { + header "PropertyAccessor.h" + export * +} //--- include/Override.h #import @@ -52,3 +61,31 @@ void triggerOverrideCheck(SubClass *sc) { [sc potentialOverride]; } + +//--- include/PropertyAccessor.h +#import +@interface PropertySubClass: NSObject +- (int)potentialProperty; +- (void)setPotentialProperty:(int)p; +@end + +//--- PropertyAccessor_Internal.h +#import +@interface PropertySubClass() +@property int potentialProperty; +@end + +//--- test-property_accessor.m +//expected-no-diagnostics +// Get a version of `PropertySubClass` where `-[PropertySubClass potentialProperty]` +// is a property accessor. +#import "PropertyAccessor_