[clang] 18530e5 - [ODRHash] Stop hashing `ObjCMethodDecl::isOverriding` as it doesn't capture inherent method quality.

Volodymyr Sapsai via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 5 18:10:20 PDT 2023


Author: Volodymyr Sapsai
Date: 2023-07-05T18:04:32-07:00
New Revision: 18530e5d0770098bc33ff6f02a7b63ea887692a6

URL: https://github.com/llvm/llvm-project/commit/18530e5d0770098bc33ff6f02a7b63ea887692a6
DIFF: https://github.com/llvm/llvm-project/commit/18530e5d0770098bc33ff6f02a7b63ea887692a6.diff

LOG: [ODRHash] Stop hashing `ObjCMethodDecl::isOverriding` as it doesn't capture inherent method quality.

`isOverriding` 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

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

Added: 
    clang/test/Modules/compare-objc-nonisolated-methods.m

Modified: 
    clang/lib/AST/ODRDiagsEmitter.cpp
    clang/lib/AST/ODRHash.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ODRDiagsEmitter.cpp b/clang/lib/AST/ODRDiagsEmitter.cpp
index d1c59bd1f2ca23..1994f0865428bd 100644
--- a/clang/lib/AST/ODRDiagsEmitter.cpp
+++ b/clang/lib/AST/ODRDiagsEmitter.cpp
@@ -2096,7 +2096,8 @@ bool ODRDiagsEmitter::diagnoseMismatch(
       << FirstDecl->getSourceRange();
   Diag(SecondDecl->getLocation(),
        diag::note_module_odr_violation_mismatch_decl_unknown)
-      << SecondModule << FirstDiffType << SecondDecl->getSourceRange();
+      << SecondModule.empty() << SecondModule << FirstDiffType
+      << SecondDecl->getSourceRange();
   return true;
 }
 

diff  --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp
index 0ead0940479d85..3ea023d3cee66a 100644
--- a/clang/lib/AST/ODRHash.cpp
+++ b/clang/lib/AST/ODRHash.cpp
@@ -377,7 +377,6 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
     Hash.AddBoolean(Method->isVariadic());
     Hash.AddBoolean(Method->isSynthesizedAccessorStub());
     Hash.AddBoolean(Method->isDefined());
-    Hash.AddBoolean(Method->isOverriding());
     Hash.AddBoolean(Method->isDirectMethod());
     Hash.AddBoolean(Method->isThisDeclarationADesignatedInitializer());
     Hash.AddBoolean(Method->hasSkippedBody());

diff  --git a/clang/test/Modules/compare-objc-nonisolated-methods.m b/clang/test/Modules/compare-objc-nonisolated-methods.m
new file mode 100644
index 00000000000000..a60114148420bd
--- /dev/null
+++ b/clang/test/Modules/compare-objc-nonisolated-methods.m
@@ -0,0 +1,54 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Test that 
diff erent values of `ObjCMethodDecl::isOverriding` in 
diff erent 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=Override %t/test-overriding.m
+
+//--- include/Common.h
+ at interface NSObject
+ at end
+
+//--- include/Indirection.h
+#import <Override.h>
+
+//--- include/module.modulemap
+module Common {
+  header "Common.h"
+  export *
+}
+module Indirection {
+  header "Indirection.h"
+  export *
+}
+module Override {
+  header "Override.h"
+  export *
+}
+
+//--- include/Override.h
+#import <Common.h>
+ at interface SubClass: NSObject
+- (void)potentialOverride;
+ at end
+
+//--- Override_Internal.h
+#import <Common.h>
+ at interface NSObject(InternalCategory)
+- (void)potentialOverride;
+ at end
+
+//--- test-overriding.m
+//expected-no-diagnostics
+// Get non-modular version of `SubClass`, so that `-[SubClass potentialOverride]`
+// is an override of a method in `InternalCategory`.
+#import "Override_Internal.h"
+#import <Override.h>
+
+// Get modular version of `SubClass` where `-[SubClass potentialOverride]` is
+// not an override because module "Override" doesn't know about Override_Internal.h.
+#import <Indirection.h>
+
+void triggerOverrideCheck(SubClass *sc) {
+  [sc potentialOverride];
+}


        


More information about the cfe-commits mailing list