From 9a798383691c4e5d6ce38dbff91897dccaa339da Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 20 Jul 2024 18:58:29 +0200 Subject: [PATCH 1/9] Bugfix for issue-1793: using the heap to properly store the default values --- src/gsi/gsi/gsiExpression.cc | 8 ++++---- src/pya/pya/pyaCallables.cc | 8 ++++---- src/rba/rba/rba.cc | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/gsi/gsi/gsiExpression.cc b/src/gsi/gsi/gsiExpression.cc index 3d77ddada..46ca33374 100644 --- a/src/gsi/gsi/gsiExpression.cc +++ b/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 (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")); } diff --git a/src/pya/pya/pyaCallables.cc b/src/pya/pya/pyaCallables.cc index 05b463f40..5422c9968 100644 --- a/src/pya/pya/pyaCallables.cc +++ b/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 (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"))); } diff --git a/src/rba/rba/rba.cc b/src/rba/rba/rba.cc index e51abfeb4..21743409b 100644 --- a/src/rba/rba/rba.cc +++ b/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 (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"))); } From a8eaead404f4004b1dd67a9411974e9e4b1b798d Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 20 Jul 2024 19:57:11 +0200 Subject: [PATCH 2/9] Enhanced tl::Variant so it can capture all possible default values. Added a self-diagnosis step in debug builds. --- src/db/db/dbLayoutToNetlist.cc | 4 +- src/gsi/gsi/gsiClassBase.cc | 14 +++- src/klayout.pri | 3 + src/tl/tl/tlVariant.cc | 40 +++++++--- src/tl/tl/tlVariant.h | 105 ++++++++++++++++++++++++- src/tl/unit_tests/tlExpressionTests.cc | 2 +- src/tl/unit_tests/tlVariantTests.cc | 76 ++++++++++++++++++ 7 files changed, 225 insertions(+), 19 deletions(-) diff --git a/src/db/db/dbLayoutToNetlist.cc b/src/db/db/dbLayoutToNetlist.cc index 924b11a04..0a2e9b235 100644 --- a/src/db/db/dbLayoutToNetlist.cc +++ b/src/db/db/dbLayoutToNetlist.cc @@ -397,7 +397,7 @@ void LayoutToNetlist::join_nets (const tl::GlobPattern &cell, const std::set > &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::const_iterator mc = method_counts.begin (); mc != method_counts.end (); ++mc) { diff --git a/src/klayout.pri b/src/klayout.pri index a370e8490..14f394e2f 100644 --- a/src/klayout.pri +++ b/src/klayout.pri @@ -350,3 +350,6 @@ DEFINES += \ VERSION = $$KLAYOUT_VERSION +debug { + DEFINES += HAVE_DEBUG +} diff --git a/src/tl/tl/tlVariant.cc b/src/tl/tl/tlVariant.cc index 4ceacfb4b..830c5d656 100644 --- a/src/tl/tl/tlVariant.cc +++ b/src/tl/tl/tlVariant.cc @@ -317,15 +317,19 @@ Variant::Variant (const std::vector &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 &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); @@ -1761,9 +1775,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) { @@ -2398,7 +2412,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 ()) { diff --git a/src/tl/tl/tlVariant.h b/src/tl/tl/tlVariant.h index 4b12a892a..15b66f06b 100644 --- a/src/tl/tl/tlVariant.h +++ b/src/tl/tl/tlVariant.h @@ -29,6 +29,8 @@ #include #include #include +#include +#include #include #include @@ -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 + 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 + 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 + Variant (const std::vector &list) + : m_type (t_list), m_string (0) + { + m_var.m_list = new std::vector (); + 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 + Variant (const std::list &list) + : m_type (t_list), m_string (0) + { + m_var.m_list = new std::vector (); + 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 + Variant (const std::set &list) + : m_type (t_list), m_string (0) + { + m_var.m_list = new std::vector (); + 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 + Variant (const std::pair &pair) + : m_type (t_list), m_string (0) + { + m_var.m_list = new std::vector (); + 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 + Variant (const std::map &map) + : m_type (t_array), m_string (0) + { + m_var.m_array = new std::map (); + 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 &list) : m_type (t_list), m_string (0) @@ -366,6 +460,15 @@ public: m_var.m_list = new std::vector (list); } + /** + * @brief Initialize the Variant with an explicit map of variants + */ + Variant (const std::map &map) + : m_type (t_array), m_string (0) + { + m_var.m_array = new std::map (map); + } + /** * @brief Initialize the Variant with a list */ diff --git a/src/tl/unit_tests/tlExpressionTests.cc b/src/tl/unit_tests/tlExpressionTests.cc index 3d8719900..3f4710ad4 100644 --- a/src/tl/unit_tests/tlExpressionTests.cc +++ b/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 (); diff --git a/src/tl/unit_tests/tlVariantTests.cc b/src/tl/unit_tests/tlVariantTests.cc index 3ff19545d..9240a8339 100644 --- a/src/tl/unit_tests/tlVariantTests.cc +++ b/src/tl/unit_tests/tlVariantTests.cc @@ -1090,4 +1090,80 @@ 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 vi; + vi.push_back (17); + vi.push_back (1); + std::list li; + li.push_back (42); + li.push_back (-17); + std::set si; + si.insert (31); + si.insert (63); + std::pair pi (1, 3); + std::map 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}"); +} + } From 00aeeade68f8f39d5a64c671fbe29b1e585a7c55 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 20 Jul 2024 21:02:45 +0200 Subject: [PATCH 3/9] Updating antenna DRC golden data - needed as the precision of the double-typed tl::Variant output was increased --- testdata/drc/drcSimpleTests_au5.gds | Bin 9892 -> 9952 bytes testdata/drc/drcSimpleTests_au6.gds | Bin 3494 -> 3516 bytes testdata/drc/drcSimpleTests_au7.gds | Bin 22060 -> 22166 bytes 3 files changed, 0 insertions(+), 0 deletions(-) diff --git a/testdata/drc/drcSimpleTests_au5.gds b/testdata/drc/drcSimpleTests_au5.gds index c77c4e703d1679a2730ad4e832a64f384fcf1132..e89e303303c27ad24916b01fe95b98c5d9d57600 100644 GIT binary patch delta 664 zcmZ4D`@lDffsKKQDS|{L4=vr&aum2zw jq{k^aIbT&u(7@DKM delta 706 zcmaFhyTmt&fsKKQDS|Q-#303>i_B)=U}E#}bYfr-VP>^+>@@d2w)}&o z%MSeov!g;7WLRYKuIYl?$Qe|TV03@%t)&Kwi diff --git a/testdata/drc/drcSimpleTests_au6.gds b/testdata/drc/drcSimpleTests_au6.gds index 0b311b67b2640b2d3c786375f59dfebd3832b586..752f13b5a6096b4066d73e1403e9fd2f777a31c5 100644 GIT binary patch delta 544 zcmZ1`y+^v9fsKKQDS|{L4=vr&au;FS8(TCP8Qmsv8QB|~S(xZ35EofkBjB4QIdz?~} z4LI4s77Afn2$JV;6<}as+QR^H6Luwvc(of85YR3)`2ZJw7f4^gYeXJ5eiyJC;MIPQ SQ)03o4<{~9Y@Wr#!w3Kv9(}$5 delta 531 zcmdlZy-d2EfsKKQDS|Q-#303>hs^+>@@d2w)}&o z%MSeov!g;7WLRhA33BucW?*1pXTYJ76>Qzc7EMM*m&psbjZ92*6mk(opAD>EV)8o#CO2?Oarh@NFfi?509zt}ZV6C@BE)`d zQc{z+*zw467!=?)T8cwWfshL63xre%HV{xT`5!kPXG(DW|NrL!9wRny;Nf8e0E0Y! A!vFvP diff --git a/testdata/drc/drcSimpleTests_au7.gds b/testdata/drc/drcSimpleTests_au7.gds index b96c9dd3cc9da25cf363ce92a1c5546d668b5468..c7a3cc36d4b56a5098a95eb78e774a1c1c52aa75 100644 GIT binary patch delta 1427 zcmaKrOK1~O6o&7eOy)gf8YeY0S&Rsov?fhL8uMrjjk@W>wzOSTH#VDs;0q~Y7Gfzb z6uLXojZoT+Lah&f znlQJ55c3JGlAa4J={MFDpqa^BFcfO{fpr=%q*A&o((!To zr)v*dr%xDnYmaX|tgYR?UotYyL2cI$XT&nJVj{H9swdGCgBom#S7BvbhIL(XlUqVS z@(^Bn31OK^gaF8F>I_4n5*k#_@$gphpbm(bQmLW>6JfD?9_>E%!n-PNkR>qSPOQ%6 z3&kANrlRo8R6#R&yhr}G0Y~N|won|)lU0ZiNvD@p*p5?)8^fN&=m?egJ>p4xzDOnN z2@gVsnWX3*IB4bQTZ(q_yaFLJ2&vKro6F}j_7!|D>G(B_UIt$wXC;rr_Bh{L>Fj>D zE!AgnHon^uheoBple6cpO`GWL6mz%x#MIoXzUT3CXk9Nuy_A5)r0UCNt+yP3CsS(Y zEn9{)mZZoyif%JKB^A=s|628yh4cgaMV;!Q3QjVL5@~rcPR!E`+?%D0m)+1d60}0} K{k@&JP3|uhwWO&4 delta 1660 zcmbQXmT}D*#(D-e1}3Hm1{p>s_Gb*-K%NkT6oWo8n}LIg&BxP;fkA|s)y}cg-22$_ z50)-F^cT#I3Sp38#jTHvjV&a|(Jz>RfrXs`he}qkbsJkW85tWUZ{XILe2r0mG6$2o z3ImHEK7C+qn}eA4vrRrLY%}>j7x&}{PF{W;h1|r7_@cy;%zP^aLx#y8IHV>|;AEeC zRRp9)T)+gIf^#CyxFv%`@kn07CmAJ%N2eYS0Y@c?R62+e<+^)fM7d6|ffzf1-sNC7AVy_?1R>XLz9O-Zce1IrBrpyd z87(GX(AMDaPXL>}Vv^1@M$5^<>Y5y`0%#(WFKDncA}N3d7LtgX0=gDGZ4@m~zanXY z`Ux(g&h`KQp9gSb1T4{V%w`)sMIOc_lP5StPuAj82Br@#9%DTtOCuv=LkmMAv&jh# z8k6O$*}+OC1HA&|!@L601rvg%2_&J(1-7WVpuRy;!XbSD>@s8_sFx5z`bbu*P4;o* zg!2`cL^e6iW@m1bWShLvL3i?LPdAJbqsmi?3mO+NWzycR7{M_4l$Rph9IOgJ20=ZE z*B~h$A`F5C1zv-)e2Fj!8d`V_+U190kO_JcfE3L<3=C|%OpIR{*u Date: Sat, 20 Jul 2024 21:04:51 +0200 Subject: [PATCH 4/9] Properly selecting debug mode in .pri files for self diagnosis. Touching files for recompile. --- src/db/db/dbLayoutToNetlist.cc | 1 - src/gsi/gsi/gsiClassBase.cc | 1 - src/klayout.pri | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/db/db/dbLayoutToNetlist.cc b/src/db/db/dbLayoutToNetlist.cc index 0a2e9b235..4970ac952 100644 --- a/src/db/db/dbLayoutToNetlist.cc +++ b/src/db/db/dbLayoutToNetlist.cc @@ -1,5 +1,4 @@ - /* KLayout Layout Viewer diff --git a/src/gsi/gsi/gsiClassBase.cc b/src/gsi/gsi/gsiClassBase.cc index 52f9c8fb9..c9824a46d 100644 --- a/src/gsi/gsi/gsiClassBase.cc +++ b/src/gsi/gsi/gsiClassBase.cc @@ -935,4 +935,3 @@ bool has_class (const std::type_info &ti) } } - diff --git a/src/klayout.pri b/src/klayout.pri index 14f394e2f..b87ac9531 100644 --- a/src/klayout.pri +++ b/src/klayout.pri @@ -350,6 +350,6 @@ DEFINES += \ VERSION = $$KLAYOUT_VERSION -debug { +CONFIG(debug, debug|release) { DEFINES += HAVE_DEBUG } From 0dd6fca51ee0611f4ffcbecab18593a596902ef1 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 20 Jul 2024 22:35:54 +0200 Subject: [PATCH 5/9] Implemented a fix for #1794 (giant meta info produces invalid GDS) This fix solves two problems: * Too large meta info data * Too many meta info entries The first problem is fixed by splitting the strings that serialize the meta info. The second problem is fixed by introducing prefixed strings that indicate the attribute index within the string, not inside the PROPATTR record. The solution is backward compatible, although old versions will not read all meta info and skip entries that exceed the GDS capacity. Caveat: the produced GDS files may contain duplicate PROPATTR keys. This is not strictly illegal, but some third-party processors may drop such entries. --- .../gds2/db_plugin/dbGDS2ReaderBase.cc | 53 ++++++++- .../gds2/db_plugin/dbGDS2WriterBase.cc | 68 ++++++++--- .../gds2/db_plugin/dbGDS2WriterBase.h | 1 + .../gds2/unit_tests/dbGDS2WriterTests.cc | 109 +++++++++++++++++- 4 files changed, 210 insertions(+), 21 deletions(-) diff --git a/src/plugins/streamers/gds2/db_plugin/dbGDS2ReaderBase.cc b/src/plugins/streamers/gds2/db_plugin/dbGDS2ReaderBase.cc index 6435fd011..2edd16847 100644 --- a/src/plugins/streamers/gds2/db_plugin/dbGDS2ReaderBase.cc +++ b/src/plugins/streamers/gds2/db_plugin/dbGDS2ReaderBase.cc @@ -461,6 +461,7 @@ GDS2ReaderBase::read_context_info_cell () if (valid_hook) { std::vector &strings = m_context_info.insert (std::make_pair (cn, std::vector ())).first->second; + std::map > strings_ex; size_t attr = 0; @@ -474,16 +475,60 @@ GDS2ReaderBase::read_context_info_cell () attr = size_t (get_ushort ()); } else if (rec_id == sPROPVALUE) { - if (strings.size () <= attr) { - strings.resize (attr + 1, std::string ()); + const char *str = get_string (); + + // To embed long strings and more than 64k attributes, a separate notation is used: + // "#,

:" + // where is a string index and

is the part index (zero-based). + // For such properties, the PROPATTR value is ignored. This means however, that the + // attribute numbers may not be unique. + // See issue #1794. + + if (str[0] == '#') { + + tl::Extractor ex (str + 1); + size_t n = 0, p = 0; + if (ex.try_read (n) && ex.test (",") && ex.try_read (p) && ex.test (":")) { + if (strings.size () <= n) { + strings.resize (n + 1, std::string ()); + } + std::vector &sv = strings_ex[n]; + if (sv.size () <= p) { + sv.resize (p + 1, std::string ()); + } + sv[p] = ex.get (); + } + + } else { + + if (strings.size () <= attr) { + strings.resize (attr + 1, std::string ()); + } + strings [attr] = str; + } - strings [attr] = get_string (); } else { error (tl::to_string (tr ("ENDEL, PROPATTR or PROPVALUE record expected"))); } - } + } + + // combine the multipart strings (#1794) + for (auto es = strings_ex.begin (); es != strings_ex.end (); ++es) { + if (es->first < strings.size ()) { + std::string &s = strings [es->first]; + s.clear (); + size_t sz = 0; + for (auto i = es->second.begin (); i != es->second.end (); ++i) { + sz += i->size (); + } + s.reserve (sz); + for (auto i = es->second.begin (); i != es->second.end (); ++i) { + s += *i; + } + } + } } else { error (tl::to_string (tr ("Invalid record inside a context info cell"))); diff --git a/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.cc b/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.cc index 2b49a1491..9f1552bc6 100644 --- a/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.cc +++ b/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.cc @@ -72,6 +72,54 @@ inline int scale (double sf, int value) } } +void +GDS2WriterBase::write_context_string (size_t n, const std::string &s) +{ + // max. size for GDS strings used as payload carrier + size_t chunk_size = 32000; + short max_short = std::numeric_limits::max (); + + if (n > size_t (max_short) || s.size () > chunk_size) { + + // Split strings and use a separate notation: "#,<+p>:..." for the partial + // strings. n is the string index and p the part index (zero based). + // The property number is not defined in that case. There may be properties with + // the same number. See issue #1794. + + size_t nchunks = (s.size () + (chunk_size - 1)) / chunk_size; + while (nchunks > 0) { + + --nchunks; + + std::string partial; + partial.reserve (chunk_size + 100); // approx. + partial += "#"; + partial += tl::to_string (n); + partial += ","; + partial += tl::to_string (nchunks); + partial += ":"; + size_t pos = nchunks * chunk_size; + partial += std::string (s, pos, std::min (s.size (), pos + chunk_size) - pos); + + write_record_size (6); + write_record (sPROPATTR); + write_short (n <= size_t (max_short) ? short (n) : max_short); + + write_string_record (sPROPVALUE, partial); + + } + + } else { + + write_record_size (6); + write_record (sPROPATTR); + write_short (short (n)); + + write_string_record (sPROPVALUE, s); + + } +} + void GDS2WriterBase::write_context_cell (db::Layout &layout, const short *time_data, const std::vector &cells) { @@ -112,15 +160,9 @@ GDS2WriterBase::write_context_cell (db::Layout &layout, const short *time_data, // Hint: write in the reverse order since this way, the reader is more efficient (it knows how many strings // will arrive) for (std::vector ::const_iterator s = context_prop_strings.end (); s != context_prop_strings.begin (); ) { - --s; - - write_record_size (6); - write_record (sPROPATTR); - write_short (short (std::distance (std::vector ::const_iterator (context_prop_strings.begin ()), s))); // = user string - - write_string_record (sPROPVALUE, *s); - + size_t n = std::distance (std::vector ::const_iterator (context_prop_strings.begin ()), s); + write_context_string (n, *s); } } @@ -151,15 +193,9 @@ GDS2WriterBase::write_context_cell (db::Layout &layout, const short *time_data, // Hint: write in the reverse order since this way, the reader is more efficient (it knows how many strings // will arrive) for (std::vector ::const_iterator s = context_prop_strings.end (); s != context_prop_strings.begin (); ) { - --s; - - write_record_size (6); - write_record (sPROPATTR); - write_short (short (std::distance (std::vector ::const_iterator (context_prop_strings.begin ()), s))); // = user string - - write_string_record (sPROPVALUE, *s); - + size_t n = std::distance (std::vector ::const_iterator (context_prop_strings.begin ()), s); + write_context_string (n, *s); } } diff --git a/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.h b/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.h index a70f3e626..0d058e7fe 100644 --- a/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.h +++ b/src/plugins/streamers/gds2/db_plugin/dbGDS2WriterBase.h @@ -169,6 +169,7 @@ private: void write_properties (const db::Layout &layout, db::properties_id_type prop_id); void write_context_cell (db::Layout &layout, const short *time_data, const std::vector &cells); + void write_context_string (size_t n, const std::string &s); }; } // namespace db diff --git a/src/plugins/streamers/gds2/unit_tests/dbGDS2WriterTests.cc b/src/plugins/streamers/gds2/unit_tests/dbGDS2WriterTests.cc index a840ef810..4fce6a91e 100644 --- a/src/plugins/streamers/gds2/unit_tests/dbGDS2WriterTests.cc +++ b/src/plugins/streamers/gds2/unit_tests/dbGDS2WriterTests.cc @@ -1196,7 +1196,7 @@ TEST(121) } // Meta info -TEST(130a) +TEST(130) { db::Layout layout_org; @@ -1312,6 +1312,113 @@ TEST(130a) EXPECT_EQ (layout_read.meta_info ("b").value.to_string (), "nil"); } +// Giant meta info (issue #1794) +TEST(131) +{ + db::Layout layout_org; + + layout_org.add_cell ("U"); + db::cell_index_type ci = layout_org.add_cell ("X"); + + std::vector ll1; + std::vector ll2; + + for (unsigned int i = 0; i < 100000; ++i) { + ll1.push_back (tl::Variant (i)); + ll2.push_back ("C" + tl::to_string (i)); + } + + layout_org.add_meta_info ("a", db::MetaInfo ("", ll1, true)); + layout_org.add_meta_info ("b", db::MetaInfo ("", "value", true)); + + layout_org.add_meta_info (ci, "a", db::MetaInfo ("", ll2, true)); + layout_org.add_meta_info (ci, "c", db::MetaInfo ("", -1, true)); + + std::string tmp_file = tl::TestBase::tmp_file ("tmp_GDS2Writer_131.gds"); + + { + tl::OutputStream out (tmp_file); + db::SaveLayoutOptions options; + db::Writer writer (options); + writer.write (layout_org, out); + } + + db::Layout layout_read; + + { + tl::InputStream in (tmp_file); + db::Reader reader (in); + reader.read (layout_read); + } + + EXPECT_EQ (layout_read.has_meta_info ("x"), false); + EXPECT_EQ (layout_read.has_meta_info ("a"), true); + EXPECT_EQ (layout_read.meta_info ("x").value.to_string (), "nil"); + EXPECT_EQ (layout_read.meta_info ("a").value == ll1, true); + EXPECT_EQ (layout_read.has_meta_info ("b"), true); + EXPECT_EQ (layout_read.meta_info ("b").value.to_string (), "value"); + + db::cell_index_type ci2 = layout_read.cell_by_name ("X").second; + + EXPECT_EQ (layout_read.meta_info (ci2, "x").value.to_string (), "nil"); + EXPECT_EQ (layout_read.meta_info (ci2, "a").value == ll2, true); + EXPECT_EQ (layout_read.meta_info (ci2, "c").value.to_string (), "-1"); +} + +// Many meta info (issue #1794) +TEST(132) +{ + db::Layout layout_org; + + layout_org.add_cell ("U"); + db::cell_index_type ci = layout_org.add_cell ("X"); + + for (unsigned int i = 0; i < 100000; ++i) { + layout_org.add_meta_info ("a" + tl::to_string (i), db::MetaInfo ("", i, true)); + } + layout_org.add_meta_info ("b", db::MetaInfo ("", "value", true)); + + for (unsigned int i = 0; i < 100000; ++i) { + layout_org.add_meta_info (ci, "a" + tl::to_string (i * 2), db::MetaInfo ("", i * 2, true)); + } + layout_org.add_meta_info (ci, "c", db::MetaInfo ("", -1, true)); + + std::string tmp_file = tl::TestBase::tmp_file ("tmp_GDS2Writer_132.gds"); + + { + tl::OutputStream out (tmp_file); + db::SaveLayoutOptions options; + db::Writer writer (options); + writer.write (layout_org, out); + } + + db::Layout layout_read; + + { + tl::InputStream in (tmp_file); + db::Reader reader (in); + reader.read (layout_read); + } + + EXPECT_EQ (layout_read.has_meta_info ("x"), false); + EXPECT_EQ (layout_read.meta_info ("x").value.to_string (), "nil"); + for (unsigned int i = 0; i < 10; ++i) { + EXPECT_EQ (layout_read.has_meta_info ("a" + tl::to_string (i)), true); + EXPECT_EQ (layout_read.meta_info ("a" + tl::to_string (i)).value.to_string (), tl::Variant (i).to_string ()); + } + EXPECT_EQ (layout_read.has_meta_info ("b"), true); + EXPECT_EQ (layout_read.meta_info ("b").value.to_string (), "value"); + + db::cell_index_type ci2 = layout_read.cell_by_name ("X").second; + + EXPECT_EQ (layout_read.meta_info (ci2, "x").value.to_string (), "nil"); + for (unsigned int i = 0; i < 10; ++i) { + EXPECT_EQ (layout_read.has_meta_info (ci2, "a" + tl::to_string (i * 2)), true); + EXPECT_EQ (layout_read.meta_info (ci2, "a" + tl::to_string (i * 2)).value.to_string (), tl::Variant (i * 2).to_string ()); + } + EXPECT_EQ (layout_read.meta_info (ci2, "c").value.to_string (), "-1"); +} + // Extreme fracturing by max. points TEST(166) { From 15eebb032fd329eaeef84711368057fd91710431 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 21 Jul 2024 10:23:49 +0200 Subject: [PATCH 6/9] Enabling MSVC build --- src/tl/unit_tests/tlVariantTests.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tl/unit_tests/tlVariantTests.cc b/src/tl/unit_tests/tlVariantTests.cc index 58a7b0595..84ca9755b 100644 --- a/src/tl/unit_tests/tlVariantTests.cc +++ b/src/tl/unit_tests/tlVariantTests.cc @@ -28,6 +28,7 @@ #include "tlTypeTraits.h" #include "tlUnitTest.h" +#define _USE_MATH_DEFINES // for MSVC #include #include #include From 3bd02c023fef8a803c03e87895451e255f8a2c9c Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 21 Jul 2024 10:47:09 +0200 Subject: [PATCH 7/9] Enabling MSVC build (2) --- src/tl/unit_tests/tlVariantTests.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tl/unit_tests/tlVariantTests.cc b/src/tl/unit_tests/tlVariantTests.cc index 84ca9755b..d03a56d1c 100644 --- a/src/tl/unit_tests/tlVariantTests.cc +++ b/src/tl/unit_tests/tlVariantTests.cc @@ -28,11 +28,12 @@ #include "tlTypeTraits.h" #include "tlUnitTest.h" -#define _USE_MATH_DEFINES // for MSVC -#include #include #include +#define _USE_MATH_DEFINES // for MSVC +#include + struct A { std::string a; From 8e7d9669af40cf6d6c7952958421aa00e33b5615 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Tue, 23 Jul 2024 17:37:03 +1200 Subject: [PATCH 8/9] Initialize m_verbosity_level on demand to avoid depending on the order of dynamic initialization Resolves #1797 --- src/tl/tl/tlLog.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/tl/tl/tlLog.cc b/src/tl/tl/tlLog.cc index 4fcd7226c..eb680e14c 100644 --- a/src/tl/tl/tlLog.cc +++ b/src/tl/tl/tlLog.cc @@ -49,18 +49,22 @@ static int default_verbosity () return verbosity; } -static int m_verbosity_level = default_verbosity (); +static int &verbosity_level () +{ + static int level = default_verbosity (); + return level; +} void verbosity (int level) { - m_verbosity_level = level; + verbosity_level () = level; } int verbosity () { - return m_verbosity_level; + return verbosity_level (); } // ------------------------------------------------ From d6ee473d72bebd2585573de74eb4bd6d957194f0 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Thu, 25 Jul 2024 22:36:46 +0200 Subject: [PATCH 9/9] Fixed issue #1799 (Can't set cell properties by script) --- src/db/db/dbPropertiesRepository.cc | 10 ++++++- .../oasis/unit_tests/dbOASISReaderTests.cc | 25 ++++++++++++++++++ testdata/oasis/issue_1799.oas | Bin 0 -> 546 bytes 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 testdata/oasis/issue_1799.oas diff --git a/src/db/db/dbPropertiesRepository.cc b/src/db/db/dbPropertiesRepository.cc index 3f3d10049..3d6628fe7 100644 --- a/src/db/db/dbPropertiesRepository.cc +++ b/src/db/db/dbPropertiesRepository.cc @@ -89,6 +89,11 @@ PropertiesRepository::prop_name_id (const tl::Variant &name) void PropertiesRepository::change_properties (property_names_id_type id, const properties_set &new_props) { + // NOTE: change_properties MAY put the property map into a state where there is + // more than one property ID per set. For example, 1 and 5 may be valid property + // ids for the same set. "properties(1)" and "properties(5)" returns the same + // property set "S", while "properties_id(S)" only returns 1. + const properties_set &old_props = properties (id); std::map ::const_iterator pi = m_properties_ids_by_set.find (old_props); @@ -149,7 +154,10 @@ PropertiesRepository::properties_id (const properties_set &props) std::map ::const_iterator pi = m_properties_ids_by_set.find (props); if (pi == m_properties_ids_by_set.end ()) { - properties_id_type id = m_properties_ids_by_set.size (); + properties_id_type id = 0; + if (! m_properties_by_id.empty ()) { + id = (--m_properties_by_id.end ())->first + 1; + } m_properties_ids_by_set.insert (std::make_pair (props, id)); m_properties_by_id.insert (std::make_pair (id, props)); for (properties_set::const_iterator nv = props.begin (); nv != props.end (); ++nv) { diff --git a/src/plugins/streamers/oasis/unit_tests/dbOASISReaderTests.cc b/src/plugins/streamers/oasis/unit_tests/dbOASISReaderTests.cc index ae287e6f0..a519ff1a7 100644 --- a/src/plugins/streamers/oasis/unit_tests/dbOASISReaderTests.cc +++ b/src/plugins/streamers/oasis/unit_tests/dbOASISReaderTests.cc @@ -639,6 +639,31 @@ TEST(Bug_1474) } } +TEST(Bug_1799) +{ + db::Manager m (false); + db::Layout layout (&m); + + { + tl::InputStream file (tl::testdata () + "/oasis/issue_1799.oas"); + db::OASISReader reader (file); + reader.read (layout); + } + + db::properties_id_type pn = layout.properties_repository ().prop_name_id (tl::Variant (1)); + db::PropertiesRepository::properties_set ps; + ps.insert (std::make_pair (pn, tl::Variant ("hello, world!"))); + + auto pid = layout.properties_repository ().properties_id (ps); + + auto ps2 = layout.properties_repository ().properties (pid); + EXPECT_EQ (ps2.size (), size_t (1)); + EXPECT_EQ (ps2.find (pn) != ps2.end (), true); + if (ps2.find (pn) != ps2.end ()) { + EXPECT_EQ (ps2.find (pn)->second.to_string (), "hello, world!"); + } +} + TEST(DuplicateCellname) { db::Manager m (false); diff --git a/testdata/oasis/issue_1799.oas b/testdata/oasis/issue_1799.oas new file mode 100644 index 0000000000000000000000000000000000000000..32ae303d8b61d4023b3a574226109ad2c95cd0de GIT binary patch literal 546 zcmY!lcJ=kt^>+;R4CduxWH!_@V0gjKC?n3q!6L)YEF;ds&md#Q%FoEp#Luk6P!^th z#*nMQfXD5jp~8yY6*H5Pb+0Wm5iI@nzMk7P{Q3KjD?)wd2&K%M+H2+dp(|)^7)nZCu28Bt5wdroTtPvpE2dJh>qqdecw}_ zy64XO_Ue1;c<4Sd`BchVcqQPxpSO?RN&Qt@wET26PoDAC-*)Pp=bAJ6x}Eyox?1PX zoDaUfCDi!oTB>foRmIRwcfW@DMwTV}c;-im6!V__s&j>rmqApSgMo#$ z=@(-|?ZYd~Z~2b#JO)wBf+a=yrAaxd#re6Z@g<1`sYNp43>+UAnHW2n7#SFK7~2_v L6hp(v0t^fQ4J_y0 literal 0 HcmV?d00001