[analyzer] [RetainCountChecker] [NFC] Remove redundant enum items *Msg, as the object type is already communicated by a separate field

Differential Revision: https://reviews.llvm.org/D56070

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

5 files changed, 69 insertions, 82 deletions

diff --git a/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h b/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
index 5461716f9d..099b20f611 100644
--- a/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
+++ b/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
@@ -68,21 +68,11 @@ enum ArgEffectKind {
   /// if CFRelease has been called on the argument.
   DecRef,
 
-  /// The argument has its reference count decreased by 1.  This is as
-  /// if a -release message has been sent to the argument.  This differs
-  /// in behavior from DecRef when ARC is enabled.
-  DecRefMsg,
-
   /// The argument has its reference count decreased by 1 to model
   /// a transferred bridge cast under ARC.
   DecRefBridgedTransferred,
 
   /// The argument has its reference count increased by 1.  This is as
-  /// if a -retain message has been sent to the argument.  This differs
-  /// in behavior from IncRef when ARC is enabled.
-  IncRefMsg,
-
-  /// The argument has its reference count increased by 1.  This is as
   /// if CFRetain has been called on the argument.
   IncRef,
 
@@ -122,13 +112,6 @@ enum ArgEffectKind {
   /// count of the argument and all typestate tracking on that argument
   /// should cease.
   DecRefAndStopTrackingHard,
-
-  /// Performs the combined functionality of DecRefMsg and StopTrackingHard.
-  ///
-  /// The models the effect that the called function decrements the reference
-  /// count of the argument and all typestate tracking on that argument
-  /// should cease.
-  DecRefMsgAndStopTrackingHard
 };
 
 /// An ArgEffect summarizes the retain count behavior on an argument or receiver
diff --git a/lib/ARCMigrate/ObjCMT.cpp b/lib/ARCMigrate/ObjCMT.cpp
index c3f849b7fa..6950ce0e12 100644
--- a/lib/ARCMigrate/ObjCMT.cpp
+++ b/lib/ARCMigrate/ObjCMT.cpp
@@ -1484,14 +1484,15 @@ void ObjCMigrateASTConsumer::AddCFAnnotations(ASTContext &Ctx,
        pe = FuncDecl->param_end(); pi != pe; ++pi, ++i) {
     const ParmVarDecl *pd = *pi;
     ArgEffect AE = AEArgs[i];
-    if (AE.getKind() == DecRef && !pd->hasAttr<CFConsumedAttr>() &&
+    if (AE.getKind() == DecRef && AE.getObjKind() == ObjKind::CF &&
+        !pd->hasAttr<CFConsumedAttr>() &&
         NSAPIObj->isMacroDefined("CF_CONSUMED")) {
       edit::Commit commit(*Editor);
       commit.insertBefore(pd->getLocation(), "CF_CONSUMED ");
       Editor->commit(commit);
-    }
-    else if (AE.getKind() == DecRefMsg && !pd->hasAttr<NSConsumedAttr>() &&
-             NSAPIObj->isMacroDefined("NS_CONSUMED")) {
+    } else if (AE.getKind() == DecRef && AE.getObjKind() == ObjKind::ObjC &&
+               !pd->hasAttr<NSConsumedAttr>() &&
+               NSAPIObj->isMacroDefined("NS_CONSUMED")) {
       edit::Commit commit(*Editor);
       commit.insertBefore(pd->getLocation(), "NS_CONSUMED ");
       Editor->commit(commit);
@@ -1536,8 +1537,8 @@ ObjCMigrateASTConsumer::CF_BRIDGING_KIND
        pe = FuncDecl->param_end(); pi != pe; ++pi, ++i) {
     const ParmVarDecl *pd = *pi;
     ArgEffect AE = AEArgs[i];
-    if (AE.getKind() == DecRef /*CFConsumed annotated*/ ||
-        AE.getKind() == IncRef) {
+    if ((AE.getKind() == DecRef /*CFConsumed annotated*/ ||
+         AE.getKind() == IncRef) && AE.getObjKind() == ObjKind::CF) {
       if (AE.getKind() == DecRef && !pd->hasAttr<CFConsumedAttr>())
         ArgCFAudited = true;
       else if (AE.getKind() == IncRef)
@@ -1610,7 +1611,9 @@ void ObjCMigrateASTConsumer::AddCFAnnotations(ASTContext &Ctx,
        pe = MethodDecl->param_end(); pi != pe; ++pi, ++i) {
     const ParmVarDecl *pd = *pi;
     ArgEffect AE = AEArgs[i];
-    if (AE.getKind() == DecRef && !pd->hasAttr<CFConsumedAttr>() &&
+    if (AE.getKind() == DecRef
+        && AE.getObjKind() == ObjKind::CF
+        && !pd->hasAttr<CFConsumedAttr>() &&
         NSAPIObj->isMacroDefined("CF_CONSUMED")) {
       edit::Commit commit(*Editor);
       commit.insertBefore(pd->getLocation(), "CF_CONSUMED ");
@@ -1633,7 +1636,7 @@ void ObjCMigrateASTConsumer::migrateAddMethodAnnotation(
        MethodDecl->hasAttr<NSReturnsNotRetainedAttr>() ||
        MethodDecl->hasAttr<NSReturnsAutoreleasedAttr>());
 
-  if (CE.getReceiver().getKind() == DecRefMsg &&
+  if (CE.getReceiver().getKind() == DecRef &&
       !MethodDecl->hasAttr<NSConsumesSelfAttr>() &&
       MethodDecl->getMethodFamily() != OMF_init &&
       MethodDecl->getMethodFamily() != OMF_release &&
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index 4e5fac815e..73de9f5b50 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -248,17 +248,17 @@ void RetainCountChecker::checkPostStmt(const CastExpr *CE,
   if (!BE)
     return;
 
-  ArgEffectKind AE = IncRef;
+  ArgEffect AE = ArgEffect(IncRef, ObjKind::ObjC);
 
   switch (BE->getBridgeKind()) {
     case OBC_Bridge:
       // Do nothing.
       return;
     case OBC_BridgeRetained:
-      AE = IncRef;
+      AE = AE.withKind(IncRef);
       break;
     case OBC_BridgeTransfer:
-      AE = DecRefBridgedTransferred;
+      AE = AE.withKind(DecRefBridgedTransferred);
       break;
   }
 
@@ -290,7 +290,8 @@ void RetainCountChecker::processObjCLiterals(CheckerContext &C,
     if (SymbolRef sym = V.getAsSymbol())
       if (const RefVal* T = getRefBinding(state, sym)) {
         RefVal::Kind hasErr = (RefVal::Kind) 0;
-        state = updateSymbol(state, sym, *T, MayEscape, hasErr, C);
+        state = updateSymbol(state, sym, *T,
+                             ArgEffect(MayEscape, ObjKind::ObjC), hasErr, C);
         if (hasErr) {
           processNonLeakError(state, Child->getSourceRange(), hasErr, sym, C);
           return;
@@ -512,7 +513,7 @@ static bool isPointerToObject(QualType QT) {
 
 /// Whether the tracked value should be escaped on a given call.
 /// OSObjects are escaped when passed to void * / etc.
-static bool shouldEscapeArgumentOnCall(const CallEvent &CE, unsigned ArgIdx,
+static bool shouldEscapeOSArgumentOnCall(const CallEvent &CE, unsigned ArgIdx,
                                        const RefVal *TrackedValue) {
   if (TrackedValue->getObjKind() != ObjKind::OS)
     return false;
@@ -536,7 +537,7 @@ void RetainCountChecker::processSummaryOfInlined(const RetainSummary &Summ,
     if (SymbolRef Sym = V.getAsLocSymbol()) {
       bool ShouldRemoveBinding = Summ.getArg(idx).getKind() == StopTrackingHard;
       if (const RefVal *T = getRefBinding(state, Sym))
-        if (shouldEscapeArgumentOnCall(CallOrMsg, idx, T))
+        if (shouldEscapeOSArgumentOnCall(CallOrMsg, idx, T))
           ShouldRemoveBinding = true;
 
       if (ShouldRemoveBinding)
@@ -611,14 +612,15 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ,
   for (unsigned idx = 0, e = CallOrMsg.getNumArgs(); idx != e; ++idx) {
     SVal V = CallOrMsg.getArgSVal(idx);
 
-    ArgEffectKind Effect = Summ.getArg(idx).getKind();
-    if (Effect == RetainedOutParameter || Effect == UnretainedOutParameter) {
-      state = updateOutParameter(state, V, Effect);
+    ArgEffect Effect = Summ.getArg(idx);
+    if (Effect.getKind() == RetainedOutParameter ||
+        Effect.getKind() == UnretainedOutParameter) {
+      state = updateOutParameter(state, V, Effect.getKind());
     } else if (SymbolRef Sym = V.getAsLocSymbol()) {
       if (const RefVal *T = getRefBinding(state, Sym)) {
 
-        if (shouldEscapeArgumentOnCall(CallOrMsg, idx, T))
-          Effect = StopTrackingHard;
+        if (shouldEscapeOSArgumentOnCall(CallOrMsg, idx, T))
+          Effect = ArgEffect(StopTrackingHard, ObjKind::OS);
 
         state = updateSymbol(state, Sym, *T, Effect, hasErr, C);
         if (hasErr) {
@@ -638,7 +640,7 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ,
         if (const RefVal *T = getRefBinding(state, Sym)) {
           ReceiverIsTracked = true;
           state = updateSymbol(state, Sym, *T,
-                               Summ.getReceiverEffect().getKind(), hasErr, C);
+                               Summ.getReceiverEffect(), hasErr, C);
           if (hasErr) {
             ErrorRange = MsgInvocation->getOriginExpr()->getReceiverRange();
             ErrorSym = Sym;
@@ -648,7 +650,7 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ,
     } else if (const auto *MCall = dyn_cast<CXXMemberCall>(&CallOrMsg)) {
       if (SymbolRef Sym = MCall->getCXXThisVal().getAsLocSymbol()) {
         if (const RefVal *T = getRefBinding(state, Sym)) {
-          state = updateSymbol(state, Sym, *T, Summ.getThisEffect().getKind(),
+          state = updateSymbol(state, Sym, *T, Summ.getThisEffect(),
                                hasErr, C);
           if (hasErr) {
             ErrorRange = MCall->getOriginExpr()->getSourceRange();
@@ -709,25 +711,27 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ,
 
 ProgramStateRef RetainCountChecker::updateSymbol(ProgramStateRef state,
                                                  SymbolRef sym, RefVal V,
-                                                 ArgEffectKind E,
+                                                 ArgEffect AE,
                                                  RefVal::Kind &hasErr,
                                                  CheckerContext &C) const {
   bool IgnoreRetainMsg = (bool)C.getASTContext().getLangOpts().ObjCAutoRefCount;
-  switch (E) {
-  default:
-    break;
-  case IncRefMsg:
-    E = IgnoreRetainMsg ? DoNothing : IncRef;
-    break;
-  case DecRefMsg:
-    E = IgnoreRetainMsg ? DoNothing: DecRef;
-    break;
-  case DecRefMsgAndStopTrackingHard:
-    E = IgnoreRetainMsg ? StopTracking : DecRefAndStopTrackingHard;
-    break;
-  case MakeCollectable:
-    E = DoNothing;
+  if (AE.getObjKind() == ObjKind::ObjC && IgnoreRetainMsg) {
+    switch (AE.getKind()) {
+    default:
+      break;
+    case IncRef:
+      AE = AE.withKind(DoNothing);
+      break;
+    case DecRef:
+      AE = AE.withKind(DoNothing);
+      break;
+    case DecRefAndStopTrackingHard:
+      AE = AE.withKind(StopTracking);
+      break;
+    }
   }
+  if (AE.getKind() == MakeCollectable)
+    AE = AE.withKind(DoNothing);
 
   // Handle all use-after-releases.
   if (V.getKind() == RefVal::Released) {
@@ -736,12 +740,9 @@ ProgramStateRef RetainCountChecker::updateSymbol(ProgramStateRef state,
     return setRefBinding(state, sym, V);
   }
 
-  switch (E) {
-    case DecRefMsg:
-    case IncRefMsg:
+  switch (AE.getKind()) {
     case MakeCollectable:
-    case DecRefMsgAndStopTrackingHard:
-      llvm_unreachable("DecRefMsg/IncRefMsg/MakeCollectable already converted");
+      llvm_unreachable("MakeCollectable already converted");
 
     case UnretainedOutParameter:
     case RetainedOutParameter:
@@ -806,13 +807,13 @@ ProgramStateRef RetainCountChecker::updateSymbol(ProgramStateRef state,
         case RefVal::Owned:
           assert(V.getCount() > 0);
           if (V.getCount() == 1) {
-            if (E == DecRefBridgedTransferred ||
+            if (AE.getKind() == DecRefBridgedTransferred ||
                 V.getIvarAccessHistory() ==
                   RefVal::IvarAccessHistory::AccessedDirectly)
               V = V ^ RefVal::NotOwned;
             else
               V = V ^ RefVal::Released;
-          } else if (E == DecRefAndStopTrackingHard) {
+          } else if (AE.getKind() == DecRefAndStopTrackingHard) {
             return removeRefBinding(state, sym);
           }
 
@@ -821,14 +822,14 @@ ProgramStateRef RetainCountChecker::updateSymbol(ProgramStateRef state,
 
         case RefVal::NotOwned:
           if (V.getCount() > 0) {
-            if (E == DecRefAndStopTrackingHard)
+            if (AE.getKind() == DecRefAndStopTrackingHard)
               return removeRefBinding(state, sym);
             V = V - 1;
           } else if (V.getIvarAccessHistory() ==
                        RefVal::IvarAccessHistory::AccessedDirectly) {
             // Assume that the instance variable was holding on the object at
             // +1, and we just didn't know.
-            if (E == DecRefAndStopTrackingHard)
+            if (AE.getKind() == DecRefAndStopTrackingHard)
               return removeRefBinding(state, sym);
             V = V.releaseViaIvar() ^ RefVal::Released;
           } else {
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
index 80d1b0573c..d8bbe6fcdd 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
@@ -347,7 +347,7 @@ public:
   void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
 
   ProgramStateRef updateSymbol(ProgramStateRef state, SymbolRef sym,
-                               RefVal V, ArgEffectKind E, RefVal::Kind &hasErr,
+                               RefVal V, ArgEffect E, RefVal::Kind &hasErr,
                                CheckerContext &C) const;
 
   void processNonLeakError(ProgramStateRef St, SourceRange ErrorRange,
diff --git a/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp b/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
index 9f0e60ec3e..20dc900e25 100644
--- a/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
+++ b/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
@@ -457,7 +457,6 @@ static ArgEffect getStopTrackingHardEquivalent(ArgEffect E) {
   case Autorelease:
   case DecRefBridgedTransferred:
   case IncRef:
-  case IncRefMsg:
   case MakeCollectable:
   case UnretainedOutParameter:
   case RetainedOutParameter:
@@ -468,9 +467,6 @@ static ArgEffect getStopTrackingHardEquivalent(ArgEffect E) {
   case DecRef:
   case DecRefAndStopTrackingHard:
     return E.withKind(DecRefAndStopTrackingHard);
-  case DecRefMsg:
-  case DecRefMsgAndStopTrackingHard:
-    return E.withKind(DecRefMsgAndStopTrackingHard);
   case Dealloc:
     return E.withKind(Dealloc);
   }
@@ -649,6 +645,8 @@ RetainSummaryManager::canEval(const CallExpr *CE, const FunctionDecl *FD,
   return None;
 }
 
+// TODO: UnaryFuncKind is a very funny enum, it really should not exist:
+// just pass the needed effect directly!
 const RetainSummary *
 RetainSummaryManager::getUnarySummary(const FunctionType* FT,
                                       UnaryFuncKind func) {
@@ -662,7 +660,7 @@ RetainSummaryManager::getUnarySummary(const FunctionType* FT,
   if (!FTP || FTP->getNumParams() != 1)
     return getPersistentStopSummary();
 
-  ArgEffect Effect;
+  ArgEffect Effect(DoNothing, ObjKind::CF);
   switch (func) {
   case cfretain: Effect = Effect.withKind(IncRef); break;
   case cfrelease: Effect = Effect.withKind(DecRef); break;
@@ -778,7 +776,7 @@ bool RetainSummaryManager::applyFunctionParamAnnotationEffect(
     const ParmVarDecl *pd, unsigned parm_idx, const FunctionDecl *FD,
     RetainSummaryTemplate &Template) {
   if (hasEnabledAttr<NSConsumedAttr>(pd)) {
-    Template->addArg(AF, parm_idx, ArgEffect(DecRefMsg, ObjKind::ObjC));
+    Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::ObjC));
     return true;
   } else if (hasEnabledAttr<CFConsumedAttr>(pd)) {
     Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::CF));
@@ -857,7 +855,7 @@ RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ,
 
   // Effects on the receiver.
   if (MD->hasAttr<NSConsumesSelfAttr>())
-    Template->setReceiverEffect(ArgEffect(DecRefMsg, ObjKind::ObjC));
+    Template->setReceiverEffect(ArgEffect(DecRef, ObjKind::ObjC));
 
   // Effects on the parameters.
   unsigned parm_idx = 0;
@@ -865,7 +863,7 @@ RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ,
        pi != pe; ++pi, ++parm_idx) {
     const ParmVarDecl *pd = *pi;
     if (pd->hasAttr<NSConsumedAttr>()) {
-      Template->addArg(AF, parm_idx, ArgEffect(DecRefMsg, ObjKind::ObjC));
+      Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::ObjC));
     } else if (pd->hasAttr<CFConsumedAttr>()) {
       Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::CF));
     } else if (pd->hasAttr<OSConsumedAttr>()) {
@@ -931,7 +929,7 @@ RetainSummaryManager::getStandardMethodSummary(const ObjCMethodDecl *MD,
       break;
     case OMF_init:
       ResultEff = ObjCInitRetE;
-      ReceiverEff = ArgEffect(DecRefMsg, ObjKind::ObjC);
+      ReceiverEff = ArgEffect(DecRef, ObjKind::ObjC);
       break;
     case OMF_alloc:
     case OMF_new:
@@ -946,10 +944,10 @@ RetainSummaryManager::getStandardMethodSummary(const ObjCMethodDecl *MD,
       ReceiverEff = ArgEffect(Autorelease, ObjKind::ObjC);
       break;
     case OMF_retain:
-      ReceiverEff = ArgEffect(IncRefMsg, ObjKind::ObjC);
+      ReceiverEff = ArgEffect(IncRef, ObjKind::ObjC);
       break;
     case OMF_release:
-      ReceiverEff = ArgEffect(DecRefMsg, ObjKind::ObjC);
+      ReceiverEff = ArgEffect(DecRef, ObjKind::ObjC);
       break;
     case OMF_dealloc:
       ReceiverEff = ArgEffect(Dealloc, ObjKind::ObjC);
@@ -1062,9 +1060,8 @@ void RetainSummaryManager::InitializeMethodSummaries() {
   ArgEffects ScratchArgs = AF.getEmptyMap();
   // Create the "init" selector.  It just acts as a pass-through for the
   // receiver.
-  const RetainSummary *InitSumm = getPersistentSummary(ObjCInitRetE,
-                                                       ScratchArgs,
-                                                       ArgEffect(DecRefMsg));
+  const RetainSummary *InitSumm = getPersistentSummary(
+      ObjCInitRetE, ScratchArgs, ArgEffect(DecRef, ObjKind::ObjC));
   addNSObjectMethSummary(GetNullarySelector("init", Ctx), InitSumm);
 
   // awakeAfterUsingCoder: behaves basically like an 'init' method.  It
@@ -1080,20 +1077,23 @@ void RetainSummaryManager::InitializeMethodSummaries() {
 
   // Create the "retain" selector.
   RetEffect NoRet = RetEffect::MakeNoRet();
-  const RetainSummary *Summ =
-      getPersistentSummary(NoRet, ScratchArgs, ArgEffect(IncRefMsg));
+  const RetainSummary *Summ = getPersistentSummary(
+      NoRet, ScratchArgs, ArgEffect(IncRef, ObjKind::ObjC));
   addNSObjectMethSummary(GetNullarySelector("retain", Ctx), Summ);
 
   // Create the "release" selector.
-  Summ = getPersistentSummary(NoRet, ScratchArgs, ArgEffect(DecRefMsg));
+  Summ = getPersistentSummary(NoRet, ScratchArgs,
+                              ArgEffect(DecRef, ObjKind::ObjC));
   addNSObjectMethSummary(GetNullarySelector("release", Ctx), Summ);
 
   // Create the -dealloc summary.
-  Summ = getPersistentSummary(NoRet, ScratchArgs, ArgEffect(Dealloc));
+  Summ = getPersistentSummary(NoRet, ScratchArgs, ArgEffect(Dealloc,
+                                                            ObjKind::ObjC));
   addNSObjectMethSummary(GetNullarySelector("dealloc", Ctx), Summ);
 
   // Create the "autorelease" selector.
-  Summ = getPersistentSummary(NoRet, ScratchArgs, ArgEffect(Autorelease));
+  Summ = getPersistentSummary(NoRet, ScratchArgs, ArgEffect(Autorelease,
+                                                            ObjKind::ObjC));
   addNSObjectMethSummary(GetNullarySelector("autorelease", Ctx), Summ);
 
   // For NSWindow, allocated objects are (initially) self-owned.