[clang] 29444f0 - [modules] Merge ObjC interface ivars with anonymous types.
Volodymyr Sapsai via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 4 18:49:15 PDT 2022
Author: Volodymyr Sapsai
Date: 2022-04-04T18:48:30-07:00
New Revision: 29444f0444c6d3586969fd6fbe49c8188c02c244
URL: https://github.com/llvm/llvm-project/commit/29444f0444c6d3586969fd6fbe49c8188c02c244
DIFF: https://github.com/llvm/llvm-project/commit/29444f0444c6d3586969fd6fbe49c8188c02c244.diff
LOG: [modules] Merge ObjC interface ivars with anonymous types.
Without the fix ivars with anonymous types can trigger errors like
> error: 'TestClass::structIvar' from module 'Target' is not present in definition of 'TestClass' provided earlier
> [...]
> note: declaration of 'structIvar' does not match
It happens because types of ivars from different modules are considered
to be different. And it is caused by not merging anonymous `TagDecl`
from different modules.
To fix that I've changed `serialization::needsAnonymousDeclarationNumber`
to handle anonymous `TagDecl` inside `ObjCInterfaceDecl`. But that's not
sufficient as C code inside `ObjCInterfaceDecl` doesn't use interface
decl as a decl context but switches to its parent (TranslationUnit in
most cases). I'm changing that to make `ObjCContainerDecl` the lexical
decl context but keeping the semantic decl context intact.
Test "check-dup-decls-inside-objc.m" doesn't reflect a change in
functionality but captures the existing behavior to prevent regressions.
rdar://85563013
Differential Revision: https://reviews.llvm.org/D118525
Added:
clang/test/Modules/merge-anon-record-definition-in-objc.m
clang/test/SemaObjC/check-dup-decls-inside-objc.m
Modified:
clang/lib/Parse/ParseObjc.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Serialization/ASTCommon.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/test/AST/ast-dump-decl.mm
Removed:
################################################################################
diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp
index 719513e7bda09..c56fcb8cc3cfe 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/lib/Parse/ParseObjc.cpp
@@ -1871,9 +1871,9 @@ void Parser::HelperActionsForIvarDeclarations(Decl *interfaceDecl, SourceLocatio
if (!RBraceMissing)
T.consumeClose();
- Actions.ActOnObjCContainerStartDefinition(interfaceDecl);
+ assert(getObjCDeclContext() == interfaceDecl &&
+ "Ivars should have interfaceDecl as their decl context");
Actions.ActOnLastBitfield(T.getCloseLocation(), AllIvarDecls);
- Actions.ActOnObjCContainerFinishDefinition();
// Call ActOnFields() even if we don't have any decls. This is useful
// for code rewriting tools that need to be aware of the empty list.
Actions.ActOnFields(getCurScope(), atLoc, interfaceDecl, AllIvarDecls,
@@ -1908,8 +1908,7 @@ void Parser::ParseObjCClassInstanceVariables(Decl *interfaceDecl,
assert(Tok.is(tok::l_brace) && "expected {");
SmallVector<Decl *, 32> AllIvarDecls;
- ParseScope ClassScope(this, Scope::DeclScope|Scope::ClassScope);
- ObjCDeclContextSwitch ObjCDC(*this);
+ ParseScope ClassScope(this, Scope::DeclScope | Scope::ClassScope);
BalancedDelimiterTracker T(*this, tok::l_brace);
T.consumeOpen();
@@ -1973,13 +1972,13 @@ void Parser::ParseObjCClassInstanceVariables(Decl *interfaceDecl,
}
auto ObjCIvarCallback = [&](ParsingFieldDeclarator &FD) {
- Actions.ActOnObjCContainerStartDefinition(interfaceDecl);
+ assert(getObjCDeclContext() == interfaceDecl &&
+ "Ivar should have interfaceDecl as its decl context");
// Install the declarator into the interface decl.
FD.D.setObjCIvar(true);
Decl *Field = Actions.ActOnIvar(
getCurScope(), FD.D.getDeclSpec().getSourceRange().getBegin(), FD.D,
FD.BitfieldSize, visibility);
- Actions.ActOnObjCContainerFinishDefinition();
if (Field)
AllIvarDecls.push_back(Field);
FD.complete(Field);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 1e25346fde6f6..0b5b530bc756c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16059,9 +16059,20 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
// with C structs, unions, and enums when looking for a matching
// tag declaration or definition. See the similar lookup tweak
// in Sema::LookupName; is there a better way to deal with this?
- while (isa<RecordDecl>(SearchDC) || isa<EnumDecl>(SearchDC))
+ while (isa<RecordDecl, EnumDecl, ObjCContainerDecl>(SearchDC))
+ SearchDC = SearchDC->getParent();
+ } else if (getLangOpts().CPlusPlus) {
+ // Inside ObjCContainer want to keep it as a lexical decl context but go
+ // past it (most often to TranslationUnit) to find the semantic decl
+ // context.
+ while (isa<ObjCContainerDecl>(SearchDC))
SearchDC = SearchDC->getParent();
}
+ } else if (getLangOpts().CPlusPlus) {
+ // Don't use ObjCContainerDecl as the semantic decl context for anonymous
+ // TagDecl the same way as we skip it for named TagDecl.
+ while (isa<ObjCContainerDecl>(SearchDC))
+ SearchDC = SearchDC->getParent();
}
if (Previous.isSingleResult() &&
diff --git a/clang/lib/Serialization/ASTCommon.cpp b/clang/lib/Serialization/ASTCommon.cpp
index 673c56674f616..26b722b6b14a4 100644
--- a/clang/lib/Serialization/ASTCommon.cpp
+++ b/clang/lib/Serialization/ASTCommon.cpp
@@ -478,7 +478,9 @@ bool serialization::needsAnonymousDeclarationNumber(const NamedDecl *D) {
// Otherwise, we only care about anonymous class members / block-scope decls.
// FIXME: We need to handle lambdas and blocks within inline / templated
// variables too.
- if (D->getDeclName() || !isa<RecordDecl>(D->getLexicalDeclContext()))
+ if (D->getDeclName())
+ return false;
+ if (!isa<RecordDecl, ObjCInterfaceDecl>(D->getLexicalDeclContext()))
return false;
return isa<TagDecl>(D) || isa<FieldDecl>(D);
}
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 40208eb00c153..43aacdeda9987 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3065,6 +3065,8 @@ ASTDeclReader::getPrimaryDCForAnonymousDecl(DeclContext *LexicalDC) {
if (auto *RD = dyn_cast<CXXRecordDecl>(LexicalDC)) {
auto *DD = RD->getCanonicalDecl()->DefinitionData;
return DD ? DD->Definition : nullptr;
+ } else if (auto *OID = dyn_cast<ObjCInterfaceDecl>(LexicalDC)) {
+ return OID->getCanonicalDecl()->getDefinition();
}
// For anything else, walk its merged redeclarations looking for a definition.
diff --git a/clang/test/AST/ast-dump-decl.mm b/clang/test/AST/ast-dump-decl.mm
index 69ff46101da1e..c9282e9537e28 100644
--- a/clang/test/AST/ast-dump-decl.mm
+++ b/clang/test/AST/ast-dump-decl.mm
@@ -30,6 +30,23 @@ - (void) foo {
// CHECK-NEXT: ObjCInterface{{.*}} 'TestObjCImplementation'
// CHECK-NEXT: CXXCtorInitializer{{.*}} 'X'
// CHECK-NEXT: CXXConstructExpr
+// CHECK-NEXT: CXXRecordDecl{{.*}} struct X definition
+// CHECK-NEXT: DefinitionData
+// CHECK-NEXT: DefaultConstructor
+// CHECK-NEXT: CopyConstructor
+// CHECK-NEXT: MoveConstructor
+// CHECK-NEXT: CopyAssignment
+// CHECK-NEXT: MoveAssignment
+// CHECK-NEXT: Destructor
+// CHECK-NEXT: CXXRecordDecl{{.*}} struct X
+// CHECK-NEXT: FieldDecl{{.*}} i 'int'
+// CHECK-NEXT: CXXConstructorDecl{{.*}} 'void ()
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: CXXConstructorDecl{{.*}} 'void (const X &)
+// CHECK-NEXT: ParmVarDecl{{.*}} 'const X &'
+// CHECK-NEXT: CXXConstructorDecl{{.*}} 'void (X &&)
+// CHECK-NEXT: ParmVarDecl{{.*}} 'X &&'
+// CHECK-NEXT: CXXDestructorDecl
// CHECK-NEXT: ObjCIvarDecl{{.*}} X
// CHECK-NEXT: ObjCMethodDecl{{.*}} foo
diff --git a/clang/test/Modules/merge-anon-record-definition-in-objc.m b/clang/test/Modules/merge-anon-record-definition-in-objc.m
new file mode 100644
index 0000000000000..415ed97491339
--- /dev/null
+++ b/clang/test/Modules/merge-anon-record-definition-in-objc.m
@@ -0,0 +1,84 @@
+// UNSUPPORTED: -zos, -aix
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -F%t/Frameworks %t/test.m -Wno-objc-property-implementation -Wno-incomplete-implementation \
+// RUN: -fmodules -fmodule-name=Target -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// RUN: %clang_cc1 -fsyntax-only -F%t/Frameworks -x objective-c++ %t/test.m -Wno-objc-property-implementation -Wno-incomplete-implementation \
+// RUN: -fmodules -fmodule-name=Target -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// Test anonymous TagDecl inside Objective-C interfaces are merged and ivars with these anonymous types are merged too.
+
+//--- Frameworks/Foundation.framework/Headers/Foundation.h
+ at interface NSObject
+ at end
+
+//--- Frameworks/Foundation.framework/Modules/module.modulemap
+framework module Foundation {
+ header "Foundation.h"
+ export *
+}
+
+//--- Frameworks/Target.framework/Headers/Target.h
+#import <Foundation/Foundation.h>
+ at interface TestClass : NSObject {
+ at public
+ struct {
+ struct { int x; int y; } left;
+ struct { int w; int z; } right;
+ } structIvar;
+ union { int x; float y; } *unionIvar;
+ enum { kX = 0, } enumIvar;
+}
+ at property struct { int u; } prop;
+#ifndef __cplusplus
+- (struct { int v; })method;
+#endif
+ at end
+
+ at interface TestClass() {
+ at public
+ struct { int y; } extensionIvar;
+}
+ at end
+
+ at implementation TestClass {
+ at public
+ struct { int z; } implementationIvar;
+}
+ at end
+
+//--- Frameworks/Target.framework/Modules/module.modulemap
+framework module Target {
+ header "Target.h"
+ export *
+}
+
+//--- Frameworks/Redirect.framework/Headers/Redirect.h
+#import <Target/Target.h>
+
+//--- Frameworks/Redirect.framework/Modules/module.modulemap
+framework module Redirect {
+ header "Redirect.h"
+ export *
+}
+
+//--- test.m
+// At first import everything as non-modular.
+#import <Target/Target.h>
+// And now as modular to merge same entities obtained through
diff erent sources.
+#import <Redirect/Redirect.h>
+// Non-modular import is achieved through using the same name (-fmodule-name) as the imported framework module.
+
+void test(TestClass *obj) {
+ obj->structIvar.left.x = 0;
+ obj->unionIvar->y = 1.0f;
+ obj->enumIvar = kX;
+ int tmp = obj.prop.u;
+#ifndef __cplusplus
+ tmp += [obj method].v;
+#endif
+
+ obj->extensionIvar.y = 0;
+ obj->implementationIvar.z = 0;
+}
diff --git a/clang/test/SemaObjC/check-dup-decls-inside-objc.m b/clang/test/SemaObjC/check-dup-decls-inside-objc.m
new file mode 100644
index 0000000000000..b8245a5126e97
--- /dev/null
+++ b/clang/test/SemaObjC/check-dup-decls-inside-objc.m
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class -x objective-c++ %s
+
+// Test decls inside Objective-C entities are considered to be duplicates of same-name decls outside of these entities.
+
+ at protocol SomeProtocol
+struct InProtocol {}; // expected-note {{previous definition is here}}
+- (union MethodReturnType { int x; float y; })returningMethod; // expected-note {{previous definition is here}}
+#ifdef __cplusplus
+// expected-error at -2 {{'MethodReturnType' cannot be defined in a parameter type}}
+#endif
+ at end
+
+ at interface Container {
+ struct InInterfaceCurliesWithField {} field; // expected-note {{previous definition is here}}
+ union InInterfaceCurlies { int x; float y; }; // expected-note {{previous definition is here}}
+}
+enum InInterface { kX = 0, }; // expected-note {{previous definition is here}}
+#ifdef __cplusplus
+enum class InInterfaceScoped { kXScoped = 0, }; // expected-note {{previous definition is here}}
+#endif
+ at end
+
+ at interface Container(Category)
+union InCategory { int x; float y; }; // expected-note {{previous definition is here}}
+ at end
+
+ at interface Container() {
+ enum InExtensionCurliesWithField: int { kY = 1, } extensionField; // expected-note {{previous definition is here}}
+ struct InExtensionCurlies {}; // expected-note {{previous definition is here}}
+}
+union InExtension { int x; float y; }; // expected-note {{previous definition is here}}
+ at end
+
+ at implementation Container {
+ union InImplementationCurliesWithField { int x; float y; } implField; // expected-note {{previous definition is here}}
+ enum InImplementationCurlies { kZ = 2, }; // expected-note {{previous definition is here}}
+}
+struct InImplementation {}; // expected-note {{previous definition is here}}
+ at end
+
+ at implementation Container(Category)
+enum InCategoryImplementation { kW = 3, }; // expected-note {{previous definition is here}}
+ at end
+
+
+struct InProtocol { int a; }; // expected-error {{redefinition of 'InProtocol'}}
+union MethodReturnType { int a; long b; }; // expected-error {{redefinition of 'MethodReturnType'}}
+
+struct InInterfaceCurliesWithField { int a; }; // expected-error {{redefinition of 'InInterfaceCurliesWithField'}}
+union InInterfaceCurlies { int a; long b; }; // expected-error {{redefinition of 'InInterfaceCurlies'}}
+enum InInterface { kA = 10, }; // expected-error {{redefinition of 'InInterface'}}
+#ifdef __cplusplus
+enum class InInterfaceScoped { kAScoped = 10, }; // expected-error {{redefinition of 'InInterfaceScoped'}}
+#endif
+
+union InCategory { int a; long b; }; // expected-error {{redefinition of 'InCategory'}}
+
+enum InExtensionCurliesWithField: int { kB = 11, }; // expected-error {{redefinition of 'InExtensionCurliesWithField'}}
+struct InExtensionCurlies { int a; }; // expected-error {{redefinition of 'InExtensionCurlies'}}
+union InExtension { int a; long b; }; // expected-error {{redefinition of 'InExtension'}}
+
+union InImplementationCurliesWithField { int a; long b; }; // expected-error {{redefinition of 'InImplementationCurliesWithField'}}
+enum InImplementationCurlies { kC = 12, }; // expected-error {{redefinition of 'InImplementationCurlies'}}
+struct InImplementation { int a; }; // expected-error {{redefinition of 'InImplementation'}}
+
+enum InCategoryImplementation { kD = 13, }; // expected-error {{redefinition of 'InCategoryImplementation'}}
More information about the cfe-commits
mailing list