[cfe-commits] r57675 - in /cfe/trunk: include/clang/AST/ASTContext.h lib/AST/ASTContext.cpp

[Daniel Dunbar]

Author: ddunbar
Date: Fri Oct 17 02:30:50 2008
New Revision: 57675

URL: http://llvm.org/viewvc/llvm-project?rev=57675&view=rev
Log:
Fix bug in Obj-C type encoding for structures. 
 - Mechanism for detecting if a structure should be expanded wasn't
   reliable. Simplified by just keeping track of what we should be
   expanding.

 - This fixes a bug in using NSInvocation to invoke a method which
   returned a structure, which in used by Key Value Observing, which
   in the end, caused a miscompile in poor little Sketch.

Modified:
    cfe/trunk/include/clang/AST/ASTContext.h
    cfe/trunk/lib/AST/ASTContext.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=57675&r1=57674&r2=57675&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Fri Oct 17 02:30:50 2008
@@ -473,6 +473,13 @@
   ///  struct/union/class.  This will eventually be used by enums as well.
   void setTagDefinition(TagDecl* R);
   friend class RecordDecl;
+
+  // Return the ObjC type encoding for a given type.
+  void getObjCEncodingForTypeImpl(QualType t, std::string &S, 
+                                  bool ExpandPointedToStructures,
+                                  bool ExpandStructures,
+                              llvm::SmallVector<const RecordType*,8> &RT) const;
+  
 };
   
 }  // end namespace clang

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=57675&r1=57674&r2=57675&view=diff

==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Fri Oct 17 02:30:50 2008
@@ -1594,7 +1594,18 @@
 }
 
 void ASTContext::getObjCEncodingForType(QualType T, std::string& S,
-       llvm::SmallVector<const RecordType *, 8> &ERType) const {
+                         llvm::SmallVector<const RecordType*,8> &ERType) const {
+  // We follow the behavior of gcc, expanding structures which are
+  // directly pointed to, and expanding embedded structures. Note that
+  // these rules are sufficient to prevent recursive encoding of the
+  // same type.
+  getObjCEncodingForTypeImpl(T, S, true, true, ERType);
+}
+
+void ASTContext::getObjCEncodingForTypeImpl(QualType T, std::string& S,
+                                            bool ExpandPointedToStructures,
+                                            bool ExpandStructures,
+                         llvm::SmallVector<const RecordType*,8> &ERType) const {
   // FIXME: This currently doesn't encode:
   // @ An object (whether statically typed or typed id)
   // # A class object (Class)
@@ -1630,8 +1641,9 @@
   }
   else if (T->isObjCQualifiedIdType()) {
     // Treat id<P...> same as 'id' for encoding purposes.
-    return getObjCEncodingForType(getObjCIdType(), S, ERType);
-    
+    return getObjCEncodingForTypeImpl(getObjCIdType(), S, 
+                                      ExpandPointedToStructures,
+                                      ExpandStructures, ERType);    
   }
   else if (const PointerType *PT = T->getAsPointerType()) {
     QualType PointeeTy = PT->getPointeeType();
@@ -1656,7 +1668,8 @@
     }
     
     S += '^';
-    getObjCEncodingForType(PT->getPointeeType(), S, ERType);
+    getObjCEncodingForTypeImpl(PT->getPointeeType(), S, 
+                               false, ExpandPointedToStructures, ERType);
   } else if (const ArrayType *AT =
                // Ignore type qualifiers etc.
                dyn_cast<ArrayType>(T->getCanonicalTypeInternal())) {
@@ -1667,17 +1680,13 @@
     else
       assert(0 && "Unhandled array type!");
     
-    getObjCEncodingForType(AT->getElementType(), S, ERType);
+    getObjCEncodingForTypeImpl(AT->getElementType(), S, 
+                               false, ExpandStructures, ERType);
     S += ']';
   } else if (T->getAsFunctionType()) {
     S += '?';
   } else if (const RecordType *RTy = T->getAsRecordType()) {
-    RecordDecl *RDecl= RTy->getDecl();
-    // This mimics the behavior in gcc's encode_aggregate_within().
-    // The idea is to only inline structure definitions for top level pointers
-    // to structures and embedded structures.
-    bool inlining = (S.size() == 1 && S[0] == '^' ||
-                     S.size() > 1 && S[S.size()-1] != '^');
+    RecordDecl *RDecl = RTy->getDecl();
     S += '{';
     // Anonymous structures print as '?'
     if (const IdentifierInfo *II = RDecl->getIdentifier()) {
@@ -1691,12 +1700,12 @@
         found = true;
         break;
       }
-    if (!found && inlining) {
+    if (!found && ExpandStructures) {
       ERType.push_back(RTy);
       S += '=';
       for (int i = 0; i < RDecl->getNumMembers(); i++) {
         FieldDecl *field = RDecl->getMember(i);
-        getObjCEncodingForType(field->getType(), S, ERType);
+        getObjCEncodingForTypeImpl(field->getType(), S, false, true, ERType);
       }
       assert(ERType.back() == RTy && "Record Type stack mismatch.");
       ERType.pop_back();