[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

Author: vsapsai

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);

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() &&

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);
 }

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.

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
 

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;
+}

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'}}