Author: Yitzhak Mandelbaum Date: 2023-04-17T18:03:58Z New Revision: 09b462ef8539433f56d1a6d2cc807db9a56072e6
URL: https://github.com/llvm/llvm-project/commit/09b462ef8539433f56d1a6d2cc807db9a56072e6 DIFF: https://github.com/llvm/llvm-project/commit/09b462ef8539433f56d1a6d2cc807db9a56072e6.diff LOG: [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 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 88f3488fcb251..9745cd8a48311 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/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 isOptionalMemberCallWithName( 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 @@ 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; 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`. diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index 8aba91566d4d7..a9d2357a57fb2 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -1784,8 +1784,7 @@ TEST_P(UncheckedOptionalAccessTest, ValueOr) { )"); } -TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) { - // Pointers. +TEST_P(UncheckedOptionalAccessTest, ValueOrComparisonPointers) { ExpectDiagnosticsFor( R"code( #include "unchecked_optional_access_test.h" @@ -1798,8 +1797,9 @@ TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) { } } )code"); +} - // Integers. +TEST_P(UncheckedOptionalAccessTest, ValueOrComparisonIntegers) { ExpectDiagnosticsFor( R"code( #include "unchecked_optional_access_test.h" @@ -1812,8 +1812,9 @@ TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) { } } )code"); +} - // Strings. +TEST_P(UncheckedOptionalAccessTest, ValueOrComparisonStrings) { ExpectDiagnosticsFor( R"code( #include "unchecked_optional_access_test.h" @@ -1839,9 +1840,9 @@ TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) { } } )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( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
