Use std::enable_shared_from_this instead of thisP parameters.

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();

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;

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

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;

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;

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

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(); }
 

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

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