[ObjC] Fix encoding of ObjC pointer types that are pointers to typedefs

[ahatanak]

Summary

clang currently emits different strings for s0 and s1 in the following code because pointers to typedefs are encoded as "^{PointeeClassName=#}" while ObjC pointer types that aren't pointers to typedefs are encoded as "@".

@class Class1;

typedef NSArray<Class1 *> MyArray;

void foo1(void) {
  const char *s0 = @encode(MyArray *); // "^{NSArray=#}"
  const char *s1 = @encode(NSArray<Class1 *> *); // "@" 
}

It seems that this was a deliberate choice made in r61387 to make clang compatible with gcc (see http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20081222/010465.html), but it causes apple's runtime to crash. This patch fixes the bug by checking whether the runtime is one of the GNU runtimes before using the special encoding for typedefs.

rdar://problem/50563529

[theraven]

lib/AST/ASTContext.cpp

6975–6976

Please can we at least make this check just for the GCC runtime? I'm not sure it's even needed there. I've previously had to write code that works around this and have always considered it a bug in GCC, rather than a feature that I'd like us to copy, so I'd also be happy with just deleting this code path entirely.

[ahatanak]

lib/AST/ASTContext.cpp

6975–6976

Are we allowed to delete the code path entirely? That would clean up the code a bit, but I assume it would also break GCC runtime users' code.

[theraven]

lib/AST/ASTContext.cpp

6975–6976

I'd be happy with deleting it entirely. Nothing in the gcc runtime itself depends on it and all of the code that I've dealt with that works with the GCC runtime and this functionality has explicit work arounds for this behaviour in GCC.

[ahatanak]

Delete the entire code path that tries to work around a bug in gcc.

[smeenai]

In D61974#1508097, @ahatanak wrote:

Delete the entire code path that tries to work around a bug in gcc.

Should something be added to the release notes for this?

[ahatanak]

Yes, I think we can add something to the release note.

[ahatanak]

ping

[rjmccall]

lib/AST/ASTContext.cpp

6975

Is this option dead now?

[ahatanak]

Remove flag EncodePointerToObjCTypedef, which is no longer needed.

[rjmccall]

LGTM. David should sign off, too, though.

[theraven]

LGTM. I wonder if we have any other ugly GCC bug compatibility parts in clang's Objective-C implementation...

[ahatanak]

In D61974#1521334, @theraven wrote:

LGTM. I wonder if we have any other ugly GCC bug compatibility parts in clang's Objective-C implementation...

I see a couple of FIXME comments in ASTContext.cpp that seem to suggest clang is copying gcc's behavior that is possibly incorrect.

include/clang/AST/ASTContext.h

 #define OPT_LIST(V)                                                            \
   V(ExpandPointedToStructures, 0)                                              \
   V(ExpandStructures, 1)                                                       \
   V(IsOutermostType, 2)                                                        \
   V(EncodingProperty, 3)                                                       \
   V(IsStructField, 4)                                                          \
   V(EncodeBlockParameters, 5)                                                  \
   V(EncodeClassNames, 6)                                                       \
