[clang] 3ec7b91 - [analyzer][NFC] Refactor CallEvent::isCalled()

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2021-10-18T14:57:24+02:00
New Revision: 3ec7b91141da4b3f4dce4964ca3ea7c3549584d2

URL: https://github.com/llvm/llvm-project/commit/3ec7b91141da4b3f4dce4964ca3ea7c3549584d2
DIFF: https://github.com/llvm/llvm-project/commit/3ec7b91141da4b3f4dce4964ca3ea7c3549584d2.diff

LOG: [analyzer][NFC] Refactor CallEvent::isCalled()

Refactor the code to make it more readable.

It will set up further changes, and improvements to this code in
subsequent patches.
This is a non-functional change.

Reviewed By: martong

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

Added: 
    

Modified: 
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
    clang/lib/StaticAnalyzer/Core/CallEvent.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 060fff1a74071..e10e3509fe3d7 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1237,9 +1237,7 @@ enum CallDescriptionFlags : int {
 /// arguments and the name of the function.
 class CallDescription {
   friend CallEvent;
-
-  mutable IdentifierInfo *II = nullptr;
-  mutable bool IsLookupDone = false;
+  mutable Optional<const IdentifierInfo *> II;
   // The list of the qualified names used to identify the specified CallEvent,
   // e.g. "{a, b}" represent the qualified names, like "a::b".
   std::vector<const char *> QualifiedName;
@@ -1273,7 +1271,9 @@ class CallDescription {
                   Optional<size_t> RequiredParams = None)
       : QualifiedName(QualifiedName), RequiredArgs(RequiredArgs),
         RequiredParams(readRequiredParams(RequiredArgs, RequiredParams)),
-        Flags(Flags) {}
+        Flags(Flags) {
+    assert(!QualifiedName.empty());
+  }
 
   /// Construct a CallDescription with default flags.
   CallDescription(ArrayRef<const char *> QualifiedName,
@@ -1283,6 +1283,17 @@ class CallDescription {
 
   /// Get the name of the function that this object matches.
   StringRef getFunctionName() const { return QualifiedName.back(); }
+
+  /// Get the qualified name parts in reversed order.
+  /// E.g. { "std", "vector", "data" } -> "vector", "std"
+  auto begin_qualified_name_parts() const {
+    return std::next(QualifiedName.rbegin());
+  }
+  auto end_qualified_name_parts() const { return QualifiedName.rend(); }
+
+  /// It's false, if and only if we expect a single identifier, such as
+  /// `getenv`. It's true for `std::swap`, or `my::detail::container::data`.
+  bool hasQualifiedNameParts() const { return QualifiedName.size() > 1; }
 };
 
 /// An immutable map from CallDescriptions to arbitrary data. Provides a unified

diff  --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index 9a95921b64c3b..ec44bcf5b27ca 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -307,10 +307,7 @@ bool CallEvent::isCalled(const CallDescription &CD) const {
   if (getKind() == CE_ObjCMessage)
     return false;
 
-  const IdentifierInfo *II = getCalleeIdentifier();
-  if (!II)
-    return false;
-  const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(getDecl());
+  const auto *FD = dyn_cast_or_null<FunctionDecl>(getDecl());
   if (!FD)
     return false;
 
@@ -320,44 +317,69 @@ bool CallEvent::isCalled(const CallDescription &CD) const {
            (!CD.RequiredParams || CD.RequiredParams <= parameters().size());
   }
 
-  if (!CD.IsLookupDone) {
-    CD.IsLookupDone = true;
+  if (!CD.II.hasValue()) {
     CD.II = &getState()->getStateManager().getContext().Idents.get(
         CD.getFunctionName());
   }
 
-  if (II != CD.II)
-    return false;
+  const auto MatchNameOnly = [](const CallDescription &CD,
+                                const NamedDecl *ND) -> bool {
+    DeclarationName Name = ND->getDeclName();
+    if (const auto *II = Name.getAsIdentifierInfo())
+      return II == CD.II.getValue(); // Fast case.
 
-  // If CallDescription provides prefix names, use them to improve matching
-  // accuracy.
-  if (CD.QualifiedName.size() > 1 && FD) {
-    const DeclContext *Ctx = FD->getDeclContext();
-    // See if we'll be able to match them all.
-    size_t NumUnmatched = CD.QualifiedName.size() - 1;
-    for (; Ctx && isa<NamedDecl>(Ctx); Ctx = Ctx->getParent()) {
-      if (NumUnmatched == 0)
-        break;
+    // Simply report mismatch for:
+    // C++ overloaded operators, constructors, destructors, etc.
+    return false;
+  };
 
-      if (const auto *ND = dyn_cast<NamespaceDecl>(Ctx)) {
-        if (ND->getName() == CD.QualifiedName[NumUnmatched - 1])
-          --NumUnmatched;
-        continue;
-      }
+  const auto ExactMatchArgAndParamCounts =
+      [](const CallEvent &Call, const CallDescription &CD) -> bool {
+    const bool ArgsMatch =
+        !CD.RequiredArgs || CD.RequiredArgs == Call.getNumArgs();
+    const bool ParamsMatch =
+        !CD.RequiredParams || CD.RequiredParams == Call.parameters().size();
+    return ArgsMatch && ParamsMatch;
+  };
 
-      if (const auto *RD = dyn_cast<RecordDecl>(Ctx)) {
-        if (RD->getName() == CD.QualifiedName[NumUnmatched - 1])
-          --NumUnmatched;
+  const auto MatchQualifiedNameParts = [](const CallDescription &CD,
+                                          const Decl *D) -> bool {
+    const auto FindNextNamespaceOrRecord =
+        [](const DeclContext *Ctx) -> const DeclContext * {
+      while (Ctx && !isa<NamespaceDecl, RecordDecl>(Ctx))
+        Ctx = Ctx->getParent();
+      return Ctx;
+    };
+
+    auto QualifierPartsIt = CD.begin_qualified_name_parts();
+    const auto QualifierPartsEndIt = CD.end_qualified_name_parts();
+
+    // Match namespace and record names. Skip unrelated names if they don't
+    // match.
+    const DeclContext *Ctx = FindNextNamespaceOrRecord(D->getDeclContext());
+    for (; Ctx && QualifierPartsIt != QualifierPartsEndIt;
+         Ctx = FindNextNamespaceOrRecord(Ctx->getParent())) {
+      // If not matched just continue and try matching for the next one.
+      if (cast<NamedDecl>(Ctx)->getName() != *QualifierPartsIt)
         continue;
-      }
+      ++QualifierPartsIt;
     }
 
-    if (NumUnmatched > 0)
-      return false;
-  }
+    // We matched if we consumed all expected qualifier segments.
+    return QualifierPartsIt == QualifierPartsEndIt;
+  };
+
+  // Let's start matching...
+  if (!ExactMatchArgAndParamCounts(*this, CD))
+    return false;
+
+  if (!MatchNameOnly(CD, FD))
+    return false;
+
+  if (!CD.hasQualifiedNameParts())
+    return true;
 
-  return (!CD.RequiredArgs || CD.RequiredArgs == getNumArgs()) &&
-         (!CD.RequiredParams || CD.RequiredParams == parameters().size());
+  return MatchQualifiedNameParts(CD, FD);
 }
 
 SVal CallEvent::getArgSVal(unsigned Index) const {