r350862 - [analyzer] [RetainCountChecker] [NFC] Refactor the way attributes are handled

George Karpenkov via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 10 10:14:51 PST 2019


Author: george.karpenkov
Date: Thu Jan 10 10:14:51 2019
New Revision: 350862

URL: http://llvm.org/viewvc/llvm-project?rev=350862&view=rev
Log:
[analyzer] [RetainCountChecker] [NFC] Refactor the way attributes are handled

Make sure all checks for attributes go through a centralized function,
which checks whether attribute handling is enabled, and performs
validation.  The type of the attribute is returned.

Sadly, metaprogramming is required as attributes have no sensible static
getters.

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

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

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h?rev=350862&r1=350861&r2=350862&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h Thu Jan 10 10:14:51 2019
@@ -741,16 +741,18 @@ public:
 
   RetEffect getObjAllocRetEffect() const { return ObjCAllocRetE; }
 
-  /// \return True if the declaration has an attribute {@code T},
-  /// AND we are tracking that attribute. False otherwise.
+  /// Determine whether a declaration {@code D} of correspondent type (return
+  /// type for functions/methods) {@code QT} has any of the given attributes,
+  /// provided they pass necessary validation checks AND tracking the given
+  /// attribute is enabled.
+  /// Returns the object kind corresponding to the present attribute, or None,
+  /// if none of the specified attributes are present.
+  /// Crashes if passed an attribute which is not explicitly handled.
   template <class T>
-  bool hasEnabledAttr(const Decl *D) {
-    return isAttrEnabled<T>() && D->hasAttr<T>();
-  }
+  Optional<ObjKind> hasAnyEnabledAttrOf(const Decl *D, QualType QT);
 
-  /// Check whether we are tracking properties specified by the attributes.
-  template <class T>
-  bool isAttrEnabled();
+  template <class T1, class T2, class... Others>
+  Optional<ObjKind> hasAnyEnabledAttrOf(const Decl *D, QualType QT);
 
   friend class RetainSummaryTemplate;
 };

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=350862&r1=350861&r2=350862&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Thu Jan 10 10:14:51 2019
@@ -36,17 +36,79 @@ constexpr static bool isOneOf() {
   return std::is_same<T, P>::value || isOneOf<T, ToCompare...>();
 }
 
