Merge pull request #1795 from KLayout/bugfix/issue-1793

Fixing issue #1793 (problem with default arguments in Python)

src/db/db/dbLayoutToNetlist.cc

@@ -1,5 +1,4 @@
 
-
 /*
 
   KLayout Layout Viewer
@@ -397,7 +396,7 @@ void LayoutToNetlist::join_nets (const tl::GlobPattern &cell, const std::set<std
   m_joined_nets_per_cell.push_back (std::make_pair (cell, gp));
 }
 
-#if defined(_DEBUG)
+#if defined(HAVE_DEBUG)
 static bool check_many_pins (const db::Netlist *netlist)
 {
   bool ok = true;
@@ -430,7 +429,7 @@ void LayoutToNetlist::extract_netlist ()
   do_soft_connections ();
 
   //  implement the "join_nets" (aka "must connect") feature
-#if defined(_DEBUG)
+#if defined(HAVE_DEBUG)
   //  NOTE: the join_nets feature only works for "one pin per net" case
   //  TODO: either fix that or make sure we do not get multiple pins per net.
   //  Right now, there no known case that produces multiple pins on a net at

src/gsi/gsi/gsiClassBase.cc

@@ -432,7 +432,7 @@ static const std::set<std::pair<std::string, bool> > &name_map_for_class (const
   return cc->second;
 }
 
-#if defined(_DEBUG)
+#if defined(HAVE_DEBUG)
 static std::string type_signature (const gsi::ArgType &t)
 {
   gsi::ArgType tr (t);
@@ -622,7 +622,7 @@ ClassBase::merge_declarations ()
     tl_assert (! c->declaration () || c->declaration () == &*c);
   }
 
-#if defined(_DEBUG)
+#if defined(HAVE_DEBUG)
   //  do a sanity check
   for (gsi::ClassBase::class_iterator c = gsi::ClassBase::begin_classes (); c != gsi::ClassBase::end_classes (); ++c) {
 
@@ -634,6 +634,16 @@ ClassBase::merge_declarations ()
           method_counts [signature (*m, *s)] += 1;
         }
       }
+      //  try to obtain the default values to find potential binding issues
+      for (gsi::MethodBase::argument_iterator a = (*m)->begin_arguments (); a != (*m)->end_arguments (); ++a) {
+        if (a->spec ()) {
+          try {
+            a->spec ()->default_value ();
+          } catch (tl::Exception &ex) {
+            tl::warn << "Method " << signature (*m, *(*m)->begin_synonyms ()) << ": error obtaining default value for argument '" << a->spec ()->name () << "': " << ex.msg ();
+          }
+        }
+      }
     }
 
     for (std::map<std::string, int>::const_iterator mc = method_counts.begin (); mc != method_counts.end (); ++mc) {
@@ -925,4 +935,3 @@ bool has_class (const std::type_info &ti)
 }
 
 }
-

src/gsi/gsi/gsiExpression.cc

@@ -1036,10 +1036,10 @@ VariantUserClassImpl::execute_gsi (const tl::ExpressionParserContext & /*context
               //  leave it to the consumer to establish the default values (that is faster)
               break;
             }
-            const tl::Variant &def_value = a->spec ()->default_value ();
-            //  NOTE: this const_cast means we need to take care that we do not use default values on "out" parameters.
-            //  Otherwise there is a chance we will modify the default value.
-            gsi::push_arg (arglist, *a, const_cast<tl::Variant &> (def_value), &heap);
+            //  Note: we will use the default value variant for longer, so push it to the heap (#1793)
+            tl::Variant *def_value = new tl::Variant (a->spec ()->default_value ());
+            heap.push (def_value);
+            gsi::push_arg (arglist, *a, *def_value, &heap);
           } else {
             throw tl::Exception (tl::to_string ("No argument provided (positional or keyword) and no default value available"));
           }

src/klayout.pri

@@ -359,3 +359,6 @@ DEFINES += \
 
 VERSION = $$KLAYOUT_VERSION
 
+CONFIG(debug, debug|release) {
+  DEFINES += HAVE_DEBUG
+}

src/pya/pya/pyaCallables.cc

@@ -764,10 +764,10 @@ push_args (gsi::SerialArgs &arglist, const gsi::MethodBase *meth, PyObject *args
             //  leave it to the consumer to establish the default values (that is faster)
             break;
           }
-          const tl::Variant &def_value = a->spec ()->default_value ();
-          //  NOTE: this const_cast means we need to take care that we do not use default values on "out" parameters.
-          //  Otherwise there is a chance we will modify the default value.
-          gsi::push_arg (arglist, *a, const_cast<tl::Variant &> (def_value), &heap);
+          //  Note: we will use the default value variant for longer, so push it to the heap (#1793)
+          tl::Variant *def_value = new tl::Variant (a->spec ()->default_value ());
+          heap.push (def_value);
+          gsi::push_arg (arglist, *a, *def_value, &heap);
         } else {
           throw tl::Exception (tl::to_string (tr ("No argument provided (positional or keyword) and no default value available")));
         }

src/rba/rba/rba.cc

@@ -1125,10 +1125,10 @@ push_args (gsi::SerialArgs &arglist, const gsi::MethodBase *meth, VALUE *argv, i
             //  leave it to the consumer to establish the default values (that is faster)
             break;
           }
-          const tl::Variant &def_value = a->spec ()->default_value ();
-          //  NOTE: this const_cast means we need to take care that we do not use default values on "out" parameters.
-          //  Otherwise there is a chance we will modify the default value.
-          gsi::push_arg (arglist, *a, const_cast<tl::Variant &> (def_value), &heap);
+          //  Note: we will use the default value variant for longer, so push it to the heap (#1793)
+          tl::Variant *def_value = new tl::Variant (a->spec ()->default_value ());
+          heap.push (def_value);
+          gsi::push_arg (arglist, *a, *def_value, &heap);
         } else {
           throw tl::Exception (tl::to_string (tr ("No argument provided (positional or keyword) and no default value available")));
         }

src/tl/tl/tlVariant.cc

@@ -317,15 +317,19 @@ Variant::Variant (const std::vector<char> &ba)
 #if defined(HAVE_QT)
 
 Variant::Variant (const QByteArray &qba) 
-  : m_type (t_qbytearray), m_string (0)
+  : m_type (qba.isNull () ? t_nil : t_qbytearray), m_string (0)
 {
-  m_var.m_qbytearray = new QByteArray (qba);
+  if (! qba.isNull ()) {
+    m_var.m_qbytearray = new QByteArray (qba);
+  }
 }
 
 Variant::Variant (const QString &qs) 
-  : m_type (t_qstring), m_string (0)
+  : m_type (qs.isNull () ? t_nil : t_qstring), m_string (0)
 {
-  m_var.m_qstring = new QString (qs);
+  if (! qs.isNull ()) {
+    m_var.m_qstring = new QString (qs);
+  }
 }
 
 Variant::Variant (const QVariant &v)
@@ -526,10 +530,14 @@ Variant::Variant (const std::string &s)
 }
 
 Variant::Variant (const char *s) 
-  : m_type (t_string)
+  : m_type (s != 0 ? t_string : t_nil)
 {
-  m_string = new char [strlen (s) + 1];
-  strcpy (m_string, s);
+  if (s) {
+    m_string = new char [strlen (s) + 1];
+    strcpy (m_string, s);
+  } else {
+    m_string = 0;
+  }
 }
 
 Variant::Variant (double d)
@@ -676,7 +684,9 @@ Variant::reset ()
 Variant &
 Variant::operator= (const char *s)
 {
-  if (m_type == t_string && s == m_string) {
+  if (! s) {
+    reset ();
+  } else if (m_type == t_string && s == m_string) {
     //  we are assigning to ourselves
   } else {
     char *snew = new char [strlen (s) + 1];
@@ -721,7 +731,9 @@ Variant::operator= (const std::vector<char> &s)
 Variant &
 Variant::operator= (const QByteArray &qs)
 {
-  if (m_type == t_qbytearray && &qs == m_var.m_qbytearray) {
+  if (qs.isNull ()) {
+    reset ();
+  } else if (m_type == t_qbytearray && &qs == m_var.m_qbytearray) {
     //  we are assigning to ourselves
   } else {
     QByteArray *snew = new QByteArray (qs);
@@ -735,7 +747,9 @@ Variant::operator= (const QByteArray &qs)
 Variant &
 Variant::operator= (const QString &qs)
 {
-  if (m_type == t_qstring && &qs == m_var.m_qstring) {
+  if (qs.isNull ()) {
+    reset ();
+  } else if (m_type == t_qstring && &qs == m_var.m_qstring) {
     //  we are assigning to ourselves
   } else {
     QString *snew = new QString (qs);
@@ -1784,9 +1798,9 @@ Variant::to_string () const
     if (m_type == t_nil) {
       r = "nil";
     } else if (m_type == t_double) {
-      r = tl::to_string (m_var.m_double);
+      r = tl::to_string (m_var.m_double, 15);
     } else if (m_type == t_float) {
-      r = tl::to_string (m_var.m_float);
+      r = tl::to_string (m_var.m_float, 7);
     } else if (m_type == t_char) {
       r = tl::to_string ((int) m_var.m_char);
     } else if (m_type == t_schar) {
@@ -2421,7 +2435,7 @@ Variant::to_parsable_string () const
   } else if (is_ulonglong ()) {
     return "#lu" + tl::to_string (to_ulonglong ());
   } else if (is_double ()) {
-    return "##" + tl::to_string (to_double ());
+    return "##" + tl::to_string (to_double (), 15);
   } else if (is_bool ()) {
     return m_var.m_bool ? "true" : "false";
   } else if (is_nil ()) {

src/tl/tl/tlVariant.h

@@ -29,6 +29,8 @@
 #include <string>
 #include <vector>
 #include <map>
+#include <set>
+#include <list>
 #include <stdexcept>
 #include <typeinfo>
 
@@ -358,7 +360,99 @@ public:
   }
 
   /**
-   *  @brief Initialize the Variant with an explicit vector or variants
+   *  @brief Initialize with a const user type (will always create a reference and NOT own the object)
+   */
+  template <class T>
+  Variant (const T *obj)
+    : m_type (t_nil), m_string (0)
+  {
+    if (obj) {
+      *this = make_variant_ref (obj);
+    }
+  }
+
+  /**
+   *  @brief Initialize with a non-const user type (will always create a reference and NOT own the object)
+   */
+  template <class T>
+  Variant (T *obj)
+    : m_type (t_nil), m_string (0)
+  {
+    if (obj) {
+      *this = make_variant_ref (obj);
+    }
+  }
+
+  /**
+   *  @brief Initialize the Variant with a STL vector
+   */
+  template <class T>
+  Variant (const std::vector<T> &list)
+    : m_type (t_list), m_string (0)
+  {
+    m_var.m_list = new std::vector<tl::Variant> ();
+    m_var.m_list->reserve (list.size ());
+    for (auto i = list.begin (); i != list.end (); ++i) {
+      m_var.m_list->push_back (tl::Variant (*i));
+    }
+  }
+
+  /**
+   *  @brief Initialize the Variant with a STL list
+   */
+  template <class T>
+  Variant (const std::list<T> &list)
+    : m_type (t_list), m_string (0)
+  {
+    m_var.m_list = new std::vector<tl::Variant> ();
+    m_var.m_list->reserve (list.size ());
+    for (auto i = list.begin (); i != list.end (); ++i) {
+      m_var.m_list->push_back (tl::Variant (*i));
+    }
+  }
+
+  /**
+   *  @brief Initialize the Variant with a STL set (not maintaining the set character)
+   */
+  template <class T>
+  Variant (const std::set<T> &list)
+    : m_type (t_list), m_string (0)
+  {
+    m_var.m_list = new std::vector<tl::Variant> ();
+    m_var.m_list->reserve (list.size ());
+    for (auto i = list.begin (); i != list.end (); ++i) {
+      m_var.m_list->push_back (tl::Variant (*i));
+    }
+  }
+
+  /**
+   *  @brief Initialize the Variant with a STL pair
+   */
+  template <class A, class B>
+  Variant (const std::pair<A, B> &pair)
+    : m_type (t_list), m_string (0)
+  {
+    m_var.m_list = new std::vector<tl::Variant> ();
+    m_var.m_list->reserve (2);
+    m_var.m_list->push_back (tl::Variant (pair.first));
+    m_var.m_list->push_back (tl::Variant (pair.second));
+  }
+
+  /**
+   *  @brief Initialize the Variant with a STL map
+   */
+  template <class K, class V>
+  Variant (const std::map<K, V> &map)
+    : m_type (t_array), m_string (0)
+  {
+    m_var.m_array = new std::map<tl::Variant, tl::Variant> ();
+    for (auto i = map.begin (); i != map.end (); ++i) {
+      m_var.m_array->insert (std::make_pair (tl::Variant (i->first), tl::Variant (i->second)));
+    }
+  }
+
+  /**
+   *  @brief Initialize the Variant with an explicit vector of variants
    */
   Variant (const std::vector<tl::Variant> &list)
     : m_type (t_list), m_string (0)
