vsapsai created this revision.
vsapsai added reviewers: jansvoboda11, rsmith.
Herald added a subscriber: ributzka.
Herald added a project: All.
vsapsai requested review of this revision.
Herald added a project: clang.
Instead of emitting a redefinition error, check that definitions are
equivalent and allow such scenario. As an implementation detail we use
the previous decl to avoid disrupting existing canonical decl. And to
avoid multiple definitions in redeclaration chain we just drop the new
definition after checking equivalence.
This patch covers only duplicates encountered during parsing. We can
encounter duplicates also during deserialization but that's responsibility of
ASTReaderDecl.
rdar://82908223
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124286
Files:
clang/include/clang/AST/DeclObjC.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/DeclObjC.cpp
clang/lib/Parse/ParseObjc.cpp
clang/lib/Sema/SemaDeclObjC.cpp
clang/test/Modules/hidden-duplicates.m
Index: clang/test/Modules/hidden-duplicates.m
===================================================================
--- /dev/null
+++ clang/test/Modules/hidden-duplicates.m
@@ -0,0 +1,62 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify -DTEST_MAKE_HIDDEN_VISIBLE=1 \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify -x objective-c++ \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify -DTEST_MAKE_HIDDEN_VISIBLE=1 -x objective-c++ \
+// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// Test parsing duplicate Objective-C entities when a previous entity is defined
+// in a hidden [sub]module and cannot be used.
+//
+// Testing with header guards and includes on purpose as tracking imports in
+// modules is a separate issue.
+
+//--- include/textual.h
+#ifndef TEXTUAL_H
+#define TEXTUAL_H
+
+__attribute__((objc_root_class))
+@interface NSObject {
+@public
+ int ivarName;
+}
+@property (assign, nonatomic) int propName;
+@end
+
+@class ForwardDeclaredClassWithoutDefinition;
+
+#endif
+
+//--- include/empty.h
+//--- include/initially_hidden.h
+#include "textual.h"
+
+//--- include/module.modulemap
+module Piecewise {
+ module Empty {
+ header "empty.h"
+ }
+ module InitiallyHidden {
+ header "initially_hidden.h"
+ export *
+ }
+}
+
+//--- test.m
+// Including empty.h loads the entire module Piecewise but keeps InitiallyHidden hidden.
+#include "empty.h"
+#include "textual.h"
+#ifdef TEST_MAKE_HIDDEN_VISIBLE
+#include "initially_hidden.h"
+#endif
+// expected-no-diagnostics
+
+void testAccessingInterface(NSObject *obj) {
+ obj->ivarName = obj.propName;
+}
+
+void testForwardDecl(ForwardDeclaredClassWithoutDefinition *obj);
Index: clang/lib/Sema/SemaDeclObjC.cpp
===================================================================
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -978,7 +978,7 @@
ArrayRef<ParsedType> SuperTypeArgs, SourceRange SuperTypeArgsRange,
Decl *const *ProtoRefs, unsigned NumProtoRefs,
const SourceLocation *ProtoLocs, SourceLocation EndProtoLoc,
- const ParsedAttributesView &AttrList) {
+ const ParsedAttributesView &AttrList, SkipBodyInfo *SkipBody) {
assert(ClassName && "Missing class identifier");
// Check for another declaration kind with the same name.
@@ -1057,10 +1057,16 @@
if (PrevIDecl) {
// Class already seen. Was it a definition?
if (ObjCInterfaceDecl *Def = PrevIDecl->getDefinition()) {
- Diag(AtInterfaceLoc, diag::err_duplicate_class_def)
- << PrevIDecl->getDeclName();
- Diag(Def->getLocation(), diag::note_previous_definition);
- IDecl->setInvalidDecl();
+ if (SkipBody && !hasVisibleDefinition(Def)) {
+ SkipBody->CheckSameAsPrevious = true;
+ SkipBody->New = IDecl;
+ SkipBody->Previous = Def;
+ } else {
+ Diag(AtInterfaceLoc, diag::err_duplicate_class_def)
+ << PrevIDecl->getDeclName();
+ Diag(Def->getLocation(), diag::note_previous_definition);
+ IDecl->setInvalidDecl();
+ }
}
}
@@ -1075,7 +1081,9 @@
// Start the definition of this class. If we're in a redefinition case, there
// may already be a definition, so we'll end up adding to it.
- if (!IDecl->hasDefinition())
+ if (SkipBody && SkipBody->CheckSameAsPrevious)
+ IDecl->startDuplicateDefinitionForComparison();
+ else if (!IDecl->hasDefinition())
IDecl->startDefinition();
if (SuperName) {
Index: clang/lib/Parse/ParseObjc.cpp
===================================================================
--- clang/lib/Parse/ParseObjc.cpp
+++ clang/lib/Parse/ParseObjc.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/PrettyDeclStackTrace.h"
#include "clang/Basic/CharInfo.h"
+#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Parse/ParseDiagnostic.h"
#include "clang/Parse/Parser.h"
@@ -353,17 +354,29 @@
Actions.ActOnTypedefedProtocols(protocols, protocolLocs,
superClassId, superClassLoc);
+ Sema::SkipBodyInfo SkipBody;
ObjCInterfaceDecl *ClsType = Actions.ActOnStartClassInterface(
getCurScope(), AtLoc, nameId, nameLoc, typeParameterList, superClassId,
superClassLoc, typeArgs,
SourceRange(typeArgsLAngleLoc, typeArgsRAngleLoc), protocols.data(),
- protocols.size(), protocolLocs.data(), EndProtoLoc, attrs);
+ protocols.size(), protocolLocs.data(), EndProtoLoc, attrs, &SkipBody);
if (Tok.is(tok::l_brace))
ParseObjCClassInstanceVariables(ClsType, tok::objc_protected, AtLoc);
ParseObjCInterfaceDeclList(tok::objc_interface, ClsType);
+ if (SkipBody.CheckSameAsPrevious) {
+ if (Actions.ActOnDuplicateDefinition(SkipBody.Previous, SkipBody)) {
+ ClsType->mergeDuplicateDefinitionWithCommon(
+ cast<ObjCInterfaceDecl>(SkipBody.Previous)->getDefinition());
+ } else {
+ Diag(AtLoc, diag::err_duplicate_class_def) << ClsType->getDeclName();
+ Diag(SkipBody.Previous->getLocation(), diag::note_previous_definition);
+ ClsType->setInvalidDecl();
+ }
+ }
+
return ClsType;
}
Index: clang/lib/AST/DeclObjC.cpp
===================================================================
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -627,6 +627,17 @@
}
}
+void ObjCInterfaceDecl::startDuplicateDefinitionForComparison() {
+ Data.setPointer(nullptr);
+ allocateDefinitionData();
+ // Don't propagate data to other redeclarations.
+}
+
+void ObjCInterfaceDecl::mergeDuplicateDefinitionWithCommon(
+ const ObjCInterfaceDecl *Definition) {
+ Data = Definition->Data;
+}
+
ObjCIvarDecl *ObjCInterfaceDecl::lookupInstanceVariable(IdentifierInfo *ID,
ObjCInterfaceDecl *&clsDeclared) {
// FIXME: Should make sure no callers ever do this.
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -9750,7 +9750,7 @@
ArrayRef<ParsedType> SuperTypeArgs, SourceRange SuperTypeArgsRange,
Decl *const *ProtoRefs, unsigned NumProtoRefs,
const SourceLocation *ProtoLocs, SourceLocation EndProtoLoc,
- const ParsedAttributesView &AttrList);
+ const ParsedAttributesView &AttrList, SkipBodyInfo *SkipBody);
void ActOnSuperClassOfClassInterface(Scope *S,
SourceLocation AtInterfaceLoc,
Index: clang/include/clang/AST/DeclObjC.h
===================================================================
--- clang/include/clang/AST/DeclObjC.h
+++ clang/include/clang/AST/DeclObjC.h
@@ -1539,6 +1539,13 @@
/// a forward declaration (\@class) to a definition (\@interface).
void startDefinition();
+ /// Starts the definition without sharing it with other redeclarations.
+ /// Such definition shouldn't be used for anything but only to compare if
+ /// a duplicate is compatible with previous definition or if it is
+ /// a distinct duplicate.
+ void startDuplicateDefinitionForComparison();
+ void mergeDuplicateDefinitionWithCommon(const ObjCInterfaceDecl *Definition);
+
/// Retrieve the superclass type.
const ObjCObjectType *getSuperClassType() const {
if (TypeSourceInfo *TInfo = getSuperClassTInfo())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
