diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/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(isMakeOptionalCall(), transferMakeOptionalCall) - // optional::optional + // optional::optional (in place) .CaseOfCFGStmt( 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( 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( 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(isOptionalValueOrConversionConstructor(), transferValueOrConversionConstructor) + // optional::operator= .CaseOfCFGStmt( 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)); }) diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -1273,7 +1273,6 @@ ExpectDiagnosticsFor(SourceCode, ast_matchers::hasName("target")); } -private: template 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 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 opt; + }; + )", + cxxConstructorDecl(ofClass(hasName("Target")))); +} + // FIXME: Add support for: // - constructors (copy, move) // - assignment operators (default, copy, move)