[clang] 2893d55 - [Serialization] Don't warn when a deserialized category is equivalent to an existing one.

[Volodymyr Sapsai via cfe-commits]

Author: Volodymyr Sapsai
Date: 2023-02-22T11:01:40-08:00
New Revision: 2893d55f8f61edb2c253b960cab1107ea6c163c2

URL: https://github.com/llvm/llvm-project/commit/2893d55f8f61edb2c253b960cab1107ea6c163c2
DIFF: https://github.com/llvm/llvm-project/commit/2893d55f8f61edb2c253b960cab1107ea6c163c2.diff

LOG: [Serialization] Don't warn when a deserialized category is equivalent to an existing one.

A single class allows multiple categories to be defined for it. But if
two of such categories have the same name, we emit a warning. It's not a
hard error but a good indication of a potential mistake.

With modules, we can end up with the same category in different modules.
Diagnosing such a situation has little value as the categories in
different modules are equivalent and don't reflect the usage of the same
name for different purposes. When we deserialize a duplicate category,
compare it to an existing one and warn only when the new one is
different.

rdar://104582081

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

Added: 
    

Modified: 
    clang/lib/Serialization/ASTReaderDecl.cpp
    clang/test/Modules/compare-objc-interface.m
    clang/test/Modules/hidden-duplicates.m
    clang/test/Modules/objc-categories.m

Removed: 
    


################################################################################
diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 8cb513eff13e0..b9bbc0ec9eb28 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -14,6 +14,7 @@
 #include "ASTCommon.h"
 #include "ASTReaderInternals.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/AttrIterator.h"
 #include "clang/AST/Decl.h"
@@ -4181,23 +4182,22 @@ namespace {
       // Check for duplicate categories.
       if (Cat->getDeclName()) {
         ObjCCategoryDecl *&Existing = NameCategoryMap[Cat->getDeclName()];
-        if (Existing &&
-            Reader.getOwningModuleFile(Existing)
-                                          != Reader.getOwningModuleFile(Cat)) {
-          // FIXME: We should not warn for duplicates in diamond:
-          //
-          //   MT     //
-          //  /  \    //
-          // ML  MR   //
-          //  \  /    //
-          //   MB     //
-          //
-          // If there are duplicates in ML/MR, there will be warning when
-          // creating MB *and* when importing MB. We should not warn when
-          // importing.
-          Reader.Diag(Cat->getLocation(), diag::warn_dup_category_def)
-            << Interface->getDeclName() << Cat->getDeclName();
-          Reader.Diag(Existing->getLocation(), diag::note_previous_definition);
+        if (Existing && Reader.getOwningModuleFile(Existing) !=
+                            Reader.getOwningModuleFile(Cat)) {
+          llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
+          StructuralEquivalenceContext Ctx(
+              Cat->getASTContext(), Existing->getASTContext(),
+              NonEquivalentDecls, StructuralEquivalenceKind::Default,
+              /*StrictTypeSpelling =*/false,
+              /*Complain =*/false,
+              /*ErrorOnTagTypeMismatch =*/true);
+          if (!Ctx.IsEquivalent(Cat, Existing)) {
+            // Warn only if the categories with the same name are 
diff erent.
+            Reader.Diag(Cat->getLocation(), diag::warn_dup_category_def)
+                << Interface->getDeclName() << Cat->getDeclName();
+            Reader.Diag(Existing->getLocation(),
+                        diag::note_previous_definition);
+          }
         } else if (!Existing) {
           // Record this category.
           Existing = Cat;

diff  --git a/clang/test/Modules/compare-objc-interface.m b/clang/test/Modules/compare-objc-interface.m
index 17a03de3ce29b..cc3b5fe60b1f5 100644
--- a/clang/test/Modules/compare-objc-interface.m
+++ b/clang/test/Modules/compare-objc-interface.m
@@ -444,3 +444,54 @@ @interface CompareLastImplAttribute: NSObject
 // expected-error at first.h:* {{'CompareLastImplAttribute' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found property 'lastImplAttribute' with 'direct' attribute}}
 // expected-note-re at second.h:* {{but in {{'Second'|definition here}} found property 'lastImplAttribute' with 
diff erent 'direct' attribute}}
 #endif
+
+#if defined(FIRST)
+ at interface CompareMatchingCategories: NSObject @end
+ at interface CompareMatchingCategories(Matching)
+- (int)testMethod;
+ at end
+
+ at interface CompareMismatchingCategories1: NSObject @end
+ at interface CompareMismatchingCategories1(Category1)
+- (void)presentMethod;
+ at end
+ at interface CompareMismatchingCategories2: NSObject @end
+ at interface CompareMismatchingCategories2(Category2)
+ at end
+
+ at interface CompareDifferentCategoryNames: NSObject @end
+ at interface CompareDifferentCategoryNames(CategoryFirst)
+- (void)firstMethod:(int)x;
+ at end
+#elif defined(SECOND)
+ at interface CompareMatchingCategories: NSObject @end
+ at interface CompareMatchingCategories(Matching)
+- (int)testMethod;
+ at end
+
+ at interface CompareMismatchingCategories1: NSObject @end
+ at interface CompareMismatchingCategories1(Category1)
+ at end
+ at interface CompareMismatchingCategories2: NSObject @end
+ at interface CompareMismatchingCategories2(Category2)
+- (void)presentMethod;
+ at end
+
+ at interface CompareDifferentCategoryNames: NSObject @end
+ at interface CompareDifferentCategoryNames(CategorySecond)
+- (void)secondMethod;
+ at end
+#else
+CompareMatchingCategories *compareMatchingCategories; // no diagnostic
+CompareMismatchingCategories1 *compareMismatchingCategories1;
+#ifdef TEST_MODULAR
+// expected-warning at second.h:* {{duplicate definition of category 'Category1' on interface 'CompareMismatchingCategories1'}}
+// expected-note at first.h:* {{previous definition is here}}
+#endif
+CompareMismatchingCategories2 *compareMismatchingCategories2;
+#ifdef TEST_MODULAR
+// expected-warning at second.h:* {{duplicate definition of category 'Category2' on interface 'CompareMismatchingCategories2'}}
+// expected-note at first.h:* {{previous definition is here}}
+#endif
+CompareDifferentCategoryNames *compareDifferentCategoryNames; // no diagnostic
+#endif

diff  --git a/clang/test/Modules/hidden-duplicates.m b/clang/test/Modules/hidden-duplicates.m
index b3af9697cc0b8..62c49424d1fc4 100644
--- a/clang/test/Modules/hidden-duplicates.m
+++ b/clang/test/Modules/hidden-duplicates.m
@@ -32,6 +32,7 @@ @protocol ForwardDeclaredProtocolWithoutDefinition;
 
 @interface NSObject @end
 @class ForwardDeclaredInterfaceWithoutDefinition;
+ at interface NSObject(CategoryForTesting) @end
 
 NSObject *interfaceDefinition(NSObject *o);
 NSObject *forwardDeclaredInterface(NSObject *o);

diff  --git a/clang/test/Modules/objc-categories.m b/clang/test/Modules/objc-categories.m
index fcffe3cc96200..8bc73a29c51cb 100644
--- a/clang/test/Modules/objc-categories.m
+++ b/clang/test/Modules/objc-categories.m
@@ -8,8 +8,6 @@
 
 @import category_bottom;
 
-// expected-note at Inputs/category_left.h:14 {{previous definition}}
-// expected-warning at Inputs/category_right.h:12 {{duplicate definition of category}}
 // expected-note at Inputs/category_top.h:1 {{receiver is instance of class declared here}}
 
 @interface Foo(Source)