-template <class T> bool RetainSummaryManager::isAttrEnabled() {
+namespace {
+
+struct GeneralizedReturnsRetainedAttr {
+  static bool classof(const Attr *A) {
+    if (auto AA = dyn_cast<AnnotateAttr>(A))
+      return AA->getAnnotation() == "rc_ownership_returns_retained";
+    return false;
+  }
+};
+
+struct GeneralizedReturnsNotRetainedAttr {
+  static bool classof(const Attr *A) {
+    if (auto AA = dyn_cast<AnnotateAttr>(A))
+      return AA->getAnnotation() == "rc_ownership_returns_not_retained";
+    return false;
+  }
+};
+
+struct GeneralizedConsumedAttr {
+  static bool classof(const Attr *A) {
+    if (auto AA = dyn_cast<AnnotateAttr>(A))
+      return AA->getAnnotation() == "rc_ownership_consumed";
+    return false;
+  }
+};
+
+}
+
+template <class T>
+Optional<ObjKind> RetainSummaryManager::hasAnyEnabledAttrOf(const Decl *D,
+                                                            QualType QT) {
+  ObjKind K;
   if (isOneOf<T, CFConsumedAttr, CFReturnsRetainedAttr,
-              CFReturnsNotRetainedAttr, NSConsumedAttr, NSConsumesSelfAttr,
-              NSReturnsAutoreleasedAttr, NSReturnsRetainedAttr,
-              NSReturnsNotRetainedAttr>()) {
-    return TrackObjCAndCFObjects;
+              CFReturnsNotRetainedAttr>()) {
+    if (!TrackObjCAndCFObjects)
+      return None;
+
+    K = ObjKind::CF;
+  } else if (isOneOf<T, NSConsumedAttr, NSConsumesSelfAttr,
+                     NSReturnsAutoreleasedAttr, NSReturnsRetainedAttr,
+                     NSReturnsNotRetainedAttr, NSConsumesSelfAttr>()) {
+
+    if (!TrackObjCAndCFObjects)
+      return None;
+
+    if (isOneOf<T, NSReturnsRetainedAttr, NSReturnsAutoreleasedAttr,
+                NSReturnsNotRetainedAttr>() &&
+        !cocoa::isCocoaObjectRef(QT))
+      return None;
+    K = ObjKind::ObjC;
   } else if (isOneOf<T, OSConsumedAttr, OSConsumesThisAttr,
                      OSReturnsNotRetainedAttr, OSReturnsRetainedAttr>()) {
-    return TrackOSObjects;
+    if (!TrackOSObjects)
+      return None;
+    K = ObjKind::OS;
+  } else if (isOneOf<T, GeneralizedReturnsNotRetainedAttr,
+                     GeneralizedReturnsRetainedAttr,
+                     GeneralizedConsumedAttr>()) {
+    K = ObjKind::Generalized;
+  } else {
+    llvm_unreachable("Unexpected attribute");
   }
-  llvm_unreachable("Unexpected attribute passed");
+  if (D->hasAttr<T>())
+    return K;
+  return None;
+}
+
+template <class T1, class T2, class... Others>
+Optional<ObjKind> RetainSummaryManager::hasAnyEnabledAttrOf(const Decl *D,
+                                                            QualType QT) {
+  if (auto Out = hasAnyEnabledAttrOf<T1>(D, QT))
+    return Out;
+  return hasAnyEnabledAttrOf<T2, Others...>(D, QT);
 }
 
 const RetainSummary *
@@ -727,33 +789,18 @@ RetainSummaryManager::getCFSummaryGetRul
 Optional<RetEffect>
 RetainSummaryManager::getRetEffectFromAnnotations(QualType RetTy,
                                                   const Decl *D) {
-  if (TrackObjCAndCFObjects && cocoa::isCocoaObjectRef(RetTy)) {
-    if (D->hasAttr<NSReturnsRetainedAttr>())
-      return ObjCAllocRetE;
-
-    if (D->hasAttr<NSReturnsNotRetainedAttr>() ||
-        D->hasAttr<NSReturnsAutoreleasedAttr>())
-      return RetEffect::MakeNotOwned(ObjKind::ObjC);
-
-  } else if (!RetTy->isPointerType()) {
-    return None;
-  }
-
-  if (hasEnabledAttr<CFReturnsRetainedAttr>(D)) {
-    return RetEffect::MakeOwned(ObjKind::CF);
-  } else if (hasEnabledAttr<OSReturnsRetainedAttr>(D)) {
-    return RetEffect::MakeOwned(ObjKind::OS);
-  } else if (hasRCAnnotation(D, "rc_ownership_returns_retained")) {
-    return RetEffect::MakeOwned(ObjKind::Generalized);
-  }
+  if (hasAnyEnabledAttrOf<NSReturnsRetainedAttr>(D, RetTy))
+    return ObjCAllocRetE;
 
-  if (hasEnabledAttr<CFReturnsNotRetainedAttr>(D)) {
-    return RetEffect::MakeNotOwned(ObjKind::CF);
-  } else if (hasEnabledAttr<OSReturnsNotRetainedAttr>(D)) {
-    return RetEffect::MakeNotOwned(ObjKind::OS);
-  } else if (hasRCAnnotation(D, "rc_ownership_returns_not_retained")) {
-    return RetEffect::MakeNotOwned(ObjKind::Generalized);
-  }
+  if (auto K = hasAnyEnabledAttrOf<CFReturnsRetainedAttr, OSReturnsRetainedAttr,
+                                   GeneralizedReturnsRetainedAttr>(D, RetTy))
+    return RetEffect::MakeOwned(*K);
+
+  if (auto K = hasAnyEnabledAttrOf<
+          CFReturnsNotRetainedAttr, OSReturnsNotRetainedAttr,
+          GeneralizedReturnsNotRetainedAttr, NSReturnsNotRetainedAttr,
+          NSReturnsAutoreleasedAttr>(D, RetTy))
+    return RetEffect::MakeNotOwned(*K);
 
   if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
     for (const auto *PD : MD->overridden_methods())
@@ -766,37 +813,20 @@ RetainSummaryManager::getRetEffectFromAn
 bool RetainSummaryManager::applyFunctionParamAnnotationEffect(
     const ParmVarDecl *pd, unsigned parm_idx, const FunctionDecl *FD,
     RetainSummaryTemplate &Template) {
-  if (hasEnabledAttr<NSConsumedAttr>(pd)) {
-    Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::ObjC));
+  QualType QT = pd->getType();
+  if (auto K =
+          hasAnyEnabledAttrOf<NSConsumedAttr, CFConsumedAttr, OSConsumedAttr,
+                              GeneralizedConsumedAttr>(pd, QT)) {
+    Template->addArg(AF, parm_idx, ArgEffect(DecRef, *K));
     return true;
-  } else if (hasEnabledAttr<CFConsumedAttr>(pd)) {
-    Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::CF));
+  } else if (auto K =
+                 hasAnyEnabledAttrOf<CFReturnsRetainedAttr,
+                                     GeneralizedReturnsRetainedAttr>(pd, QT)) {
+    Template->addArg(AF, parm_idx, ArgEffect(RetainedOutParameter, *K));
     return true;
-  } else if (hasEnabledAttr<OSConsumedAttr>(pd)) {
-    Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::OS));
+  } else if (auto K = hasAnyEnabledAttrOf<CFReturnsNotRetainedAttr>(pd, QT)) {
+    Template->addArg(AF, parm_idx, ArgEffect(UnretainedOutParameter, *K));
     return true;
-  } else if (hasRCAnnotation(pd, "rc_ownership_consumed")) {
-    Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::Generalized));
-    return true;
-  } else if (hasEnabledAttr<CFReturnsRetainedAttr>(pd) ||
-             hasRCAnnotation(pd, "rc_ownership_returns_retained")) {
-    QualType PointeeTy = pd->getType()->getPointeeType();
-    if (!PointeeTy.isNull()) {
-      if (coreFoundation::isCFObjectRef(PointeeTy)) {
-        Template->addArg(AF, parm_idx, ArgEffect(RetainedOutParameter,
-                                                 ObjKind::CF));
-        return true;
-      }
-    }
-  } else if (hasEnabledAttr<CFReturnsNotRetainedAttr>(pd)) {
-    QualType PointeeTy = pd->getType()->getPointeeType();
-    if (!PointeeTy.isNull()) {
-      if (coreFoundation::isCFObjectRef(PointeeTy)) {
-        Template->addArg(AF, parm_idx, ArgEffect(UnretainedOutParameter,
-                                                 ObjKind::CF));
-        return true;
-      }
-    }
   } else {
     if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
       for (const auto *OD : MD->overridden_methods()) {
@@ -831,7 +861,7 @@ RetainSummaryManager::updateSummaryFromA
   if (Optional<RetEffect> RetE = getRetEffectFromAnnotations(RetTy, FD))
     Template->setRetEffect(*RetE);
 
-  if (hasEnabledAttr<OSConsumesThisAttr>(FD))
+  if (hasAnyEnabledAttrOf<OSConsumesThisAttr>(FD, RetTy))
     Template->setThisEffect(ArgEffect(DecRef, ObjKind::OS));
 }
 
@@ -845,35 +875,25 @@ RetainSummaryManager::updateSummaryFromA
   RetainSummaryTemplate Template(Summ, *this);
 
   // Effects on the receiver.
-  if (MD->hasAttr<NSConsumesSelfAttr>())
+  if (hasAnyEnabledAttrOf<NSConsumesSelfAttr>(MD, MD->getReturnType()))
     Template->setReceiverEffect(ArgEffect(DecRef, ObjKind::ObjC));
 
   // Effects on the parameters.
   unsigned parm_idx = 0;
-  for (auto pi=MD->param_begin(), pe=MD->param_end();
-       pi != pe; ++pi, ++parm_idx) {
+  for (auto pi = MD->param_begin(), pe = MD->param_end(); pi != pe;
+       ++pi, ++parm_idx) {
     const ParmVarDecl *pd = *pi;
-    if (pd->hasAttr<NSConsumedAttr>()) {
-      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>()) {
-      Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::OS));
-    } else if (pd->hasAttr<CFReturnsRetainedAttr>()) {
-      QualType PointeeTy = pd->getType()->getPointeeType();
-      if (!PointeeTy.isNull())
-        if (coreFoundation::isCFObjectRef(PointeeTy))
-          Template->addArg(AF, parm_idx,
-                           ArgEffect(RetainedOutParameter, ObjKind::CF));
-    } else if (pd->hasAttr<CFReturnsNotRetainedAttr>()) {
-      QualType PointeeTy = pd->getType()->getPointeeType();
-      if (!PointeeTy.isNull())
-        if (coreFoundation::isCFObjectRef(PointeeTy))
-          Template->addArg(AF, parm_idx, ArgEffect(UnretainedOutParameter,
-                                                   ObjKind::CF));
+    QualType QT = pd->getType();
+    if (auto K =
+            hasAnyEnabledAttrOf<NSConsumedAttr, CFConsumedAttr, OSConsumedAttr>(
+                pd, QT)) {
+      Template->addArg(AF, parm_idx, ArgEffect(DecRef, *K));
+    } else if (auto K = hasAnyEnabledAttrOf<CFReturnsRetainedAttr>(pd, QT)) {
+      Template->addArg(AF, parm_idx, ArgEffect(RetainedOutParameter, *K));
+    } else if (auto K = hasAnyEnabledAttrOf<CFReturnsNotRetainedAttr>(pd, QT)) {
+      Template->addArg(AF, parm_idx, ArgEffect(UnretainedOutParameter, *K));
     }
   }
-
   QualType RetTy = MD->getReturnType();
   if (Optional<RetEffect> RetE = getRetEffectFromAnnotations(RetTy, MD))
     Template->setRetEffect(*RetE);




More information about the cfe-commits mailing list