r176336 - Attempt to not place ownership qualifiers on the result type

John McCall rjmccall at apple.com
Thu Feb 28 23:58:17 PST 2013


Author: rjmccall
Date: Fri Mar  1 01:58:16 2013
New Revision: 176336

URL: http://llvm.org/viewvc/llvm-project?rev=176336&view=rev
Log:
Attempt to not place ownership qualifiers on the result type
of block declarators.  Document the rule we use.

Also document the rule that Doug implemented a few weeks ago
which drops ownership qualifiers on function result types.

rdar://10127067

Modified:
    cfe/trunk/docs/AutomaticReferenceCounting.rst
    cfe/trunk/include/clang/Sema/DeclSpec.h
    cfe/trunk/lib/Sema/SemaType.cpp
    cfe/trunk/test/SemaObjC/arc-objc-lifetime.m

Modified: cfe/trunk/docs/AutomaticReferenceCounting.rst
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/AutomaticReferenceCounting.rst?rev=176336&r1=176335&r2=176336&view=diff
==============================================================================
--- cfe/trunk/docs/AutomaticReferenceCounting.rst (original)
+++ cfe/trunk/docs/AutomaticReferenceCounting.rst Fri Mar  1 01:58:16 2013
@@ -679,6 +679,9 @@ There is a single exception to this rule
 to a substituted template type parameter, which overrides the ownership
 qualifier provided by the template argument.
 
+When forming a function type, the result type is adjusted so that any
+top-level ownership qualifier is deleted.
+
 Except as described under the :ref:`inference rules <arc.ownership.inference>`,
 a program is ill-formed if it attempts to form a pointer or reference type to a
 retainable object owner type which lacks an ownership qualifier.
