From a0aa69a2483af34f864395c67e2d2143e16e1816 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 18 Nov 2018 23:07:05 -0500 Subject: [PATCH] All tracks allocated with make_shared, no more make_unique... ... so that we can use Track::SharedPointer without undefined behavior even on tracks that don't yet belong to any TrackList. Also fix the return type of function template TrackList::Add and remove some casts. --- src/LabelTrack.cpp | 9 +++--- src/LabelTrack.h | 2 +- src/Mix.h | 3 +- src/NoteTrack.cpp | 17 +++++------ src/NoteTrack.h | 2 +- src/TimeTrack.cpp | 9 +++--- src/Track.cpp | 44 ++++------------------------- src/Track.h | 26 ++++++++--------- src/WaveTrack.cpp | 13 ++++----- src/WaveTrack.h | 12 +++----- src/effects/Effect.cpp | 8 +++--- src/effects/Effect.h | 2 +- src/effects/SBSMSEffect.cpp | 4 +-- src/effects/nyquist/Nyquist.cpp | 2 +- src/import/ImportRaw.h | 2 +- src/menus/LabelMenus.cpp | 8 ++---- src/toolbars/ControlToolBar.cpp | 4 +-- src/toolbars/TranscriptionToolBar.h | 2 +- 18 files changed, 60 insertions(+), 109 deletions(-) diff --git a/src/LabelTrack.cpp b/src/LabelTrack.cpp index 5601912ff..bc7a5a5dd 100644 --- a/src/LabelTrack.cpp +++ b/src/LabelTrack.cpp @@ -99,7 +99,7 @@ int LabelTrack::mFontHeight=-1; LabelTrack::Holder TrackFactory::NewLabelTrack() { - return std::make_unique(mDirManager); + return std::make_shared(mDirManager); } LabelTrack::LabelTrack(const std::shared_ptr &projDirManager): @@ -1142,7 +1142,7 @@ double LabelTrack::GetEndTime() const Track::Holder LabelTrack::Duplicate() const { - return std::make_unique( *this ); + return std::make_shared( *this ); } void LabelTrack::SetSelected(bool s) @@ -2489,7 +2489,7 @@ Track::Holder LabelTrack::SplitCut(double t0, double t1) Track::Holder LabelTrack::Copy(double t0, double t1, bool) const { - auto tmp = std::make_unique(GetDirManager()); + auto tmp = std::make_shared(GetDirManager()); const auto lt = static_cast(tmp.get()); for (auto &labelStruct: mLabels) { @@ -2534,8 +2534,7 @@ Track::Holder LabelTrack::Copy(double t0, double t1, bool) const } lt->mClipLen = (t1 - t0); - // This std::move is needed to "upcast" the pointer type - return std::move(tmp); + return tmp; } diff --git a/src/LabelTrack.h b/src/LabelTrack.h index 867647c15..2e38f0e28 100644 --- a/src/LabelTrack.h +++ b/src/LabelTrack.h @@ -161,7 +161,7 @@ class AUDACITY_DLL_API LabelTrack final : public Track double GetStartTime() const override; double GetEndTime() const override; - using Holder = std::unique_ptr; + using Holder = std::shared_ptr; Track::Holder Duplicate() const override; void SetSelected(bool s) override; diff --git a/src/Mix.h b/src/Mix.h index 59e319fe7..30bb79101 100644 --- a/src/Mix.h +++ b/src/Mix.h @@ -49,7 +49,8 @@ class WaveTrackCache; void MixAndRender(TrackList * tracks, TrackFactory *factory, double rate, sampleFormat format, double startTime, double endTime, - std::unique_ptr &uLeft, std::unique_ptr &uRight); + std::shared_ptr &uLeft, + std::shared_ptr &uRight); void MixBuffers(unsigned numChannels, int *channelFlags, float *gains, samplePtr src, diff --git a/src/NoteTrack.cpp b/src/NoteTrack.cpp index 4cb99f463..bd9f522bc 100644 --- a/src/NoteTrack.cpp +++ b/src/NoteTrack.cpp @@ -104,7 +104,7 @@ SONFNS(AutoSave) NoteTrack::Holder TrackFactory::NewNoteTrack() { - return std::make_unique(mDirManager); + return std::make_shared(mDirManager); } NoteTrack::NoteTrack(const std::shared_ptr &projDirManager) @@ -155,7 +155,7 @@ Alg_seq &NoteTrack::GetSeq() const Track::Holder NoteTrack::Duplicate() const { - auto duplicate = std::make_unique(mDirManager); + auto duplicate = std::make_shared(mDirManager); duplicate->Init(*this); // The duplicate begins life in serialized state. Often the duplicate is // pushed on the Undo stack. Then we want to un-serialize it (or a further @@ -190,8 +190,7 @@ Track::Holder NoteTrack::Duplicate() const #ifdef EXPERIMENTAL_MIDI_OUT duplicate->SetVelocity(GetVelocity()); #endif - // This std::move is needed to "upcast" the pointer type - return std::move(duplicate); + return duplicate; } @@ -463,7 +462,7 @@ Track::Holder NoteTrack::Cut(double t0, double t1) //( std::min( t1, GetEndTime() ) ) - ( std::max( t0, GetStartTime() ) ) //); - auto newTrack = std::make_unique(mDirManager); + auto newTrack = std::make_shared(mDirManager); newTrack->Init(*this); @@ -480,8 +479,7 @@ Track::Holder NoteTrack::Cut(double t0, double t1) //(mBottomNote, mDirManager, // mSerializationBuffer, mSerializationLength, mVisibleChannels) - // This std::move is needed to "upcast" the pointer type - return std::move(newTrack); + return newTrack; } Track::Holder NoteTrack::Copy(double t0, double t1, bool) const @@ -491,7 +489,7 @@ Track::Holder NoteTrack::Copy(double t0, double t1, bool) const double len = t1-t0; - auto newTrack = std::make_unique(mDirManager); + auto newTrack = std::make_shared(mDirManager); newTrack->Init(*this); @@ -504,8 +502,7 @@ Track::Holder NoteTrack::Copy(double t0, double t1, bool) const // (mBottomNote, mDirManager, mSerializationBuffer, // mSerializationLength, mVisibleChannels) - // This std::move is needed to "upcast" the pointer type - return std::move(newTrack); + return newTrack; } bool NoteTrack::Trim(double t0, double t1) diff --git a/src/NoteTrack.h b/src/NoteTrack.h index 6179c03cd..5b4bd89a7 100644 --- a/src/NoteTrack.h +++ b/src/NoteTrack.h @@ -75,7 +75,7 @@ class AUDACITY_DLL_API NoteTrack final const AudacityProject *pProject, int currentTool, bool bMultiTool) override; - using Holder = std::unique_ptr; + using Holder = std::shared_ptr; Track::Holder Duplicate() const override; double GetOffset() const override; diff --git a/src/TimeTrack.cpp b/src/TimeTrack.cpp index f6f51fd45..56e863f62 100644 --- a/src/TimeTrack.cpp +++ b/src/TimeTrack.cpp @@ -33,9 +33,9 @@ #define TIMETRACK_MIN 0.01 #define TIMETRACK_MAX 10.0 -std::unique_ptr TrackFactory::NewTimeTrack() +std::shared_ptr TrackFactory::NewTimeTrack() { - return std::make_unique(mDirManager, mZoomInfo); + return std::make_shared(mDirManager, mZoomInfo); } TimeTrack::TimeTrack(const std::shared_ptr &projDirManager, const ZoomInfo *zoomInfo): @@ -108,8 +108,7 @@ Track::Holder TimeTrack::Cut( double t0, double t1 ) Track::Holder TimeTrack::Copy( double t0, double t1, bool ) const { - auto result = std::make_unique( *this, &t0, &t1 ); - return Track::Holder{ std::move( result ) }; + return std::make_shared( *this, &t0, &t1 ); } void TimeTrack::Clear(double t0, double t1) @@ -143,7 +142,7 @@ void TimeTrack::InsertSilence(double t, double len) Track::Holder TimeTrack::Duplicate() const { - return std::make_unique(*this); + return std::make_shared(*this); } bool TimeTrack::GetInterpolateLog() const diff --git a/src/Track.cpp b/src/Track.cpp index 72a245a59..7a21cce39 100644 --- a/src/Track.cpp +++ b/src/Track.cpp @@ -728,35 +728,10 @@ Track *TrackList::FindById( TrackId id ) return it->get(); } -template -Track *TrackList::Add(std::unique_ptr &&t) +Track *TrackList::DoAddToHead(const std::shared_ptr &t) { - Track *pTrack; - push_back(ListOfTracks::value_type(pTrack = t.release())); - - auto n = getPrev( getEnd() ); - - pTrack->SetOwner(mSelf, n); - pTrack->SetId( TrackId{ ++sCounter } ); - RecalcPositions(n); - AdditionEvent(n); - return back().get(); -} - -// Make instantiations for the linker to find -template Track *TrackList::Add(std::unique_ptr &&); -#if defined(USE_MIDI) -template Track *TrackList::Add(std::unique_ptr &&); -#endif -template Track *TrackList::Add(std::unique_ptr &&); -template Track *TrackList::Add(std::unique_ptr &&); -template Track *TrackList::Add(std::unique_ptr &&); - -template -Track *TrackList::AddToHead(std::unique_ptr &&t) -{ - Track *pTrack; - push_front(ListOfTracks::value_type(pTrack = t.release())); + Track *pTrack = t.get(); + push_front(ListOfTracks::value_type(t)); auto n = getBegin(); pTrack->SetOwner(mSelf, n); pTrack->SetId( TrackId{ ++sCounter } ); @@ -765,11 +740,7 @@ Track *TrackList::AddToHead(std::unique_ptr &&t) return front().get(); } -// Make instantiations for the linker to find -template Track *TrackList::AddToHead(std::unique_ptr &&); - -template -Track *TrackList::Add(std::shared_ptr &&t) +Track *TrackList::DoAdd(const std::shared_ptr &t) { push_back(t); @@ -782,10 +753,6 @@ Track *TrackList::Add(std::shared_ptr &&t) return back().get(); } -// Make instantiations for the linker to find -template Track *TrackList::Add(std::shared_ptr &&); -template Track *TrackList::Add(std::shared_ptr &&); - void TrackList::GroupChannels( Track &track, size_t groupSize, bool resetChannels ) { @@ -1234,8 +1201,7 @@ TrackList::RegisterPendingChangedTrack( Updater updater, Track *src ) { std::shared_ptr pTrack; if (src) - // convert from unique_ptr to shared_ptr - pTrack.reset( src->Duplicate().release() ); + pTrack = src->Duplicate(); if (pTrack) { mUpdaters.push_back( updater ); diff --git a/src/Track.h b/src/Track.h index b2af7f7d5..2e2a08556 100644 --- a/src/Track.h +++ b/src/Track.h @@ -367,7 +367,7 @@ private: void Init(const Track &orig); - using Holder = std::unique_ptr; + using Holder = std::shared_ptr; virtual Holder Duplicate() const = 0; // Called when this track is merged to stereo with another, and should @@ -1330,6 +1330,9 @@ class TrackList final : public wxEvtHandler, public ListOfTracks } private: + Track *DoAddToHead(const std::shared_ptr &t); + Track *DoAdd(const std::shared_ptr &t); + template< typename TrackType, typename InTrackType > static TrackIterRange< TrackType > Channels_( TrackIter< InTrackType > iter1 ) @@ -1369,15 +1372,12 @@ public: /// Add a Track, giving it a fresh id template - Track *Add(std::unique_ptr &&t); + TrackKind *AddToHead( const std::shared_ptr< TrackKind > &t ) + { return static_cast< TrackKind* >( DoAddToHead( t ) ); } - /// Add a Track, giving it a fresh id template - Track *AddToHead(std::unique_ptr &&t); - - /// Add a Track, giving it a fresh id - template - Track *Add(std::shared_ptr &&t); + TrackKind *Add( const std::shared_ptr< TrackKind > &t ) + { return static_cast< TrackKind* >( DoAdd( t ) ); } /** \brief Define a group of channels starting at the given track * @@ -1632,13 +1632,13 @@ class AUDACITY_DLL_API TrackFactory public: // These methods are defined in WaveTrack.cpp, NoteTrack.cpp, // LabelTrack.cpp, and TimeTrack.cpp respectively - std::unique_ptr DuplicateWaveTrack(const WaveTrack &orig); - std::unique_ptr NewWaveTrack(sampleFormat format = (sampleFormat)0, + std::shared_ptr DuplicateWaveTrack(const WaveTrack &orig); + std::shared_ptr NewWaveTrack(sampleFormat format = (sampleFormat)0, double rate = 0); - std::unique_ptr NewLabelTrack(); - std::unique_ptr NewTimeTrack(); + std::shared_ptr NewLabelTrack(); + std::shared_ptr NewTimeTrack(); #if defined(USE_MIDI) - std::unique_ptr NewNoteTrack(); + std::shared_ptr NewNoteTrack(); #endif }; diff --git a/src/WaveTrack.cpp b/src/WaveTrack.cpp index a737d7d60..30d117c1d 100644 --- a/src/WaveTrack.cpp +++ b/src/WaveTrack.cpp @@ -70,15 +70,13 @@ using std::max; WaveTrack::Holder TrackFactory::DuplicateWaveTrack(const WaveTrack &orig) { - return std::unique_ptr - { static_cast(orig.Duplicate().release()) }; + return std::static_pointer_cast( orig.Duplicate() ); } WaveTrack::Holder TrackFactory::NewWaveTrack(sampleFormat format, double rate) { - return std::unique_ptr - { safenew WaveTrack(mDirManager, format, rate) }; + return std::make_shared ( mDirManager, format, rate ); } WaveTrack::WaveTrack(const std::shared_ptr &projDirManager, sampleFormat format, double rate) : @@ -395,7 +393,7 @@ int WaveTrack::ZeroLevelYCoordinate(wxRect rect) const Track::Holder WaveTrack::Duplicate() const { - return Track::Holder{ safenew WaveTrack{ *this } }; + return std::make_shared( *this ); } double WaveTrack::GetRate() const @@ -629,9 +627,8 @@ Track::Holder WaveTrack::Copy(double t0, double t1, bool forClipboard) const if (t1 < t0) THROW_INCONSISTENCY_EXCEPTION; - WaveTrack *newTrack; - Track::Holder result - { newTrack = safenew WaveTrack{ mDirManager } }; + auto result = std::make_shared( mDirManager ); + WaveTrack *newTrack = result.get(); newTrack->Init(*this); diff --git a/src/WaveTrack.h b/src/WaveTrack.h index b0f098fa4..acd0c0506 100644 --- a/src/WaveTrack.h +++ b/src/WaveTrack.h @@ -58,28 +58,24 @@ using Regions = std::vector < Region >; class Envelope; class AUDACITY_DLL_API WaveTrack final : public PlayableTrack { - - private: +public: // // Constructor / Destructor / Duplicator // - // Private since only factories are allowed to construct WaveTracks - // WaveTrack(const std::shared_ptr &projDirManager, sampleFormat format = (sampleFormat)0, double rate = 0); WaveTrack(const WaveTrack &orig); - void Init(const WaveTrack &orig); - -public: // overwrite data excluding the sample sequence but including display // settings void Reinit(const WaveTrack &orig); private: + void Init(const WaveTrack &orig); + Track::Holder Duplicate() const override; friend class TrackFactory; @@ -87,7 +83,7 @@ private: public: typedef WaveTrackLocation Location; - using Holder = std::unique_ptr; + using Holder = std::shared_ptr; virtual ~WaveTrack(); diff --git a/src/effects/Effect.cpp b/src/effects/Effect.cpp index edfeb4971..2b768f949 100644 --- a/src/effects/Effect.cpp +++ b/src/effects/Effect.cpp @@ -1207,7 +1207,7 @@ bool Effect::DoEffect(wxWindow *parent, // We don't yet know the effect type for code in the Nyquist Prompt, so // assume it requires a track and handle errors when the effect runs. if ((GetType() == EffectTypeGenerate || GetPath() == NYQUIST_PROMPT_ID) && (mNumTracks == 0)) { - newTrack = static_cast(mTracks->Add(mFactory->NewWaveTrack())); + newTrack = mTracks->Add(mFactory->NewWaveTrack()); newTrack->SetSelected(true); } @@ -1553,7 +1553,7 @@ bool Effect::ProcessTrack(int count, auto chans = std::min(mNumAudioOut, mNumChannels); - std::unique_ptr genLeft, genRight; + std::shared_ptr genLeft, genRight; decltype(len) genLength = 0; bool isGenerator = GetType() == EffectTypeGenerate; @@ -2073,11 +2073,11 @@ void Effect::CopyInputTracks(bool allSyncLockSelected) } } -Track *Effect::AddToOutputTracks(std::unique_ptr &&t) +Track *Effect::AddToOutputTracks(const std::shared_ptr &t) { mIMap.push_back(NULL); mOMap.push_back(t.get()); - return mOutputTracks->Add(std::move(t)); + return mOutputTracks->Add(t); } Effect::AddedAnalysisTrack::AddedAnalysisTrack(Effect *pEffect, const wxString &name) diff --git a/src/effects/Effect.h b/src/effects/Effect.h index 9390b7164..38b69ea65 100644 --- a/src/effects/Effect.h +++ b/src/effects/Effect.h @@ -443,7 +443,7 @@ protected: void ReplaceProcessedTracks(const bool bGoodResult); // Use this to append a NEW output track. - Track *AddToOutputTracks(std::unique_ptr &&t); + Track *AddToOutputTracks(const std::shared_ptr &t); // // protected data diff --git a/src/effects/SBSMSEffect.cpp b/src/effects/SBSMSEffect.cpp index 7e91e0398..c61cfc606 100644 --- a/src/effects/SBSMSEffect.cpp +++ b/src/effects/SBSMSEffect.cpp @@ -59,8 +59,8 @@ public: // Not required by callbacks, but makes for easier cleanup std::unique_ptr resampler; std::unique_ptr quality; - std::unique_ptr outputLeftTrack; - std::unique_ptr outputRightTrack; + std::shared_ptr outputLeftTrack; + std::shared_ptr outputRightTrack; std::exception_ptr mpException {}; }; diff --git a/src/effects/nyquist/Nyquist.cpp b/src/effects/nyquist/Nyquist.cpp index d0666246e..a8437b607 100644 --- a/src/effects/nyquist/Nyquist.cpp +++ b/src/effects/nyquist/Nyquist.cpp @@ -1419,7 +1419,7 @@ bool NyquistEffect::ProcessOne() return false; } - std::unique_ptr outputTrack[2]; + std::shared_ptr outputTrack[2]; double rate = mCurTrack[0]->GetRate(); for (int i = 0; i < outChannels; i++) { diff --git a/src/import/ImportRaw.h b/src/import/ImportRaw.h index f65f88ca0..b48ab7866 100644 --- a/src/import/ImportRaw.h +++ b/src/import/ImportRaw.h @@ -23,7 +23,7 @@ class wxWindow; // Newly constructed WaveTracks that are not yet owned by a TrackList // are held in unique_ptr not shared_ptr -using NewChannelGroup = std::vector< std::unique_ptr >; +using NewChannelGroup = std::vector< std::shared_ptr >; using TrackHolders = std::vector< NewChannelGroup >; diff --git a/src/menus/LabelMenus.cpp b/src/menus/LabelMenus.cpp index 4d69308fc..ecd3e230b 100644 --- a/src/menus/LabelMenus.cpp +++ b/src/menus/LabelMenus.cpp @@ -41,10 +41,8 @@ int DoAddLabel( auto lt = * iter.Filter< LabelTrack >(); // If none found, start a NEW label track and use it - if (!lt) { - lt = static_cast - (tracks->Add(trackFactory->NewLabelTrack())); - } + if (!lt) + lt = tracks->Add(trackFactory->NewLabelTrack()); // LLL: Commented as it seemed a little forceful to remove users // selection when adding the label. This does not happen if @@ -158,7 +156,7 @@ void EditByLabel( } } -using EditDestFunction = std::unique_ptr (WaveTrack::*)(double, double); +using EditDestFunction = std::shared_ptr (WaveTrack::*)(double, double); //Executes the edit function on all selected wave tracks with //regions specified by selected labels diff --git a/src/toolbars/ControlToolBar.cpp b/src/toolbars/ControlToolBar.cpp index 4bf454ad4..82cf95029 100644 --- a/src/toolbars/ControlToolBar.cpp +++ b/src/toolbars/ControlToolBar.cpp @@ -1168,9 +1168,7 @@ bool ControlToolBar::DoRecord(AudacityProject &project, Track *first {}; for (int c = 0; c < recordingChannels; c++) { - std::shared_ptr newTrack{ - p->GetTrackFactory()->NewWaveTrack().release() - }; + auto newTrack = p->GetTrackFactory()->NewWaveTrack(); if (!first) first = newTrack.get(); diff --git a/src/toolbars/TranscriptionToolBar.h b/src/toolbars/TranscriptionToolBar.h index fc9a101fa..4b520df8e 100644 --- a/src/toolbars/TranscriptionToolBar.h +++ b/src/toolbars/TranscriptionToolBar.h @@ -156,7 +156,7 @@ class TranscriptionToolBar final : public ToolBar { int mBackgroundWidth; int mBackgroundHeight; - std::unique_ptr mTimeTrack; + std::shared_ptr mTimeTrack; public: