[clang] cd22e0d - [clang][dataflow] Refine matching of optional types to anchor at top level.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2023-04-17T18:02:51Z
New Revision: cd22e0dc9d0b5487eb2d54dd028a2aa439627159

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

LOG: [clang][dataflow] Refine matching of optional types to anchor at top level.

This patch refines the matching of the relevant optional types to anchor on the
global namespace. Previously, we could match anything with the right name
(e.g. `base::Optional`) even if nested within other namespaces. This over
matching resulted in an assertion violation when _different_ `base::Optional`
was encountered nested inside another namespace.

Fixes issue #57036.

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

Added: 
    

Modified: 
    clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
    clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 1512023383a68..88f3488fcb251 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -43,9 +43,9 @@ using LatticeTransferState = TransferState<NoopLattice>;
 
 DeclarationMatcher optionalClass() {
   return classTemplateSpecializationDecl(
-      anyOf(hasName("std::optional"), hasName("std::__optional_storage_base"),
-            hasName("__optional_destruct_base"), hasName("absl::optional"),
-            hasName("base::Optional")),
+      hasAnyName("::std::optional", "::std::__optional_storage_base",
+                 "::std::__optional_destruct_base", "::absl::optional",
+                 "::base::Optional"),
       hasTemplateArgument(0, refersToType(type().bind("T"))));
 }
 
@@ -251,14 +251,34 @@ QualType stripReference(QualType Type) {
   return Type->isReferenceType() ? Type->getPointeeType() : Type;
 }
 
+bool isTopLevelNamespaceWithName(const NamespaceDecl &NS,
+                                 llvm::StringRef Name) {
+  return NS.getDeclName().isIdentifier() && NS.getName() == Name &&
+         NS.getParent() != nullptr && NS.getParent()->isTranslationUnit();
+}
+
 /// Returns true if and only if `Type` is an optional type.
 bool isOptionalType(QualType Type) {
   if (!Type->isRecordType())
     return false;
-  // FIXME: Optimize this by avoiding the `getQualifiedNameAsString` call.
-  auto TypeName = Type->getAsCXXRecordDecl()->getQualifiedNameAsString();
-  return TypeName == "std::optional" || TypeName == "absl::optional" ||
-         TypeName == "base::Optional";
+  const CXXRecordDecl *D = Type->getAsCXXRecordDecl();
+  if (D == nullptr || !D->getDeclName().isIdentifier())
+    return false;
+  if (D->getName() == "optional") {
+    if (const auto *N =
+        dyn_cast_or_null<NamespaceDecl>(D->getDeclContext()))
+      return N->isStdNamespace() || isTopLevelNamespaceWithName(*N, "absl");
+    return false;
+  }
+
+  if (D->getName() == "Optional") {
+    // Check whether namespace is "::base".
+    const auto *N =
+        dyn_cast_or_null<NamespaceDecl>(D->getDeclContext());
+    return N != nullptr && isTopLevelNamespaceWithName(*N, "base");
+  }
+
+  return false;
 }
 
 /// Returns the number of optional wrappers in `Type`.

diff  --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index c7e2ad6263f5b..8aba91566d4d7 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1353,6 +1353,25 @@ INSTANTIATE_TEST_SUITE_P(
       return Info.param.NamespaceName;
     });
 
+// Verifies that similarly-named types are ignored.
+TEST_P(UncheckedOptionalAccessTest, NonTrackedOptionalType) {
+  ExpectDiagnosticsFor(
+      R"(
+    namespace other {
+    namespace $ns {
+    template <typename T>
+    struct $optional {
+      T value();
+    };
+    }
+
+    void target($ns::$optional<int> opt) {
+      opt.value();
+    }
+    }
+  )");
+}
+
 TEST_P(UncheckedOptionalAccessTest, EmptyFunctionBody) {
   ExpectDiagnosticsFor(R"(
     void target() {