diff --git a/src/Menus.cpp b/src/Menus.cpp index b7e6acaa9..46d030459 100644 --- a/src/Menus.cpp +++ b/src/Menus.cpp @@ -2909,13 +2909,16 @@ void AudacityProject::SortTracks(int flags) { size_t ndx = 0; int cmpValue; + // This one place outside of TrackList where we must use undisguised + // std::list iterators! Avoid this elsewhere! std::vector arr; arr.reserve(mTracks->GetCount()); bool lastTrackLinked = false; //sort by linked tracks. Assumes linked track follows owner in list. // First find the permutation. - for (auto iter = mTracks->begin(), end = mTracks->end(); iter != end; ++iter) { + for (auto iter = mTracks->ListOfTracks::begin(), + end = mTracks->ListOfTracks::end(); iter != end; ++iter) { const auto &track = *iter; if(lastTrackLinked) { //insert after the last track since this track should be linked to it. @@ -6242,7 +6245,7 @@ int AudacityProject::FindClips(double t0, double t1, bool next, std::vectorend() != std::find_if(tracks->begin(), tracks->end(), - [] (const std::shared_ptr& t) { + [] (const Track * t) { return t->GetSelected() && t->GetKind() == Track::Wave; }); // first search the tracks individually @@ -7320,7 +7323,7 @@ int AudacityProject::FindClipBoundaries(double time, bool next, std::vectorend() != std::find_if(tracks->begin(), tracks->end(), - [] (const std::shared_ptr& t) { + [] (const Track *t) { return t->GetSelected() && t->GetKind() == Track::Wave; }); diff --git a/src/Track.cpp b/src/Track.cpp index 07b8d8e1b..f84895ee1 100644 --- a/src/Track.cpp +++ b/src/Track.cpp @@ -321,12 +321,12 @@ bool PlayableTrack::HandleXMLAttribute(const wxChar *attr, const wxChar *value) } // TrackListIterator -TrackListIterator::TrackListIterator(TrackList * val) - : l(val) - , cur{} +TrackListIterator::TrackListIterator(TrackList * val, TrackNodePointer p) + : l{ val } + , cur{ p } { - if (l) - cur = l->begin(); + if ( l && p == TrackNodePointer{} ) + cur = l->ListOfTracks::begin(); } Track *TrackListIterator::StartWith(Track * val) @@ -356,7 +356,7 @@ Track *TrackListIterator::First(TrackList * val) return NULL; } - cur = l->begin(); + cur = l->ListOfTracks::begin(); if (!l->isNull(cur)) { return cur->get(); @@ -371,7 +371,7 @@ Track *TrackListIterator::Last(bool skiplinked) return NULL; } - cur = l->end(); + cur = l->ListOfTracks::end(); if (l->hasPrev(cur)) --cur; else @@ -442,6 +442,14 @@ Track *TrackListIterator::Prev(bool skiplinked) return cur->get(); } +Track *TrackListIterator::operator *() const +{ + if ( !l || l->isNull( cur ) ) + return nullptr; + else + return cur->get(); +} + Track *TrackListIterator::RemoveCurrent() { if (!l || l->isNull(cur)) @@ -460,6 +468,21 @@ Track *TrackListIterator::RemoveCurrent() return NULL; } +bool TrackListIterator::operator == (const TrackListIterator &other) const +{ + if (cur == other.cur) + // sufficient but not necessary + return true; + + // Also return true whenever each contains either an end iterator or is + // in default-constructed state. + bool isEnd = !l || l->isNull( cur ); + if (isEnd) + return !other.l || other.l->isNull( other.cur ); + + return false; +} + // // TrackListCondIterator (base class for iterators that iterate over all tracks // that meet a condition) @@ -741,9 +764,11 @@ void TrackList::DoAssign(const TrackList &that) void TrackList::Swap(TrackList &that) { ListOfTracks::swap(that); - for (auto it = begin(), last = end(); it != last; ++it) + for (auto it = ListOfTracks::begin(), last = ListOfTracks::end(); + it != last; ++it) (*it)->SetOwner(this->mSelf, it); - for (auto it = that.begin(), last = that.end(); it != last; ++it) + for (auto it = that.ListOfTracks::begin(), last = that.ListOfTracks::end(); + it != last; ++it) (*it)->SetOwner(that.mSelf, it); } @@ -770,8 +795,8 @@ void TrackList::RecalcPositions(TrackNodePointer node) } const auto theEnd = end(); - for (auto n = node; n != theEnd; ++n) { - t = n->get(); + for (auto n = TrackListIterator{ this, node }; n != theEnd; ++n) { + t = *n; t->SetIndex(i++); t->SetY(y); y += t->GetHeight(); @@ -803,12 +828,12 @@ void TrackList::ResizingEvent(TrackNodePointer node) void TrackList::Permute(const std::vector &permutation) { for (const auto iter : permutation) { - value_type track = std::move(*iter); + ListOfTracks::value_type track = std::move(*iter); erase(iter); Track *pTrack = track.get(); - pTrack->SetOwner(mSelf, insert(end(), std::move(track))); + pTrack->SetOwner(mSelf, insert(ListOfTracks::end(), std::move(track))); } - auto n = begin(); + auto n = ListOfTracks::begin(); RecalcPositions(n); PermutationEvent(); } @@ -817,8 +842,8 @@ template Track *TrackList::Add(std::unique_ptr &&t) { Track *pTrack; - push_back(value_type(pTrack = t.release())); - auto n = end(); + push_back(ListOfTracks::value_type(pTrack = t.release())); + auto n = ListOfTracks::end(); --n; pTrack->SetOwner(mSelf, n); RecalcPositions(n); @@ -839,8 +864,8 @@ template Track *TrackList::AddToHead(std::unique_ptr &&t) { Track *pTrack; - push_front(value_type(pTrack = t.release())); - auto n = begin(); + push_front(ListOfTracks::value_type(pTrack = t.release())); + auto n = ListOfTracks::begin(); pTrack->SetOwner(mSelf, n); RecalcPositions(n); ResizingEvent(n); @@ -854,7 +879,7 @@ template Track *TrackList::Add(std::shared_ptr &&t) { push_back(t); - auto n = end(); + auto n = ListOfTracks::end(); --n; t->SetOwner(mSelf, n); RecalcPositions(n); @@ -866,9 +891,10 @@ Track *TrackList::Add(std::shared_ptr &&t) template Track *TrackList::Add(std::shared_ptr &&); template Track *TrackList::Add(std::shared_ptr &&); -auto TrackList::Replace(Track * t, value_type &&with) -> value_type +auto TrackList::Replace(Track * t, ListOfTracks::value_type &&with) -> + ListOfTracks::value_type { - value_type holder; + ListOfTracks::value_type holder; if (t && with) { auto node = t->GetNode(); t->SetOwner({}, {}); @@ -888,13 +914,13 @@ auto TrackList::Replace(Track * t, value_type &&with) -> value_type TrackNodePointer TrackList::Remove(Track *t) { - TrackNodePointer result(end()); + auto result = ListOfTracks::end(); if (t) { auto node = t->GetNode(); t->SetOwner({}, {}); if (!isNull(node)) { - value_type holder = std::move( *node ); + ListOfTracks::value_type holder = std::move( *node ); result = erase(node); if (!isNull(result)) { @@ -1052,7 +1078,7 @@ void TrackList::SwapNodes(TrackNodePointer s1, TrackNodePointer s2) } // Remove tracks - value_type save11 = std::move(*s1), save12{}; + ListOfTracks::value_type save11 = std::move(*s1), save12{}; s1 = erase(s1); if (linked1) { wxASSERT(s1 != s2); @@ -1060,7 +1086,7 @@ void TrackList::SwapNodes(TrackNodePointer s1, TrackNodePointer s2) } const bool same = (s1 == s2); - value_type save21 = std::move(*s2), save22{}; + ListOfTracks::value_type save21 = std::move(*s2), save22{}; s2 = erase(s2); if (linked2) save22 = std::move(*s2), s2 = erase(s2); @@ -1112,9 +1138,7 @@ bool TrackList::MoveDown(Track * t) bool TrackList::Contains(const Track * t) const { - return std::find_if(begin(), end(), - [=](const value_type &track) { return t == track.get(); } - ) != end(); + return make_iterator_range( *this ).contains( t ); } bool TrackList::IsEmpty() const @@ -1136,12 +1160,12 @@ int TrackList::GetCount() const TimeTrack *TrackList::GetTimeTrack() { auto iter = std::find_if(begin(), end(), - [] (const value_type &t) { return t->GetKind() == Track::Time; } + [] ( Track *t ) { return t->GetKind() == Track::Time; } ); if (iter == end()) return nullptr; else - return static_cast(iter->get()); + return static_cast(*iter); } const TimeTrack *TrackList::GetTimeTrack() const @@ -1214,7 +1238,7 @@ unsigned TrackList::GetNumExportChannels(bool selectionOnly) const namespace { template - Array GetWaveTracks(ListOfTracks::const_iterator p, ListOfTracks::const_iterator end, + Array GetWaveTracks(TrackListIterator p, const TrackListIterator end, bool selectionOnly, bool includeMuted) { Array waveTrackArray; @@ -1225,8 +1249,7 @@ namespace { if (track->GetKind() == Track::Wave && (includeMuted || !wt->GetMute()) && (track->GetSelected() || !selectionOnly)) { - waveTrackArray.push_back( - std::static_pointer_cast(track)); + waveTrackArray.push_back( Track::Pointer< WaveTrack >( track ) ); } } @@ -1241,7 +1264,9 @@ WaveTrackArray TrackList::GetWaveTrackArray(bool selectionOnly, bool includeMute WaveTrackConstArray TrackList::GetWaveTrackConstArray(bool selectionOnly, bool includeMuted) const { - return GetWaveTracks(begin(), end(), selectionOnly, includeMuted); + auto list = const_cast(this); + return GetWaveTracks( + list->begin(), list->end(), selectionOnly, includeMuted); } #if defined(USE_MIDI) @@ -1252,7 +1277,7 @@ NoteTrackArray TrackList::GetNoteTrackArray(bool selectionOnly) for(const auto &track : *this) { if (track->GetKind() == Track::Note && (track->GetSelected() || !selectionOnly)) { - noteTrackArray.push_back(std::static_pointer_cast(track)); + noteTrackArray.push_back( Track::Pointer(track) ); } } diff --git a/src/Track.h b/src/Track.h index 8f05c8792..d40a0db47 100644 --- a/src/Track.h +++ b/src/Track.h @@ -337,13 +337,19 @@ protected: }; class AUDACITY_DLL_API TrackListIterator /* not final */ +: public std::iterator< std::forward_iterator_tag, Track *const > { public: - TrackListIterator(TrackList * val = NULL); + // The default-constructed value can serve as the end iterator for + // traversal over any track list. + TrackListIterator() {} + explicit TrackListIterator(TrackList * val, TrackNodePointer p = {}); + TrackListIterator(const TrackListIterator&) = default; + TrackListIterator& operator=(const TrackListIterator&) = default; virtual ~TrackListIterator() {} // Iterate functions - virtual Track *First(TrackList * val = NULL); + virtual Track *First(TrackList * val = nullptr); virtual Track *StartWith(Track * val); virtual Track *Next(bool skiplinked = false); virtual Track *Prev(bool skiplinked = false); @@ -351,19 +357,38 @@ class AUDACITY_DLL_API TrackListIterator /* not final */ Track *RemoveCurrent(); // deletes track, returns next + // Provide minimal STL forward-iterator idiom: + + // unlike Next, this is non-mutating. + // An end iterator may be safely dereferenced, returning nullptr. + Track *operator * () const; + + TrackListIterator &operator++ () { (void) Next(); return *this; } + + bool operator == (const TrackListIterator &other) const; + bool operator != (const TrackListIterator &other) const + { return !(*this == other); } + protected: friend TrackList; - TrackList *l; + TrackList *l {}; TrackNodePointer cur{}; }; class AUDACITY_DLL_API TrackListConstIterator +: public std::iterator< std::forward_iterator_tag, const Track *const > { public: - TrackListConstIterator(const TrackList * val = NULL) - : mIter(const_cast(val)) + // The default-constructed value can serve as the end iterator for + // traversal over any track list. + TrackListConstIterator() {} + explicit TrackListConstIterator( + const TrackList * val, TrackNodePointer p = {}) + : mIter(const_cast(val), p) {} + TrackListConstIterator(const TrackListConstIterator&) = default; + TrackListConstIterator& operator=(const TrackListConstIterator&) = default; ~TrackListConstIterator() {} // Iterate functions @@ -378,6 +403,19 @@ public: const Track *Last(bool skiplinked = false) { return mIter.Last(skiplinked); } + // Provide minimal STL forward-iterator idiom: + + // unlike Next, this is non-mutating. + // An end iterator may be safely dereferenced, returning nullptr. + const Track *operator * () const { return *mIter; } + + TrackListConstIterator &operator++ () { (void) Next(); return *this; } + + bool operator == (const TrackListConstIterator &other) const + { return mIter == other.mIter; } + bool operator != (const TrackListConstIterator &other) const + { return !(*this == other); } + private: TrackListIterator mIter; }; @@ -532,6 +570,17 @@ class TrackList final : public wxEvtHandler, public ListOfTracks // Destructor virtual ~TrackList(); + // Hide the inherited begin() and end() + using iterator = TrackListIterator; + using const_iterator = TrackListConstIterator; + using value_type = Track *; + iterator begin() { return iterator{ this, ListOfTracks::begin() }; } + iterator end() { return {}; } + const_iterator begin() const { return const_iterator{ this }; } + const_iterator end() const { return {}; } + const_iterator cbegin() const { return begin(); } + const_iterator cend() const { return end(); } + friend class Track; friend class TrackListIterator; friend class SyncLockedTracksIterator; @@ -549,7 +598,7 @@ class TrackList final : public wxEvtHandler, public ListOfTracks Track *Add(std::shared_ptr &&t); /// Replace first track with second track, give back a holder - value_type Replace(Track * t, value_type &&with); + ListOfTracks::value_type Replace(Track * t, ListOfTracks::value_type &&with); /// Remove this Track or all children of this TrackList. /// Return an iterator to what followed the removed track. @@ -629,11 +678,11 @@ class TrackList final : public wxEvtHandler, public ListOfTracks private: bool isNull(TrackNodePointer p) const - { return p == end(); } + { return p == TrackNodePointer{} || p == ListOfTracks::end(); } void setNull(TrackNodePointer &p) - { p = end(); } + { p = ListOfTracks::end(); } bool hasPrev(TrackNodePointer p) const - { return p != begin(); } + { return p != ListOfTracks::begin(); } void DoAssign(const TrackList &that); diff --git a/src/effects/Effect.cpp b/src/effects/Effect.cpp index 1eed69969..3a4b5a6e9 100644 --- a/src/effects/Effect.cpp +++ b/src/effects/Effect.cpp @@ -2170,7 +2170,8 @@ void Effect::ReplaceProcessedTracks(const bool bGoodResult) // Assume resources need to be freed. wxASSERT(mOutputTracks); // Make sure we at least did the CopyInputTracks(). - auto iterOut = mOutputTracks->begin(), iterEnd = mOutputTracks->end(); + auto iterOut = mOutputTracks->ListOfTracks::begin(), + iterEnd = mOutputTracks->ListOfTracks::end(); size_t cnt = mOMap.size(); size_t i = 0;