vsapsai created this revision.
vsapsai added reviewers: ChuanqiXu, jansvoboda11.
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.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D144149
Files:
clang/lib/Serialization/ASTReaderDecl.cpp
clang/test/Modules/compare-objc-interface.m
clang/test/Modules/hidden-duplicates.m
clang/test/Modules/objc-categories.m
Index: clang/test/Modules/objc-categories.m
===================================================================
--- clang/test/Modules/objc-categories.m
+++ 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)
Index: clang/test/Modules/hidden-duplicates.m
===================================================================
--- clang/test/Modules/hidden-duplicates.m
+++ clang/test/Modules/hidden-duplicates.m
@@ -32,6 +32,7 @@
@interface NSObject @end
@class ForwardDeclaredInterfaceWithoutDefinition;
+@interface NSObject(CategoryForTesting) @end
NSObject *interfaceDefinition(NSObject *o);
NSObject *forwardDeclaredInterface(NSObject *o);
Index: clang/test/Modules/compare-objc-interface.m
===================================================================
--- clang/test/Modules/compare-objc-interface.m
+++ clang/test/Modules/compare-objc-interface.m
@@ -444,3 +444,54 @@
// 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
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ 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 @@
// 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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
