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

[george.karpenkov]

Summary

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.

[NoQ]

Gathered weirdness is better than scattered weirdness!

clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

106–112

Is it possible to skip T2 here? I.e., template <class T1, class... Others> and then hasAnyEnabledAttrOf<Others...>.

[george.karpenkov]

clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

106–112

Nope, it's ambiguous otherwise.

clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h

public:
 
   void updateSummaryForCall(const RetainSummary *&Summ,
                             const CallEvent &Call);
 
   bool isARCEnabled() const { return ARCEnabled; }
 
   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;
 };
 
 
 // Used to avoid allocating long-term (BPAlloc'd) memory for default retain
 // summaries. If a function or method looks like it has a default summary, but
 // it has annotations, the annotations are added to the stack-based template

clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

 
 /// Helper function to check whether the class is one of the
 /// rest of varargs.
 template <class T, class P, class... ToCompare>
 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,
+              CFReturnsNotRetainedAttr>()) {
+    if (!TrackObjCAndCFObjects)
+      return None;
+
+    K = ObjKind::CF;
+  } else if (isOneOf<T, NSConsumedAttr, NSConsumesSelfAttr,
                      NSReturnsAutoreleasedAttr, NSReturnsRetainedAttr,
-              NSReturnsNotRetainedAttr>()) {
-    return TrackObjCAndCFObjects;
+                     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 *
 RetainSummaryManager::getPersistentSummary(const RetainSummary &OldSumm) {
   // Unique "simple" summaries -- those without ArgEffects.
   if (OldSumm.isSimple()) {
     ::llvm::FoldingSetNodeID ID;
     OldSumm.Profile(ID);
 

 
 //===----------------------------------------------------------------------===//
 // Summary creation for Selectors.
 //===----------------------------------------------------------------------===//
 
 Optional<RetEffect>
 RetainSummaryManager::getRetEffectFromAnnotations(QualType RetTy,
                                                   const Decl *D) {
-  if (TrackObjCAndCFObjects && cocoa::isCocoaObjectRef(RetTy)) {
-    if (D->hasAttr<NSReturnsRetainedAttr>())
-      return ObjCAllocRetE;
+  if (hasAnyEnabledAttrOf<NSReturnsRetainedAttr>(D, RetTy))
 
-    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 (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())
       if (auto RE = getRetEffectFromAnnotations(RetTy, PD))
         return RE;
 
   return None;
 }
 
 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));
-    return true;
-  } else if (hasEnabledAttr<CFConsumedAttr>(pd)) {
-    Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::CF));
-    return true;
-  } else if (hasEnabledAttr<OSConsumedAttr>(pd)) {
-    Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::OS));
-    return true;
-  } else if (hasRCAnnotation(pd, "rc_ownership_consumed")) {
-    Template->addArg(AF, parm_idx, ArgEffect(DecRef, ObjKind::Generalized));
+  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<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 (auto K =
+                 hasAnyEnabledAttrOf<CFReturnsRetainedAttr,
+                                     GeneralizedReturnsRetainedAttr>(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));
   } else {
     if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
       for (const auto *OD : MD->overridden_methods()) {
         const ParmVarDecl *OP = OD->parameters()[parm_idx];
         if (applyFunctionParamAnnotationEffect(OP, parm_idx, OD, Template))
           return true;
       }
     }
for (auto pi = FD->param_begin(),
     const ParmVarDecl *pd = *pi;
     applyFunctionParamAnnotationEffect(pd, parm_idx, FD, Template);
   }
 
   QualType RetTy = FD->getReturnType();
   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));
 }
 
 void
 RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ,
                                                    const ObjCMethodDecl *MD) {
   if (!MD)
     return;
 
   assert(Summ && "Must have a valid summary to add annotations to");
   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);
 }
 
 const RetainSummary *
 RetainSummaryManager::getStandardMethodSummary(const ObjCMethodDecl *MD,
                                                Selector S, QualType RetTy) {