vsapsai updated this revision to Diff 417468.
vsapsai added a comment.
Fix bug with anonymous enum constant lookup in C++.
Turns out the lookup was broken by using `ObjCInterfaceDecl` as a semantic decl
context for anonymous enums. Fixing that fixed the lookup.
Also added in tests checking scoped enums in C++. Scoped enums cannot be
anonymous, so test only named enum definitions inside Objective-C code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118525/new/
https://reviews.llvm.org/D118525
Files:
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
clang/test/Modules/merge-anon-record-definition-in-objc.m
clang/test/SemaObjC/check-dup-decls-inside-objc.m
Index: clang/test/SemaObjC/check-dup-decls-inside-objc.m
===================================================================
--- /dev/null
+++ 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'}}
Index: clang/test/Modules/merge-anon-record-definition-in-objc.m
===================================================================
--- /dev/null
+++ 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 different 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;
+}
Index: clang/test/AST/ast-dump-decl.mm
===================================================================
--- clang/test/AST/ast-dump-decl.mm
+++ clang/test/AST/ast-dump-decl.mm
@@ -30,6 +30,23 @@
// 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
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3053,6 +3053,8 @@
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.
Index: clang/lib/Serialization/ASTCommon.cpp
===================================================================
--- clang/lib/Serialization/ASTCommon.cpp
+++ clang/lib/Serialization/ASTCommon.cpp
@@ -477,7 +477,9 @@
// 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);
}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16056,9 +16056,20 @@
// 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() &&
Index: clang/lib/Parse/ParseObjc.cpp
===================================================================
--- clang/lib/Parse/ParseObjc.cpp
+++ clang/lib/Parse/ParseObjc.cpp
@@ -1872,9 +1872,9 @@
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,
@@ -1909,8 +1909,7 @@
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();
@@ -1974,13 +1973,13 @@
}
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);
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
