vsapsai created this revision.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124289
Files:
clang/include/clang/AST/DeclObjC.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/test/Modules/compare-objc-interface.m
clang/test/Modules/interface-diagnose-missing-import.m
clang/test/Modules/method_pool.m
Index: clang/test/Modules/method_pool.m
===================================================================
--- clang/test/Modules/method_pool.m
+++ clang/test/Modules/method_pool.m
@@ -28,6 +28,9 @@
}
@import MethodPoolB;
+//FIXME: main definition should come from MethodPoolA and definition in MethodPoolB should be a duplicate
+// expected-error@MethodPoolA.h:* {{duplicate interface definition for class 'B'}}
+// expected-note@MethodPoolB.h:* {{previous definition is here}}
void testMethod1Again(id object) {
[object method1];
Index: clang/test/Modules/interface-diagnose-missing-import.m
===================================================================
--- clang/test/Modules/interface-diagnose-missing-import.m
+++ clang/test/Modules/interface-diagnose-missing-import.m
@@ -1,7 +1,9 @@
// RUN: rm -rf %t
// RUN: %clang_cc1 %s -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F%S/Inputs/interface-diagnose-missing-import -verify
// expected-no-diagnostics
-@interface Buggy
+@interface NSObject
+@end
+@interface Buggy: NSObject
@end
@import Foo.Bar;
Index: clang/test/Modules/compare-objc-interface.m
===================================================================
--- clang/test/Modules/compare-objc-interface.m
+++ clang/test/Modules/compare-objc-interface.m
@@ -11,13 +11,21 @@
// RUN: cat %t/test.m >> %t/include/second-textual.h
// RUN: echo "#undef SECOND" >> %t/include/second-textual.h
+// Build second modular header file
+// RUN: echo "#define SECOND" >> %t/include/second-modular.h
+// RUN: cat %t/test.m >> %t/include/second-modular.h
+// RUN: echo "#undef SECOND" >> %t/include/second-modular.h
+
// Test that each header can compile
// RUN: %clang_cc1 -fsyntax-only -x objective-c %t/include/first.h -fblocks -fobjc-arc
// RUN: %clang_cc1 -fsyntax-only -x objective-c %t/include/second-textual.h -fblocks -fobjc-arc
+// RUN: %clang_cc1 -fsyntax-only -x objective-c %t/include/second-modular.h -fblocks -fobjc-arc
// Run test
// RUN: %clang_cc1 -I%t/include -verify %t/test.m -fblocks -fobjc-arc \
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: %clang_cc1 -I%t/include -verify %t/test.m -fblocks -fobjc-arc -DTEST_MODULAR=1 \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
// In non-modular case we don't allow interface redefinitions. But with modules
// previous definition can come from a hidden [sub]module. And in this case we
@@ -54,14 +62,23 @@
export *
}
}
+module Second {
+ header "second-modular.h"
+ export *
+}
//--- test.m
#if defined(FIRST) || defined(SECOND)
-#include "common.h"
+# include "common.h"
#endif
+
#if !defined(FIRST) && !defined(SECOND)
-#include "first-empty.h"
-#include "second-textual.h"
+# include "first-empty.h"
+# ifdef TEST_MODULAR
+# include "second-modular.h"
+# else
+# include "second-textual.h"
+# endif
#endif
#if defined(FIRST)
@@ -81,6 +98,6 @@
@interface CompareSuperClass : CommonClass @end
#else
CompareSuperClass *compareSuperClass;
-// expected-error@second-textual.h:* {{duplicate interface definition for class 'CompareSuperClass'}}
+// expected-error@*:* {{duplicate interface definition for class 'CompareSuperClass'}}
// expected-note@first.h:* {{previous definition is here}}
#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -761,6 +761,8 @@
Record.AddSourceLocation(D->getEndOfDefinitionLoc());
Record.push_back(Data.HasDesignatedInitializers);
+ Record.push_back(D->getODRHash());
+
// Write out the protocols that are directly referenced by the @interface.
Record.push_back(Data.ReferencedProtocols.size());
for (const auto *P : D->protocols())
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1152,6 +1152,8 @@
Data.EndLoc = readSourceLocation();
Data.HasDesignatedInitializers = Record.readInt();
+ Data.ODRHash = Record.readInt();
+ Data.HasODRHash = true;
// Read the directly referenced protocols and their SourceLocations.
unsigned NumProtocols = Record.readInt();
@@ -1183,9 +1185,12 @@
Reader.MergedDeclContexts.insert(
std::make_pair(NewDD.Definition, DD.Definition));
Reader.mergeDefinitionVisibility(DD.Definition, NewDD.Definition);
+ if (D->getODRHash() != NewDD.Definition->getODRHash())
+ Reader.PendingObjCInterfaceOdrMergeFailures[D].push_back(
+ NewDD.Definition);
}
- // FIXME: odr checking?
+ // FIXME: odr checking for categories and extensions?
}
void ASTDeclReader::VisitObjCInterfaceDecl(ObjCInterfaceDecl *ID) {
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -42,6 +42,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticError.h"
#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/ExceptionSpecificationType.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
@@ -9400,7 +9401,8 @@
void ASTReader::diagnoseOdrViolations() {
if (PendingOdrMergeFailures.empty() && PendingOdrMergeChecks.empty() &&
PendingFunctionOdrMergeFailures.empty() &&
- PendingEnumOdrMergeFailures.empty())
+ PendingEnumOdrMergeFailures.empty() &&
+ PendingObjCInterfaceOdrMergeFailures.empty())
return;
// Trigger the import of the full definition of each class that had any
@@ -9422,6 +9424,17 @@
}
}
+ // Trigger the import of the full interface definition.
+ auto ObjCInterfaceOdrMergeFailures =
+ std::move(PendingObjCInterfaceOdrMergeFailures);
+ PendingObjCInterfaceOdrMergeFailures.clear();
+ for (auto &Merge : ObjCInterfaceOdrMergeFailures) {
+ Merge.first->decls_begin();
+ for (auto &Interface : Merge.second) {
+ Interface->decls_begin();
+ }
+ }
+
// Trigger the import of functions.
auto FunctionOdrMergeFailures = std::move(PendingFunctionOdrMergeFailures);
PendingFunctionOdrMergeFailures.clear();
@@ -9529,7 +9542,7 @@
}
if (OdrMergeFailures.empty() && FunctionOdrMergeFailures.empty() &&
- EnumOdrMergeFailures.empty())
+ EnumOdrMergeFailures.empty() && ObjCInterfaceOdrMergeFailures.empty())
return;
// Ensure we don't accidentally recursively enter deserialization while
@@ -11117,6 +11130,14 @@
}
}
+ for (auto &Merge : ObjCInterfaceOdrMergeFailures) {
+ for (auto &Interface : Merge.second) {
+ Diag(Interface->getLocation(), diag::err_duplicate_class_def)
+ << Interface->getDeclName();
+ Diag(Merge.first->getLocation(), diag::note_previous_definition);
+ }
+ }
+
// Issue ODR failures diagnostics for functions.
for (auto &Merge : FunctionOdrMergeFailures) {
enum ODRFunctionDifference {
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -1129,6 +1129,11 @@
llvm::SmallDenseMap<EnumDecl *, llvm::SmallVector<EnumDecl *, 2>, 2>
PendingEnumOdrMergeFailures;
+ /// ObjCInterfaceDecl in which we found an ODR violation.
+ llvm::SmallDenseMap<ObjCInterfaceDecl *,
+ llvm::SmallVector<ObjCInterfaceDecl *, 2>, 2>
+ PendingObjCInterfaceOdrMergeFailures;
+
/// DeclContexts in which we have diagnosed an ODR violation.
llvm::SmallPtrSet<DeclContext*, 2> DiagnosedOdrMergeFailures;
Index: clang/include/clang/AST/DeclObjC.h
===================================================================
--- clang/include/clang/AST/DeclObjC.h
+++ clang/include/clang/AST/DeclObjC.h
@@ -1150,6 +1150,7 @@
class ObjCInterfaceDecl : public ObjCContainerDecl
, public Redeclarable<ObjCInterfaceDecl> {
friend class ASTContext;
+ friend class ASTReader;
/// TypeForDecl - This indicates the Type object that represents this
/// TypeDecl. It is a cache maintained by ASTContext::getObjCInterfaceType
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
