r352533 - [analyzer] [RetainSummaryManager] [NFC] Split one function into two, as it's really doing two things

George Karpenkov via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 29 11:29:46 PST 2019


Author: george.karpenkov
Date: Tue Jan 29 11:29:45 2019
New Revision: 352533

URL: http://llvm.org/viewvc/llvm-project?rev=352533&view=rev
Log:
[analyzer] [RetainSummaryManager] [NFC] Split one function into two, as it's really doing two things

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

Modified:
    cfe/trunk/include/clang/Analysis/RetainSummaryManager.h
    cfe/trunk/lib/Analysis/RetainSummaryManager.cpp

Modified: cfe/trunk/include/clang/Analysis/RetainSummaryManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/RetainSummaryManager.h?rev=352533&r1=352532&r2=352533&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/RetainSummaryManager.h (original)
+++ cfe/trunk/include/clang/Analysis/RetainSummaryManager.h Tue Jan 29 11:29:45 2019
@@ -693,10 +693,22 @@ private:
   void updateSummaryFromAnnotations(const RetainSummary *&Summ,
                                     const FunctionDecl *FD);
 
-  void updateSummaryForCall(const RetainSummary *&Summ,
-                            AnyCall C,
-                            bool HasNonZeroCallbackArg,
-                            bool IsReceiverUnconsumedSelf);
+  const RetainSummary *updateSummaryForNonZeroCallbackArg(const RetainSummary *S,
+                                                          AnyCall &C);
+
+  /// Special case '[super init];' and '[self init];'
+  ///
+  /// Even though calling '[super init]' without assigning the result to self
+  /// and checking if the parent returns 'nil' is a bad pattern, it is common.
+  /// Additionally, our Self Init checker already warns about it. To avoid
+  /// overwhelming the user with messages from both checkers, we model the case
+  /// of '[super init]' in cases when it is not consumed by another expression
+  /// as if the call preserves the value of 'self'; essentially, assuming it can
+  /// never fail and return 'nil'.
+  /// Note, we don't want to just stop tracking the value since we want the
+  /// RetainCount checker to report leaks and use-after-free if SelfInit checker
+  /// is turned off.
+  void updateSummaryForReceiverUnconsumedSelf(const RetainSummary *&S);
 
   /// Determine whether a declaration {@code D} of correspondent type (return
   /// type for functions/methods) {@code QT} has any of the given attributes,

Modified: cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RetainSummaryManager.cpp?rev=352533&r1=352532&r2=352533&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/Analysis/RetainSummaryManager.cpp Tue Jan 29 11:29:45 2019
@@ -557,64 +557,47 @@ static ArgEffect getStopTrackingHardEqui
   llvm_unreachable("Unknown ArgEffect kind");
 }
 
