This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0e8d4a6df959: [clang][dataflow] Simplify handling of nullopt-optionals. (authored by ymandel).
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140506/new/ https://reviews.llvm.org/D140506 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 @@ -1273,7 +1273,6 @@ ExpectDiagnosticsFor(SourceCode, ast_matchers::hasName("target")); } -private: template <typename FuncDeclMatcher> void ExpectDiagnosticsFor(std::string SourceCode, FuncDeclMatcher FuncMatcher) { @@ -2939,6 +2938,38 @@ )"); } +TEST_P(UncheckedOptionalAccessTest, CtorInitializerNullopt) { + using namespace ast_matchers; + ExpectDiagnosticsFor( + R"( + #include "unchecked_optional_access_test.h" + + struct Target { + Target(): opt($ns::nullopt) { + opt.value(); // [[unsafe]] + } + $ns::$optional<int> opt; + }; + )", + cxxConstructorDecl(ofClass(hasName("Target")))); +} + +TEST_P(UncheckedOptionalAccessTest, CtorInitializerValue) { + using namespace ast_matchers; + ExpectDiagnosticsFor( + R"( + #include "unchecked_optional_access_test.h" + + struct Target { + Target(): opt(3) { + opt.value(); + } + $ns::$optional<int> opt; + }; + )", + cxxConstructorDecl(ofClass(hasName("Target")))); +} + // FIXME: Add support for: // - constructors (copy, move) // - assignment operators (default, copy, move) Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -438,12 +438,11 @@ Loc, createOptionalValue(State.Env, State.Env.makeAtomicBoolValue())); } -void assignOptionalValue(const Expr &E, LatticeTransferState &State, +void assignOptionalValue(const Expr &E, Environment &Env, BoolValue &HasValueVal) { if (auto *OptionalLoc = - State.Env.getStorageLocation(E, SkipPast::ReferenceThenPointer)) { - State.Env.setValue(*OptionalLoc, - createOptionalValue(State.Env, HasValueVal)); + Env.getStorageLocation(E, SkipPast::ReferenceThenPointer)) { + Env.setValue(*OptionalLoc, createOptionalValue(Env, HasValueVal)); } } @@ -479,7 +478,7 @@ LatticeTransferState &State) { assert(E->getNumArgs() > 0); - assignOptionalValue(*E, State, + assignOptionalValue(*E, State.Env, valueOrConversionHasValue(*E->getConstructor(), *E->getArg(0), MatchRes, State)); @@ -647,35 +646,35 @@ // make_optional .CaseOfCFGStmt<CallExpr>(isMakeOptionalCall(), transferMakeOptionalCall) - // optional::optional + // optional::optional (in place) .CaseOfCFGStmt<CXXConstructExpr>( isOptionalInPlaceConstructor(), [](const CXXConstructExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { - assignOptionalValue(*E, State, State.Env.getBoolLiteralValue(true)); + assignOptionalValue(*E, State.Env, + State.Env.getBoolLiteralValue(true)); }) + // nullopt_t::nullopt_t .CaseOfCFGStmt<CXXConstructExpr>( isNulloptConstructor(), [](const CXXConstructExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { - assignOptionalValue(*E, State, + assignOptionalValue(*E, State.Env, State.Env.getBoolLiteralValue(false)); }) + // optional::optional(nullopt_t) .CaseOfCFGStmt<CXXConstructExpr>( isOptionalNulloptConstructor(), [](const CXXConstructExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { - // Shares a temporary with the underlying `nullopt_t` instance. - if (auto *OptionalLoc = - State.Env.getStorageLocation(*E, SkipPast::None)) { - State.Env.setValue( - *OptionalLoc, - *State.Env.getValue(*E->getArg(0), SkipPast::None)); - } + assignOptionalValue(*E, State.Env, + State.Env.getBoolLiteralValue(false)); }) + // optional::optional (value/conversion) .CaseOfCFGStmt<CXXConstructExpr>(isOptionalValueOrConversionConstructor(), transferValueOrConversionConstructor) + // optional::operator= .CaseOfCFGStmt<CXXOperatorCallExpr>( isOptionalValueOrConversionAssignment(), @@ -714,7 +713,7 @@ isOptionalMemberCallWithName("emplace"), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { - assignOptionalValue(*E->getImplicitObjectArgument(), State, + assignOptionalValue(*E->getImplicitObjectArgument(), State.Env, State.Env.getBoolLiteralValue(true)); }) @@ -723,7 +722,7 @@ isOptionalMemberCallWithName("reset"), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { - assignOptionalValue(*E->getImplicitObjectArgument(), State, + assignOptionalValue(*E->getImplicitObjectArgument(), State.Env, State.Env.getBoolLiteralValue(false)); })
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits