[clang] 02899d7 - [clang] Don't make ObjCIvarDecl visible twice when adding them to an implicit ObjCInterfaceDecl

[Raphael Isemann via cfe-commits]

Author: Raphael Isemann
Date: 2020-08-11T16:24:32+02:00
New Revision: 02899d7f1b9ae7f6da30bd020a714c7b3eb2c59f

URL: https://github.com/llvm/llvm-project/commit/02899d7f1b9ae7f6da30bd020a714c7b3eb2c59f
DIFF: https://github.com/llvm/llvm-project/commit/02899d7f1b9ae7f6da30bd020a714c7b3eb2c59f.diff

LOG: [clang] Don't make ObjCIvarDecl visible twice when adding them to an implicit ObjCInterfaceDecl

`addDecl` is making the ivar visible in its primary context. The primary context
of the ivar here is in a 'fragile' ABI the ObjCInterfaceDecl and in a
'non-fragile' ABI the current ObjCImplementationDecl. The additional call to
`makeDeclVisibleInContext` to make the ivar visible in the ObjCInterfaceDecl is
only necessary in the 'non-fragile' case (as in the 'fragile' case the Decl
becomes automatically visible in the ObjCInterfaceDecl with the `addDecl` call
as thats its primary context). See `Sema::ActOnIvar` for where the ivar is put
into a different context depending on the ABI.

To put this into an example:

```
lang=c++
@implementation SomeClass
{
  id ivar1;
}
@end

fragile case:
implicit ObjCInterfaceDecl 'SomeClass'
`- ivar1 (in primary context and will be automatically made visible)
ObjCImplementationDecl 'SomeClass'

non-fragile case:
implicit ObjCInterfaceDecl 'SomeClass'
`-<<<ivar1 not visible here and needs to be manually marked as visible.>>>
ObjCImplementationDecl 'SomeClass'
`- ivar1 (in its primary context and will be automatically made visible here)
```

Making a Decl visible multiple times in the same context is inefficient and
potentially can lead to crashes. See D84827 for more info and what this is
breaking.

Reviewed By: aprantl

Differential Revision: https://reviews.llvm.org/D84829

Added: 
    

Modified: 
    clang/lib/Sema/SemaDeclObjC.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index 89815b838500..6ef6fd1d8c1c 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -2122,7 +2122,12 @@ void Sema::CheckImplementationIvars(ObjCImplementationDecl *ImpDecl,
     // Add ivar's to class's DeclContext.
     for (unsigned i = 0, e = numIvars; i != e; ++i) {
       ivars[i]->setLexicalDeclContext(ImpDecl);
-      IDecl->makeDeclVisibleInContext(ivars[i]);
+      // In a 'fragile' runtime the ivar was added to the implicit
+      // ObjCInterfaceDecl while in a 'non-fragile' runtime the ivar is
+      // only in the ObjCImplementationDecl. In the non-fragile case the ivar
+      // therefore also needs to be propagated to the ObjCInterfaceDecl.
+      if (!LangOpts.ObjCRuntime.isFragile())
+        IDecl->makeDeclVisibleInContext(ivars[i]);
       ImpDecl->addDecl(ivars[i]);
     }