vsapsai created this revision.
vsapsai added reviewers: jansvoboda11, rsmith, bruno.
Herald added a subscriber: ributzka.
Herald added a project: All.
vsapsai requested review of this revision.
Herald added a project: clang.
The purpose of the change is to start using ODR hash for comparison
instead of ASTStructuralEquivalence and ODR hash calculation is,
unfortunately, incomplete. The decision to use ODR hash is made because
we can use the same mechanism while checking duplicates encountered
during deserialization. For that case ASTStructuralEquivalence is not a
good fit because it can lead to excessive deserialization and
deserialized decls would be used only for ASTStructuralEquivalence.
rdar://82908223
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124287
Files:
clang/include/clang/AST/DeclObjC.h
clang/include/clang/AST/ODRHash.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/DeclObjC.cpp
clang/lib/AST/ODRHash.cpp
clang/lib/Parse/ParseObjc.cpp
clang/test/Modules/compare-objc-interface.m
Index: clang/test/Modules/compare-objc-interface.m
===================================================================
--- /dev/null
+++ clang/test/Modules/compare-objc-interface.m
@@ -0,0 +1,86 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Build first header file
+// RUN: echo "#define FIRST" >> %t/include/first.h
+// RUN: cat %t/test.m >> %t/include/first.h
+// RUN: echo "#undef FIRST" >> %t/include/first.h
+
+// Build second header file
+// RUN: echo "#define SECOND" >> %t/include/second-textual.h
+// RUN: cat %t/test.m >> %t/include/second-textual.h
+// RUN: echo "#undef SECOND" >> %t/include/second-textual.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 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
+
+// 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
+// allow a new definition if it is equivalent to the hidden one.
+//
+// This test case is to verify equivalence checks.
+
+//--- include/common.h
+#ifndef COMMON_H
+#define COMMON_H
+__attribute__((objc_root_class))
+@interface NSObject
+@end
+
+@interface CommonClass: NSObject
+@end
+
+@protocol ProtoP @end
+@protocol ProtoQ @end
+#endif
+
+//--- include/first-empty.h
+//--- include/module.modulemap
+module Common {
+ header "common.h"
+ export *
+}
+module First {
+ module Empty {
+ header "first-empty.h"
+ }
+ module InitiallyHidden {
+ header "first.h"
+ export *
+ }
+}
+
+//--- test.m
+#if defined(FIRST) || defined(SECOND)
+#include "common.h"
+#endif
+#if !defined(FIRST) && !defined(SECOND)
+#include "first-empty.h"
+#include "second-textual.h"
+#endif
+
+#if defined(FIRST)
+@class CompareForwardDeclaration1;
+@interface CompareForwardDeclaration2: NSObject @end
+#elif defined(SECOND)
+@interface CompareForwardDeclaration1: NSObject @end
+@class CompareForwardDeclaration2;
+#else
+CompareForwardDeclaration1 *compareForwardDeclaration1;
+CompareForwardDeclaration2 *compareForwardDeclaration2;
+#endif
+
+#if defined(FIRST)
+@interface CompareSuperClass : NSObject @end
+#elif defined(SECOND)
+@interface CompareSuperClass : CommonClass @end
+#else
+CompareSuperClass *compareSuperClass;
+// expected-error@second-textual.h:* {{duplicate interface definition for class 'CompareSuperClass'}}
+// expected-note@first.h:* {{previous definition is here}}
+#endif
Index: clang/lib/Parse/ParseObjc.cpp
===================================================================
--- clang/lib/Parse/ParseObjc.cpp
+++ clang/lib/Parse/ParseObjc.cpp
@@ -367,9 +367,9 @@
ParseObjCInterfaceDeclList(tok::objc_interface, ClsType);
if (SkipBody.CheckSameAsPrevious) {
- if (Actions.ActOnDuplicateDefinition(SkipBody.Previous, SkipBody)) {
- ClsType->mergeDuplicateDefinitionWithCommon(
- cast<ObjCInterfaceDecl>(SkipBody.Previous)->getDefinition());
+ auto *PreviousDef = cast<ObjCInterfaceDecl>(SkipBody.Previous);
+ if (Actions.ActOnDuplicateDefinition(ClsType, PreviousDef)) {
+ ClsType->mergeDuplicateDefinitionWithCommon(PreviousDef->getDefinition());
} else {
Diag(AtLoc, diag::err_duplicate_class_def) << ClsType->getDeclName();
Diag(SkipBody.Previous->getLocation(), diag::note_previous_definition);
Index: clang/lib/AST/ODRHash.cpp
===================================================================
--- clang/lib/AST/ODRHash.cpp
+++ clang/lib/AST/ODRHash.cpp
@@ -517,6 +517,28 @@
}
}
+void ODRHash::AddObjCInterfaceDecl(const ObjCInterfaceDecl *IF) {
+ AddDecl(IF);
+
+ ObjCInterfaceDecl *SuperClass = IF->getSuperClass();
+ AddBoolean(SuperClass);
+ if (SuperClass)
+ ID.AddInteger(SuperClass->getODRHash());
+
+ //FIXME: Hash other interface-specific elements like protocols, etc.
+
+ // Filter out sub-Decls which will not be processed in order to get an
+ // accurate count of Decl's.
+ llvm::SmallVector<const Decl *, 16> Decls;
+ for (Decl *SubDecl : IF->decls())
+ if (isDeclToBeProcessed(SubDecl, IF))
+ Decls.push_back(SubDecl);
+
+ ID.AddInteger(Decls.size());
+ for (auto *SubDecl : Decls)
+ AddSubDecl(SubDecl);
+}
+
void ODRHash::AddFunctionDecl(const FunctionDecl *Function,
bool SkipBody) {
assert(Function && "Expecting non-null pointer.");
Index: clang/lib/AST/DeclObjC.cpp
===================================================================
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -16,6 +16,7 @@
#include "clang/AST/Attr.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
+#include "clang/AST/ODRHash.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/Type.h"
#include "clang/AST/TypeLoc.h"
@@ -792,6 +793,33 @@
return Method;
}
+unsigned ObjCInterfaceDecl::getODRHash() {
+ assert(hasDefinition() && "ODRHash only for interfaces with definitions");
+
+ // Previously calculated hash is stored in DefinitionData.
+ if (hasODRHash())
+ return data().ODRHash;
+
+ // Only calculate hash on the first call of getODRHash per interface.
+ ODRHash Hasher;
+ Hasher.AddObjCInterfaceDecl(getDefinition());
+ data().ODRHash = Hasher.CalculateHash();
+ setHasODRHash(true);
+
+ return data().ODRHash;
+}
+
+bool ObjCInterfaceDecl::hasODRHash() const {
+ if (!hasDefinition())
+ return false;
+ return data().HasODRHash;
+}
+
+void ObjCInterfaceDecl::setHasODRHash(bool HasHash) {
+ assert(hasDefinition() && "Cannot set ODRHash without definition");
+ data().HasODRHash = HasHash;
+}
+
//===----------------------------------------------------------------------===//
// ObjCMethodDecl
//===----------------------------------------------------------------------===//
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3292,6 +3292,20 @@
/// in case of a structural mismatch.
bool ActOnDuplicateDefinition(Decl *Prev, SkipBodyInfo &SkipBody);
+ /// Check ODR hashes for C/ObjC when merging types from modules.
+ /// Differently from C++, actually parse the body and reject in case
+ /// of a mismatch.
+ template <typename T,
+ typename = std::enable_if_t<std::is_base_of<NamedDecl, T>::value>>
+ bool ActOnDuplicateDefinition(T *Duplicate, T *Previous) {
+ if (Duplicate->getODRHash() != Previous->getODRHash())
+ return false;
+
+ // Make the previous decl visible.
+ makeMergedDefinitionVisible(Previous);
+ return true;
+ }
+
typedef void *SkippedDefinitionContext;
/// Invoked when we enter a tag definition that we're skipping.
Index: clang/include/clang/AST/ODRHash.h
===================================================================
--- clang/include/clang/AST/ODRHash.h
+++ clang/include/clang/AST/ODRHash.h
@@ -55,6 +55,10 @@
// more information than the AddDecl class.
void AddCXXRecordDecl(const CXXRecordDecl *Record);
+ // Use this for ODR checking ObjC interfaces. This
+ // method compares more information than the AddDecl class.
+ void AddObjCInterfaceDecl(const ObjCInterfaceDecl *Interface);
+
// Use this for ODR checking functions between modules. This method compares
// more information than the AddDecl class. SkipBody will process the
// hash as if the function has no body.
Index: clang/include/clang/AST/DeclObjC.h
===================================================================
--- clang/include/clang/AST/DeclObjC.h
+++ clang/include/clang/AST/DeclObjC.h
@@ -1207,6 +1207,12 @@
/// One of the \c InheritedDesignatedInitializersState enumeratos.
mutable unsigned InheritedDesignatedInitializers : 2;
+ /// Tracks whether a ODR hash has been computed for this interface.
+ unsigned HasODRHash : 1;
+
+ /// A hash of parts of the class to help in ODR checking.
+ unsigned ODRHash = 0;
+
/// The location of the last location in this declaration, before
/// the properties/methods. For example, this will be the '>', '}', or
/// identifier,
@@ -1215,7 +1221,7 @@
DefinitionData()
: ExternallyCompleted(false), IvarListMissingImplementation(true),
HasDesignatedInitializers(false),
- InheritedDesignatedInitializers(IDI_Unknown) {}
+ InheritedDesignatedInitializers(IDI_Unknown), HasODRHash(false) {}
};
/// The type parameters associated with this class, if any.
@@ -1904,12 +1910,19 @@
const Type *getTypeForDecl() const { return TypeForDecl; }
void setTypeForDecl(const Type *TD) const { TypeForDecl = TD; }
+ /// Get precomputed ODRHash or add a new one.
+ unsigned getODRHash();
+
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == ObjCInterface; }
private:
const ObjCInterfaceDecl *findInterfaceWithDesignatedInitializers() const;
bool inheritsDesignatedInitializers() const;
+
+ /// True if a valid hash is stored in ODRHash.
+ bool hasODRHash() const;
+ void setHasODRHash(bool HasHash);
};
/// ObjCIvarDecl - Represents an ObjC instance variable. In general, ObjC
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
