[analyzer] RetainCountChecker: remove unused enum value; NFC.

[NoQ]

Summary

OwnedSymbol is not used anywhere; OwnedAllocatedSymbol is used instead. I couldn't grasp the intended difference between the two, maybe we should remove it?

[zaks.anna]

I suspect allocated owned means that the analyzer saw the allocation site.

Removing dead code is great! Thanks. This looks good to me other than the name of the method that I commented about.

include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h

179 ↗

(On Diff #80257)

Should we rename into MakeAllocatedOwned()?

[dcoughlin]

include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h

179 ↗

(On Diff #80257)

I think the current use of 'OwnedAllocatedSymbol' more closely matches what is conveyed by the name and documentation of 'OwnedSymbol', so it might be better to remove 'OwnedSymbol' (as Artem has done) and then immediately rename all occurrences of 'OwnedAllocatedSymbol' to 'OwnedSymbol'. There are plenty of places where the analyzer doesn't see the allocation site and yet yields 'OwnedAllocatedSymbol', so I don't think the current implementation matches its name.

[NoQ]

Remove OwnedAllocatedSymbol instead.

[zaks.anna]

LGTM.

cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h

 class RetEffect {
 public:
   enum Kind {
     /// Indicates that no retain count information is tracked for
     /// the return value.
     NoRet,
     /// Indicates that the returned value is an owned (+1) symbol.
     OwnedSymbol,
-    /// Indicates that the returned value is an owned (+1) symbol and
-    /// that it should be treated as freshly allocated.
-    OwnedAllocatedSymbol,
     /// Indicates that the returned value is an object with retain count
     /// semantics but that it is not owned (+0).  This is the default
     /// for getters, etc.
     NotOwnedSymbol,
     /// Indicates that the object is not owned and controlled by the
     /// Garbage collector.
     GCNotOwnedSymbol,
     /// Indicates that the return value is an owned object when the
private:
   RetEffect(Kind k, ObjKind o = AnyObj) : K(k), O(o) {}
   
 public:
   Kind getKind() const { return K; }
   
   ObjKind getObjKind() const { return O; }
   
   bool isOwned() const {
-    return K == OwnedSymbol || K == OwnedAllocatedSymbol ||
-    K == OwnedWhenTrackedReceiver;
+    return K == OwnedSymbol || K == OwnedWhenTrackedReceiver;
   }
   
   bool notOwned() const {
     return K == NotOwnedSymbol;
   }
   
   bool operator==(const RetEffect &Other) const {
     return K == Other.K && O == Other.O;
   }
   
   static RetEffect MakeOwnedWhenTrackedReceiver() {
     return RetEffect(OwnedWhenTrackedReceiver, ObjC);
   }
   
-  static RetEffect MakeOwned(ObjKind o, bool isAllocated = false) {
-    return RetEffect(isAllocated ? OwnedAllocatedSymbol : OwnedSymbol, o);
+  static RetEffect MakeOwned(ObjKind o) {
+    return RetEffect(OwnedSymbol, o);
   }
   static RetEffect MakeNotOwned(ObjKind o) {
     return RetEffect(NotOwnedSymbol, o);
   }
   static RetEffect MakeGCNotOwned() {
     return RetEffect(GCNotOwnedSymbol, ObjC);
   }
   static RetEffect MakeNoRet() {

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

public:
   RetainSummaryManager(ASTContext &ctx, bool gcenabled, bool usesARC)
    : Ctx(ctx),
      GCEnabled(gcenabled),
      ARCEnabled(usesARC),
      AF(BPAlloc), ScratchArgs(AF.getEmptyMap()),
      ObjCAllocRetE(gcenabled
                     ? RetEffect::MakeGCNotOwned()
                     : (usesARC ? RetEffect::MakeNotOwned(RetEffect::ObjC)
-                               : RetEffect::MakeOwned(RetEffect::ObjC, true))),
+                               : RetEffect::MakeOwned(RetEffect::ObjC))),
      ObjCInitRetE(gcenabled
                     ? RetEffect::MakeGCNotOwned()
                     : (usesARC ? RetEffect::MakeNotOwned(RetEffect::ObjC)
                                : RetEffect::MakeOwnedWhenTrackedReceiver())) {
     InitializeClassMethodSummaries();
     InitializeMethodSummaries();
   }
 
do {
     } else if (FName == "IOBSDNameMatching" ||
                FName == "IOServiceMatching" ||
                FName == "IOServiceNameMatching" ||
                FName == "IORegistryEntrySearchCFProperty" ||
                FName == "IORegistryEntryIDMatching" ||
                FName == "IOOpenFirmwarePathMatching") {
       // Part of <rdar://problem/6961230>. (IOKit)
       // This should be addressed using a API table.
-      S = getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF, true),
+      S = getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF),
                                DoNothing, DoNothing);
     } else if (FName == "IOServiceGetMatchingService" ||
                FName == "IOServiceGetMatchingServices") {
       // FIXES: <rdar://problem/6326900>
       // This should be addressed using a API table.  This strcmp is also
       // a little gross, but there is no need to super optimize here.
       ScratchArgs = AF.add(ScratchArgs, 1, DecRef);
       S = getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing);
if (FName == "pthread_create" || FName == "pthread_setspecific") {
       ScratchArgs = AF.add(ScratchArgs, 7, StopTracking);
       S = getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing);
     } else if (FName == "CGBitmapContextCreateWithData") {
       // FIXES: <rdar://problem/7358899>
       // Eventually this can be improved by recognizing that 'releaseInfo'
       // passed to CGBitmapContextCreateWithData is released via
       // a callback and doing full IPA to make sure this is done correctly.
       ScratchArgs = AF.add(ScratchArgs, 8, StopTracking);
-      S = getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF, true),
+      S = getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF),
                                DoNothing, DoNothing);
     } else if (FName == "CVPixelBufferCreateWithPlanarBytes") {
       // FIXES: <rdar://problem/7283567>
       // Eventually this can be improved by recognizing that the pixel
       // buffer passed to CVPixelBufferCreateWithPlanarBytes is released
       // via a callback and doing full IPA to make sure this is done
       // correctly.
       ScratchArgs = AF.add(ScratchArgs, 12, StopTracking);
RetainSummaryManager::getUnarySummary(const FunctionType* FT,
   ScratchArgs = AF.add(ScratchArgs, 0, Effect);
   return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing);
 }
 
 const RetainSummary *
 RetainSummaryManager::getCFSummaryCreateRule(const FunctionDecl *FD) {
   assert (ScratchArgs.isEmpty());
 
-  return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF, true));
+  return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF));
 }
 
 const RetainSummary *
 RetainSummaryManager::getCFSummaryGetRule(const FunctionDecl *FD) {
   assert (ScratchArgs.isEmpty());
   return getPersistentSummary(RetEffect::MakeNotOwned(RetEffect::CF),
                               DoNothing, DoNothing);
 }
