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

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL288917: [analyzer] Remove an unused enum value in 
RetainCountChecker. (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D27408?vs=80431=80608#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27408

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -740,7 +740,7 @@
  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)
@@ -1086,7 +1086,7 @@
FName == "IOOpenFirmwarePathMatching") {
   // Part of . (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") {
@@ -1116,7 +1116,7 @@
   // 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: 
@@ -1292,7 +1292,7 @@
 RetainSummaryManager::getCFSummaryCreateRule(const FunctionDecl *FD) {
   assert (ScratchArgs.isEmpty());
 
-  return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF, true));
+  return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF));
 }
 
 const RetainSummary *
@@ -1322,7 +1322,7 @@
   }
 
   if (D->hasAttr())
-return RetEffect::MakeOwned(RetEffect::CF, true);
+return RetEffect::MakeOwned(RetEffect::CF);
 
   if (D->hasAttr())
 return RetEffect::MakeNotOwned(RetEffect::CF);
@@ -1435,7 +1435,7 @@
   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);
@@ -1457,7 +1457,7 @@
   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;
@@ -1588,7 +1588,7 @@
   // 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();
@@ -3076,7 +3076,6 @@
   // No work necessary.
   break;
 
-case RetEffect::OwnedAllocatedSymbol:
 case RetEffect::OwnedSymbol: {
   SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol();
   if (!Sym)
Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
@@ -120,9 +120,6 @@
 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.
@@ -163,8 +160,7 @@
   ObjKind getObjKind() const { return O; }
   
   bool isOwned() const {
-return K == OwnedSymbol || K == OwnedAllocatedSymbol ||
-K == OwnedWhenTrackedReceiver;
+return K == OwnedSymbol || K == 

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

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

LGTM.


https://reviews.llvm.org/D27408



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 80431.
NoQ added a comment.

Remove OwnedAllocatedSymbol instead.


https://reviews.llvm.org/D27408

Files:
  include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
  lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -740,7 +740,7 @@
  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)
@@ -1086,7 +1086,7 @@
FName == "IOOpenFirmwarePathMatching") {
   // Part of . (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") {
@@ -1116,7 +1116,7 @@
   // 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: 
@@ -1292,7 +1292,7 @@
 RetainSummaryManager::getCFSummaryCreateRule(const FunctionDecl *FD) {
   assert (ScratchArgs.isEmpty());
 
-  return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF, true));
+  return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF));
 }
 
 const RetainSummary *
@@ -1322,7 +1322,7 @@
   }
 
   if (D->hasAttr())
-return RetEffect::MakeOwned(RetEffect::CF, true);
+return RetEffect::MakeOwned(RetEffect::CF);
 
   if (D->hasAttr())
 return RetEffect::MakeNotOwned(RetEffect::CF);
@@ -1435,7 +1435,7 @@
   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);
@@ -1457,7 +1457,7 @@
   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;
@@ -1588,7 +1588,7 @@
   // 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();
@@ -3076,7 +3076,6 @@
   // No work necessary.
   break;
 
-case RetEffect::OwnedAllocatedSymbol:
 case RetEffect::OwnedSymbol: {
   SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol();
   if (!Sym)
Index: include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
===
--- include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
+++ include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
@@ -120,9 +120,6 @@
 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.
@@ -163,8 +160,7 @@
   ObjKind getObjKind() const { return O; }
   
   bool isOwned() const {
-return K == OwnedSymbol || K == OwnedAllocatedSymbol ||
-K == OwnedWhenTrackedReceiver;
+return K == OwnedSymbol || K == OwnedWhenTrackedReceiver;
   }
   
   bool notOwned() const {
@@ -179,8 +175,8 @@
 return RetEffect(OwnedWhenTrackedReceiver, ObjC);
   }
   
-  static RetEffect MakeOwned(ObjKind o, bool isAllocated = false) {
-return RetEffect(isAllocated ? OwnedAllocatedSymbol 

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

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added inline comments.



Comment at: include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h:179
   
-  static RetEffect MakeOwned(ObjKind o, bool isAllocated = false) {
-return RetEffect(isAllocated ? OwnedAllocatedSymbol : OwnedSymbol, o);
+  static RetEffect MakeOwned(ObjKind o) {
+return RetEffect(OwnedAllocatedSymbol, o);

zaks.anna wrote:
> Should we rename into MakeAllocatedOwned()?
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.


https://reviews.llvm.org/D27408



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.

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.




Comment at: include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h:179
   
-  static RetEffect MakeOwned(ObjKind o, bool isAllocated = false) {
-return RetEffect(isAllocated ? OwnedAllocatedSymbol : OwnedSymbol, o);
+  static RetEffect MakeOwned(ObjKind o) {
+return RetEffect(OwnedAllocatedSymbol, o);

Should we rename into MakeAllocatedOwned()?


https://reviews.llvm.org/D27408



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin.
NoQ added a subscriber: cfe-commits.

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


https://reviews.llvm.org/D27408

Files:
  include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
  lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -740,7 +740,7 @@
  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)
@@ -1086,7 +1086,7 @@
FName == "IOOpenFirmwarePathMatching") {
   // Part of . (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") {
@@ -1116,7 +1116,7 @@
   // 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: 
@@ -1284,7 +1284,7 @@
 RetainSummaryManager::getCFSummaryCreateRule(const FunctionDecl *FD) {
   assert (ScratchArgs.isEmpty());
 
-  return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF, true));
+  return getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF));
 }
 
 const RetainSummary *
@@ -1314,7 +1314,7 @@
   }
 
   if (D->hasAttr())
-return RetEffect::MakeOwned(RetEffect::CF, true);
+return RetEffect::MakeOwned(RetEffect::CF);
 
   if (D->hasAttr())
 return RetEffect::MakeNotOwned(RetEffect::CF);
@@ -1427,7 +1427,7 @@
   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);
@@ -1449,7 +1449,7 @@
   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;
@@ -1580,7 +1580,7 @@
   // 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();
@@ -3067,8 +3067,7 @@
   // No work necessary.
   break;
 
-case RetEffect::OwnedAllocatedSymbol:
-case RetEffect::OwnedSymbol: {
+case RetEffect::OwnedAllocatedSymbol: {
   SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol();
   if (!Sym)
 break;
Index: include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
===
--- include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
+++ include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
@@ -118,8 +118,6 @@
 /// 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,
@@ -163,8 +161,7 @@
   ObjKind getObjKind() const { return O; }
   
   bool isOwned() const {
-return K == OwnedSymbol || K == OwnedAllocatedSymbol ||
-K == OwnedWhenTrackedReceiver;
+return K == OwnedAllocatedSymbol || K == OwnedWhenTrackedReceiver;
   }
   
   bool notOwned() const {
@@ -179,8 +176,8 @@
 return