[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

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 different.
+            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;

clang/test/Modules/compare-objc-interface.m

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

clang/test/Modules/hidden-duplicates.m

@@ -32,6 +32,7 @@ @protocol ForwardDeclaredProtocolWithoutDefinition;
 
 @interface NSObject @end
 @class ForwardDeclaredInterfaceWithoutDefinition;
+@interface NSObject(CategoryForTesting) @end
 
 NSObject *interfaceDefinition(NSObject *o);
 NSObject *forwardDeclaredInterface(NSObject *o);

clang/test/Modules/objc-categories.m

@@ -8,8 +8,6 @@
 
 @import category_bottom;
 
-// expected-note@Inputs/category_left.h:14 {{previous definition}}
-// expected-warning@Inputs/category_right.h:12 {{duplicate definition of category}}
 // expected-note@Inputs/category_top.h:1 {{receiver is instance of class declared here}}
 
 @interface Foo(Source)