@@ -689,7 +692,9 @@ retainable object owner type which lacks
   lvalues of retainable object pointer type have an ownership qualifier.  The
   ability to override an ownership qualifier during template substitution is
   required to counteract the :ref:`inference of __strong for template type
-  arguments <arc.ownership.inference.template.arguments>`.
+  arguments <arc.ownership.inference.template.arguments>`.  Ownership qualifiers
+  on return types are dropped because they serve no purpose there except to
+  cause spurious problems with overloading and templates.
 
 There are four ownership qualifiers:
 
@@ -717,17 +722,37 @@ If an ownership qualifier appears in the
 following rules apply:
 
 * if the type specifier is a retainable object owner type, the qualifier
-  applies to that type;
-* if the outermost non-array part of the declarator is a pointer or block
-  pointer, the qualifier applies to that type;
+  initially applies to that type;
+
+* otherwise, if the outermost non-array declarator is a pointer
+  or block pointer declarator, the qualifier initially applies to
+  that type;
+
 * otherwise the program is ill-formed.
 
+* If the qualifier is so applied at a position in the declaration
+  where the next-innermost declarator is a function declarator, and
+  there is an block declarator within that function declarator, then
+  the qualifier applies instead to that block declarator and this rule
+  is considered afresh beginning from the new position.
+
 If an ownership qualifier appears on the declarator name, or on the declared
-object, it is applied to outermost pointer or block-pointer type.
+object, it is applied to the innermost pointer or block-pointer type.
 
 If an ownership qualifier appears anywhere else in a declarator, it applies to
 the type there.
 
+.. admonition:: Rationale
+
+  Ownership qualifiers are like ``const`` and ``volatile`` in the sense
+  that they may sensibly apply at multiple distinct positions within a
+  declarator.  However, unlike those qualifiers, there are many
+  situations where they are not meaningful, and so we make an effort
+  to "move" the qualifier to a place where it will be meaningful.  The
+  general goal is to allow the programmer to write, say, ``__strong``
+  before the entire declaration and have it apply in the leftmost
+  sensible place.
+
 .. _arc.ownership.spelling.property:
 
 Property declarations

Modified: cfe/trunk/include/clang/Sema/DeclSpec.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DeclSpec.h?rev=176336&r1=176335&r2=176336&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/DeclSpec.h (original)
+++ cfe/trunk/include/clang/Sema/DeclSpec.h Fri Mar  1 01:58:16 2013
@@ -1435,6 +1435,9 @@ struct DeclaratorChunk {
     return I;
   }
 
+  bool isParen() const {
+    return Kind == Paren;
+  }
 };
 
 /// \brief Described the kind of function definition (if any) provided for
@@ -1803,26 +1806,35 @@ public:
     DeclTypeInfo.erase(DeclTypeInfo.begin());
   }
 
+  /// Return the innermost (closest to the declarator) chunk of this
+  /// declarator that is not a parens chunk, or null if there are no
+  /// non-parens chunks.
+  const DeclaratorChunk *getInnermostNonParenChunk() const {
+    for (unsigned i = 0, i_end = DeclTypeInfo.size(); i < i_end; ++i) {
+      if (!DeclTypeInfo[i].isParen())
+        return &DeclTypeInfo[i];
+    }
+    return 0;
+  }
+
+  /// Return the outermost (furthest from the declarator) chunk of
+  /// this declarator that is not a parens chunk, or null if there are
+  /// no non-parens chunks.
+  const DeclaratorChunk *getOutermostNonParenChunk() const {
+    for (unsigned i = DeclTypeInfo.size(), i_end = 0; i != i_end; --i) {
+      if (!DeclTypeInfo[i-1].isParen())
+        return &DeclTypeInfo[i-1];
+    }
+    return 0;
+  }
+
   /// isArrayOfUnknownBound - This method returns true if the declarator
   /// is a declarator for an array of unknown bound (looking through
   /// parentheses).
   bool isArrayOfUnknownBound() const {
-    for (unsigned i = 0, i_end = DeclTypeInfo.size(); i < i_end; ++i) {
-      switch (DeclTypeInfo[i].Kind) {
-      case DeclaratorChunk::Paren:
-        continue;
-      case DeclaratorChunk::Function:
-      case DeclaratorChunk::Pointer:
-      case DeclaratorChunk::Reference:
-      case DeclaratorChunk::BlockPointer:
-      case DeclaratorChunk::MemberPointer:
-        return false;
-      case DeclaratorChunk::Array:
-        return !DeclTypeInfo[i].Arr.NumElts;
-      }
-      llvm_unreachable("Invalid type chunk");
-    }
-    return false;
+    const DeclaratorChunk *chunk = getInnermostNonParenChunk();
+    return (chunk && chunk->Kind == DeclaratorChunk::Array &&
+            !chunk->Arr.NumElts);
   }
 
   /// isFunctionDeclarator - This method returns true if the declarator

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=176336&r1=176335&r2=176336&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Fri Mar  1 01:58:16 2013
@@ -150,6 +150,10 @@ namespace {
       return declarator;
     }
 
+    bool isProcessingDeclSpec() const {
+      return chunkIndex == declarator.getNumTypeObjects();
+    }
+
     unsigned getCurrentChunkIndex() const {
       return chunkIndex;
     }
@@ -160,8 +164,7 @@ namespace {
     }
 
     AttributeList *&getCurrentAttrListRef() const {
-      assert(chunkIndex <= declarator.getNumTypeObjects());
-      if (chunkIndex == declarator.getNumTypeObjects())
+      if (isProcessingDeclSpec())
         return getMutableDeclSpec().getAttributes().getListRef();
       return declarator.getTypeObject(chunkIndex).getAttrListRef();
     }
@@ -302,6 +305,66 @@ static bool handleObjCPointerTypeAttr(Ty
   return handleObjCOwnershipTypeAttr(state, attr, type);
 }
 
+/// Given the index of a declarator chunk, check whether that chunk
+/// directly specifies the return type of a function and, if so, find
+/// an appropriate place for it.
+///
+/// \param i - a notional index which the search will start
+///   immediately inside
+static DeclaratorChunk *maybeMovePastReturnType(Declarator &declarator,
+                                                unsigned i) {
+  assert(i <= declarator.getNumTypeObjects());
+
+  DeclaratorChunk *result = 0;
+
+  // First, look inwards past parens for a function declarator.
+  for (; i != 0; --i) {
+    DeclaratorChunk &fnChunk = declarator.getTypeObject(i-1);
+    switch (fnChunk.Kind) {
+    case DeclaratorChunk::Paren:
+      continue;
+
+    // If we find anything except a function, bail out.
+    case DeclaratorChunk::Pointer:
+    case DeclaratorChunk::BlockPointer:
+    case DeclaratorChunk::Array:
+    case DeclaratorChunk::Reference:
+    case DeclaratorChunk::MemberPointer:
+      return result;
+
+    // If we do find a function declarator, scan inwards from that,
+    // looking for a block-pointer declarator.
+    case DeclaratorChunk::Function:
+      for (--i; i != 0; --i) {
+        DeclaratorChunk &blockChunk = declarator.getTypeObject(i-1);
+        switch (blockChunk.Kind) {
+        case DeclaratorChunk::Paren:
+        case DeclaratorChunk::Pointer:
+        case DeclaratorChunk::Array:
+        case DeclaratorChunk::Function:
+        case DeclaratorChunk::Reference:
+        case DeclaratorChunk::MemberPointer:
+          continue;
+        case DeclaratorChunk::BlockPointer:
+          result = &blockChunk;
+          goto continue_outer;
+        }
+        llvm_unreachable("bad declarator chunk kind");
+      }
+
+      // If we run out of declarators doing that, we're done.
+      return result;
+    }
+    llvm_unreachable("bad declarator chunk kind");
+
+    // Okay, reconsider from our new point.
+  continue_outer: ;
+  }
+
+  // Ran out of chunks, bail out.
+  return result;
+}
+
 /// Given that an objc_gc attribute was written somewhere on a
 /// declaration *other* than on the declarator itself (for which, use
 /// distributeObjCPointerTypeAttrFromDeclarator), and given that it
@@ -311,22 +374,44 @@ static void distributeObjCPointerTypeAtt
                                           AttributeList &attr,
                                           QualType type) {
   Declarator &declarator = state.getDeclarator();
+
+  // Move it to the outermost normal or block pointer declarator.
   for (unsigned i = state.getCurrentChunkIndex(); i != 0; --i) {
     DeclaratorChunk &chunk = declarator.getTypeObject(i-1);
     switch (chunk.Kind) {
     case DeclaratorChunk::Pointer:
-    case DeclaratorChunk::BlockPointer:
+    case DeclaratorChunk::BlockPointer: {
+      // But don't move an ARC ownership attribute to the return type
+      // of a block.
+      DeclaratorChunk *destChunk = 0;
+      if (state.isProcessingDeclSpec() &&
+          attr.getKind() == AttributeList::AT_ObjCOwnership)
+        destChunk = maybeMovePastReturnType(declarator, i - 1);
+      if (!destChunk) destChunk = &chunk;
+
       moveAttrFromListToList(attr, state.getCurrentAttrListRef(),
-                             chunk.getAttrListRef());
+                             destChunk->getAttrListRef());
       return;
+    }
 
     case DeclaratorChunk::Paren:
     case DeclaratorChunk::Array:
       continue;
 
+    // We may be starting at the return type of a block.
+    case DeclaratorChunk::Function:
+      if (state.isProcessingDeclSpec() &&
+          attr.getKind() == AttributeList::AT_ObjCOwnership) {
+        if (DeclaratorChunk *dest = maybeMovePastReturnType(declarator, i)) {
+          moveAttrFromListToList(attr, state.getCurrentAttrListRef(),
+                                 dest->getAttrListRef());
+          return;
+        }
+      }
+      goto error;
+
     // Don't walk through these.
     case DeclaratorChunk::Reference:
-    case DeclaratorChunk::Function:
     case DeclaratorChunk::MemberPointer:
       goto error;
     }
@@ -3638,6 +3723,14 @@ static bool handleObjCOwnershipTypeAttr(
     } else if (!type->isObjCRetainableType()) {
       return false;
     }
+
+    // Don't accept an ownership attribute in the declspec if it would
+    // just be the return type of a block pointer.
+    if (state.isProcessingDeclSpec()) {
+      Declarator &D = state.getDeclarator();
+      if (maybeMovePastReturnType(D, D.getNumTypeObjects()))
+        return false;
+    }
   }
 
   Sema &S = state.getSema();
@@ -3744,10 +3837,8 @@ static bool handleObjCOwnershipTypeAttr(
   // Forbid __weak for class objects marked as
   // objc_arc_weak_reference_unavailable
   if (lifetime == Qualifiers::OCL_Weak) {
-    QualType T = type;
-    while (const PointerType *ptr = T->getAs<PointerType>())
-      T = ptr->getPointeeType();
-    if (const ObjCObjectPointerType *ObjT = T->getAs<ObjCObjectPointerType>()) {
+    if (const ObjCObjectPointerType *ObjT =
+          type->getAs<ObjCObjectPointerType>()) {
       if (ObjCInterfaceDecl *Class = ObjT->getInterfaceDecl()) {
         if (Class->isArcWeakrefUnavailable()) {
             S.Diag(AttrLoc, diag::err_arc_unsupported_weak_class);

Modified: cfe/trunk/test/SemaObjC/arc-objc-lifetime.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-objc-lifetime.m?rev=176336&r1=176335&r2=176336&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/arc-objc-lifetime.m (original)
+++ cfe/trunk/test/SemaObjC/arc-objc-lifetime.m Fri Mar  1 01:58:16 2013
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc -fblocks -Wexplicit-ownership-type  -verify -Wno-objc-root-class %s
-// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc -fblocks -Wexplicit-ownership-type -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc -fblocks -fobjc-runtime-has-weak -Wexplicit-ownership-type  -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc -fblocks -fobjc-runtime-has-weak -Wexplicit-ownership-type -verify -Wno-objc-root-class %s
 // rdar://10244607
 
 typedef const struct __CFString * CFStringRef;
@@ -80,9 +80,48 @@ NSObject * __strong f4(void); // expecte
 NSObject_ptr __strong f5(); // expected-warning{{ARC __strong lifetime qualifier on return type is ignored}}
 
 typedef __strong id (*fptr)(int); // expected-warning{{ARC __strong lifetime qualifier on return type is ignored}}
-typedef __strong id (^block_ptr)(int); // expected-warning{{ARC __strong lifetime qualifier on return type is ignored}}
 
 // Don't warn
 strong_id f6();
 strong_NSObject_ptr f7();
+typedef __strong id (^block_ptr)(int);
 
+// rdar://10127067
+void test8_a() {
+  __weak id *(^myBlock)(void);
+  __weak id *var = myBlock();
+  (void) (__strong id *) &myBlock;
+  (void) (__weak id *) &myBlock; // expected-error {{cast}}
+}
+void test8_b() {
+  __weak id (^myBlock)(void);
+  (void) (__weak id *) &myBlock;
+  (void) (__strong id *) &myBlock; // expected-error {{cast}}
+}
+void test8_c() {
+  __weak id (^*(^myBlock)(void))(void);
+  (void) (__weak id*) myBlock();
+  (void) (__strong id*) myBlock(); // expected-error {{cast}}
+  (void) (__weak id*) &myBlock; // expected-error {{cast}}
+  (void) (__strong id*) &myBlock;
+}
+
+ at class Test9;
+void test9_a() {
+  __weak Test9 **(^myBlock)(void);
+  __weak Test9 **var = myBlock();
+  (void) (__strong Test9 **) &myBlock;
+  (void) (__weak Test9 **) &myBlock; // expected-error {{cast}}
+}
+void test9_b() {
+  __weak Test9 *(^myBlock)(void);
+  (void) (__weak Test9**) &myBlock;
+  (void) (__strong Test9**) &myBlock; // expected-error {{cast}}
+}
+void test9_c() {
+  __weak Test9 *(^*(^myBlock)(void))(void);
+  (void) (__weak Test9 **) myBlock();
+  (void) (__strong Test9 **) myBlock(); // expected-error {{cast}}
+  (void) (__weak Test9 **) &myBlock; // expected-error {{cast}}
+  (void) (__strong Test9 **) &myBlock;
+}





More information about the cfe-commits mailing list