diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 2330697299fdd..15ce37c13a6c7 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -500,6 +500,9 @@ class Environment { return cast_or_null(getValue(E)); } + std::optional getAtomValue(Atom A) const; + void setAtomValue(Atom A, bool); + // FIXME: should we deprecate the following & call arena().create() directly? /// Creates a `T` (some subclass of `Value`), forwarding `args` to the @@ -720,6 +723,7 @@ class Environment { // deterministic sequence. This in turn produces deterministic SAT formulas. llvm::MapVector ExprToVal; llvm::MapVector LocToVal; + llvm::MapVector AtomToVal; Atom FlowConditionToken; }; diff --git a/clang/include/clang/Analysis/FlowSensitive/Transfer.h b/clang/include/clang/Analysis/FlowSensitive/Transfer.h index ed148250d8eb2..938ac5f3439ca 100644 --- a/clang/include/clang/Analysis/FlowSensitive/Transfer.h +++ b/clang/include/clang/Analysis/FlowSensitive/Transfer.h @@ -48,6 +48,12 @@ class StmtToEnvMap { const TypeErasedDataflowAnalysisState &CurState; }; +namespace bool_model { + +BoolValue &freshBoolValue(Environment &Env); + +} // namespace bool_model + /// Evaluates `S` and updates `Env` accordingly. /// /// Requirements: diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index cc1ebd511191a..8a41c9b948d44 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -29,6 +29,169 @@ namespace clang { namespace dataflow { +namespace simple_bool_model { +static std::optional getLiteralValue(const Formula &F, + const Environment &Env) { + switch (F.kind()) { + case Formula::AtomRef: + return Env.getAtomValue(F.getAtom()); + case Formula::Literal: + return F.literal(); + case Formula::Not: { + ArrayRef Operands = F.operands(); + assert(Operands.size() == 1); + if (std::optional Maybe = getLiteralValue(*Operands[0], Env)) + return !*Maybe; + return std::nullopt; + } + case Formula::And: { + ArrayRef Operands = F.operands(); + assert(Operands.size() == 2); + auto &LHS = *Operands[0]; + auto &RHS = *Operands[1]; + if (std::optional Left = getLiteralValue(LHS, Env)) + return !*Left ? Left : getLiteralValue(RHS, Env); + if (std::optional Right = getLiteralValue(RHS, Env); Right == false) + return Right; + return std::nullopt; + } + case Formula::Or: { + ArrayRef Operands = F.operands(); + assert(Operands.size() == 2); + auto &LHS = *Operands[0]; + auto &RHS = *Operands[1]; + if (std::optional Left = getLiteralValue(LHS, Env)) + return *Left ? Left : getLiteralValue(RHS, Env); + if (std::optional Right = getLiteralValue(RHS, Env); Right == true) + return Right; + return std::nullopt; + } + case Formula::Equal: { + ArrayRef Operands = F.operands(); + assert(Operands.size() == 2); + auto &LHS = *Operands[0]; + auto &RHS = *Operands[1]; + if (&LHS == &RHS) + return true; + auto V_L = getLiteralValue(LHS, Env); + return V_L.has_value() && V_L == getLiteralValue(RHS, Env); + } + default: + return std::nullopt; + } +} + +// Updates atom settings in `Env` based on the formula. `AtomVal` indicates the +// value to use for atoms encountered in the formula. +static void assumeFormula(bool AtomVal, const Formula &F, Environment &Env) { + switch (F.kind()) { + case Formula::AtomRef: + // FIXME: if the atom is set to a different value, mark the block as + // unreachable. + Env.setAtomValue(F.getAtom(), AtomVal); + break; + case Formula::Literal: + // FIXME: if the literal is `false`, mark the block as unreachable. + break; + case Formula::Not: { + ArrayRef Operands = F.operands(); + assert(Operands.size() == 1); + assumeFormula(!AtomVal, *Operands[0], Env); + break; + } + case Formula::And: { + if (AtomVal == true) { + ArrayRef Operands = F.operands(); + assert(Operands.size() == 2); + assumeFormula(true, *Operands[0], Env); + assumeFormula(true, *Operands[1], Env); + } + break; + } + case Formula::Or: { + if (AtomVal == false) { + // Interpret the negated "or" as "and" of negated operands. + ArrayRef Operands = F.operands(); + assert(Operands.size() == 2); + assumeFormula(false, *Operands[0], Env); + assumeFormula(false, *Operands[1], Env); + } + break; + } + case Formula::Equal: { + ArrayRef Operands = F.operands(); + assert(Operands.size() == 2); + auto &LHS = *Operands[0]; + auto &RHS = *Operands[1]; + if (auto V = getLiteralValue(LHS, Env)) { + assumeFormula(AtomVal == *V, RHS, Env); + } else if (auto V = getLiteralValue(RHS, Env)) { + assumeFormula(AtomVal == *V, LHS, Env); + } + break; + } + default: + break; + } +} + +BoolValue &join(BoolValue &Val1, const Environment &Env1, BoolValue &Val2, + const Environment &Env2, Environment &JoinedEnv) { + if (auto V1 = getLiteralValue(Val1.formula(), Env1); + V1.has_value() && V1 == getLiteralValue(Val2.formula(), Env2)) + return JoinedEnv.getBoolLiteralValue(*V1); + return JoinedEnv.makeAtomicBoolValue(); +} + +void assume(Environment &Env, const Formula &F) { + assumeFormula(/*AtomVal*/ true, F, Env); +} + +bool allows(const Environment &Env, const Formula &F) { + auto V = getLiteralValue(F, Env); + // We are conservative in the negative direction: if we can't determine the + // value, assume it is allowed. + return V.value_or(true); +} + +bool proves(const Environment &Env, const Formula &F) { + auto V = getLiteralValue(F, Env); + return V.value_or(false); +} +} // namespace simple_bool_model + +namespace sat_bool_model { +BoolValue &join(BoolValue &Val1, const Environment &Env1, BoolValue &Val2, + const Environment &Env2, Environment &JoinedEnv) { + auto &A = JoinedEnv.arena(); + auto &JoinedVal = JoinedEnv.makeAtomicBoolValue(); + auto &JoinedFormula = JoinedVal.formula(); + JoinedEnv.assume( + A.makeOr(A.makeAnd(A.makeAtomRef(Env1.getFlowConditionToken()), + A.makeEquals(JoinedFormula, Val1.formula())), + A.makeAnd(A.makeAtomRef(Env2.getFlowConditionToken()), + A.makeEquals(JoinedFormula, Val2.formula())))); + return JoinedVal; +} + +void assume(Environment &Env, const Formula &F) { + Env.getDataflowAnalysisContext().addFlowConditionConstraint( + Env.getFlowConditionToken(), F); +} + +bool allows(const Environment &Env, const Formula &F) { + return Env.getDataflowAnalysisContext().flowConditionAllows( + Env.getFlowConditionToken(), F); +} + +bool proves(const Environment &Env, const Formula &F) { + return Env.getDataflowAnalysisContext().flowConditionImplies( + Env.getFlowConditionToken(), F); +} +} // namespace sat_bool_model + +namespace bool_model = simple_bool_model; + // FIXME: convert these to parameters of the analysis or environment. Current // settings have been experimentaly validated, but only for a particular // analysis. @@ -129,16 +292,9 @@ static Value *joinDistinctValues(QualType Type, Value &Val1, // if (o.has_value()) // x = o.value(); // ``` - auto &Expr1 = cast(Val1).formula(); - auto &Expr2 = cast(Val2).formula(); - auto &A = JoinedEnv.arena(); - auto &JoinedVal = A.makeAtomRef(A.makeAtom()); - JoinedEnv.assume( - A.makeOr(A.makeAnd(A.makeAtomRef(Env1.getFlowConditionToken()), - A.makeEquals(JoinedVal, Expr1)), - A.makeAnd(A.makeAtomRef(Env2.getFlowConditionToken()), - A.makeEquals(JoinedVal, Expr2)))); - return &A.makeBoolValue(JoinedVal); + auto &B1 = cast(Val1); + auto &B2 = cast(Val2); + return &bool_model::join(B1, Env1, B2, Env2, JoinedEnv); } Value *JoinedVal = nullptr; @@ -706,6 +862,12 @@ Environment Environment::join(const Environment &EnvA, const Environment &EnvB, JoinedEnv.ExprToLoc = joinExprMaps(EnvA.ExprToLoc, EnvB.ExprToLoc); } + for (auto &Entry : EnvA.AtomToVal) { + auto It = EnvB.AtomToVal.find(Entry.first); + if (It != EnvB.AtomToVal.end() && Entry.second == It->second) + JoinedEnv.AtomToVal.insert({Entry.first, Entry.second}); + } + return JoinedEnv; } @@ -1056,16 +1218,14 @@ StorageLocation &Environment::createObjectInternal(const ValueDecl *D, return Loc; } -void Environment::assume(const Formula &F) { - DACtx->addFlowConditionConstraint(FlowConditionToken, F); -} +void Environment::assume(const Formula &F) { bool_model::assume(*this, F); } bool Environment::proves(const Formula &F) const { - return DACtx->flowConditionImplies(FlowConditionToken, F); + return bool_model::proves(*this, F); } bool Environment::allows(const Formula &F) const { - return DACtx->flowConditionAllows(FlowConditionToken, F); + return bool_model::allows(*this, F); } void Environment::dump(raw_ostream &OS) const { @@ -1120,6 +1280,15 @@ void Environment::dump() const { dump(llvm::dbgs()); } +std::optional Environment::getAtomValue(Atom A) const { + auto It = AtomToVal.find(A); + if (It == AtomToVal.end()) + return std::nullopt; + return It->second; +} + +void Environment::setAtomValue(Atom A, bool b) { AtomToVal[A] = b; } + RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE, const Environment &Env) { Expr *ImplicitObject = MCE.getImplicitObjectArgument(); diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index dbf4878622eba..f840ccd382815 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -653,8 +653,7 @@ const Formula &evaluateEquality(Arena &A, const Formula &EqVal, // If neither is set, then they are equal. // We rewrite b) as !EqVal => (LHS v RHS), for a more compact formula. return A.makeAnd( - A.makeImplies(EqVal, A.makeOr(A.makeAnd(LHS, RHS), - A.makeAnd(A.makeNot(LHS), A.makeNot(RHS)))), + A.makeImplies(EqVal, A.makeEquals(LHS, RHS)), A.makeImplies(A.makeNot(EqVal), A.makeOr(LHS, RHS))); } diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 960e9688ffb72..4ad58f82353fe 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -50,21 +50,6 @@ const Environment *StmtToEnvMap::getEnvironment(const Stmt &S) const { return &State->Env; } -static BoolValue &evaluateBooleanEquality(const Expr &LHS, const Expr &RHS, - Environment &Env) { - Value *LHSValue = Env.getValue(LHS); - Value *RHSValue = Env.getValue(RHS); - - if (LHSValue == RHSValue) - return Env.getBoolLiteralValue(true); - - if (auto *LHSBool = dyn_cast_or_null(LHSValue)) - if (auto *RHSBool = dyn_cast_or_null(RHSValue)) - return Env.makeIff(*LHSBool, *RHSBool); - - return Env.makeAtomicBoolValue(); -} - static BoolValue &unpackValue(BoolValue &V, Environment &Env) { if (auto *Top = llvm::dyn_cast(&V)) { auto &A = Env.getDataflowAnalysisContext().arena(); @@ -73,6 +58,59 @@ static BoolValue &unpackValue(BoolValue &V, Environment &Env) { return V; } +static void propagateValue(const Expr &From, const Expr &To, Environment &Env) { + if (auto *Val = Env.getValue(From)) + Env.setValue(To, *Val); +} + +static void propagateStorageLocation(const Expr &From, const Expr &To, + Environment &Env) { + if (auto *Loc = Env.getStorageLocation(From)) + Env.setStorageLocation(To, *Loc); +} + +// Propagates the value or storage location of `From` to `To` in cases where +// `From` may be either a glvalue or a prvalue. `To` must be a glvalue iff +// `From` is a glvalue. +static void propagateValueOrStorageLocation(const Expr &From, const Expr &To, + Environment &Env) { + assert(From.isGLValue() == To.isGLValue()); + if (From.isGLValue()) + propagateStorageLocation(From, To, Env); + else + propagateValue(From, To, Env); +} + +namespace bool_model { + +BoolValue &freshBoolValue(Environment &Env) { + return Env.makeAtomicBoolValue(); +} + +BoolValue &rValueFromLValue(BoolValue &V, Environment &Env) { + return unpackValue(V, Env); +} + +BoolValue &logicalOrOp(BoolValue &LHS, BoolValue &RHS, Environment &Env) { + return Env.makeOr(LHS, RHS); +} + +BoolValue &logicalAndOp(BoolValue &LHS, BoolValue &RHS, Environment &Env) { + return Env.makeAnd(LHS, RHS); +} + +BoolValue &eqOp(BoolValue &LHS, BoolValue &RHS, Environment &Env) { + return Env.makeIff(LHS, RHS); +} + +BoolValue &neOp(BoolValue &LHS, BoolValue &RHS, Environment &Env) { + return Env.makeNot(Env.makeIff(LHS, RHS)); +} + +BoolValue ¬Op(BoolValue &Sub, Environment &Env) { return Env.makeNot(Sub); } + +} // namespace bool_model + // Unpacks the value (if any) associated with `E` and updates `E` to the new // value, if any unpacking occured. Also, does the lvalue-to-rvalue conversion, // by skipping past the reference. @@ -86,34 +124,45 @@ static Value *maybeUnpackLValueExpr(const Expr &E, Environment &Env) { if (B == nullptr) return Val; - auto &UnpackedVal = unpackValue(*B, Env); + auto &UnpackedVal = bool_model::rValueFromLValue(*B, Env); if (&UnpackedVal == Val) return Val; Env.setValue(*Loc, UnpackedVal); return &UnpackedVal; } -static void propagateValue(const Expr &From, const Expr &To, Environment &Env) { - if (auto *Val = Env.getValue(From)) - Env.setValue(To, *Val); +static BoolValue &evaluateEquality(const Expr &LHS, const Expr &RHS, + Environment &Env) { + Value *LHSValue = Env.getValue(LHS); + Value *RHSValue = Env.getValue(RHS); + + // Bug! + if (LHSValue == RHSValue) + return Env.getBoolLiteralValue(true); + + if (auto *LHSBool = dyn_cast_or_null(LHSValue)) + if (auto *RHSBool = dyn_cast_or_null(RHSValue)) + return bool_model::eqOp(*LHSBool, *RHSBool, Env); + + // TODO Why this eager construcoitn of an atomic? + return Env.makeAtomicBoolValue(); } -static void propagateStorageLocation(const Expr &From, const Expr &To, +static BoolValue &evaluateInequality(const Expr &LHS, const Expr &RHS, Environment &Env) { - if (auto *Loc = Env.getStorageLocation(From)) - Env.setStorageLocation(To, *Loc); -} + Value *LHSValue = Env.getValue(LHS); + Value *RHSValue = Env.getValue(RHS); -// Propagates the value or storage location of `From` to `To` in cases where -// `From` may be either a glvalue or a prvalue. `To` must be a glvalue iff -// `From` is a glvalue. -static void propagateValueOrStorageLocation(const Expr &From, const Expr &To, - Environment &Env) { - assert(From.isGLValue() == To.isGLValue()); - if (From.isGLValue()) - propagateStorageLocation(From, To, Env); - else - propagateValue(From, To, Env); + // Bug! + if (LHSValue == RHSValue) + return Env.getBoolLiteralValue(false); + + if (auto *LHSBool = dyn_cast_or_null(LHSValue)) + if (auto *RHSBool = dyn_cast_or_null(RHSValue)) + return bool_model::neOp(*LHSBool, *RHSBool, Env); + + // TODO Why this eager construcoitn of an atomic? + return Env.makeAtomicBoolValue(); } namespace { @@ -153,22 +202,20 @@ class TransferVisitor : public ConstStmtVisitor { BoolValue &RHSVal = getLogicOperatorSubExprValue(*RHS); if (S->getOpcode() == BO_LAnd) - Env.setValue(*S, Env.makeAnd(LHSVal, RHSVal)); + Env.setValue(*S, bool_model::logicalAndOp(LHSVal, RHSVal, Env)); else - Env.setValue(*S, Env.makeOr(LHSVal, RHSVal)); + Env.setValue(*S, bool_model::logicalOrOp(LHSVal, RHSVal, Env)); break; } case BO_NE: - case BO_EQ: { - auto &LHSEqRHSValue = evaluateBooleanEquality(*LHS, *RHS, Env); - Env.setValue(*S, S->getOpcode() == BO_EQ ? LHSEqRHSValue - : Env.makeNot(LHSEqRHSValue)); + Env.setValue(*S, evaluateInequality(*LHS, *RHS, Env)); break; - } - case BO_Comma: { + case BO_EQ: + Env.setValue(*S, evaluateEquality(*LHS, *RHS, Env)); + break; + case BO_Comma: propagateValueOrStorageLocation(*RHS, *S, Env); break; - } default: break; } @@ -273,7 +320,7 @@ class TransferVisitor : public ConstStmtVisitor { else // FIXME: If integer modeling is added, then update this code to create // the boolean based on the integer model. - Env.setValue(*S, Env.makeAtomicBoolValue()); + Env.setValue(*S, bool_model::freshBoolValue(Env)); break; } @@ -362,7 +409,7 @@ class TransferVisitor : public ConstStmtVisitor { if (SubExprVal == nullptr) break; - Env.setValue(*S, Env.makeNot(*SubExprVal)); + Env.setValue(*S, bool_model::notOp(*SubExprVal, Env)); break; } default: diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp index 595f70f819ddb..bea19776daba1 100644 --- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp +++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp @@ -284,7 +284,6 @@ computeBlockInputState(const CFGBlock &Block, AnalysisContext &AC) { Builder.addUnowned(PredState); continue; } - bool BranchVal = blockIndexInPredecessor(*Pred, Block) == 0; // `transferBranch` may need to mutate the environment to describe the @@ -297,6 +296,7 @@ computeBlockInputState(const CFGBlock &Block, AnalysisContext &AC) { // assert ourselves instead of asserting via `cast()` so that we get // a more meaningful line number if the assertion fails. assert(CondVal != nullptr); + // Invert the condition if this is the successor for the "else" branch. BoolValue *AssertedVal = BranchVal ? CondVal : &Copy.Env.makeNot(*CondVal); Copy.Env.assume(AssertedVal->formula()); @@ -465,7 +465,8 @@ transferCFGBlock(const CFGBlock &Block, AnalysisContext &AC, // we have *some* value for the condition expression. This ensures that // when we extend the flow condition, it actually changes. if (State.Env.getValue(*TerminatorCond) == nullptr) - State.Env.setValue(*TerminatorCond, State.Env.makeAtomicBoolValue()); + State.Env.setValue(*TerminatorCond, + bool_model::freshBoolValue(State.Env)); AC.Log.recordState(State); } diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 1d3b268976a76..3765bdcfb23d0 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "clang/Analysis/FlowSensitive/Transfer.h" #include "TestingSupport.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" @@ -35,6 +36,7 @@ using ::testing::Eq; using ::testing::IsNull; using ::testing::Ne; using ::testing::NotNull; +using ::testing::Optional; using ::testing::UnorderedElementsAre; // Declares a minimal coroutine library. @@ -3747,6 +3749,11 @@ TEST(TransferTest, AssignFromBoolLiteral) { }); } +// TODO: this looks like a change-detector test. And, I don't see the value of +// the three different cases. Why not 12? It's really not clear what we're +// getting at here. Is this a test for correct formula construction? If so, we +// should name it (and comment) rather than tieing it to assignment. +// Also, this should be "InitializeFrom...". TEST(TransferTest, AssignFromCompositeBoolExpression) { { std::string Code = R"( @@ -4412,12 +4419,12 @@ TEST(TransferTest, LoopWithAssignmentConverges) { bool foo(); void target() { - do { + do { bool Bar = foo(); if (Bar) break; (void)Bar; /*[[p]]*/ - } while (true); + } while (true); } )"; // The key property that we are verifying is implicit in `runDataflow` -- @@ -4434,6 +4441,8 @@ TEST(TransferTest, LoopWithAssignmentConverges) { ASSERT_THAT(BarDecl, NotNull()); auto &BarVal = getFormula(*BarDecl, Env); + ASSERT_EQ(BarVal.kind(), Formula::AtomRef); + ASSERT_THAT(Env.getAtomValue(BarVal.getAtom()), Optional(false)); EXPECT_TRUE(Env.proves(Env.arena().makeNot(BarVal))); }); } @@ -5115,6 +5124,10 @@ TEST(TransferTest, WhileStmtBranchExtendsFlowCondition) { }); } +// TODO: this test is overly complicated for what its name implies it's +// testing. It involves a complex condition of (A or B), where neither holds +// separately. But, that involves join machinery and SAT solving, which is well +// more than the simple test for DoWhile support in flow-condition extenionsion. TEST(TransferTest, DoWhileStmtBranchExtendsFlowCondition) { std::string Code = R"( void target(bool Foo) { diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp index bea00ab1a1f06..d12e79c7848dd 100644 --- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp @@ -286,6 +286,10 @@ TEST_F(DiscardExprStateTest, BooleanOperator) { const auto &AndOpState = blockStateForStmt(BlockStates, AndOp); EXPECT_EQ(AndOpState.Env.getValue(*AndOp.getLHS()), LHSValue); EXPECT_EQ(AndOpState.Env.getValue(*AndOp.getRHS()), RHSValue); + // FIXME: this test is too strict. We want to check equivalence not equality; + // as is, its a change detector test. Notice that we only evaluate `b1 && b2` + // in a context where we know that `b1` is true, so there's a potential + // optimization to store only `RHSValue` as the operation's value. EXPECT_EQ(AndOpState.Env.getValue(AndOp), &AndOpState.Env.makeAnd(*LHSValue, *RHSValue)); diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index 9430730004dbd..7011345053e9a 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -1382,9 +1382,10 @@ class UncheckedOptionalAccessTest INSTANTIATE_TEST_SUITE_P( UncheckedOptionalUseTestInst, UncheckedOptionalAccessTest, - ::testing::Values(OptionalTypeIdentifier{"std", "optional"}, - OptionalTypeIdentifier{"absl", "optional"}, - OptionalTypeIdentifier{"base", "Optional"}), + ::testing::Values(OptionalTypeIdentifier{"std", "optional"}// , + // OptionalTypeIdentifier{"absl", "optional"}, + // OptionalTypeIdentifier{"base", "Optional"} + ), [](const ::testing::TestParamInfo &Info) { return Info.param.NamespaceName; }); @@ -2954,7 +2955,7 @@ TEST_P(UncheckedOptionalAccessTest, CorrelatedBranches) { )"); } -TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) { +TEST_P(UncheckedOptionalAccessTest, JoinDistinctValuesThenCheck) { ExpectDiagnosticsFor( R"code( #include "unchecked_optional_access_test.h" @@ -2973,7 +2974,9 @@ TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) { } } )code"); +} +TEST_P(UncheckedOptionalAccessTest, JoinDistinctValuesCheckInBranches) { ExpectDiagnosticsFor(R"code( #include "unchecked_optional_access_test.h" @@ -2989,7 +2992,9 @@ TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) { opt.value(); } )code"); +} +TEST_P(UncheckedOptionalAccessTest, JoinDistinctValuesCheckInOneBranch) { ExpectDiagnosticsFor( R"code( #include "unchecked_optional_access_test.h" @@ -3005,7 +3010,9 @@ TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) { opt.value(); // [[unsafe]] } )code"); +} +TEST_P(UncheckedOptionalAccessTest, JoinDistinctValuesSetInBothBranches) { ExpectDiagnosticsFor( R"code( #include "unchecked_optional_access_test.h" @@ -3020,7 +3027,9 @@ TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) { opt.value(); } )code"); +} +TEST_P(UncheckedOptionalAccessTest, JoinDistinctValuesSetInOneBranch) { ExpectDiagnosticsFor( R"code( #include "unchecked_optional_access_test.h"