-  V(EncodePointerToObjCTypedef, 7)
 
 #define V(N,I) ObjCEncOptions& set##N() { Bits |= 1 << I; return *this; }
 OPT_LIST(V)
 #undef V
 
 #define V(N,I) bool N() const { return Bits & 1 << I; }
 OPT_LIST(V)
 #undef V
 
 #undef OPT_LIST
 
     LLVM_NODISCARD ObjCEncOptions keepingOnly(ObjCEncOptions Mask) const {
       return Bits & Mask.Bits;
     }
 
     LLVM_NODISCARD ObjCEncOptions forComponentType() const {
       ObjCEncOptions Mask = ObjCEncOptions()
                                 .setIsOutermostType()
-                                .setIsStructField()
-                                .setEncodePointerToObjCTypedef();
+                                .setIsStructField();
       return Bits & ~Mask.Bits;
     }
   };
 
   // Return the Objective-C type encoding for a given type.
   void getObjCEncodingForTypeImpl(QualType t, std::string &S,
                                   ObjCEncOptions Options,
                                   const FieldDecl *Field,

lib/AST/ASTContext.cpp

if (Options.ExpandStructures()) {
       SmallVector<const ObjCIvarDecl*, 32> Ivars;
       DeepCollectObjCIvars(OI, true, Ivars);
       for (unsigned i = 0, e = Ivars.size(); i != e; ++i) {
         const FieldDecl *Field = Ivars[i];
         if (Field->isBitField())
           getObjCEncodingForTypeImpl(Field->getType(), S,
                                      ObjCEncOptions().setExpandStructures(),
                                      Field);
-        else {
-          ObjCEncOptions NewOptions = ObjCEncOptions().setExpandStructures();
-          if (Options.EncodePointerToObjCTypedef())
-            NewOptions.setEncodePointerToObjCTypedef();
-          getObjCEncodingForTypeImpl(Field->getType(), S, NewOptions, FD,
+        else
+          getObjCEncodingForTypeImpl(Field->getType(), S,
+                                     ObjCEncOptions().setExpandStructures(), FD,
                                      NotEncodedT);
-        }
-      }
-    }
     S += '}';
     return;
   }
 
   case Type::ObjCObjectPointer: {
     const auto *OPT = T->castAs<ObjCObjectPointerType>();
     if (OPT->isObjCIdType()) {
       S += '@';
if (OPT->isObjCQualifiedIdType()) {
         // Only when doing ivar or property encoding.
         S += '"';
         for (const auto *I : OPT->quals()) {
           S += '<';
           S += I->getObjCRuntimeNameAsString();
           S += '>';
         }
         S += '"';
       }
       return;
     }
 
-    QualType PointeeTy = OPT->getPointeeType();
-    if (!Options.EncodingProperty() &&
-        isa<TypedefType>(PointeeTy.getTypePtr()) &&
-        !Options.EncodePointerToObjCTypedef()) {
-      // Another historical/compatibility reason.
-      // We encode the underlying type which comes out as
-      // {...};
-      S += '^';
-      if (FD && OPT->getInterfaceDecl()) {
-        // Prevent recursive encoding of fields in some rare cases.
-        ObjCInterfaceDecl *OI = OPT->getInterfaceDecl();
-        SmallVector<const ObjCIvarDecl*, 32> Ivars;
-        DeepCollectObjCIvars(OI, true, Ivars);
-        for (unsigned i = 0, e = Ivars.size(); i != e; ++i) {
-          if (Ivars[i] == FD) {
-            S += '{';
-            S += OI->getObjCRuntimeNameAsString();
-            S += '}';
-            return;
-          }
-        }
-      }
-      ObjCEncOptions NewOptions =
-          ObjCEncOptions().setEncodePointerToObjCTypedef();
-      if (Options.ExpandPointedToStructures())
-        NewOptions.setExpandStructures();
-      getObjCEncodingForTypeImpl(PointeeTy, S, NewOptions, /*Field=*/nullptr);
-      return;
-    }
-
     S += '@';
     if (OPT->getInterfaceDecl() &&
         (FD || Options.EncodingProperty() || Options.EncodeClassNames())) {
       S += '"';
       S += OPT->getInterfaceDecl()->getObjCRuntimeNameAsString();
       for (const auto *I : OPT->quals()) {
         S += '<';
         S += I->getObjCRuntimeNameAsString();
         S += '>';

test/CodeGenObjC/encode-test-6.m

 }
 @property (copy) BABugExampleRedefinition *property;
 @end
 
 @implementation BABugExample
 @synthesize property = _property;
 @end
 
-// CHECK: private unnamed_addr constant [24 x i8] c"^{BABugExample=@}16
+// CHECK: private unnamed_addr constant [8 x i8] c"@16
 
 // rdar://14408244
 @class SCNCamera;
 typedef SCNCamera C3DCamera;
 typedef struct
 {
     C3DCamera *presentationInstance;
 }  C3DCameraStorage;
 
 @interface SCNCamera
 @end
 
 @implementation SCNCamera
 {
     C3DCameraStorage _storage;
 }
 @end
-// CHECK: private unnamed_addr constant [39 x i8] c"{?=\22presentationInstance\22^{SCNCamera}}\00"
+// CHECK: private unnamed_addr constant [39 x i8] c"{?=\22presentationInstance\22@\22SCNCamera\22}\00"
 
 // rdar://16655340
 int i;
 typeof(@encode(typeof(i))) e = @encode(typeof(i));
 const char * Test()
 {
     return e;
 }
 // CHECK: @e = global [2 x i8] c"i\00", align 1
 // CHECK: define i8* @Test()
 // CHECK: ret i8* getelementptr inbounds ([2 x i8], [2 x i8]* @e, i64 0, i64 0)

test/CodeGenObjC/encode-test.m

 const char g2[] = @encode(B1);
 
 // CHECK: @g3 = constant [8 x i8] c"r^{S=i}\00"
 const char g3[] = @encode(const struct S *);
 
 // CHECK: @g4 = constant [6 x i8] c"{S=i}\00"
 const char g4[] = @encode(const struct S);
 
-// CHECK: @g5 = constant [12 x i8] c"^{Object=#}\00"
+// CHECK: @g5 = constant [2 x i8] c"@\00"
 const char g5[] = @encode(MyObj * const);
 
 ////
 
 enum Enum1X { one, two, three, four };
 
 @interface Base1X {
   unsigned a: 2;

test/CodeGenObjCXX/encode.mm

@interface N
   S<N> a;
 }
 @end
 
 @implementation N
 @end
 
 const char *expand_struct() {
-  // CHECK: @{{.*}} = private unnamed_addr constant [16 x i8] c"{N={S<N>=^{N}}}\00"
+  // CHECK: @{{.*}} = private unnamed_addr constant [13 x i8] c"{N={S<N>=@}}\00"
   return @encode(N);
 }