Skip to content

Commit cd22e0d

Browse files
committed
[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 llvm#57036. Differential Revision: https://reviews.llvm.org/D148344
1 parent 774703e commit cd22e0d

File tree

2 files changed

+46
-7
lines changed

2 files changed

+46
-7
lines changed

clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ using LatticeTransferState = TransferState<NoopLattice>;
4343

4444
DeclarationMatcher optionalClass() {
4545
return classTemplateSpecializationDecl(
46-
anyOf(hasName("std::optional"), hasName("std::__optional_storage_base"),
47-
hasName("__optional_destruct_base"), hasName("absl::optional"),
48-
hasName("base::Optional")),
46+
hasAnyName("::std::optional", "::std::__optional_storage_base",
47+
"::std::__optional_destruct_base", "::absl::optional",
48+
"::base::Optional"),
4949
hasTemplateArgument(0, refersToType(type().bind("T"))));
5050
}
5151

@@ -251,14 +251,34 @@ QualType stripReference(QualType Type) {
251251
return Type->isReferenceType() ? Type->getPointeeType() : Type;
252252
}
253253

254+
bool isTopLevelNamespaceWithName(const NamespaceDecl &NS,
255+
llvm::StringRef Name) {
256+
return NS.getDeclName().isIdentifier() && NS.getName() == Name &&
257+
NS.getParent() != nullptr && NS.getParent()->isTranslationUnit();
258+
}
259+
254260
/// Returns true if and only if `Type` is an optional type.
255261
bool isOptionalType(QualType Type) {
256262
if (!Type->isRecordType())
257263
return false;
258-
// FIXME: Optimize this by avoiding the `getQualifiedNameAsString` call.
259-
auto TypeName = Type->getAsCXXRecordDecl()->getQualifiedNameAsString();
260-
return TypeName == "std::optional" || TypeName == "absl::optional" ||
261-
TypeName == "base::Optional";
264+
const CXXRecordDecl *D = Type->getAsCXXRecordDecl();
265+
if (D == nullptr || !D->getDeclName().isIdentifier())
266+
return false;
267+
if (D->getName() == "optional") {
268+
if (const auto *N =
269+
dyn_cast_or_null<NamespaceDecl>(D->getDeclContext()))
270+
return N->isStdNamespace() || isTopLevelNamespaceWithName(*N, "absl");
271+
return false;
272+
}
273+
274+
if (D->getName() == "Optional") {
275+
// Check whether namespace is "::base".
276+
const auto *N =
277+
dyn_cast_or_null<NamespaceDecl>(D->getDeclContext());
278+
return N != nullptr && isTopLevelNamespaceWithName(*N, "base");
279+
}
280+
281+
return false;
262282
}
263283

264284
/// Returns the number of optional wrappers in `Type`.

clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,25 @@ INSTANTIATE_TEST_SUITE_P(
13531353
return Info.param.NamespaceName;
13541354
});
13551355

1356+
// Verifies that similarly-named types are ignored.
1357+
TEST_P(UncheckedOptionalAccessTest, NonTrackedOptionalType) {
1358+
ExpectDiagnosticsFor(
1359+
R"(
1360+
namespace other {
1361+
namespace $ns {
1362+
template <typename T>
1363+
struct $optional {
1364+
T value();
1365+
};
1366+
}
1367+
1368+
void target($ns::$optional<int> opt) {
1369+
opt.value();
1370+
}
1371+
}
1372+
)");
1373+
}
1374+
13561375
TEST_P(UncheckedOptionalAccessTest, EmptyFunctionBody) {
13571376
ExpectDiagnosticsFor(R"(
13581377
void target() {

0 commit comments

Comments
 (0)