From e005d47d563f5bd1b34184c0e887a4c2f150d31d Mon Sep 17 00:00:00 2001
From: Twan van Laarhoven <twanvl@gmail.com>
Date: Wed, 6 May 2020 22:59:10 +0200
Subject: [PATCH] Use std::enable_shared_from_this instead of thisP parameters.

---
 src/data/field.cpp              |  4 +--
 src/data/field.hpp              | 51 ++++++++++++++++-----------------
 src/gfx/generated_image.cpp     |  4 +--
 src/gfx/generated_image.hpp     |  4 +--
 src/gui/control/card_editor.cpp |  2 +-
 src/gui/set/console_panel.cpp   |  2 +-
 src/gui/value/choice.cpp        |  2 +-
 src/gui/value/editor.hpp        |  5 ++--
 src/render/card/viewer.cpp      |  2 +-
 src/render/value/viewer.hpp     |  7 ++---
 src/script/context.cpp          |  2 +-
 src/script/dependency.cpp       |  6 ++--
 src/script/functions/export.cpp |  2 +-
 src/script/functions/image.cpp  |  4 ---
 src/script/image.cpp            |  9 +-----
 src/script/image.hpp            |  3 --
 src/script/to_value.hpp         | 23 ++++++++-------
 src/script/value.cpp            | 45 +++++++++++++++++------------
 src/script/value.hpp            |  7 ++---
 src/util/smart_ptr.hpp          | 48 +++++++++++++++++++------------
 20 files changed, 114 insertions(+), 118 deletions(-)