if (D->hasAttr<NSReturnsNotRetainedAttr>() ||
         D->hasAttr<NSReturnsAutoreleasedAttr>())
       return RetEffect::MakeNotOwned(RetEffect::ObjC);
 
   } else if (!RetTy->isPointerType()) {
     return None;
   }
 
   if (D->hasAttr<CFReturnsRetainedAttr>())
-    return RetEffect::MakeOwned(RetEffect::CF, true);
+    return RetEffect::MakeOwned(RetEffect::CF);
 
   if (D->hasAttr<CFReturnsNotRetainedAttr>())
     return RetEffect::MakeNotOwned(RetEffect::CF);
 
   return None;
 }
 
 void
case OMF_performSelector:
         // double-check with the selector. This is ugly, but there aren't that
         // many Objective-C methods that return CF objects, right?
         if (MD) {
           switch (S.getMethodFamily()) {
           case OMF_alloc:
           case OMF_new:
           case OMF_copy:
           case OMF_mutableCopy:
-            ResultEff = RetEffect::MakeOwned(RetEffect::CF, true);
+            ResultEff = RetEffect::MakeOwned(RetEffect::CF);
             break;
           default:
             ResultEff = RetEffect::MakeNotOwned(RetEffect::CF);
             break;
           }
         } else {
           ResultEff = RetEffect::MakeNotOwned(RetEffect::CF);
         }
       }
       break;
     case OMF_init:
       ResultEff = ObjCInitRetE;
       ReceiverEff = DecRefMsg;
       break;
     case OMF_alloc:
     case OMF_new:
     case OMF_copy:
     case OMF_mutableCopy:
       if (cocoa::isCocoaObjectRef(RetTy))
         ResultEff = ObjCAllocRetE;
       else if (coreFoundation::isCFObjectRef(RetTy))
-        ResultEff = RetEffect::MakeOwned(RetEffect::CF, true);
+        ResultEff = RetEffect::MakeOwned(RetEffect::CF);
       break;
     case OMF_autorelease:
       ReceiverEff = Autorelease;
       break;
     case OMF_retain:
       ReceiverEff = IncRefMsg;
       break;
     case OMF_release:
void RetainSummaryManager::InitializeMethodSummaries() {
   // awakeAfterUsingCoder: behaves basically like an 'init' method.  It
   // claims the receiver and returns a retained object.
   addNSObjectMethSummary(GetUnarySelector("awakeAfterUsingCoder", Ctx),
                          InitSumm);
 
   // The next methods are allocators.
   const RetainSummary *AllocSumm = getPersistentSummary(ObjCAllocRetE);
   const RetainSummary *CFAllocSumm =
-    getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF, true));
+    getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF));
 
   // Create the "retain" selector.
   RetEffect NoRet = RetEffect::MakeNoRet();
   const RetainSummary *Summ = getPersistentSummary(NoRet, IncRefMsg);
   addNSObjectMethSummary(GetNullarySelector("retain", Ctx), Summ);
 
   // Create the "release" selector.
   Summ = getPersistentSummary(NoRet, DecRefMsg);
switch (RE.getKind()) {
     default:
       llvm_unreachable("Unhandled RetEffect.");
 
     case RetEffect::NoRet:
     case RetEffect::NoRetHard:
       // No work necessary.
       break;
 
-    case RetEffect::OwnedAllocatedSymbol:
     case RetEffect::OwnedSymbol: {
       SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol();
       if (!Sym)
         break;
 
       // Use the result type from the CallEvent as it automatically adjusts
       // for methods/functions that return references.
       QualType ResultTy = CallOrMsg.getResultType();