Revert "Provide STL idiom for iterating tracks..."

Unsafe on MSVC 2013.

src/Menus.cpp

@@ -2909,16 +2909,13 @@ 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<ListOfTracks::iterator> 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->ListOfTracks::begin(),
+   for (auto iter = mTracks->begin(), end = mTracks->end(); iter != end; ++iter) {
-        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.
@@ -6245,7 +6242,7 @@ int AudacityProject::FindClips(double t0, double t1, bool next, std::vector<Foun
 
    bool anyWaveTracksSelected =
       tracks->end() != std::find_if(tracks->begin(), tracks->end(),
-         [] (const Track * t) {
+         [] (const std::shared_ptr<Track>& t) {
             return t->GetSelected() && t->GetKind() == Track::Wave; });
 
    // first search the tracks individually
@@ -7323,7 +7320,7 @@ int AudacityProject::FindClipBoundaries(double time, bool next, std::vector<Foun
 
    bool anyWaveTracksSelected =
       tracks->end() != std::find_if(tracks->begin(), tracks->end(),
-         [] (const Track *t) {
+         [] (const std::shared_ptr<Track>& t) {
             return t->GetSelected() && t->GetKind() == Track::Wave; });
 
 

src/Track.cpp

@@ -321,12 +321,12 @@ bool PlayableTrack::HandleXMLAttribute(const wxChar *attr, const wxChar *value)
 }
 
 // TrackListIterator
-TrackListIterator::TrackListIterator(TrackList * val, TrackNodePointer p)
+TrackListIterator::TrackListIterator(TrackList * val)
-   : l{ val }
+   : l(val)
-   , cur{ p }
+   , cur{}
 {
-   if ( l && p == TrackNodePointer{} )
+   if (l)
-      cur = l->ListOfTracks::begin();
+      cur = l->begin();
 }
 
 Track *TrackListIterator::StartWith(Track * val)
@@ -356,7 +356,7 @@ Track *TrackListIterator::First(TrackList * val)
       return NULL;
    }
 
-   cur = l->ListOfTracks::begin();
+   cur = l->begin();
 
    if (!l->isNull(cur)) {
       return cur->get();
@@ -371,7 +371,7 @@ Track *TrackListIterator::Last(bool skiplinked)
       return NULL;
    }
 
-   cur = l->ListOfTracks::end();
+   cur = l->end();
    if (l->hasPrev(cur))
       --cur;
    else
@@ -442,14 +442,6 @@ 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))
@@ -468,21 +460,6 @@ 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)
@@ -764,11 +741,9 @@ void TrackList::DoAssign(const TrackList &that)
 void TrackList::Swap(TrackList &that)
 {
    ListOfTracks::swap(that);
-   for (auto it = ListOfTracks::begin(), last = ListOfTracks::end();
+   for (auto it = begin(), last = end(); it != last; ++it)
-        it != last; ++it)
       (*it)->SetOwner(this->mSelf, it);
-   for (auto it = that.ListOfTracks::begin(), last = that.ListOfTracks::end();
+   for (auto it = that.begin(), last = that.end(); it != last; ++it)
-        it != last; ++it)
       (*it)->SetOwner(that.mSelf, it);
 }
 
@@ -795,8 +770,8 @@ void TrackList::RecalcPositions(TrackNodePointer node)
    }
 
    const auto theEnd = end();
