[clang] d9eca33 - [modules] Fix tracking ObjCInterfaceType decl when there are multiple definitions.
Volodymyr Sapsai via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 21 12:08:27 PDT 2021
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 at Inputs/def.h:5 0-1{{here}}
+// expected-note at Inputs/def.h:11 0-1{{here}}
// expected-note at Inputs/def.h:16 0-1{{here}}
// expected-note at 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 at -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;
- at 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.
+ at interface Buggy (MyExt)
@end
-
-// expected-note at Foo/RandoPriv.h:3{{definition here is not reachable}}
More information about the cfe-commits
mailing list