Remove OwnershipAttr::Kind, since it's essentially redundant with attr::Kind the way it's being used. Also fix isa<OwnershipAttr> support, break more-than-80-char lines, and other miscellaneous ownership attr cleanup.

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

include/clang/AST/Attr.h

@@ -379,23 +379,14 @@ class OwnershipAttr: public AttrWithString {
   unsigned* ArgNums;
   unsigned Size;
 public:
-  enum OwnershipKind { Holds, Takes, Returns, None };
-  OwnershipKind OKind;
   attr::Kind AKind;
 public:
   OwnershipAttr(attr::Kind AK, ASTContext &C, unsigned* arg_nums, unsigned size,
-                       llvm::StringRef module, OwnershipKind kind);
+                llvm::StringRef module);
 
 
   virtual void Destroy(ASTContext &C);
 
-  OwnershipKind getKind() const {
-    return OKind;
-  }
-  bool isKind(const OwnershipKind k) const {
-    return OKind == k;
-  }
-
   /// Ownership attributes have a 'module', which is the name of a kind of
   /// resource that can be checked.
   /// The Malloc checker uses the module 'malloc'.
@@ -423,7 +414,14 @@ public:
   virtual Attr *clone(ASTContext &C) const;
 
   static bool classof(const Attr *A) {
-    return true;
+    switch (A->getKind()) {
+    case attr::OwnershipTakes:
+    case attr::OwnershipHolds:
+    case attr::OwnershipReturns:
+      return true;
+    default:
+      return false;
+    }
   }
   static bool classof(const OwnershipAttr *A) {
     return true;
@@ -443,9 +441,6 @@ public:
   static bool classof(const OwnershipTakesAttr *A) {
     return true;
   }
-  static bool classof(const OwnershipAttr *A) {
-    return A->OKind == Takes;
-  }
 };
 
 class OwnershipHoldsAttr: public OwnershipAttr {
@@ -461,9 +456,6 @@ public:
   static bool classof(const OwnershipHoldsAttr *A) {
     return true;
   }
-  static bool classof(const OwnershipAttr *A) {
-    return A->OKind == Holds;
-  }
 };
 
 class OwnershipReturnsAttr: public OwnershipAttr {
@@ -479,9 +471,6 @@ public:
   static bool classof(const OwnershipReturnsAttr *A) {
     return true;
   }
-  static bool classof(const OwnershipAttr *A) {
-    return A->OKind == Returns;
-  }
 };
 
 class FormatAttr : public AttrWithString {

lib/AST/AttrImpl.cpp

@@ -49,17 +49,13 @@ NonNullAttr::NonNullAttr(ASTContext &C, unsigned* arg_nums, unsigned size)
 }
 
 OwnershipAttr::OwnershipAttr(attr::Kind AK, ASTContext &C, unsigned* arg_nums,
-                                           unsigned size,
+                             unsigned size, llvm::StringRef module)
-                                           llvm::StringRef module,
+  : AttrWithString(AK, C, module), ArgNums(0), Size(0) {
-                                           OwnershipKind kind) :
-  AttrWithString(AK, C, module), ArgNums(0), Size(0), OKind(kind) {
   if (size == 0)
     return;
   assert(arg_nums);
   ArgNums = new (C) unsigned[size];
   Size = size;
-  AKind = AK;
-  OKind = kind;
   memcpy(ArgNums, arg_nums, sizeof(*ArgNums) * size);
 }
 
