Skip to content

Commit 09b462e

Browse files
committed
[clang][dataflow] Drop optional model's dependency on libc++ internals.
Adjusts the matchers in the optional model to avoid dependency on internal implementation details of libc++'s `std::optional`. In the process, factors out the code to check the name of these types so that it's shared throughout. Differential Revision: https://reviews.llvm.org/D148377
1 parent cd22e0d commit 09b462e

File tree

2 files changed

+44
-34
lines changed

2 files changed

+44
-34
lines changed

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

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "clang/AST/ExprCXX.h"
1919
#include "clang/AST/Stmt.h"
2020
#include "clang/ASTMatchers/ASTMatchers.h"
21+
#include "clang/ASTMatchers/ASTMatchersMacros.h"
2122
#include "clang/Analysis/CFG.h"
2223
#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
2324
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
@@ -36,16 +37,44 @@
3637

3738
namespace clang {
3839
namespace dataflow {
40+
41+
static bool isTopLevelNamespaceWithName(const NamespaceDecl &NS,
42+
llvm::StringRef Name) {
43+
return NS.getDeclName().isIdentifier() && NS.getName() == Name &&
44+
NS.getParent() != nullptr && NS.getParent()->isTranslationUnit();
45+
}
46+
47+
static bool hasOptionalClassName(const CXXRecordDecl &RD) {
48+
if (!RD.getDeclName().isIdentifier())
49+
return false;
50+
51+
if (RD.getName() == "optional") {
52+
if (const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext()))
53+
return N->isStdNamespace() || isTopLevelNamespaceWithName(*N, "absl");
54+
return false;
55+
}
56+
57+
if (RD.getName() == "Optional") {
58+
// Check whether namespace is "::base".
59+
const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext());
60+
return N != nullptr && isTopLevelNamespaceWithName(*N, "base");
61+
}
62+
63+
return false;
64+
}
65+
3966
namespace {
4067

4168
using namespace ::clang::ast_matchers;
4269
using LatticeTransferState = TransferState<NoopLattice>;
4370

71+
AST_MATCHER(CXXRecordDecl, hasOptionalClassNameMatcher) {
72+
return hasOptionalClassName(Node);
73+
}
74+
4475
DeclarationMatcher optionalClass() {
4576
return classTemplateSpecializationDecl(
46-
hasAnyName("::std::optional", "::std::__optional_storage_base",
47-
"::std::__optional_destruct_base", "::absl::optional",
48-
"::base::Optional"),
77+
hasOptionalClassNameMatcher(),
4978
hasTemplateArgument(0, refersToType(type().bind("T"))));
5079
}
5180

@@ -63,8 +92,10 @@ auto isOptionalMemberCallWithName(
6392
auto Exception = unless(Ignorable ? expr(anyOf(*Ignorable, cxxThisExpr()))
6493
: cxxThisExpr());
6594
return cxxMemberCallExpr(
66-
on(expr(Exception)),
67-
callee(cxxMethodDecl(hasName(MemberName), ofClass(optionalClass()))));
95+
on(expr(Exception,
96+
anyOf(hasOptionalType(),
97+
hasType(pointerType(pointee(optionalOrAliasType())))))),
98+
callee(cxxMethodDecl(hasName(MemberName))));
6899
}
69100

70101
auto isOptionalOperatorCallWithName(
@@ -251,34 +282,12 @@ QualType stripReference(QualType Type) {
251282
return Type->isReferenceType() ? Type->getPointeeType() : Type;
252283
}
253284

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-
260285
/// Returns true if and only if `Type` is an optional type.
261286
bool isOptionalType(QualType Type) {
262287
if (!Type->isRecordType())
263288
return false;
264289
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;
290+
return D != nullptr && hasOptionalClassName(*D);
282291
}
283292

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

clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1784,8 +1784,7 @@ TEST_P(UncheckedOptionalAccessTest, ValueOr) {
17841784
)");
17851785
}
17861786

1787-
TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
1788-
// Pointers.
1787+
TEST_P(UncheckedOptionalAccessTest, ValueOrComparisonPointers) {
17891788
ExpectDiagnosticsFor(
17901789
R"code(
17911790
#include "unchecked_optional_access_test.h"
@@ -1798,8 +1797,9 @@ TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
17981797
}
17991798
}
18001799
)code");
1800+
}
18011801

1802-
// Integers.
1802+
TEST_P(UncheckedOptionalAccessTest, ValueOrComparisonIntegers) {
18031803
ExpectDiagnosticsFor(
18041804
R"code(
18051805
#include "unchecked_optional_access_test.h"
@@ -1812,8 +1812,9 @@ TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
18121812
}
18131813
}
18141814
)code");
1815+
}
18151816

1816-
// Strings.
1817+
TEST_P(UncheckedOptionalAccessTest, ValueOrComparisonStrings) {
18171818
ExpectDiagnosticsFor(
18181819
R"code(
18191820
#include "unchecked_optional_access_test.h"
@@ -1839,9 +1840,9 @@ TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
18391840
}
18401841
}
18411842
)code");
1843+
}
18421844

1843-
// Pointer-to-optional.
1844-
//
1845+
TEST_P(UncheckedOptionalAccessTest, ValueOrComparisonPointerToOptional) {
18451846
// FIXME: make `opt` a parameter directly, once we ensure that all `optional`
18461847
// values have a `has_value` property.
18471848
ExpectDiagnosticsFor(

0 commit comments

Comments
 (0)