Author: Volodymyr Sapsai Date: 2021-10-21T12:08:06-07:00 New Revision: d9eca3320a4d8db11ad65229ef6f564d134fc894
URL: https://github.com/llvm/llvm-project/commit/d9eca3320a4d8db11ad65229ef6f564d134fc894 DIFF: https://github.com/llvm/llvm-project/commit/d9eca3320a4d8db11ad65229ef6f564d134fc894.diff LOG: [modules] Fix tracking ObjCInterfaceType decl when there are multiple definitions. With the old approach we were updating `ObjCInterfaceType.Decl` to the last encountered definition. But during loading modules `ASTDeclReader::VisitObjCInterfaceDecl` keeps the *first* encountered definition. So with multiple definitions imported there would be a disagreement between expected definition in `ObjCInterfaceType.Decl` and actual definition `ObjCInterfaceDecl::getDefinition` which can lead to incorrect diagnostic. Fix by not tracking definition in `ObjCInterfaceType` explicitly but by getting it from redeclaration chain. Partially reverted 919fc50034b44c48aae8b80283f253ec2ee17f45 keeping the modified test case as the correct behavior is achieved in a different way. Differential Revision: https://reviews.llvm.org/D110452 Added: Modified: clang/include/clang/AST/Type.h clang/lib/AST/DeclObjC.cpp clang/lib/AST/Type.cpp clang/lib/Sema/SemaLookup.cpp clang/test/Modules/decldef.mm clang/test/Modules/interface-diagnose-missing-import.m Removed: ################################################################################ diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index f8c1fe91085f0..722add6cd8777 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -6016,10 +6016,9 @@ inline ObjCProtocolDecl **ObjCTypeParamType::getProtocolStorageImpl() { class ObjCInterfaceType : public ObjCObjectType { friend class ASTContext; // ASTContext creates these. friend class ASTReader; - friend class ObjCInterfaceDecl; template <class T> friend class serialization::AbstractTypeReader; - mutable ObjCInterfaceDecl *Decl; + ObjCInterfaceDecl *Decl; ObjCInterfaceType(const ObjCInterfaceDecl *D) : ObjCObjectType(Nonce_ObjCInterface), @@ -6027,7 +6026,7 @@ class ObjCInterfaceType : public ObjCObjectType { public: /// Get the declaration of this interface. - ObjCInterfaceDecl *getDecl() const { return Decl; } + ObjCInterfaceDecl *getDecl() const; bool isSugared() const { return false; } QualType desugar() const { return QualType(this, 0); } diff --git a/clang/lib/AST/DeclObjC.cpp b/clang/lib/AST/DeclObjC.cpp index 64eefc245f04a..ba827a79c0225 100644 --- a/clang/lib/AST/DeclObjC.cpp +++ b/clang/lib/AST/DeclObjC.cpp @@ -603,10 +603,6 @@ void ObjCInterfaceDecl::allocateDefinitionData() { assert(!hasDefinition() && "ObjC class already has a definition"); Data.setPointer(new (getASTContext()) DefinitionData()); Data.getPointer()->Definition = this; - - // Make the type point at the definition, now that we have one. - if (TypeForDecl) - cast<ObjCInterfaceType>(TypeForDecl)->Decl = this; } void ObjCInterfaceDecl::startDefinition() { diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp index e7cdf58839631..ead3944284aec 100644 --- a/clang/lib/AST/Type.cpp +++ b/clang/lib/AST/Type.cpp @@ -821,6 +821,13 @@ QualType ObjCObjectType::stripObjCKindOfTypeAndQuals( /*isKindOf=*/false); } +ObjCInterfaceDecl *ObjCInterfaceType::getDecl() const { + ObjCInterfaceDecl *Canon = Decl->getCanonicalDecl(); + if (ObjCInterfaceDecl *Def = Canon->getDefinition()) + return Def; + return Canon; +} + const ObjCObjectPointerType *ObjCObjectPointerType::stripObjCKindOfTypeAndQuals( const ASTContext &ctx) const { if (!isKindOfType() && qual_empty()) diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp index e4e5f57745c61..05529d0556211 100644 --- a/clang/lib/Sema/SemaLookup.cpp +++ b/clang/lib/Sema/SemaLookup.cpp @@ -5323,11 +5323,8 @@ static NamedDecl *getDefinitionToImport(NamedDecl *D) { return FD->getDefinition(); if (TagDecl *TD = dyn_cast<TagDecl>(D)) return TD->getDefinition(); - // The first definition for this ObjCInterfaceDecl might be in the TU - // and not associated with any module. Use the one we know to be complete - // and have just seen in a module. if (ObjCInterfaceDecl *ID = dyn_cast<ObjCInterfaceDecl>(D)) - return ID; + return ID->getDefinition(); if (ObjCProtocolDecl *PD = dyn_cast<ObjCProtocolDecl>(D)) return PD->getDefinition(); if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D)) diff --git a/clang/test/Modules/decldef.mm b/clang/test/Modules/decldef.mm index e8f070b511591..02883dc1d31e7 100644 --- a/clang/test/Modules/decldef.mm +++ b/clang/test/Modules/decldef.mm @@ -1,10 +1,12 @@ // RUN: rm -rf %t -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_1 -DUSE_2 -DUSE_3 -DUSE_4 -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_2 -DUSE_3 -DUSE_4 -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_3 -DUSE_4 -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_4 +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_1 -DUSE_2 -DUSE_3 -DUSE_4 -DUSE_5 +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_2 -DUSE_3 -DUSE_4 -DUSE_5 +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_3 -DUSE_4 -DUSE_5 +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_4 -DUSE_5 +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_5 // expected-note@Inputs/def.h:5 0-1{{here}} +// expected-note@Inputs/def.h:11 0-1{{here}} // expected-note@Inputs/def.h:16 0-1{{here}} // expected-note@Inputs/def-include.h:11 0-1{{here}} @@ -51,11 +53,17 @@ void testB() { } void testDef() { +#ifdef USE_4 [def defMethod]; + #ifndef USED + // expected-error@-2{{definition of 'Def' must be imported from module 'decldef.Def' before it is required}} + #define USED + #endif +#endif } void testDef2() { -#ifdef USE_4 +#ifdef USE_5 def2->func(); def3->func(); #ifndef USED diff --git a/clang/test/Modules/interface-diagnose-missing-import.m b/clang/test/Modules/interface-diagnose-missing-import.m index c455b1501c9cc..a6b8c1b03a9dd 100644 --- a/clang/test/Modules/interface-diagnose-missing-import.m +++ b/clang/test/Modules/interface-diagnose-missing-import.m @@ -1,11 +1,11 @@ // RUN: rm -rf %t // RUN: %clang_cc1 %s -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F%S/Inputs/interface-diagnose-missing-import -verify +// expected-no-diagnostics @interface Buggy @end @import Foo.Bar; -@interface Buggy (MyExt) // expected-error {{definition of 'Buggy' must be imported from module 'Foo' before it is required}} +// No diagnostic for inaccessible 'Buggy' definition because we have another definition right in this file. +@interface Buggy (MyExt) @end - -// expected-note@Foo/RandoPriv.h:3{{definition here is not reachable}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