@@ -70,19 +66,19 @@ void OwnershipAttr::Destroy(ASTContext &C) {
 }
 
 OwnershipTakesAttr::OwnershipTakesAttr(ASTContext &C, unsigned* arg_nums,
-                                       unsigned size, llvm::StringRef module) :
+                                       unsigned size, llvm::StringRef module)
-  OwnershipAttr(attr::OwnershipTakes, C, arg_nums, size, module, Takes) {
+  : OwnershipAttr(attr::OwnershipTakes, C, arg_nums, size, module) {
 }
 
 OwnershipHoldsAttr::OwnershipHoldsAttr(ASTContext &C, unsigned* arg_nums,
-                                       unsigned size, llvm::StringRef module) :
+                                       unsigned size, llvm::StringRef module)
-  OwnershipAttr(attr::OwnershipHolds, C, arg_nums, size, module, Holds) {
+  : OwnershipAttr(attr::OwnershipHolds, C, arg_nums, size, module) {
 }
 
 OwnershipReturnsAttr::OwnershipReturnsAttr(ASTContext &C, unsigned* arg_nums,
                                            unsigned size,
-                                           llvm::StringRef module) :
+                                           llvm::StringRef module)
-  OwnershipAttr(attr::OwnershipReturns, C, arg_nums, size, module, Returns) {
+  : OwnershipAttr(attr::OwnershipReturns, C, arg_nums, size, module) {
 }
 
 #define DEF_SIMPLE_ATTR_CLONE(ATTR)                                     \
@@ -187,7 +183,7 @@ Attr *NonNullAttr::clone(ASTContext &C) const {
 }
 
 Attr *OwnershipAttr::clone(ASTContext &C) const {
-  return ::new (C) OwnershipAttr(AKind, C, ArgNums, Size, getModule(), OKind);
+  return ::new (C) OwnershipAttr(AKind, C, ArgNums, Size, getModule());
 }
 
 Attr *OwnershipReturnsAttr::clone(ASTContext &C) const {

lib/Checker/MallocChecker.cpp

@@ -100,7 +100,8 @@ private:
                               const GRState *state);
 
   void FreeMem(CheckerContext &C, const CallExpr *CE);
-  void FreeMemAttr(CheckerContext &C, const CallExpr *CE, const OwnershipAttr* Att);
+  void FreeMemAttr(CheckerContext &C, const CallExpr *CE,
+                   const OwnershipAttr* Att);
   const GRState *FreeMemAux(CheckerContext &C, const CallExpr *CE,
                             const GRState *state, unsigned Num, bool Hold);
 
@@ -176,22 +177,19 @@ bool MallocChecker::EvalCallExpr(CheckerContext &C, const CallExpr *CE) {
   bool rv = false;
   if (FD->hasAttrs()) {
     for (const Attr *attr = FD->getAttrs(); attr; attr = attr->getNext()) {
-      if(const OwnershipAttr* Att = dyn_cast<OwnershipAttr>(attr)) {
+      switch (attr->getKind()) {
-        switch (Att->getKind()) {
+      case attr::OwnershipReturns:
-        case OwnershipAttr::Returns: {
+        MallocMemReturnsAttr(C, CE, cast<OwnershipAttr>(attr));
-          MallocMemReturnsAttr(C, CE, Att);
+        rv = true;
-          rv = true;
+        break;
-          break;
+      case attr::OwnershipTakes:
-        }
+      case attr::OwnershipHolds:
-        case OwnershipAttr::Takes:
+        FreeMemAttr(C, CE, cast<OwnershipAttr>(attr));
-        case OwnershipAttr::Holds: {
+        rv = true;
-          FreeMemAttr(C, CE, Att);
+        break;
-          rv = true;
+      default:
-          break;
+        // Ignore non-ownership attributes.
-        }
+        break;
-        default:
-          break;
-        }
       }
     }
   }
@@ -259,13 +257,13 @@ void MallocChecker::FreeMem(CheckerContext &C, const CallExpr *CE) {
 }
 
 void MallocChecker::FreeMemAttr(CheckerContext &C, const CallExpr *CE,
-                                     const OwnershipAttr* Att) {
+                                const OwnershipAttr* Att) {
   if (!Att->isModule("malloc"))
     return;
 
   for (const unsigned *I = Att->begin(), *E = Att->end(); I != E; ++I) {
     const GRState *state =
-        FreeMemAux(C, CE, C.getState(), *I, Att->isKind(OwnershipAttr::Holds));
+        FreeMemAux(C, CE, C.getState(), *I, isa<OwnershipHoldsAttr>(Att));
   if (state)
     C.addTransition(state);
   }

lib/Sema/SemaDeclAttr.cpp

@@ -357,8 +357,8 @@ static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) {
   // The following arguments must be argument indexes, the arguments must be
   // of integer type for Returns, otherwise of pointer type.
   // The difference between Holds and Takes is that a pointer may still be used
-  // after being held.  free() should be __attribute((ownership_takes)), whereas a list
+  // after being held.  free() should be __attribute((ownership_takes)), whereas
-  // append function may well be __attribute((ownership_holds)).
+  // a list append function may well be __attribute((ownership_holds)).
 
   if (!AL.getParameterName()) {
     S.Diag(AL.getLoc(), diag::err_attribute_argument_n_not_string)
@@ -366,30 +366,33 @@ static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) {
     return;
   }
   // Figure out our Kind, and check arguments while we're at it.
-  OwnershipAttr::OwnershipKind K = OwnershipAttr::None;
+  attr::Kind K;
-  if (AL.getName()->getName().equals("ownership_takes")) {
+  switch (AL.getKind()) {
-    K = OwnershipAttr::Takes;
+  case AttributeList::AT_ownership_takes:
+    K = attr::OwnershipTakes;
     if (AL.getNumArgs() < 1) {
       S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << 2;
       return;
     }
-  } else if (AL.getName()->getName().equals("ownership_holds")) {
+    break;
-    K = OwnershipAttr::Holds;
+  case AttributeList::AT_ownership_holds:
+    K = attr::OwnershipHolds;
     if (AL.getNumArgs() < 1) {
       S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << 2;
       return;
     }
-  } else if (AL.getName()->getName().equals("ownership_returns")) {
+    break;
-    K = OwnershipAttr::Returns;
+  case AttributeList::AT_ownership_returns:
+    K = attr::OwnershipReturns;
     if (AL.getNumArgs() > 1) {
       S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments)
           << AL.getNumArgs() + 1;
       return;
     }
-  }
+    break;
-  // This should never happen given how we are called.
+  default:
-  if (K == OwnershipAttr::None) {
+    // This should never happen given how we are called.
-    return;
+    llvm_unreachable("Unknown ownership attribute");
   }
 
   if (!isFunction(d) || !hasFunctionProto(d)) {
@@ -408,7 +411,8 @@ static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) {
 
   llvm::SmallVector<unsigned, 10> OwnershipArgs;
 
-  for (AttributeList::arg_iterator I = AL.arg_begin(), E = AL.arg_end(); I != E; ++I) {
+  for (AttributeList::arg_iterator I = AL.arg_begin(), E = AL.arg_end(); I != E;
+       ++I) {
 
     Expr *IdxExpr = static_cast<Expr *>(*I);
     llvm::APSInt ArgNum(32);
@@ -428,21 +432,21 @@ static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) {
     }
     --x;
     switch (K) {
-    case OwnershipAttr::Takes:
+    case attr::OwnershipTakes:
-    case OwnershipAttr::Holds: {
+    case attr::OwnershipHolds: {
       // Is the function argument a pointer type?
       QualType T = getFunctionOrMethodArgType(d, x);
       if (!T->isAnyPointerType() && !T->isBlockPointerType()) {
         // FIXME: Should also highlight argument in decl.
         S.Diag(AL.getLoc(), diag::err_ownership_type)
-            << ((K==OwnershipAttr::Takes)?"ownership_takes":"ownership_holds")
+            << ((K==attr::OwnershipTakes)?"ownership_takes":"ownership_holds")
             << "pointer"
             << IdxExpr->getSourceRange();
         continue;
       }
       break;
     }
-    case OwnershipAttr::Returns: {
+    case attr::OwnershipReturns: {
       if (AL.getNumArgs() > 1) {
           // Is the function argument an integer type?
           Expr *IdxExpr = static_cast<Expr *>(AL.getArg(0));
@@ -457,20 +461,18 @@ static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) {
       }
       break;
     }
-    // Should never happen, here to silence a warning.
+    default:
-    default: {
+      llvm_unreachable("Unknown ownership attribute");
-      return;
-    }
     } // switch
 
     // Check we don't have a conflict with another ownership attribute.
-    if (d->hasAttrs()) {
+    if (K != attr::OwnershipReturns && d->hasAttrs()) {
       for (const Attr *attr = d->getAttrs(); attr; attr = attr->getNext()) {
         if (const OwnershipAttr* Att = dyn_cast<OwnershipAttr>(attr)) {
           // Two ownership attributes of the same kind can't conflict,
           // except returns attributes.
-          if (K == OwnershipAttr::Returns || Att->getKind() != K) {
+          if (Att->getKind() != K) {
-            for (const unsigned *I = Att->begin(), *E = Att->end(); I != E; ++I) {
+            for (const unsigned *I = Att->begin(), *E = Att->end(); I!=E; ++I) {
               if (x == *I) {
                 S.Diag(AL.getLoc(), diag::err_attributes_are_not_compatible)
                     << AL.getName()->getName() << "ownership_*";
@@ -487,7 +489,7 @@ static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) {
   unsigned size = OwnershipArgs.size();
   llvm::array_pod_sort(start, start + size);
   switch (K) {
-  case OwnershipAttr::Takes: {
+  case attr::OwnershipTakes: {
     if (OwnershipArgs.empty()) {
       S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << 2;
       return;
@@ -496,7 +498,7 @@ static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) {
                                                     Module));
     break;
   }
-  case OwnershipAttr::Holds: {
+  case attr::OwnershipHolds: {
     if (OwnershipArgs.empty()) {
       S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << 2;
       return;
@@ -505,13 +507,13 @@ static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) {
                                                     Module));
     break;
   }
-  case OwnershipAttr::Returns: {
+  case attr::OwnershipReturns: {
     d->addAttr(::new (S.Context) OwnershipReturnsAttr(S.Context, start, size,
                                                       Module));
     break;
   }
   default:
-    break;
+    llvm_unreachable("Unknown ownership attribute");
   }
 }