ymandel created this revision.
ymandel added reviewers: xazax.hun, gribozavr2.
Herald added subscribers: martong, rnkovacs.
Herald added a reviewer: NoQ.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D148377
Files:
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1784,8 +1784,7 @@
)");
}
-TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
- // Pointers.
+TEST_P(UncheckedOptionalAccessTest, ValueOrComparisonPointers) {
ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -1798,8 +1797,9 @@
}
}
)code");
+}
- // Integers.
+TEST_P(UncheckedOptionalAccessTest, ValueOrComparisonIntegers) {
ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -1812,8 +1812,9 @@
}
}
)code");
+}
- // Strings.
+TEST_P(UncheckedOptionalAccessTest, ValueOrComparisonStrings) {
ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -1839,9 +1840,9 @@
}
}
)code");
+}
- // Pointer-to-optional.
- //
+TEST_P(UncheckedOptionalAccessTest, ValueOrComparisonPointerToOptional) {
// FIXME: make `opt` a parameter directly, once we ensure that all `optional`
// values have a `has_value` property.
ExpectDiagnosticsFor(
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -18,6 +18,7 @@
#include "clang/AST/ExprCXX.h"
#include "clang/AST/Stmt.h"
#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
@@ -36,16 +37,44 @@
namespace clang {
namespace dataflow {
+
+static bool isTopLevelNamespaceWithName(const NamespaceDecl &NS,
+ llvm::StringRef Name) {
+ return NS.getDeclName().isIdentifier() && NS.getName() == Name &&
+ NS.getParent() != nullptr && NS.getParent()->isTranslationUnit();
+}
+
+static bool hasOptionalClassName(const CXXRecordDecl &RD) {
+ if (!RD.getDeclName().isIdentifier())
+ return false;
+
+ if (RD.getName() == "optional") {
+ if (const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext()))
+ return N->isStdNamespace() || isTopLevelNamespaceWithName(*N, "absl");
+ return false;
+ }
+
+ if (RD.getName() == "Optional") {
+ // Check whether namespace is "::base".
+ const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext());
+ return N != nullptr && isTopLevelNamespaceWithName(*N, "base");
+ }
+
+ return false;
+}
+
namespace {
using namespace ::clang::ast_matchers;
using LatticeTransferState = TransferState<NoopLattice>;
+AST_MATCHER(CXXRecordDecl, hasOptionalClassNameMatcher) {
+ return hasOptionalClassName(Node);
+}
+
DeclarationMatcher optionalClass() {
return classTemplateSpecializationDecl(
- hasAnyName("::std::optional", "::std::__optional_storage_base",
- "::std::__optional_destruct_base", "::absl::optional",
- "::base::Optional"),
+ hasOptionalClassNameMatcher(),
hasTemplateArgument(0, refersToType(type().bind("T"))));
}
@@ -63,8 +92,10 @@
auto Exception = unless(Ignorable ? expr(anyOf(*Ignorable, cxxThisExpr()))
: cxxThisExpr());
return cxxMemberCallExpr(
- on(expr(Exception)),
- callee(cxxMethodDecl(hasName(MemberName), ofClass(optionalClass()))));
+ on(expr(Exception,
+ anyOf(hasOptionalType(),
+ hasType(pointerType(pointee(optionalOrAliasType())))))),
+ callee(cxxMethodDecl(hasName(MemberName))));
}
auto isOptionalOperatorCallWithName(
@@ -251,34 +282,12 @@
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;
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;
+ return D != nullptr && hasOptionalClassName(*D);
}
/// Returns the number of optional wrappers in `Type`.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