diff --git a/src/data/field.cpp b/src/data/field.cpp
index 6a191b26..3c4b30ca 100644
--- a/src/data/field.cpp
+++ b/src/data/field.cpp
@@ -125,7 +125,7 @@ IMPLEMENT_REFLECTION(Style) {
 }
 
 void init_object(const FieldP& field, StyleP& style) {
-  if (!style) style = field->newStyle(field);
+  if (!style) style = field->newStyle();
 }
 template <> StyleP read_new<Style>(Reader&) {
   throw InternalError(_("IndexMap contains nullptr StyleP the application should have crashed already"));
@@ -290,7 +290,7 @@ void Value::updateSortValue(Context& ctx) {
 
 void init_object(const FieldP& field, ValueP& value) {
   if (!value)
-    value = field->newValue(field);
+    value = field->newValue();
 }
 template <> ValueP read_new<Value>(Reader&) {
   throw InternalError(_("IndexMap contains nullptr ValueP the application should have crashed already"));
diff --git a/src/data/field.hpp b/src/data/field.hpp
index d6695876..b6954348 100644
--- a/src/data/field.hpp
+++ b/src/data/field.hpp
@@ -36,7 +36,7 @@ DECLARE_DYNAMIC_ARG(Value*, value_being_updated);
 // ----------------------------------------------------------------------------- : Field
 
 /// Information on how to store a value
-class Field : public IntrusivePtrVirtualBase {
+class Field : public IntrusivePtrVirtualBase, public IntrusiveFromThis<Field> {
   public:
   Field();
   virtual ~Field();
@@ -61,11 +61,9 @@ class Field : public IntrusivePtrVirtualBase {
   Dependencies dependent_scripts; ///< Scripts that depend on values of this field
   
   /// Creates a new Value corresponding to this Field
-  /** thisP is a smart pointer to this */
-  virtual ValueP newValue(const FieldP& thisP) const = 0;
+  virtual ValueP newValue() = 0;
   /// Creates a new Style corresponding to this Field
-  /** thisP is a smart pointer to this */
-  virtual StyleP newStyle(const FieldP& thisP) const = 0;
+  virtual StyleP newStyle() = 0;
   /// Type of this field
   virtual String typeName() const = 0;
   
@@ -95,7 +93,7 @@ inline String type_name(const Field&) {
 // ----------------------------------------------------------------------------- : Style
 
 /// Style information needed to display a Value in a Field.
-class Style : public IntrusivePtrVirtualBase {
+class Style : public IntrusivePtrVirtualBase, public IntrusiveFromThis<Style> {
   public:
   Style(const FieldP&);
   virtual ~Style();
@@ -134,11 +132,9 @@ class Style : public IntrusivePtrVirtualBase {
   virtual StyleP clone() const = 0;
   
   /// Make a viewer object for values using this style
-  /** thisP is a smart pointer to this */
-  virtual ValueViewerP makeViewer(DataViewer& parent, const StyleP& thisP) = 0;
+  virtual ValueViewerP makeViewer(DataViewer& parent) = 0;
   /// Make an editor object for values using this style
-  /** thisP is a smart pointer to this */
-  virtual ValueViewerP makeEditor(DataEditor& parent, const StyleP& thisP) = 0;
+  virtual ValueViewerP makeEditor(DataEditor& parent) = 0;
   
   /// Update scripted values of this style, return nonzero if anything has changed.
   /** The caller should tellListeners()
@@ -264,20 +260,19 @@ inline String type_name(const Value&) {
 // ----------------------------------------------------------------------------- : Utilities
 
 #define DECLARE_FIELD_TYPE(Type) \
-  DECLARE_REFLECTION_OVERRIDE(); public: \
-  virtual ValueP newValue(const FieldP& thisP) const; \
-  virtual StyleP newStyle(const FieldP& thisP) const; \
-  virtual String typeName() const
+  DECLARE_REFLECTION_OVERRIDE(); \
+  public: \
+  virtual ValueP newValue() override; \
+  virtual StyleP newStyle() override; \
+  virtual String typeName() const override
 
 // implement newStyle and newValue
 #define IMPLEMENT_FIELD_TYPE(Type, NAME) \
-  StyleP Type ## Field::newStyle(const FieldP& thisP) const { \
-    assert(thisP.get() == this); \
-    return make_intrusive<Type ## Style>(static_pointer_cast<Type ## Field>(thisP)); \
+  StyleP Type ## Field::newStyle() { \
+    return make_intrusive<Type ## Style>(static_pointer_cast<Type ## Field>(intrusive_from_this())); \
   } \
-  ValueP Type ## Field::newValue(const FieldP& thisP) const { \
-    assert(thisP.get() == this); \
-    return make_intrusive<Type ## Value>(static_pointer_cast<Type ## Field>(thisP)); \
+  ValueP Type ## Field::newValue() { \
+    return make_intrusive<Type ## Value>(static_pointer_cast<Type ## Field>(intrusive_from_this())); \
   } \
   StyleP Type ## Style::clone() const { \
     return make_intrusive<Type ## Style>(*this); \
@@ -290,17 +285,19 @@ inline String type_name(const Value&) {
   }
 
 #define DECLARE_STYLE_TYPE(Type) \
-  DECLARE_REFLECTION_OVERRIDE(); public: \
+  DECLARE_REFLECTION_OVERRIDE(); \
+  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)
+  StyleP clone() const override; \
+  ValueViewerP makeViewer(DataViewer& parent) override; \
+  ValueViewerP makeEditor(DataEditor& parent) override
 
 #define DECLARE_VALUE_TYPE(Type,ValueType_) \
-  DECLARE_REFLECTION_OVERRIDE(); public: \
+  DECLARE_REFLECTION_OVERRIDE(); \
+  public: \
   DECLARE_HAS_FIELD(Type) \
-  virtual ValueP clone() const; \
-  virtual String toString() const; \
+  ValueP clone() const override; \
+  String toString() const override; \
   typedef ValueType_ ValueType
 
 // implement field() which returns a field with the right (derived) type
diff --git a/src/gfx/generated_image.cpp b/src/gfx/generated_image.cpp
index 73b12b93..14f45939 100644
--- a/src/gfx/generated_image.cpp
+++ b/src/gfx/generated_image.cpp
@@ -19,8 +19,8 @@
 
 ScriptType GeneratedImage::type() const { return SCRIPT_IMAGE; }
 String GeneratedImage::typeName() const { return _TYPE_("image"); }
-GeneratedImageP GeneratedImage::toImage(const ScriptValueP& thisP) const {
-  return static_pointer_cast<GeneratedImage>(thisP);
+GeneratedImageP GeneratedImage::toImage() const {
+  return const_cast<GeneratedImage*>(this)->intrusive_from_this();
 }
 
 Image GeneratedImage::generateConform(const Options& options) const {
diff --git a/src/gfx/generated_image.hpp b/src/gfx/generated_image.hpp
index 108cf513..35e2d775 100644
--- a/src/gfx/generated_image.hpp
+++ b/src/gfx/generated_image.hpp
@@ -23,7 +23,7 @@ class Package;
 /// An image that is generated from a script.
 /** The actual generation is independend of the script execution
  */
-class GeneratedImage : public ScriptValue {
+class GeneratedImage : public ScriptValue, public IntrusiveFromThis<GeneratedImage> {
   public:
   /// Options for generating the image
   struct Options {
@@ -62,7 +62,7 @@ class GeneratedImage : public ScriptValue {
   
   ScriptType type() const override;
   String typeName() const override;
-  GeneratedImageP toImage(const ScriptValueP& thisP) const override;
+  GeneratedImageP toImage() const override;
 };
 
 /// Resize an image to conform to the options
diff --git a/src/gui/control/card_editor.cpp b/src/gui/control/card_editor.cpp
index ca147f91..5acb64e3 100644
--- a/src/gui/control/card_editor.cpp
+++ b/src/gui/control/card_editor.cpp
@@ -34,7 +34,7 @@ DataEditor::DataEditor(Window* parent, int id, long style)
 }
 
 ValueViewerP DataEditor::makeViewer(const StyleP& style) {
-  return style->makeEditor(*this, style);
+  return style->makeEditor(*this);
 }
 
 // ----------------------------------------------------------------------------- : Utility for ValueViewers
diff --git a/src/gui/set/console_panel.cpp b/src/gui/set/console_panel.cpp
index 93de5023..f34e232a 100644
--- a/src/gui/set/console_panel.cpp
+++ b/src/gui/set/console_panel.cpp
@@ -504,7 +504,7 @@ void ConsolePanel::exec(String const& command) {
     ScriptType type = result->type();
     if (type == SCRIPT_IMAGE) {
       GeneratedImage::Options options(0,0, set->stylesheet.get(), set.get());
-      wxImage image = result->toImage(result)->generate(options);
+      wxImage image = result->toImage()->generate(options);
       message->bitmap = wxBitmap(image);
     } else if (type == SCRIPT_COLOR) {
       message->text = result->toCode();
diff --git a/src/gui/value/choice.cpp b/src/gui/value/choice.cpp
index 23193aab..639425cd 100644
--- a/src/gui/value/choice.cpp
+++ b/src/gui/value/choice.cpp
@@ -152,7 +152,7 @@ void DropDownChoiceListBase::generateThumbnailImages() {
       try {
         String name = canonical_name_form(field().choices->choiceName(i));
         ctx.setVariable(_("input"), to_script(name));
-        GeneratedImageP img = image_from_script(style().image.getValidScriptP()->eval(ctx));
+        GeneratedImageP img = style().image.getValidScriptP()->eval(ctx)->toImage();
         style().choice_images.insert(make_pair(name, ScriptableImage(img)));
       } catch (const Error& e) {
         handle_error(Error(e.what() + _("\n  while generating choice images for drop down list")));
diff --git a/src/gui/value/editor.hpp b/src/gui/value/editor.hpp
index d456924e..c723cee7 100644
--- a/src/gui/value/editor.hpp
+++ b/src/gui/value/editor.hpp
@@ -144,9 +144,8 @@ class ValueEditor {
     public:
 
 #define IMPLEMENT_VALUE_EDITOR(Type) \
-  ValueViewerP Type##Style::makeEditor(DataEditor& parent, const StyleP& thisP) { \
-    assert(thisP.get() == this); \
-    return ValueViewerP(new Type##ValueEditor(parent, static_pointer_cast<Type##Style>(thisP))); \
+  ValueViewerP Type##Style::makeEditor(DataEditor& parent) { \
+    return ValueViewerP(new Type##ValueEditor(parent, static_pointer_cast<Type##Style>(intrusive_from_this()))); \
   } \
   Type##ValueEditor::Type##ValueEditor(DataEditor& parent, const Type##StyleP& style) \
     : Type##ValueViewer(parent, style)
diff --git a/src/render/card/viewer.cpp b/src/render/card/viewer.cpp
index 27e16e1a..eb6ff655 100644
--- a/src/render/card/viewer.cpp
+++ b/src/render/card/viewer.cpp
@@ -198,7 +198,7 @@ void DataViewer::setData(IndexMap<FieldP,ValueP>& values, IndexMap<FieldP,ValueP
 
 
 ValueViewerP DataViewer::makeViewer(const StyleP& style) {
-  return style->makeViewer(*this, style);
+  return style->makeViewer(*this);
 }
 
 void DataViewer::onAction(const Action& action, bool undone) {
diff --git a/src/render/value/viewer.hpp b/src/render/value/viewer.hpp
index 271880e1..f74f2c5a 100644
--- a/src/render/value/viewer.hpp
+++ b/src/render/value/viewer.hpp
@@ -113,9 +113,6 @@ class ValueViewer : public StyleListener {
     Type##ValueViewer(DataViewer& parent, const Type ## StyleP& style)
 
 #define IMPLEMENT_VALUE_VIEWER(Type) \
-  ValueViewerP Type##Style::makeViewer(DataViewer& parent, const StyleP& thisP) { \
-    assert(thisP.get() == this); \
-    return ValueViewerP(new Type##ValueViewer(parent, static_pointer_cast<Type##Style>(thisP))); \
+  ValueViewerP Type##Style::makeViewer(DataViewer& parent) { \
+    return ValueViewerP(new Type##ValueViewer(parent, static_pointer_cast<Type##Style>(intrusive_from_this()))); \
   }
-
-
diff --git a/src/script/context.cpp b/src/script/context.cpp
index a5d1f557..a8c39b8e 100644
--- a/src/script/context.cpp
+++ b/src/script/context.cpp
@@ -362,7 +362,7 @@ void Context::closeScope(size_t scope) {
 void instrUnary(UnaryInstructionType i, ScriptValueP& a) {
   switch (i) {
     case I_ITERATOR_C:
-      a = a->makeIterator(a);
+      a = a->makeIterator();
       break;
     case I_NEGATE: {
       ScriptType at = a->type();
diff --git a/src/script/dependency.cpp b/src/script/dependency.cpp
index f03960d1..69911589 100644
--- a/src/script/dependency.cpp
+++ b/src/script/dependency.cpp
@@ -49,8 +49,8 @@ public:
   ScriptValueP dependencies(Context& ctx, const Dependency& dep) const override {
     return unified( a->dependencies(ctx,dep), b->dependencies(ctx,dep));
   }
-  ScriptValueP makeIterator(const ScriptValueP& thisP) const override {
-    return unified(a->makeIterator(a), b->makeIterator(b));
+  ScriptValueP makeIterator() const override {
+    return unified(a->makeIterator(), b->makeIterator());
   }
   ScriptValueP dependencyMember(const String& name, const Dependency& dep) const override {
     return unified(a->dependencyMember(name,dep), b->dependencyMember(name,dep));
@@ -307,7 +307,7 @@ ScriptValueP Context::dependencies(const Dependency& dep, const Script& script)
           ScriptValueP& a = stack.back();
           switch (i.instr1) {
             case I_ITERATOR_C:
-              a = a->makeIterator(a); // as normal
+              a = a->makeIterator(); // as normal
               break;
             default:
               a = dependency_dummy;
diff --git a/src/script/functions/export.cpp b/src/script/functions/export.cpp
index 8b767e1d..124a939f 100644
--- a/src/script/functions/export.cpp
+++ b/src/script/functions/export.cpp
@@ -417,7 +417,7 @@ SCRIPT_FUNCTION(write_image_file) {
   if (card) {
     image = conform_image(export_bitmap(ei.set, card->getValue()).ConvertToImage(), options);
   } else {
-    image = image_from_script(input)->generateConform(options);
+    image = input->toImage()->generateConform(options);
   }
   if (!image.Ok()) throw Error(_("Unable to generate image for file ") + file);
   // write
diff --git a/src/script/functions/image.cpp b/src/script/functions/image.cpp
index f467b61d..3e9caf65 100644
--- a/src/script/functions/image.cpp
+++ b/src/script/functions/image.cpp
@@ -23,10 +23,6 @@ void parse_enum(const String&, ImageCombine& out);
 
 // ----------------------------------------------------------------------------- : Utility
 
-template <> inline GeneratedImageP from_script<GeneratedImageP>(const ScriptValueP& value) {
-  return image_from_script(value);
-}
-
 SCRIPT_FUNCTION(to_image) {
   SCRIPT_PARAM_C(GeneratedImageP, input);
   return input;
diff --git a/src/script/image.cpp b/src/script/image.cpp
index 161087f2..f22f8d6d 100644
--- a/src/script/image.cpp
+++ b/src/script/image.cpp
@@ -15,13 +15,6 @@
 #include <gfx/generated_image.hpp>
 #include <data/field/image.hpp>
 
-// ----------------------------------------------------------------------------- : Utility
-
-// convert any script value to a GeneratedImageP
-GeneratedImageP image_from_script(const ScriptValueP& value) {
-  return value->toImage(value);
-}
-
 // ----------------------------------------------------------------------------- : ScriptableImage
 
 Image ScriptableImage::generate(const GeneratedImage::Options& options) const {
@@ -50,7 +43,7 @@ ImageCombine ScriptableImage::combine() const {
 
 bool ScriptableImage::update(Context& ctx) {
   if (!isScripted()) return false;
-  GeneratedImageP new_value = image_from_script(script.invoke(ctx));
+  GeneratedImageP new_value = script.invoke(ctx)->toImage();
   if (!new_value || !value || *new_value != *value) {
     value = new_value;
     return true;
diff --git a/src/script/image.hpp b/src/script/image.hpp
index 2937a83e..d80f4e84 100644
--- a/src/script/image.hpp
+++ b/src/script/image.hpp
@@ -70,9 +70,6 @@ class ScriptableImage {
 /// Missing for now
 inline ScriptValueP to_script(const ScriptableImage&) { return script_nil; }
 
-/// Convert a script value to a GeneratedImageP
-GeneratedImageP image_from_script(const ScriptValueP& value);
-
 // ----------------------------------------------------------------------------- : CachedScriptableImage
 
 /// A version of ScriptableImage that does caching
diff --git a/src/script/to_value.hpp b/src/script/to_value.hpp
index 6915a2b2..69fa40a2 100644
--- a/src/script/to_value.hpp
+++ b/src/script/to_value.hpp
@@ -69,14 +69,14 @@ inline ScriptValueP delay_error(const String& m) {
 // ----------------------------------------------------------------------------- : Iterators
 
 // Iterator over a collection
-struct ScriptIterator : public ScriptValue {
+struct ScriptIterator : public ScriptValue, public IntrusiveFromThis<ScriptIterator> {
   ScriptType type() const override;
   String typeName() const override;
   CompareWhat compareAs(String&, void const*&) const override;
 
   /// Return the next item for this iterator, or ScriptValueP() if there is no such item
   ScriptValueP next(ScriptValueP* key_out = nullptr, int* index_out = nullptr) override = 0;
-  ScriptValueP makeIterator(const ScriptValueP& thisP) const override;
+  ScriptValueP makeIterator() const override;
 };
 
 // make an iterator over a range
@@ -127,7 +127,7 @@ public:
       return ScriptValue::getIndex(index);
     }
   }
-  ScriptValueP makeIterator(const ScriptValueP& thisP) const override {
+  ScriptValueP makeIterator() const override {
     return make_intrusive<ScriptCollectionIterator<Collection>>(value);
   }
   int itemCount() const override {
@@ -193,11 +193,11 @@ private:
 // ----------------------------------------------------------------------------- : Collections : from script
 
 /// Script value containing a custom collection, returned from script functions
-class ScriptCustomCollection : public ScriptCollectionBase {
+class ScriptCustomCollection : public ScriptCollectionBase, public IntrusiveFromThis<ScriptCustomCollection>{
 public:
   ScriptValueP getMember(const String& name) const override;
   ScriptValueP getIndex(int index) const override;
-  ScriptValueP makeIterator(const ScriptValueP& thisP) const override;
+  ScriptValueP makeIterator() const override;
   int itemCount() const override {
     return (int)(value.size() + key_value.size());
   }
@@ -223,7 +223,7 @@ public:
   inline ScriptConcatCollection(ScriptValueP a, ScriptValueP b) : a(a), b(b) {}
   ScriptValueP getMember(const String& name) const override;
   ScriptValueP getIndex(int index) const override;
-  ScriptValueP makeIterator(const ScriptValueP& thisP) const override;
+  ScriptValueP makeIterator() const override;
   int itemCount() const override { return a->itemCount() + b->itemCount(); }
   /// Collections can be compared by comparing pointers
   CompareWhat compareAs(String&, void const*& compare_ptr) const override {
@@ -263,8 +263,8 @@ public:
   String toCode() const override {
     ScriptValueP d = getDefault(); return d ? d->toCode() : to_code(*value);
   }
-  GeneratedImageP toImage(const ScriptValueP& thisP) const override {
-    ScriptValueP d = getDefault(); return d ? d->toImage(d) : ScriptValue::toImage(thisP);
+  GeneratedImageP toImage() const override {
+    ScriptValueP d = getDefault(); return d ? d->toImage() : ScriptValue::toImage();
   }
   ScriptValueP getMember(const String& name) const override {
     #if USE_SCRIPT_PROFILING
@@ -295,12 +295,12 @@ public:
   void dependencyThis(const Dependency& dep) override {
     mark_dependency_value(*value, dep);
   }
-  ScriptValueP makeIterator(const ScriptValueP& thisP) const override {
+  ScriptValueP makeIterator() const override {
     ScriptValueP it = make_iterator(*value);
     if (it) return it;
     ScriptValueP d = getDefault();
-    if (d) return d->makeIterator(d);
-    return ScriptValue::makeIterator(thisP);
+    if (d) return d->makeIterator();
+    return ScriptValue::makeIterator();
   }
   int itemCount() const override {
     int i = item_count(*value);
@@ -436,4 +436,5 @@ template <> inline double       from_script<double>      (const ScriptValueP& va
 template <> inline bool         from_script<bool>        (const ScriptValueP& value) { return value->toBool(); }
 template <> inline Color        from_script<Color>       (const ScriptValueP& value) { return value->toColor(); }
 template <> inline wxDateTime   from_script<wxDateTime>  (const ScriptValueP& value) { return value->toDateTime(); }
+template <> inline GeneratedImageP from_script<GeneratedImageP>(const ScriptValueP& value) { return value->toImage(); }
 
diff --git a/src/script/value.cpp b/src/script/value.cpp
index ebee11a5..86225dd2 100644
--- a/src/script/value.cpp
+++ b/src/script/value.cpp
@@ -35,7 +35,7 @@ Color ScriptValue::toColor() const {
 wxDateTime ScriptValue::toDateTime() const {
   throw ScriptErrorConversion(typeName(), _TYPE_("date"));
 }
-GeneratedImageP ScriptValue::toImage(const ScriptValueP&) const {
+GeneratedImageP ScriptValue::toImage() const {
   throw ScriptErrorConversion(typeName(), _TYPE_("image"   ));
 }
 String ScriptValue::toCode() const {
@@ -49,7 +49,7 @@ ScriptValueP ScriptValue::eval(Context&, bool) const {
 ScriptValueP ScriptValue::next(ScriptValueP* key_out, int* index_out) {
   throw InternalError(_("Can't convert from ")+typeName()+_(" to iterator"));
 }
-ScriptValueP ScriptValue::makeIterator(const ScriptValueP&) const {
+ScriptValueP ScriptValue::makeIterator() const {
   return delay_error(ScriptErrorConversion(typeName(), _TYPE_("collection")));
 }
 int ScriptValue::itemCount() const {
@@ -150,7 +150,7 @@ public:
   bool toBool() const override { throw error; }
   Color toColor() const override { throw error; }
   wxDateTime toDateTime() const override { throw error; }
-  GeneratedImageP toImage(const ScriptValueP& thisP) const override { throw error; }
+  GeneratedImageP toImage() const override { throw error; }
   int itemCount() const override { throw error; }
   CompareWhat compareAs(String&, void const*&) const override { throw error; }
 
@@ -158,7 +158,7 @@ public:
   ScriptValueP getMember(const String& name) const override { return delay_error(error); }
   ScriptValueP dependencyMember(const String& name, const Dependency&) const override { return delay_error(error); }
   ScriptValueP dependencies(Context&, const Dependency&) const override { return delay_error(error); }
-  ScriptValueP makeIterator(const ScriptValueP&) const override { return delay_error(error); }
+  ScriptValueP makeIterator() const override { return delay_error(error); }
   ScriptValueP eval(Context&, bool openScope) const override { return delay_error(error); }
 
 private:
@@ -171,10 +171,18 @@ ScriptValueP delay_error(const ScriptError& error) {
 
 // ----------------------------------------------------------------------------- : Iterators
 
-ScriptType ScriptIterator::type() const { return SCRIPT_ITERATOR; }
-String ScriptIterator::typeName() const { return _("iterator"); }
-CompareWhat ScriptIterator::compareAs(String&, void const*&) const { return COMPARE_NO; }
-ScriptValueP ScriptIterator::makeIterator(const ScriptValueP& thisP) const { return thisP; }
+ScriptType ScriptIterator::type() const {
+  return SCRIPT_ITERATOR;
+}
+String ScriptIterator::typeName() const {
+  return _("iterator");
+}
+CompareWhat ScriptIterator::compareAs(String&, void const*&) const {
+  return COMPARE_NO;
+}
+ScriptValueP ScriptIterator::makeIterator() const {
+  return const_cast<ScriptIterator*>(this)->intrusive_from_this();
+}
 
 // Iterator over a range of integers
 class ScriptRangeIterator : public ScriptIterator {
@@ -357,7 +365,7 @@ public:
     }
     return date;
   }
-  GeneratedImageP toImage(const ScriptValueP&) const override {
+  GeneratedImageP toImage() const override {
     if (value.empty()) {
       return make_intrusive<BlankImage>();
     } else {
@@ -437,8 +445,8 @@ public:
   double toDouble() const override { return 0.0; }
   int toInt() const override { return 0; }
   bool toBool() const override { return false; }
-  Color toColor() const override { return wxTransparentColour; }
-  GeneratedImageP toImage(const ScriptValueP&) const {
+  Color toColor() const override { return Color(); }
+  GeneratedImageP toImage() const override {
     return make_intrusive<BlankImage>();
   }
   String toCode() const override {
@@ -479,8 +487,8 @@ String ScriptCollectionBase::toCode() const {
 // Iterator over a custom collection
 class ScriptCustomCollectionIterator : public ScriptIterator {
 public:
-  ScriptCustomCollectionIterator(ScriptCustomCollection const* col, ScriptValueP const& col_owned)
-    : col(col), col_owned(col_owned), pos(0), it(col->key_value.begin()) {}
+  ScriptCustomCollectionIterator(intrusive_ptr<ScriptCustomCollection const> const& col)
+    : col(col), pos(0), it(col->key_value.begin()) {}
   ScriptValueP next(ScriptValueP* key_out, int* index_out) override {
     if (pos < col->value.size()) {
       if (key_out) *key_out = to_script((int)pos);
@@ -495,8 +503,7 @@ public:
     }
   }
 private:
-  ScriptCustomCollection const* col;
-  ScriptValueP col_owned; // own the collection so it doesn't get deleted
+  intrusive_ptr<ScriptCustomCollection const> col;
   size_t pos;
   map<String,ScriptValueP>::const_iterator it;
 };
@@ -516,8 +523,8 @@ ScriptValueP ScriptCustomCollection::getIndex(int index) const {
     return ScriptValue::getIndex(index);
   }
 }
-ScriptValueP ScriptCustomCollection::makeIterator(const ScriptValueP& thisP) const {
-  return make_intrusive<ScriptCustomCollectionIterator>(this, thisP);
+ScriptValueP ScriptCustomCollection::makeIterator() const {
+  return make_intrusive<ScriptCustomCollectionIterator>(const_cast<ScriptCustomCollection*>(this)->intrusive_from_this());
 }
 
 // ----------------------------------------------------------------------------- : Concat collection
@@ -560,8 +567,8 @@ ScriptValueP ScriptConcatCollection::getIndex(int index) const {
     return b->getIndex(index - itemsInA);
   }
 }
-ScriptValueP ScriptConcatCollection::makeIterator(const ScriptValueP& thisP) const {
-  return make_intrusive<ScriptConcatCollectionIterator>(a->makeIterator(a), b->makeIterator(b));
+ScriptValueP ScriptConcatCollection::makeIterator() const {
+  return make_intrusive<ScriptConcatCollectionIterator>(a->makeIterator(), b->makeIterator());
 }
 
 // ----------------------------------------------------------------------------- : Default arguments / closure
diff --git a/src/script/value.hpp b/src/script/value.hpp
index 268896ad..56e864d4 100644
--- a/src/script/value.hpp
+++ b/src/script/value.hpp
@@ -70,7 +70,7 @@ public:
   /// Convert this value to a wxDateTime
   virtual wxDateTime toDateTime() const;
   /// Convert this value to an image
-  virtual GeneratedImageP toImage(const ScriptValueP& thisP) const;
+  virtual GeneratedImageP toImage() const;
 
   /// Script code to generate this value
   virtual String toCode() const;
@@ -99,10 +99,7 @@ public:
   virtual ScriptValueP simplifyClosure(ScriptClosure&) const;
 
   /// Return an iterator for the current collection, an iterator is a value that has next()
-  /** thisP is a shared_ptr for this collection, needed for the iterator to take ownership of it.
-    * It can be null if the iterator always lives shorter than the collection.
-    */
-  virtual ScriptValueP makeIterator(const ScriptValueP& thisP = ScriptValueP()) const;
+  virtual ScriptValueP makeIterator() const;
   /// Return the next item for this iterator, or ScriptValueP() if there is no such item
   /** If key_out != nullptr, then it will receive the key of the item
    *  If index_out != nullptr, then it will receive to index of the item if it is in an indexable container
diff --git a/src/util/smart_ptr.hpp b/src/util/smart_ptr.hpp
index 97b8c621..66f94872 100644
--- a/src/util/smart_ptr.hpp
+++ b/src/util/smart_ptr.hpp
@@ -29,35 +29,47 @@ using std::make_unique;
   class Type; \
   typedef shared_ptr<Type> Type##P;
 
-// ----------------------------------------------------------------------------- : Intrusive pointers
+// ----------------------------------------------------------------------------- : Shared pointers
 
 #define DECLARE_POINTER_TYPE DECLARE_SHARED_POINTER_TYPE
 
-template <typename T> class IntrusivePtrBase {};
 template <typename T> using intrusive_ptr = shared_ptr<T>;
 
+/// Base class for types that can be pointed to
+template <typename T> class IntrusivePtrBase {};
+
+/// IntrusivePtrBase with a virtual destructor
+class IntrusivePtrVirtualBase : public IntrusivePtrBase<IntrusivePtrVirtualBase> {
+public:
+  virtual ~IntrusivePtrVirtualBase() {}
+};
+
+class IntrusivePtrBaseWithDelete : public IntrusivePtrBase<IntrusivePtrBaseWithDelete> {
+public:
+  virtual ~IntrusivePtrBaseWithDelete() {}
+protected:
+  /// Delete this object
+  virtual void destroy() {
+    delete this;
+  }
+};
+
+template <typename T>
+class IntrusiveFromThis : public std::enable_shared_from_this<T> {
+protected:
+  inline intrusive_ptr<T> intrusive_from_this() {
+    return shared_from_this();
+  }
+};
+
 /// Allocate an object of type T and store it in a new intrusive_ptr, similar to std::make_shared
 template <typename T, class... Args>
 inline intrusive_ptr<T> make_intrusive(Args&&... args) {
   return std::make_shared<T>(std::forward<Args>(args)...);
 }
 
-/// IntrusivePtrBase with a virtual destructor
-class IntrusivePtrVirtualBase : public IntrusivePtrBase<IntrusivePtrVirtualBase> {
-  public:
-  virtual ~IntrusivePtrVirtualBase() {}
-};
-
-class IntrusivePtrBaseWithDelete : public IntrusivePtrBase<IntrusivePtrBaseWithDelete> {
-  public:
-  virtual ~IntrusivePtrBaseWithDelete() {}
-  protected:
-  /// Delete this object
-  virtual void destroy() {
-    delete this;
-  }
-};
-
 /// Pointer to 'anything'
 typedef intrusive_ptr<IntrusivePtrVirtualBase> VoidP;
 
+// ----------------------------------------------------------------------------- : Intrusive pointers
+