diff --git a/src/corelib/statemachine/qstatemachine.cpp b/src/corelib/statemachine/qstatemachine.cpp index 477fa83705..eec3febbfe 100644 --- a/src/corelib/statemachine/qstatemachine.cpp +++ b/src/corelib/statemachine/qstatemachine.cpp @@ -251,10 +251,17 @@ static QSet getEffectiveTargetStates(QAbstractTransition *tran foreach (QAbstractState *s, transition->targetStates()) { if (QHistoryState *historyState = QStateMachinePrivate::toHistoryState(s)) { QList historyConfiguration = QHistoryStatePrivate::get(historyState)->configuration; - if (!historyConfiguration.isEmpty()) + if (!historyConfiguration.isEmpty()) { + // There is a saved history, so apply that. targets.unite(historyConfiguration.toSet()); - else if (QAbstractState *defaultState = historyState->defaultState()) - targets.insert(defaultState); // Qt does not support initial transitions, but uses the default state of the history state for this. + } else if (QAbstractState *defaultState = historyState->defaultState()) { + // Qt does not support initial transitions, but uses the default state of the history state for this. + targets.insert(defaultState); + } else { + // Woops, we found a history state without a default state. That's not valid! + QStateMachinePrivate *m = QStateMachinePrivate::get(historyState->machine()); + m->setError(QStateMachine::NoDefaultStateInHistoryStateError, historyState); + } } else { targets.insert(s); } @@ -338,6 +345,15 @@ static int indexOfDescendant(QState *s, QAbstractState *desc) return -1; } +bool QStateMachinePrivate::transitionStateEntryLessThan(QAbstractTransition *t1, QAbstractTransition *t2) +{ + QState *s1 = t1->sourceState(), *s2 = t2->sourceState(); + if (s1 == s2) + return QStatePrivate::get(s1)->transitions().indexOf(t1) < QStatePrivate::get(s2)->transitions().indexOf(t2); + else + return stateEntryLessThan(t1->sourceState(), t2->sourceState()); +} + bool QStateMachinePrivate::stateEntryLessThan(QAbstractState *s1, QAbstractState *s2) { if (s1->parent() == s2->parent()) { @@ -401,40 +417,21 @@ QState *QStateMachinePrivate::findLCCA(const QList &states) con return findLCA(states, true); } -bool QStateMachinePrivate::isPreempted(const QAbstractState *s, const QSet &transitions) const -{ - QSet::const_iterator it; - for (it = transitions.constBegin(); it != transitions.constEnd(); ++it) { - QAbstractTransition *t = *it; - QList lst = t->targetStates(); - if (!lst.isEmpty()) { - lst.prepend(t->sourceState()); - QAbstractState *lca = findLCA(lst); - if (isDescendant(s, lca)) { -#ifdef QSTATEMACHINE_DEBUG - qDebug() << q_func() << ':' << transitions << "preempts selection of a transition from" - << s << "because" << s << "is a descendant of" << lca; -#endif - return true; - } - } - } - return false; -} - -QSet QStateMachinePrivate::selectTransitions(QEvent *event) const +QList QStateMachinePrivate::selectTransitions(QEvent *event) { Q_Q(const QStateMachine); - QSet enabledTransitions; - QSet::const_iterator it; + + QVarLengthArray configuration_sorted; + foreach (QAbstractState *s, configuration) { + if (isAtomic(s)) + configuration_sorted.append(s); + } + std::sort(configuration_sorted.begin(), configuration_sorted.end(), stateEntryLessThan); + + QList enabledTransitions; const_cast(q)->beginSelectTransitions(event); - for (it = configuration.constBegin(); it != configuration.constEnd(); ++it) { - QAbstractState *state = *it; - if (!isAtomic(state)) - continue; - if (isPreempted(state, enabledTransitions)) - continue; - QVector lst = getProperAncestors(state, rootState()->parentState()); + foreach (QAbstractState *state, configuration_sorted) { + QVector lst = getProperAncestors(state, Q_NULLPTR); if (QState *grp = toStandardState(state)) lst.prepend(grp); bool found = false; @@ -447,29 +444,94 @@ QSet QStateMachinePrivate::selectTransitions(QEvent *event #ifdef QSTATEMACHINE_DEBUG qDebug() << q << ": selecting transition" << t; #endif - enabledTransitions.insert(t); + enabledTransitions.append(t); found = true; break; } } } } + + if (!enabledTransitions.isEmpty()) { + removeConflictingTransitions(enabledTransitions); +#ifdef QSTATEMACHINE_DEBUG + qDebug() << q << ": enabled transitions after removing conflicts:" << enabledTransitions; +#endif + } const_cast(q)->endSelectTransitions(event); return enabledTransitions; } +/* The function as described in http://www.w3.org/TR/2014/WD-scxml-20140529/ : + +function removeConflictingTransitions(enabledTransitions): + filteredTransitions = new OrderedSet() + // toList sorts the transitions in the order of the states that selected them + for t1 in enabledTransitions.toList(): + t1Preempted = false; + transitionsToRemove = new OrderedSet() + for t2 in filteredTransitions.toList(): + if computeExitSet([t1]).hasIntersection(computeExitSet([t2])): + if isDescendant(t1.source, t2.source): + transitionsToRemove.add(t2) + else: + t1Preempted = true + break + if not t1Preempted: + for t3 in transitionsToRemove.toList(): + filteredTransitions.delete(t3) + filteredTransitions.add(t1) + + return filteredTransitions +*/ +void QStateMachinePrivate::removeConflictingTransitions(QList &enabledTransitions) +{ + QList filteredTransitions; + filteredTransitions.reserve(enabledTransitions.size()); + std::sort(enabledTransitions.begin(), enabledTransitions.end(), transitionStateEntryLessThan); + + Q_FOREACH (QAbstractTransition *t1, enabledTransitions) { + bool t1Preempted = false; + QVarLengthArray transitionsToRemove; + QSet exitSetT1 = computeExitSet_Unordered(QList() << t1); + Q_FOREACH (QAbstractTransition *t2, filteredTransitions) { + QSet exitSetT2 = computeExitSet_Unordered(QList() << t2); + if (!exitSetT1.intersect(exitSetT2).isEmpty()) { + if (isDescendant(t1->sourceState(), t2->sourceState())) { + transitionsToRemove.append(t2); + } else { + t1Preempted = true; + break; + } + } + } + if (!t1Preempted) { + Q_FOREACH (QAbstractTransition *t3, transitionsToRemove) + filteredTransitions.removeAll(t3); + filteredTransitions.append(t1); + } + } + + enabledTransitions = filteredTransitions; +} + void QStateMachinePrivate::microstep(QEvent *event, const QList &enabledTransitions) { #ifdef QSTATEMACHINE_DEBUG qDebug() << q_func() << ": begin microstep( enabledTransitions:" << enabledTransitions << ')'; qDebug() << q_func() << ": configuration before exiting states:" << configuration; #endif - QList exitedStates = computeStatesToExit(enabledTransitions); + QList exitedStates = computeExitSet(enabledTransitions); QHash pendingRestorables = computePendingRestorables(exitedStates); QSet statesForDefaultEntry; QList enteredStates = computeEntrySet(enabledTransitions, statesForDefaultEntry); +#ifdef QSTATEMACHINE_DEBUG + qDebug() << q_func() << ": computed exit set:" << exitedStates; + qDebug() << q_func() << ": computed entry set:" << enteredStates; +#endif + QHash > assignmentsForEnteredStates = computePropertyAssignments(enteredStates, pendingRestorables); if (!pendingRestorables.isEmpty()) { @@ -502,40 +564,65 @@ void QStateMachinePrivate::microstep(QEvent *event, const QList QStateMachinePrivate::computeStatesToExit(const QList &enabledTransitions) +/* The function as described in http://www.w3.org/TR/2014/WD-scxml-20140529/ : + +procedure computeExitSet(enabledTransitions) + +For each transition t in enabledTransitions, if t is targetless then do nothing, else compute the +transition's domain. (This will be the source state in the case of internal transitions) or the +least common compound ancestor state of the source state and target states of t (in the case of +external transitions. Add to the statesToExit set all states in the configuration that are +descendants of the domain. + +function computeExitSet(transitions) + statesToExit = new OrderedSet + for t in transitions: + if (t.target): + domain = getTransitionDomain(t) + for s in configuration: + if isDescendant(s,domain): + statesToExit.add(s) + return statesToExit +*/ +QList QStateMachinePrivate::computeExitSet(const QList &enabledTransitions) { - QSet statesToExit; -// QSet statesToSnapshot; - for (int i = 0; i < enabledTransitions.size(); ++i) { - QAbstractTransition *t = enabledTransitions.at(i); - QList lst = t->targetStates(); - if (lst.isEmpty()) - continue; - lst.prepend(t->sourceState()); - QAbstractState *lca = findLCA(lst); - if (lca == 0) { - setError(QStateMachine::NoCommonAncestorForTransitionError, t->sourceState()); - lst = pendingErrorStates.toList(); - lst.prepend(t->sourceState()); - - lca = findLCA(lst); - Q_ASSERT(lca != 0); - } - - { - QSet::const_iterator it; - for (it = configuration.constBegin(); it != configuration.constEnd(); ++it) { - QAbstractState *s = *it; - if (isDescendant(s, lca)) - statesToExit.insert(s); - } - } - } - QList statesToExit_sorted = statesToExit.toList(); + QList statesToExit_sorted = computeExitSet_Unordered(enabledTransitions).toList(); std::sort(statesToExit_sorted.begin(), statesToExit_sorted.end(), stateExitLessThan); return statesToExit_sorted; } +QSet QStateMachinePrivate::computeExitSet_Unordered(const QList &enabledTransitions) +{ + QSet statesToExit; + for (int i = 0; i < enabledTransitions.size(); ++i) { + QAbstractTransition *t = enabledTransitions.at(i); + QList effectiveTargetStates = getEffectiveTargetStates(t).toList(); + QAbstractState *domain = getTransitionDomain(t, effectiveTargetStates); + if (domain == Q_NULLPTR && !t->targetStates().isEmpty()) { + // So we didn't find the least common ancestor for the source and target states of the + // transition. If there were not target states, that would be fine: then the transition + // will fire any events or signals, but not exit the state. + // + // However, there are target states, so it's either a node without a parent (or parent's + // parent, etc), or the state belongs to a different state machine. Either way, this + // makes the state machine invalid. + if (error == QStateMachine::NoError) + setError(QStateMachine::NoCommonAncestorForTransitionError, t->sourceState()); + QList lst = pendingErrorStates.toList(); + lst.prepend(t->sourceState()); + + domain = findLCCA(lst); + Q_ASSERT(domain != 0); + } + + Q_FOREACH (QAbstractState* s, configuration) { + if (isDescendant(s, domain)) + statesToExit.insert(s); + } + } + return statesToExit; +} + void QStateMachinePrivate::exitStates(QEvent *event, const QList &statesToExit_sorted, const QHash > &assignmentsForEnteredStates) { @@ -602,9 +689,7 @@ QList QStateMachinePrivate::computeEntrySet(const QListtargetStates()) { addDescendantStatesToEnter(s, statesToEnter, statesForDefaultEntry); } -#ifdef QSTATEMACHINE_DEBUG - qDebug() << "computed entry set after descendants:" << statesToEnter; -#endif + QList effectiveTargetStates = getEffectiveTargetStates(t).toList(); QAbstractState *ancestor = getTransitionDomain(t, effectiveTargetStates); Q_FOREACH (QAbstractState *s, effectiveTargetStates) { @@ -643,7 +728,7 @@ function getTransitionDomain(t) else: return findLCCA([t.source].append(tstates)) */ -QAbstractState *QStateMachinePrivate::getTransitionDomain(QAbstractTransition *t, const QList &effectiveTargetStates) +QAbstractState *QStateMachinePrivate::getTransitionDomain(QAbstractTransition *t, const QList &effectiveTargetStates) const { if (effectiveTargetStates.isEmpty()) return 0; @@ -1285,6 +1370,9 @@ void QStateMachinePrivate::setError(QStateMachine::Error errorCode, QAbstractSta Q_ASSERT(currentErrorState != rootState()); if (currentErrorState != 0) { +#ifdef QSTATEMACHINE_DEBUG + qDebug() << q << ": entering error state" << currentErrorState << "from" << currentContext; +#endif QState *lca = findLCA(QList() << currentErrorState << currentContext); addStatesToEnter(currentErrorState, lca, pendingErrorStates, pendingErrorStatesForDefaultEntry); } else { @@ -1640,7 +1728,7 @@ void QStateMachinePrivate::_q_process() processing = false; break; } - QSet enabledTransitions; + QList enabledTransitions; QEvent *e = new QEvent(QEvent::None); enabledTransitions = selectTransitions(e); if (enabledTransitions.isEmpty()) { @@ -1676,7 +1764,7 @@ void QStateMachinePrivate::_q_process() } if (!enabledTransitions.isEmpty()) { q->beginMicrostep(e); - microstep(e, enabledTransitions.toList()); + microstep(e, enabledTransitions); q->endMicrostep(e); } #ifdef QSTATEMACHINE_DEBUG diff --git a/src/corelib/statemachine/qstatemachine_p.h b/src/corelib/statemachine/qstatemachine_p.h index a8f0745730..a88f95f1f5 100644 --- a/src/corelib/statemachine/qstatemachine_p.h +++ b/src/corelib/statemachine/qstatemachine_p.h @@ -103,6 +103,7 @@ public: QState *findLCA(const QList &states, bool onlyCompound = false) const; QState *findLCCA(const QList &states) const; + static bool transitionStateEntryLessThan(QAbstractTransition *t1, QAbstractTransition *t2); static bool stateEntryLessThan(QAbstractState *s1, QAbstractState *s2); static bool stateExitLessThan(QAbstractState *s1, QAbstractState *s2); @@ -123,12 +124,13 @@ public: void clearHistory(); QAbstractTransition *createInitialTransition() const; + void removeConflictingTransitions(QList &enabledTransitions); void microstep(QEvent *event, const QList &transitionList); - bool isPreempted(const QAbstractState *s, const QSet &transitions) const; - QSet selectTransitions(QEvent *event) const; + QList selectTransitions(QEvent *event); void exitStates(QEvent *event, const QList &statesToExit_sorted, const QHash > &assignmentsForEnteredStates); - QList computeStatesToExit(const QList &enabledTransitions); + QList computeExitSet(const QList &enabledTransitions); + QSet computeExitSet_Unordered(const QList &enabledTransitions); void executeTransitionContent(QEvent *event, const QList &transitionList); void enterStates(QEvent *event, const QList &exitedStates_sorted, const QList &statesToEnter_sorted, @@ -141,7 +143,7 @@ public: QList computeEntrySet(const QList &enabledTransitions, QSet &statesForDefaultEntry); QAbstractState *getTransitionDomain(QAbstractTransition *t, - const QList &effectiveTargetStates); + const QList &effectiveTargetStates) const; void addDescendantStatesToEnter(QAbstractState *state, QSet &statesToEnter, QSet &statesForDefaultEntry); diff --git a/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp b/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp index 605d1e7393..96d0a62f6b 100644 --- a/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp +++ b/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp @@ -246,6 +246,7 @@ private slots: void childModeConstructor(); void qtbug_44963(); + void qtbug_44783(); }; class TestState : public QState @@ -910,8 +911,11 @@ void tst_QStateMachine::historyStateHasNowhereToGo() QStateMachine machine; QState *initialState = new QState(&machine); + initialState->setObjectName("initialState"); machine.setInitialState(initialState); - machine.setErrorState(new QState(&machine)); // avoid warnings + QState *errorState = new QState(&machine); + errorState->setObjectName("errorState"); + machine.setErrorState(errorState); // avoid warnings QState *brokenState = new QState(&machine); brokenState->setObjectName("brokenState"); @@ -919,7 +923,9 @@ void tst_QStateMachine::historyStateHasNowhereToGo() QHistoryState *historyState = new QHistoryState(brokenState); historyState->setObjectName("historyState"); - initialState->addTransition(new EventTransition(QEvent::User, historyState)); + EventTransition *t = new EventTransition(QEvent::User, historyState); + t->setObjectName("initialState->historyState"); + initialState->addTransition(t); machine.start(); QCoreApplication::processEvents(); @@ -4274,6 +4280,11 @@ void tst_QStateMachine::transitionsFromParallelStateWithNoChildren() void tst_QStateMachine::parallelStateTransition() { + // This test checks if the parallel state is exited and re-entered if one compound state + // is exited and subsequently re-entered. When the parallel state is exited, the other compound + // state in the parallel state has to be exited too. When the parallel state is re-entered, the + // other state also needs to be re-entered. + QStateMachine machine; QState *parallelState = new QState(QState::ParallelStates, &machine); @@ -4302,12 +4313,16 @@ void tst_QStateMachine::parallelStateTransition() s1OtherChild->setObjectName("s1OtherChild"); DEFINE_ACTIVE_SPY(s1OtherChild); + // The following transition will exit s1 (which means that parallelState is also exited), and + // subsequently re-entered (which means that parallelState is also re-entered). EventTransition *et = new EventTransition(QEvent::User, s1OtherChild); et->setObjectName("s1->s1OtherChild"); s1->addTransition(et); machine.start(); QCoreApplication::processEvents(); + + // Initial entrance of the parallel state and its sub-states: TEST_ACTIVE_CHANGED(parallelState, 1); TEST_ACTIVE_CHANGED(s1, 1); TEST_ACTIVE_CHANGED(s1InitialChild, 1); @@ -4325,22 +4340,22 @@ void tst_QStateMachine::parallelStateTransition() machine.postEvent(new QEvent(QEvent::User)); QCoreApplication::processEvents(); - TEST_ACTIVE_CHANGED(parallelState, 1); - TEST_ACTIVE_CHANGED(s1, 3); - TEST_ACTIVE_CHANGED(s1InitialChild, 2); - TEST_ACTIVE_CHANGED(s2, 3); - TEST_ACTIVE_CHANGED(s2InitialChild, 3); - TEST_ACTIVE_CHANGED(s1OtherChild, 1); + TEST_ACTIVE_CHANGED(parallelState, 3); // initial + exit + entry + TEST_ACTIVE_CHANGED(s1, 3); // initial + exit + entry + TEST_ACTIVE_CHANGED(s1InitialChild, 2); // initial + exit + TEST_ACTIVE_CHANGED(s2, 3); // initial + exit due to parent exit + entry due to parent re-entry + TEST_ACTIVE_CHANGED(s2InitialChild, 3); // initial + exit due to parent exit + re-entry due to parent re-entry + TEST_ACTIVE_CHANGED(s1OtherChild, 1); // entry due to transition QVERIFY(machine.isRunning()); + // Check that s1InitialChild is not in the configuration, because although s1 is re-entered, + // another child state (s1OtherChild) is active, so the initial child should not be activated. QVERIFY(machine.configuration().contains(parallelState)); - QVERIFY(machine.configuration().contains(s1)); QVERIFY(machine.configuration().contains(s2)); QVERIFY(machine.configuration().contains(s1OtherChild)); QVERIFY(machine.configuration().contains(s2InitialChild)); QCOMPARE(machine.configuration().size(), 5); - } void tst_QStateMachine::nestedRestoreProperties() @@ -6256,5 +6271,69 @@ void tst_QStateMachine::qtbug_44963() QVERIFY(machine.isRunning()); } +void tst_QStateMachine::qtbug_44783() +{ + SignalEmitter emitter; + + QStateMachine machine; + QState s(&machine); + QState p(QState::ParallelStates, &s); + QState p1(&p); + QState p1_1(&p1); + QState p1_2(&p1); + QState p2(&p); + QState s1(&machine); + + machine.setInitialState(&s); + s.setInitialState(&p); + p1.setInitialState(&p1_1); + p1_1.addTransition(&emitter, SIGNAL(signalWithNoArg()), &p1_2)->setObjectName("p1_1->p1_2"); + p2.addTransition(&emitter, SIGNAL(signalWithNoArg()), &s1)->setObjectName("p2->s1"); + + s.setObjectName("s"); + p.setObjectName("p"); + p1.setObjectName("p1"); + p1_1.setObjectName("p1_1"); + p1_2.setObjectName("p1_2"); + p2.setObjectName("p2"); + s1.setObjectName("s1"); + + machine.start(); + + QTRY_COMPARE(machine.configuration().contains(&s), true); + QTRY_COMPARE(machine.configuration().contains(&p), true); + QTRY_COMPARE(machine.configuration().contains(&p1), true); + QTRY_COMPARE(machine.configuration().contains(&p1_1), true); + QTRY_COMPARE(machine.configuration().contains(&p1_2), false); + QTRY_COMPARE(machine.configuration().contains(&p2), true); + QTRY_COMPARE(machine.configuration().contains(&s1), false); + + emitter.emitSignalWithNoArg(); + + // Only one of the following two can be true, because the two possible transitions conflict. + if (machine.configuration().contains(&s1)) { + // the transition p2 -> s1 was taken, not p1_1 -> p1_2, so: + // the parallel state exited, so none of the states inside it are active + QTRY_COMPARE(machine.configuration().contains(&s), false); + QTRY_COMPARE(machine.configuration().contains(&p), false); + QTRY_COMPARE(machine.configuration().contains(&p1), false); + QTRY_COMPARE(machine.configuration().contains(&p1_1), false); + QTRY_COMPARE(machine.configuration().contains(&p1_2), false); + QTRY_COMPARE(machine.configuration().contains(&p2), false); + } else { + // the transition p1_1 -> p1_2 was taken, not p2 -> s1, so: + // the parallel state was not exited and the state is the same as the start state with one + // difference: p1_1 inactive and p1_2 active: + QTRY_COMPARE(machine.configuration().contains(&s), true); + QTRY_COMPARE(machine.configuration().contains(&p), true); + QTRY_COMPARE(machine.configuration().contains(&p1), true); + QTRY_COMPARE(machine.configuration().contains(&p1_1), false); + QTRY_COMPARE(machine.configuration().contains(&p1_2), true); + QTRY_COMPARE(machine.configuration().contains(&p2), true); + } + + QVERIFY(machine.isRunning()); +} + QTEST_MAIN(tst_QStateMachine) #include "tst_qstatemachine.moc"