-   for (auto n = TrackListIterator{ this, node }; n != theEnd; ++n) {
+   for (auto n = node; n != theEnd; ++n) {
-      t = *n;
+      t = n->get();
       t->SetIndex(i++);
       t->SetY(y);
       y += t->GetHeight();
@@ -828,12 +803,12 @@ void TrackList::ResizingEvent(TrackNodePointer node)
 void TrackList::Permute(const std::vector<TrackNodePointer> &permutation)
 {
    for (const auto iter : permutation) {
-      ListOfTracks::value_type track = std::move(*iter);
+      value_type track = std::move(*iter);
       erase(iter);
       Track *pTrack = track.get();
-      pTrack->SetOwner(mSelf, insert(ListOfTracks::end(), std::move(track)));
+      pTrack->SetOwner(mSelf, insert(end(), std::move(track)));
    }
-   auto n = ListOfTracks::begin();
+   auto n = begin();
    RecalcPositions(n);
    PermutationEvent();
 }
@@ -842,8 +817,8 @@ template<typename TrackKind>
 Track *TrackList::Add(std::unique_ptr<TrackKind> &&t)
 {
    Track *pTrack;
-   push_back(ListOfTracks::value_type(pTrack = t.release()));
+   push_back(value_type(pTrack = t.release()));
-   auto n = ListOfTracks::end();
+   auto n = end();
    --n;
    pTrack->SetOwner(mSelf, n);
    RecalcPositions(n);
@@ -864,8 +839,8 @@ template<typename TrackKind>
 Track *TrackList::AddToHead(std::unique_ptr<TrackKind> &&t)
 {
    Track *pTrack;
-   push_front(ListOfTracks::value_type(pTrack = t.release()));
+   push_front(value_type(pTrack = t.release()));
-   auto n = ListOfTracks::begin();
+   auto n = begin();
    pTrack->SetOwner(mSelf, n);
    RecalcPositions(n);
    ResizingEvent(n);
@@ -879,7 +854,7 @@ template<typename TrackKind>
 Track *TrackList::Add(std::shared_ptr<TrackKind> &&t)
 {
    push_back(t);
-   auto n = ListOfTracks::end();
+   auto n = end();
    --n;
    t->SetOwner(mSelf, n);
    RecalcPositions(n);
@@ -891,10 +866,9 @@ Track *TrackList::Add(std::shared_ptr<TrackKind> &&t)
 template Track *TrackList::Add<Track>(std::shared_ptr<Track> &&);
 template Track *TrackList::Add<WaveTrack>(std::shared_ptr<WaveTrack> &&);
 
-auto TrackList::Replace(Track * t, ListOfTracks::value_type &&with) ->
+auto TrackList::Replace(Track * t, value_type &&with) -> value_type
-   ListOfTracks::value_type
 {
-   ListOfTracks::value_type holder;
+   value_type holder;
    if (t && with) {
       auto node = t->GetNode();
       t->SetOwner({}, {});
@@ -914,13 +888,13 @@ auto TrackList::Replace(Track * t, ListOfTracks::value_type &&with) ->
 
 TrackNodePointer TrackList::Remove(Track *t)
 {
-   auto result = ListOfTracks::end();
+   TrackNodePointer result(end());
    if (t) {
       auto node = t->GetNode();
       t->SetOwner({}, {});
 
       if (!isNull(node)) {
-         ListOfTracks::value_type holder = std::move( *node );
+         value_type holder = std::move( *node );
 
          result = erase(node);
          if (!isNull(result)) {
@@ -1078,7 +1052,7 @@ void TrackList::SwapNodes(TrackNodePointer s1, TrackNodePointer s2)
    }
 
    // Remove tracks
-   ListOfTracks::value_type save11 = std::move(*s1), save12{};
+   value_type save11 = std::move(*s1), save12{};
    s1 = erase(s1);
    if (linked1) {
       wxASSERT(s1 != s2);
@@ -1086,7 +1060,7 @@ void TrackList::SwapNodes(TrackNodePointer s1, TrackNodePointer s2)
    }
    const bool same = (s1 == s2);
 
-   ListOfTracks::value_type save21 = std::move(*s2), save22{};
+   value_type save21 = std::move(*s2), save22{};
    s2 = erase(s2);
    if (linked2)
       save22 = std::move(*s2), s2 = erase(s2);
@@ -1138,7 +1112,9 @@ bool TrackList::MoveDown(Track * t)
 
 bool TrackList::Contains(const Track * t) const
 {
-   return make_iterator_range( *this ).contains( t );
+   return std::find_if(begin(), end(),
+      [=](const value_type &track) { return t == track.get(); }
+   ) != end();
 }
 
 bool TrackList::IsEmpty() const
@@ -1160,12 +1136,12 @@ int TrackList::GetCount() const
 TimeTrack *TrackList::GetTimeTrack()
 {
    auto iter = std::find_if(begin(), end(),
-      [] ( Track *t ) { return t->GetKind() == Track::Time; }
+      [] (const value_type &t) { return t->GetKind() == Track::Time; }
    );
    if (iter == end())
       return nullptr;
    else
-      return static_cast<TimeTrack*>(*iter);
+      return static_cast<TimeTrack*>(iter->get());
 }
 
 const TimeTrack *TrackList::GetTimeTrack() const
@@ -1238,7 +1214,7 @@ unsigned TrackList::GetNumExportChannels(bool selectionOnly) const
 
 namespace {
    template<typename Array>
-   Array GetWaveTracks(TrackListIterator p, const TrackListIterator end,
+   Array GetWaveTracks(ListOfTracks::const_iterator p, ListOfTracks::const_iterator end,
                        bool selectionOnly, bool includeMuted)
    {
       Array waveTrackArray;
@@ -1249,7 +1225,8 @@ namespace {
          if (track->GetKind() == Track::Wave &&
             (includeMuted || !wt->GetMute()) &&
             (track->GetSelected() || !selectionOnly)) {
-            waveTrackArray.push_back( Track::Pointer< WaveTrack >( track ) );
+            waveTrackArray.push_back(
+               std::static_pointer_cast<WaveTrack>(track));
          }
       }
 
@@ -1264,9 +1241,7 @@ WaveTrackArray TrackList::GetWaveTrackArray(bool selectionOnly, bool includeMute
 
 WaveTrackConstArray TrackList::GetWaveTrackConstArray(bool selectionOnly, bool includeMuted) const
 {
-   auto list = const_cast<TrackList*>(this);
+   return GetWaveTracks<WaveTrackConstArray>(begin(), end(), selectionOnly, includeMuted);
-   return GetWaveTracks<WaveTrackConstArray>(
-      list->begin(), list->end(), selectionOnly, includeMuted);
 }
 
 #if defined(USE_MIDI)
@@ -1277,7 +1252,7 @@ NoteTrackArray TrackList::GetNoteTrackArray(bool selectionOnly)
    for(const auto &track : *this) {
       if (track->GetKind() == Track::Note &&
          (track->GetSelected() || !selectionOnly)) {
-         noteTrackArray.push_back( Track::Pointer<NoteTrack>(track) );
+         noteTrackArray.push_back(std::static_pointer_cast<NoteTrack>(track));
       }
    }
 

src/Track.h

@@ -337,19 +337,13 @@ protected:
 };
 
 class AUDACITY_DLL_API TrackListIterator /* not final */
-: public std::iterator< std::forward_iterator_tag, Track *const >
 {
  public:
-   // The default-constructed value can serve as the end iterator for
+   TrackListIterator(TrackList * val = NULL);
-   // 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 = nullptr);
+   virtual Track *First(TrackList * val = NULL);
    virtual Track *StartWith(Track * val);
    virtual Track *Next(bool skiplinked = false);
    virtual Track *Prev(bool skiplinked = false);
@@ -357,38 +351,19 @@ 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:
-   // The default-constructed value can serve as the end iterator for
+   TrackListConstIterator(const TrackList * val = NULL)
-   // traversal over any track list.
+      : mIter(const_cast<TrackList*>(val))
-   TrackListConstIterator() {}
-   explicit TrackListConstIterator(
-      const TrackList * val, TrackNodePointer p = {})
-      : mIter(const_cast<TrackList*>(val), p)
    {}
-   TrackListConstIterator(const TrackListConstIterator&) = default;
-   TrackListConstIterator& operator=(const TrackListConstIterator&) = default;
    ~TrackListConstIterator() {}
 
    // Iterate functions
@@ -403,19 +378,6 @@ 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;
 };
@@ -570,17 +532,6 @@ 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;
@@ -598,7 +549,7 @@ class TrackList final : public wxEvtHandler, public ListOfTracks
    Track *Add(std::shared_ptr<TrackKind> &&t);
 
    /// Replace first track with second track, give back a holder
-   ListOfTracks::value_type Replace(Track * t, ListOfTracks::value_type &&with);
+   value_type Replace(Track * t, value_type &&with);
 
    /// Remove this Track or all children of this TrackList.
    /// Return an iterator to what followed the removed track.
@@ -678,11 +629,11 @@ class TrackList final : public wxEvtHandler, public ListOfTracks
 
 private:
    bool isNull(TrackNodePointer p) const
-   { return p == TrackNodePointer{} || p == ListOfTracks::end(); }
+   { return p == end(); }
    void setNull(TrackNodePointer &p)
-   { p = ListOfTracks::end(); }
+   { p = end(); }
    bool hasPrev(TrackNodePointer p) const
-   { return p != ListOfTracks::begin(); }
+   { return p != begin(); }
 
    void DoAssign(const TrackList &that);
        

src/effects/Effect.cpp

@@ -2170,8 +2170,7 @@ 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->ListOfTracks::begin(),
+   auto iterOut = mOutputTracks->begin(), iterEnd = mOutputTracks->end();
-      iterEnd = mOutputTracks->ListOfTracks::end();
 
    size_t cnt = mOMap.size();
    size_t i = 0;