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 +@interface NSObject +@end + +//--- Frameworks/Foundation.framework/Modules/module.modulemap +framework module Foundation { + header "Foundation.h" + export * +} + +//--- Frameworks/Target.framework/Headers/Target.h +#import <Foundation/Foundation.h> +@interface TestClass : NSObject { +@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; +} +@property struct { int u; } prop; +#ifndef __cplusplus +- (struct { int v; })method; +#endif +@end + +@interface TestClass() { +@public + struct { int y; } extensionIvar; +} +@end + +@implementation TestClass { +@public + struct { int z; } implementationIvar; +} +@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. + +@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@-2 {{'MethodReturnType' cannot be defined in a parameter type}} +#endif +@end + +@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 +@end + +@interface Container(Category) +union InCategory { int x; float y; }; // expected-note {{previous definition is here}} +@end + +@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}} +@end + +@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}} +@end + +@implementation Container(Category) +enum InCategoryImplementation { kW = 3, }; // expected-note {{previous definition is here}} +@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'}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
