Clean up pointer use:

* Use unique_ptr for Actions instead of manual memory management
 * Use unique_ptr in KeywordDatabase
 * Use unique_ptr instead of shared_ptr for file formats
 * Don't pass shared_ptr to Reader/Writer, use references instead
Also
 * Switch to C++17 so we can use map::try_emplace
This commit is contained in:
Twan van Laarhoven
2020-04-25 21:30:05 +02:00
parent 708b4389a0
commit 64ea1d7322
57 changed files with 363 additions and 385 deletions
+15 -6
View File
@@ -97,6 +97,7 @@ ControlPointMoveAction::ControlPointMoveAction(const set<ControlPointP>& points)
FOR_EACH(p, points) {
oldValues.push_back(p->pos);
}
perform(false);
}
String ControlPointMoveAction::getName(bool to_undo) const {
@@ -104,12 +105,15 @@ String ControlPointMoveAction::getName(bool to_undo) const {
}
void ControlPointMoveAction::perform(bool to_undo) {
if (to_undo != done) return;
done = !to_undo;
FOR_EACH_2(p,points, op,oldValues) {
swap(p->pos, op);
}
}
void ControlPointMoveAction::move (const Vector2D& deltaDelta) {
void ControlPointMoveAction::move(const Vector2D& deltaDelta) {
assert(done);
delta += deltaDelta;
// Move each point by delta, possibly constrained
set<ControlPointP>::const_iterator it = points.begin();
@@ -128,18 +132,23 @@ HandleMoveAction::HandleMoveAction(const SelectedHandle& handle)
, old_other (handle.getOther())
, constrain(false)
, snap(0)
{}
{
perform(false);
}
String HandleMoveAction::getName(bool to_undo) const {
return _ACTION_("move handle");
}
void HandleMoveAction::perform(bool to_undo) {
if (to_undo != done) return;
done = !to_undo;
swap(old_handle, handle.getHandle());
swap(old_other, handle.getOther());
}
void HandleMoveAction::move(const Vector2D& deltaDelta) {
assert(done);
delta += deltaDelta;
handle.getHandle() = constrain_snap_vector_offset(handle.point->pos, old_handle + delta, constrain, snap);
handle.getOther() = old_other;
@@ -246,7 +255,7 @@ void CurveDragAction::move(const Vector2D& delta, double t) {
ControlPointAddAction::ControlPointAddAction(const SymbolShapeP& shape, UInt insert_after, double t)
: shape(shape)
, new_point(new ControlPoint())
, new_point(make_shared<ControlPoint>())
, insert_after(insert_after)
, point1(shape->getPoint(insert_after))
, point2(shape->getPoint(insert_after + 1))
@@ -419,13 +428,13 @@ void ControlPointRemoveAction::perform(bool to_undo) {
}
Action* control_point_remove_action(const SymbolShapeP& shape, const set<ControlPointP>& to_delete) {
unique_ptr<Action> control_point_remove_action(const SymbolShapeP& shape, const set<ControlPointP>& to_delete) {
if (shape->points.size() - to_delete.size() < 2) {
// TODO : remove part?
//make_intrusive<ControlPointRemoveAllAction>(part);
return 0; // no action
return unique_ptr<Action>(); // no action
} else {
return new ControlPointRemoveAction(shape, to_delete);
return make_unique<ControlPointRemoveAction>(shape, to_delete);
}
}
+11 -3
View File
@@ -40,10 +40,18 @@ Vector2D constrain_snap_vector_offset(const Vector2D& off1, const Vector2D& d, b
/** Takes the closest snap */
Vector2D constrain_snap_vector_offset(const Vector2D& off1, const Vector2D& off2, const Vector2D& d, bool constrain, int steps);
// ----------------------------------------------------------------------------- : Base class
/// An action that by itself doesn't do anything, but can later be extended after it is performed
class ExtendableAction : public Action {
protected:
bool done = false;
};
// ----------------------------------------------------------------------------- : Move control point
/// Moving a control point in a symbol
class ControlPointMoveAction : public Action {
class ControlPointMoveAction : public ExtendableAction {
public:
ControlPointMoveAction(const set<ControlPointP>& points);
@@ -65,7 +73,7 @@ class ControlPointMoveAction : public Action {
// ----------------------------------------------------------------------------- : Move handle
/// Moving a handle(before/after) of a control point in a symbol
class HandleMoveAction : public Action {
class HandleMoveAction : public ExtendableAction {
public:
HandleMoveAction(const SelectedHandle& handle);
@@ -169,7 +177,7 @@ class ControlPointAddAction : public Action {
/// Action that removes any number of points from a symbol shape
/// TODO: If less then 3 points are left removes the entire shape?
Action* control_point_remove_action(const SymbolShapeP& shape, const set<ControlPointP>& to_delete);
unique_ptr<Action> control_point_remove_action(const SymbolShapeP& shape, const set<ControlPointP>& to_delete);
+24 -14
View File
@@ -81,14 +81,24 @@ class SimpleValueAction : public ValueAction {
typename T::ValueType new_value;
};
ValueAction* value_action(const ChoiceValueP& value, const Defaultable<String>& new_value) { return new SimpleValueAction<ChoiceValue, true> (value, new_value); }
ValueAction* value_action(const ColorValueP& value, const Defaultable<Color>& new_value) { return new SimpleValueAction<ColorValue, true> (value, new_value); }
ValueAction* value_action(const ImageValueP& value, const FileName& new_value) { return new SimpleValueAction<ImageValue, false>(value, new_value); }
ValueAction* value_action(const SymbolValueP& value, const FileName& new_value) { return new SimpleValueAction<SymbolValue, false>(value, new_value); }
ValueAction* value_action(const PackageChoiceValueP& value, const String& new_value) { return new SimpleValueAction<PackageChoiceValue, false>(value, new_value); }
ValueAction* value_action(const MultipleChoiceValueP& value, const Defaultable<String>& new_value, const String& last_change) {
unique_ptr<ValueAction> value_action(const ChoiceValueP& value, const Defaultable<String>& new_value) {
return make_unique<SimpleValueAction<ChoiceValue, true>>(value, new_value);
}
unique_ptr<ValueAction> value_action(const ColorValueP& value, const Defaultable<Color>& new_value) {
return make_unique<SimpleValueAction<ColorValue, true>>(value, new_value);
}
unique_ptr<ValueAction> value_action(const ImageValueP& value, const FileName& new_value) {
return make_unique<SimpleValueAction<ImageValue, false>>(value, new_value);
}
unique_ptr<ValueAction> value_action(const SymbolValueP& value, const FileName& new_value) {
return make_unique<SimpleValueAction<SymbolValue, false>>(value, new_value);
}
unique_ptr<ValueAction> value_action(const PackageChoiceValueP& value, const String& new_value) {
return make_unique<SimpleValueAction<PackageChoiceValue, false>>(value, new_value);
}
unique_ptr<ValueAction> value_action(const MultipleChoiceValueP& value, const Defaultable<String>& new_value, const String& last_change) {
MultipleChoiceValue::ValueType v = { new_value, last_change };
return new SimpleValueAction<MultipleChoiceValue, false>(value, v);
return make_unique<SimpleValueAction<MultipleChoiceValue, false>>(value, v);
}
@@ -133,7 +143,7 @@ TextValue& TextValueAction::value() const {
}
TextValueAction* toggle_format_action(const TextValueP& value, const String& tag, size_t start_i, size_t end_i, size_t start, size_t end, const String& action_name) {
unique_ptr<TextValueAction> toggle_format_action(const TextValueP& value, const String& tag, size_t start_i, size_t end_i, size_t start, size_t end, const String& action_name) {
if (start > end) {
swap(start, end);
swap(start_i, end_i);
@@ -165,11 +175,11 @@ TextValueAction* toggle_format_action(const TextValueP& value, const String& tag
if (value->value() == new_value) {
return nullptr; // no changes
} else {
return new TextValueAction(value, start, end, end, new_value, action_name);
return make_unique<TextValueAction>(value, start, end, end, new_value, action_name);
}
}
TextValueAction* typing_action(const TextValueP& value, size_t start_i, size_t end_i, size_t start, size_t end, const String& replacement, const String& action_name) {
unique_ptr<TextValueAction> typing_action(const TextValueP& value, size_t start_i, size_t end_i, size_t start, size_t end, const String& replacement, const String& action_name) {
bool reverse = start > end;
if (reverse) {
swap(start, end);
@@ -181,9 +191,9 @@ TextValueAction* typing_action(const TextValueP& value, size_t start_i, size_t e
return nullptr;
} else {
if (reverse) {
return new TextValueAction(value, end, start, start+untag(replacement).size(), new_value, action_name);
return make_unique<TextValueAction>(value, end, start, start+untag(replacement).size(), new_value, action_name);
} else {
return new TextValueAction(value, start, end, start+untag(replacement).size(), new_value, action_name);
return make_unique<TextValueAction>(value, start, end, start+untag(replacement).size(), new_value, action_name);
}
}
}
@@ -248,9 +258,9 @@ ValueActionPerformer::ValueActionPerformer(const ValueP& value, Card* card, cons
{}
ValueActionPerformer::~ValueActionPerformer() {}
void ValueActionPerformer::addAction(ValueAction* action) {
void ValueActionPerformer::addAction(unique_ptr<ValueAction>&& action) {
action->isOnCard(card);
set->actions.addAction(action);
set->actions.addAction(move(action));
}
Package& ValueActionPerformer::getLocalPackage() {
+9 -9
View File
@@ -55,12 +55,12 @@ class ValueAction : public Action {
// ----------------------------------------------------------------------------- : Simple
/// Action that updates a Value to a new value
ValueAction* value_action(const ChoiceValueP& value, const Defaultable<String>& new_value);
ValueAction* value_action(const MultipleChoiceValueP& value, const Defaultable<String>& new_value, const String& last_change);
ValueAction* value_action(const ColorValueP& value, const Defaultable<Color>& new_value);
ValueAction* value_action(const ImageValueP& value, const FileName& new_value);
ValueAction* value_action(const SymbolValueP& value, const FileName& new_value);
ValueAction* value_action(const PackageChoiceValueP& value, const String& new_value);
unique_ptr<ValueAction> value_action(const ChoiceValueP& value, const Defaultable<String>& new_value);
unique_ptr<ValueAction> value_action(const MultipleChoiceValueP& value, const Defaultable<String>& new_value, const String& last_change);
unique_ptr<ValueAction> value_action(const ColorValueP& value, const Defaultable<Color>& new_value);
unique_ptr<ValueAction> value_action(const ImageValueP& value, const FileName& new_value);
unique_ptr<ValueAction> value_action(const SymbolValueP& value, const FileName& new_value);
unique_ptr<ValueAction> value_action(const PackageChoiceValueP& value, const String& new_value);
// ----------------------------------------------------------------------------- : Text
@@ -86,11 +86,11 @@ class TextValueAction : public ValueAction {
};
/// Action for toggling some formating tag on or off in some range
TextValueAction* toggle_format_action(const TextValueP& value, const String& tag, size_t start_i, size_t end_i, size_t start, size_t end, const String& action_name);
unique_ptr<TextValueAction> toggle_format_action(const TextValueP& value, const String& tag, size_t start_i, size_t end_i, size_t start, size_t end, const String& action_name);
/// Typing in a TextValue, replace the selection [start...end) with replacement
/** start and end are cursor positions, start_i and end_i are indices*/
TextValueAction* typing_action(const TextValueP& value, size_t start_i, size_t end_i, size_t start, size_t end, const String& replacement, const String& action_name);
unique_ptr<TextValueAction> typing_action(const TextValueP& value, size_t start_i, size_t end_i, size_t start, size_t end, const String& replacement, const String& action_name);
// ----------------------------------------------------------------------------- : Reminder text
@@ -169,7 +169,7 @@ class ValueActionPerformer {
ValueActionPerformer(const ValueP& value, Card* card, const SetP& set);
~ValueActionPerformer();
/// Perform an action. The performer takes ownerwhip of the action.
void addAction(ValueAction* action);
void addAction(unique_ptr<ValueAction>&& action);
const ValueP value; ///< The value
Package& getLocalPackage();
+1 -1
View File
@@ -47,6 +47,6 @@ void AddCardsScript::perform(Set& set) {
// Add to set
if (!cards.empty()) {
// TODO: change the name of the action somehow
set.actions.addAction(new AddCardAction(ADD, set, cards));
set.actions.addAction(make_unique<AddCardAction>(ADD, set, cards));
}
}
+5 -5
View File
@@ -284,11 +284,11 @@ inline String type_name(const Value&) {
return _(NAME); \
}
#define DECLARE_STYLE_TYPE(Type) \
DECLARE_REFLECTION(); public: \
DECLARE_HAS_FIELD(Type) \
virtual StyleP clone() const; \
virtual ValueViewerP makeViewer(DataViewer& parent, const StyleP& thisP); \
#define DECLARE_STYLE_TYPE(Type) \
DECLARE_REFLECTION(); public: \
DECLARE_HAS_FIELD(Type) \
virtual StyleP clone() const; \
virtual ValueViewerP makeViewer(DataViewer& parent, const StyleP& thisP); \
virtual ValueViewerP makeEditor(DataEditor& parent, const StyleP& thisP)
#define DECLARE_VALUE_TYPE(Type,ValueType_) \
+1 -1
View File
@@ -190,7 +190,7 @@ void ChoiceStyle::initImage() {
// CALL 0
// PUSH_CONST nil
// OR_ELSE
ScriptCustomCollectionP lookup(new ScriptCustomCollection());
ScriptCustomCollectionP lookup = make_intrusive<ScriptCustomCollection>();
FOR_EACH(ci, choice_images) {
lookup->key_value[uncanonical_name_form(ci.first)] =
lookup->key_value[ci.first] = ci.second.getValidScriptP();
+1 -1
View File
@@ -301,7 +301,7 @@ void ApprDistroDatabase::removeSet(const String& code) {
// TODO ?
}
void ApprDistroDatabase::doRead(InputStream& in) {
void ApprDistroDatabase::doRead(wxInputStream& in) {
wxTextInputStream tin(in);
ApprDistro* last = 0;
while (!in.Eof()) {
+3 -3
View File
@@ -23,16 +23,16 @@
/// Serialize an object to a string, clipboard_package will be set to the given package.
template <typename T>
String serialize_for_clipboard(Package& package, T& object) {
shared_ptr<wxStringOutputStream> stream( new wxStringOutputStream );
wxStringOutputStream stream;
Writer writer(stream, file_version_clipboard);
WITH_DYNAMIC_ARG(clipboard_package, &package);
writer.handle(object);
return stream->GetString();
return stream.GetString();
}
template <typename T>
void deserialize_from_clipboard(T& object, Package& package, const String& data) {
shared_ptr<wxStringInputStream> stream( new wxStringInputStream(data) );
wxStringInputStream stream = {data};
Reader reader(stream, nullptr, _("clipboard"));
WITH_DYNAMIC_ARG(clipboard_package, &package);
reader.handle_greedy(object);
+7 -15
View File
@@ -21,14 +21,6 @@
#include <wx/zipstrm.h>
#include <wx/stdpaths.h>
DECLARE_TYPEOF_COLLECTION(String);
DECLARE_TYPEOF_COLLECTION(PackagedP);
DECLARE_TYPEOF_COLLECTION(PackageDependencyP);
DECLARE_TYPEOF_COLLECTION(PackageDescriptionP);
DECLARE_TYPEOF_COLLECTION(InstallablePackageP);
DECLARE_POINTER_TYPE(wxFileInputStream);
DECLARE_POINTER_TYPE(wxZipInputStream);
// Don't do this check for now, because we can't bless packages
#define USE_MODIFIED_CHECK 0
@@ -50,8 +42,8 @@ void Installer::validate(Version file_app_version) {
// TODO: support absolute icon names
try{
String filename = p->name + _("/") + p->icon_url;
InputStreamP img = openIn(p->name + _("/") + p->icon_url);
image_load_file(p->icon, *img);
auto img_stream = openIn(p->name + _("/") + p->icon_url);
image_load_file(p->icon, *img_stream);
} catch (...) {
// ignore errors, it's just an image
p->icon_url.clear();
@@ -195,9 +187,9 @@ void Installer::addPackage(Packaged& package) {
const FileInfos& file_infos = package.getFileInfos();
for (FileInfos::const_iterator it = file_infos.begin() ; it != file_infos.end() ; ++it) {
String file = it->first;
InputStreamP is = package.openIn(file);
OutputStreamP os = openOut(name + _("/") + file);
os->Write(*is);
auto in_stream = package.openIn(file);
auto out_stream = openOut(name + _("/") + file);
out_stream->Write(*in_stream);
}
}
@@ -235,8 +227,8 @@ PackageDescription::PackageDescription(const Packaged& package)
}
}
// icon
InputStreamP file = const_cast<Packaged&>(package).openIconFile();
if (file) image_load_file(icon, *file);
auto file_stream = const_cast<Packaged&>(package).openIconFile();
if (file_stream) image_load_file(icon, *file_stream);
}
IMPLEMENT_REFLECTION_NO_SCRIPT(PackageDescription) {
+27 -34
View File
@@ -256,13 +256,13 @@ class KeywordTrie {
KeywordTrie();
~KeywordTrie();
map<Char, KeywordTrie*> children; ///< children after a given character (owned)
KeywordTrie* on_any_star; ///< children on /.*/ (owned or this)
vector<const Keyword*> finished; ///< keywordss that end in this node
map<wxUniChar, unique_ptr<KeywordTrie>> children; ///< children after a given character
KeywordTrie* on_any_star; ///< children on /.*/ (owned or this)
vector<const Keyword*> finished; ///< keywords that end in this node
/// Insert nodes representing the given character
/** return the node where the evaluation will be after matching the character */
KeywordTrie* insert(Char match);
KeywordTrie* insert(wxUniChar match);
/// Insert nodes representing the given string
/** return the node where the evaluation will be after matching the string */
KeywordTrie* insert(const String& match);
@@ -276,32 +276,28 @@ class KeywordTrie {
KeywordTrie::KeywordTrie()
: on_any_star(nullptr)
{}
KeywordTrie::~KeywordTrie() {
FOR_EACH(c, children) {
delete c.second;
}
if (on_any_star != this) delete on_any_star;
}
KeywordTrie* KeywordTrie::insert(Char c) {
KeywordTrie* KeywordTrie::insert(wxUniChar c) {
#if USE_CASE_INSENSITIVE_KEYWORDS
c = toLower(c); // case insensitive matching
#endif
KeywordTrie*& child = children[c];
if (!child) child = new KeywordTrie;
return child;
unique_ptr<KeywordTrie>& child = children[c];
if (!child) child.reset(new KeywordTrie);
return child.get();
}
KeywordTrie* KeywordTrie::insert(const String& match) {
KeywordTrie* cur = this;
FOR_EACH_CONST(c, match) {
cur = cur->insert(static_cast<Char>(c));
for (wxUniChar c : match) {
cur = cur->insert(c);
}
return cur;
}
KeywordTrie* KeywordTrie::insertAnyStar() {
if (!on_any_star) on_any_star = new KeywordTrie;
if (!on_any_star) on_any_star = new KeywordTrie();
on_any_star->on_any_star = on_any_star; // circular reference to itself
return on_any_star;
}
@@ -314,14 +310,11 @@ IMPLEMENT_DYNAMIC_ARG(KeywordUsageStatistics*, keyword_usage_statistics, nullptr
KeywordDatabase::KeywordDatabase()
: root(nullptr)
{}
KeywordDatabase::~KeywordDatabase() {
clear();
}
// Note: has to be here because in the header KeywordTrie is not defined
KeywordDatabase::~KeywordDatabase() {}
void KeywordDatabase::clear() {
delete root;
root = nullptr;
root.reset();
}
void KeywordDatabase::add(const vector<KeywordP>& kws) {
@@ -334,8 +327,8 @@ void KeywordDatabase::add(const Keyword& kw) {
if (kw.match.empty() || !kw.valid) return; // can't handle empty keywords
// Create root
if (!root) {
root = new KeywordTrie;
root->on_any_star = root;
root = make_unique<KeywordTrie>();
root->on_any_star = root.get();
}
KeywordTrie* cur = root->insertAnyStar();
// Add to trie
@@ -384,7 +377,7 @@ void KeywordDatabase::prepare_parameters(const vector<KeywordParamP>& ps, const
// ----------------------------------------------------------------------------- : KeywordDatabase : matching
// transitive closure of a state, follow all on_any_star links
void closure(vector<KeywordTrie*>& state) {
void closure(vector<const KeywordTrie*>& state) {
for (size_t j = 0 ; j < state.size() ; ++j) {
if (state[j]->on_any_star && state[j]->on_any_star != state[j]) {
state.push_back(state[j]->on_any_star);
@@ -393,10 +386,10 @@ void closure(vector<KeywordTrie*>& state) {
}
#ifdef _DEBUG
void dump(int i, KeywordTrie* t) {
void dump(int i, const KeywordTrie* t) {
FOR_EACH(c, t->children) {
wxLogDebug(String(i,_(' ')) + c.first + _(" ") + String::Format(_("%p"),c.second));
dump(i+2, c.second);
wxLogDebug(String(i,_(' ')) + c.first + _(" ") + String::Format(_("%p"),c.second.get()));
dump(i+2, c.second.get());
}
if (t->on_any_star) {
wxLogDebug(String(i,_(' ')) + _(".*") + _(" ") + String::Format(_("%p"),t->on_any_star));
@@ -438,10 +431,10 @@ String KeywordDatabase::expand(const String& text,
// Find keywords
while (!tagged.empty()) {
vector<KeywordTrie*> current; // current location(s) in the trie
vector<KeywordTrie*> next; // location(s) after this step
set<const Keyword*> used; // keywords already investigated
current.push_back(root);
vector<const KeywordTrie*> current; // current location(s) in the trie
vector<const KeywordTrie*> next; // location(s) after this step
set<const Keyword*> used; // keywords already investigated
current.push_back(root.get());
closure(current);
// is the keyword expanded? From <kw-?> tag
// Possible values are:
@@ -453,7 +446,7 @@ String KeywordDatabase::expand(const String& text,
char expand_type = default_expand_type;
for (size_t i = 0 ; i < tagged.size() ;) {
Char c = tagged.GetChar(i);
wxUniChar c = tagged.GetChar(i);
// tag?
if (c == _('<')) {
if (is_substr(tagged, i, _("<kw-")) && i + 4 < tagged.size()) {
@@ -476,9 +469,9 @@ String KeywordDatabase::expand(const String& text,
}
// find 'next' trie node set matching c
FOR_EACH(kt, current) {
map<Char,KeywordTrie*>::const_iterator it = kt->children.find(c);
auto it = kt->children.find(c);
if (it != kt->children.end()) {
next.push_back(it->second);
next.push_back(it->second.get());
}
// TODO: on any star first or last?
if (kt->on_any_star) {
+3 -3
View File
@@ -140,7 +140,7 @@ DECLARE_DYNAMIC_ARG(KeywordUsageStatistics*, keyword_usage_statistics);
* The database should be rebuild.
*/
class KeywordDatabase {
public:
public:
KeywordDatabase();
~KeywordDatabase();
@@ -165,8 +165,8 @@ class KeywordDatabase {
*/
String expand(const String& text, const ScriptValueP& match_condition, const ScriptValueP& expand_default, const ScriptValueP& combine_script, Context& ctx) const;
private:
KeywordTrie* root; ///< Data structure for finding keywords
private:
unique_ptr<KeywordTrie> root; ///< Data structure for finding keywords
/// (try to) expand a single keyword
/** If the keyword matches:
+2 -1
View File
@@ -223,7 +223,8 @@ void Set::reflect_cards<Writer> (Writer& tag) {
// writeFile won't quite work because we'd need
// include file: card: filename
// to do that
Writer writer(openOut(full_name), app_version);
auto stream = openOut(full_name);
Writer writer(*stream, app_version);
writer.handle(_("card"), card);
referenceFile(full_name);
REFLECT_N("include_file", full_name);
+4 -3
View File
@@ -295,14 +295,15 @@ void Settings::read() {
String filename = settingsFile();
if (wxFileExists(filename)) {
// settings file not existing is not an error
shared_ptr<wxFileInputStream> file = make_shared<wxFileInputStream>(filename);
if (!file->Ok()) return; // failure is not an error
wxFileInputStream file(filename);
if (!file.Ok()) return; // failure is not an error
Reader reader(file, nullptr, filename);
reader.handle_greedy(*this);
}
}
void Settings::write() {
Writer writer(make_shared<wxFileOutputStream>(settingsFile()), app_version);
wxFileOutputStream file(settingsFile());
Writer writer(file, app_version);
writer.handle(*this);
}