]> granicus.if.org Git - clang/commitdiff
Attempt to not place ownership qualifiers on the result type
authorJohn McCall <rjmccall@apple.com>
Fri, 1 Mar 2013 07:58:16 +0000 (07:58 +0000)
committerJohn McCall <rjmccall@apple.com>
Fri, 1 Mar 2013 07:58:16 +0000 (07:58 +0000)
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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176336 91177308-0d34-0410-b5e6-96231b3b80d8

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

index 8993bc7b6b178c72821c177c0e7d9f66d559b851..65675a44bf695a26a786c4b08237fb250df5a758 100644 (file)
@@ -679,6 +679,9 @@ There is a single exception to this rule: an ownership qualifier may be applied
 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 an ownership qualifier.
   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 *declaration-specifiers*, 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
index 867c654435a1ed9da830d8374229d1e6b61a39b2..d20877b2d31c37175a806f07f0fa0c2a5dc121be 100644 (file)
@@ -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
index c47a7f59ac04c072a473113032eb8ff01a85df2b..1c3544283a249a455281752a0999343804afaf31 100644 (file)
@@ -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(TypeProcessingState &state,
   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 distributeObjCPointerTypeAttr(TypeProcessingState &state,
                                           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(TypeProcessingState &state,
     } 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(TypeProcessingState &state,
   // 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);
index f2fb1393222c942e6906f82b00a522215b61f798..5e252537fb889efaadf166a77707d66261b3c41c 100644 (file)
@@ -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); // expected-warning{{ARC __strong lifetime qualifi
 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;
+}
 
+@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;
+}