@@ -366,6 +460,15 @@ public:
     m_var.m_list = new std::vector<tl::Variant> (list);
   }
 
+  /**
+   *  @brief Initialize the Variant with an explicit map of variants
+   */
+  Variant (const std::map<tl::Variant, tl::Variant> &map)
+    : m_type (t_array), m_string (0)
+  {
+    m_var.m_array = new std::map<tl::Variant, tl::Variant> (map);
+  }
+
   /**
    *  @brief Initialize the Variant with a list
    */

src/tl/unit_tests/tlExpressionTests.cc

@@ -927,7 +927,7 @@ TEST(8)
   bool t;
 
   v = e.parse ("1==2?log('a'):log(2)").execute ();
-  EXPECT_EQ (v.to_string (), std::string ("0.69314718056"));
+  EXPECT_EQ (v.to_string (), std::string ("0.693147180559945"));
   t = false;
   try {
     v = e.parse ("1==1?log('a'):log(2)").execute ();

src/tl/unit_tests/tlVariantTests.cc

@@ -28,10 +28,12 @@
 #include "tlTypeTraits.h"
 #include "tlUnitTest.h"
 
-#include <cmath>
 #include <cstdio>
 #include <memory>
 
+#define _USE_MATH_DEFINES // for MSVC
+#include <math.h>
+
 struct A 
 {
   std::string a;
@@ -1092,8 +1094,84 @@ TEST(6)
   EXPECT_EQ (tl::Variant (-0.1 * (1.0 + 1.1e-13)) < tl::Variant (0.1), true);
 }
 
+//  precision of double serialization
+TEST(7a)
+{
+  tl::Variant v (M_PI);
+  tl::Variant vx;
+  std::string s (v.to_parsable_string ());
+  tl::Extractor ex (s.c_str ());
+  ex.read (vx);
+
+  EXPECT_EQ (fabs (M_PI - vx.to_double ()) < 4e-15, true);
+}
+
+TEST(7b)
+{
+  tl::Variant v ((float) M_PI);
+  tl::Variant vx;
+  std::string s (v.to_parsable_string ());
+  tl::Extractor ex (s.c_str ());
+  ex.read (vx);
+
+  EXPECT_EQ (fabs (M_PI - vx.to_double ()) < 1e-7, true);
+}
+
+//  null strings
+TEST(8)
+{
+  tl::Variant v ((const char *) 0);
+  EXPECT_EQ (v.to_parsable_string (), "nil");
+  v = tl::Variant ("abc");
+  EXPECT_EQ (v.to_parsable_string (), "'abc'");
+  v = (const char *) 0;
+  EXPECT_EQ (v.to_parsable_string (), "nil");
+
+#if defined(HAVE_QT)
+  v = tl::Variant (QString ());
+  EXPECT_EQ (v.to_parsable_string (), "nil");
+  v = tl::Variant (QString::fromUtf8 ("abc"));
+  EXPECT_EQ (v.to_parsable_string (), "'abc'");
+  v = QString ();
+  EXPECT_EQ (v.to_parsable_string (), "nil");
+
+  v = tl::Variant (QByteArray ());
+  EXPECT_EQ (v.to_parsable_string (), "nil");
+  v = tl::Variant (QByteArray ("abc"));
+  EXPECT_EQ (v.to_parsable_string (), "'abc'");
+  v = QByteArray ();
+  EXPECT_EQ (v.to_parsable_string (), "nil");
+#endif
+}
+
+//  create from STL containers
+TEST(9)
+{
+  tl::Variant v;
+
+  std::vector<int> vi;
+  vi.push_back (17);
+  vi.push_back (1);
+  std::list<int> li;
+  li.push_back (42);
+  li.push_back (-17);
+  std::set<int> si;
+  si.insert (31);
+  si.insert (63);
+  std::pair<int, int> pi (1, 3);
+  std::map<int, int> mi;
+  mi[17] = 42;
+  mi[-1] = 31;
+
+  EXPECT_EQ (tl::Variant (vi).to_parsable_string (), "(#17,#1)");
+  EXPECT_EQ (tl::Variant (li).to_parsable_string (), "(#42,#-17)");
+  EXPECT_EQ (tl::Variant (si).to_parsable_string (), "(#31,#63)");
+  EXPECT_EQ (tl::Variant (pi).to_parsable_string (), "(#1,#3)");
+  EXPECT_EQ (tl::Variant (mi).to_parsable_string (), "{#-1=>#31,#17=>#42}");
+}
+
 //  special numeric values
-TEST(7)
+TEST(10)
 {
   std::string s;
   tl::Extractor ex;