vsapsai created this revision.
vsapsai added reviewers: bruno, rsmith, arphaman.
Herald added a subscriber: ributzka.
vsapsai requested review of this revision.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D110452
Files:
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
Index: clang/test/Modules/interface-diagnose-missing-import.m
===================================================================
--- clang/test/Modules/interface-diagnose-missing-import.m
+++ 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}}
Index: clang/test/Modules/decldef.mm
===================================================================
--- clang/test/Modules/decldef.mm
+++ 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 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
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -5326,11 +5326,8 @@
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))
Index: clang/lib/AST/Type.cpp
===================================================================
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -821,6 +821,13 @@
/*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())
Index: clang/lib/AST/DeclObjC.cpp
===================================================================
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -603,10 +603,6 @@
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() {
Index: clang/include/clang/AST/Type.h
===================================================================
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6016,10 +6016,9 @@
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 @@
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); }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