-void RetainSummaryManager::updateSummaryForCall(const RetainSummary *&S,
-                                                AnyCall C,
-                                                bool HasNonZeroCallbackArg,
-                                                bool IsReceiverUnconsumedSelf) {
-
-  if (HasNonZeroCallbackArg) {
-    ArgEffect RecEffect =
-      getStopTrackingHardEquivalent(S->getReceiverEffect());
-    ArgEffect DefEffect =
-      getStopTrackingHardEquivalent(S->getDefaultArgEffect());
-
-    ArgEffects ScratchArgs(AF.getEmptyMap());
-    ArgEffects CustomArgEffects = S->getArgEffects();
-    for (ArgEffects::iterator I = CustomArgEffects.begin(),
-                              E = CustomArgEffects.end();
-         I != E; ++I) {
-      ArgEffect Translated = getStopTrackingHardEquivalent(I->second);
-      if (Translated.getKind() != DefEffect.getKind())
-        ScratchArgs = AF.add(ScratchArgs, I->first, Translated);
-    }
-
-    RetEffect RE = RetEffect::MakeNoRetHard();
+const RetainSummary *
+RetainSummaryManager::updateSummaryForNonZeroCallbackArg(const RetainSummary *S,
+                                                         AnyCall &C) {
+  ArgEffect RecEffect = getStopTrackingHardEquivalent(S->getReceiverEffect());
+  ArgEffect DefEffect = getStopTrackingHardEquivalent(S->getDefaultArgEffect());
+
+  ArgEffects ScratchArgs(AF.getEmptyMap());
+  ArgEffects CustomArgEffects = S->getArgEffects();
+  for (ArgEffects::iterator I = CustomArgEffects.begin(),
+                            E = CustomArgEffects.end();
+       I != E; ++I) {
+    ArgEffect Translated = getStopTrackingHardEquivalent(I->second);
+    if (Translated.getKind() != DefEffect.getKind())
+      ScratchArgs = AF.add(ScratchArgs, I->first, Translated);
+  }
 
-    // Special cases where the callback argument CANNOT free the return value.
-    // This can generally only happen if we know that the callback will only be
-    // called when the return value is already being deallocated.
-    if (C.getKind() == AnyCall::Function) {
-      if (const IdentifierInfo *Name = C.getIdentifier()) {
-        // When the CGBitmapContext is deallocated, the callback here will free
-        // the associated data buffer.
-        // The callback in dispatch_data_create frees the buffer, but not
-        // the data object.
-        if (Name->isStr("CGBitmapContextCreateWithData") ||
-            Name->isStr("dispatch_data_create"))
-          RE = S->getRetEffect();
-      }
-    }
+  RetEffect RE = RetEffect::MakeNoRetHard();
 
-    S = getPersistentSummary(RE, ScratchArgs, RecEffect, DefEffect);
+  // Special cases where the callback argument CANNOT free the return value.
+  // This can generally only happen if we know that the callback will only be
+  // called when the return value is already being deallocated.
+  if (const IdentifierInfo *Name = C.getIdentifier()) {
+    // When the CGBitmapContext is deallocated, the callback here will free
+    // the associated data buffer.
+    // The callback in dispatch_data_create frees the buffer, but not
+    // the data object.
+    if (Name->isStr("CGBitmapContextCreateWithData") ||
+        Name->isStr("dispatch_data_create"))
+      RE = S->getRetEffect();
   }
 
-  // Special case '[super init];' and '[self init];'
-  //
-  // Even though calling '[super init]' without assigning the result to self
-  // and checking if the parent returns 'nil' is a bad pattern, it is common.
-  // Additionally, our Self Init checker already warns about it. To avoid
-  // overwhelming the user with messages from both checkers, we model the case
-  // of '[super init]' in cases when it is not consumed by another expression
-  // as if the call preserves the value of 'self'; essentially, assuming it can
-  // never fail and return 'nil'.
-  // Note, we don't want to just stop tracking the value since we want the
-  // RetainCount checker to report leaks and use-after-free if SelfInit checker
-  // is turned off.
-  if (IsReceiverUnconsumedSelf) {
-    RetainSummaryTemplate ModifiableSummaryTemplate(S, *this);
-    ModifiableSummaryTemplate->setReceiverEffect(ArgEffect(DoNothing));
-    ModifiableSummaryTemplate->setRetEffect(RetEffect::MakeNoRet());
-  }
+  return getPersistentSummary(RE, ScratchArgs, RecEffect, DefEffect);
+}
+
+void RetainSummaryManager::updateSummaryForReceiverUnconsumedSelf(
+    const RetainSummary *&S) {
+
+  RetainSummaryTemplate Template(S, *this);
+
+  Template->setReceiverEffect(ArgEffect(DoNothing));
+  Template->setRetEffect(RetEffect::MakeNoRet());
 }
 
 const RetainSummary *
@@ -647,8 +630,11 @@ RetainSummaryManager::getSummary(AnyCall
   }
   }
 
-  updateSummaryForCall(Summ, C, HasNonZeroCallbackArg,
-                       IsReceiverUnconsumedSelf);
+  if (HasNonZeroCallbackArg)
+    Summ = updateSummaryForNonZeroCallbackArg(Summ, C);
+
+  if (IsReceiverUnconsumedSelf)
+    updateSummaryForReceiverUnconsumedSelf(Summ);
 
   assert(Summ && "Unknown call type?");
   return Summ;




More information about the cfe-commits mailing list