From 0df6339f4e0e4baf5ca953b7401d7cecd59ba2fb Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 13 Jul 2024 18:06:56 +0200 Subject: [PATCH 1/3] Fixing issue #1782 This patch establishes "nan", "inf" and "-inf" as valid values for tl::Variant, so corresponding PCell parameters can be serialized and are properly managed. --- .../pcell_declaration_helper.lym | 4 + src/tl/tl/tlString.cc | 60 +++++++++- src/tl/tl/tlVariant.cc | 29 ++++- src/tl/unit_tests/tlStringTests.cc | 59 ++++++++++ src/tl/unit_tests/tlVariantTests.cc | 106 ++++++++++++++++++ testdata/ruby/dbPCells.rb | 100 +++++++++++++++++ 6 files changed, 353 insertions(+), 5 deletions(-) diff --git a/src/db/db/built-in-macros/pcell_declaration_helper.lym b/src/db/db/built-in-macros/pcell_declaration_helper.lym index 98901fd3c2..a380b77f7b 100644 --- a/src/db/db/built-in-macros/pcell_declaration_helper.lym +++ b/src/db/db/built-in-macros/pcell_declaration_helper.lym @@ -361,6 +361,10 @@ module RBA def param(name, type, description, args = {}) + if ! args.is_a?(Hash) + raise("Too many positional arguments for 'param' (3 expected) - use named arguments for default value etc.") + end + if name !~ /^[_A-Za-z]\w*$/ raise "Invalid parameter name #{name} (needs to be a word)" end diff --git a/src/tl/tl/tlString.cc b/src/tl/tl/tlString.cc index 8a0379e6e8..d6bb80ede9 100644 --- a/src/tl/tl/tlString.cc +++ b/src/tl/tl/tlString.cc @@ -226,9 +226,28 @@ bool skip_newline (const char *&cp) } } +// ------------------------------------------------------------------------- +// Utility: case-insensitive compare of the first characters + +static bool local_compare (const char *s1, const char *s2) +{ + while (*s1 && *s2) { + uint32_t c1 = utf32_downcase (utf32_from_utf8 (s1)); + uint32_t c2 = utf32_downcase (utf32_from_utf8 (s2)); + if (c1 != c2) { + return false; + } + } + return true; +} + // ------------------------------------------------------------------------- // Utility: a strtod version that is independent of the locale +static std::string inf_string = "inf"; +static std::string ninf_string = "-inf"; +static std::string nan_string = "nan"; + static std::string micron_format ("%.5f"); static std::string dbu_format ("%.2f"); @@ -244,12 +263,24 @@ void set_db_resolution (unsigned int ndigits) std::string micron_to_string (double d) { - return tl::sprintf (micron_format.c_str (), d); + if (std::isnan (d)) { + return nan_string; + } else if (std::isinf (d)) { + return d < 0 ? ninf_string : inf_string; + } else { + return tl::sprintf (micron_format.c_str (), d); + } } std::string db_to_string (double d) { - return tl::sprintf (dbu_format.c_str (), d); + if (std::isnan (d)) { + return nan_string; + } else if (std::isinf (d)) { + return d < 0 ? ninf_string : inf_string; + } else { + return tl::sprintf (dbu_format.c_str (), d); + } } std::string to_upper_case (const std::string &s) @@ -317,6 +348,18 @@ static double local_strtod (const char *cp, const char *&cp_new) { const char *cp0 = cp; + // special numerical values + if (local_compare (cp, nan_string.c_str ())) { + cp_new = cp + nan_string.size (); + return NAN; + } else if (local_compare (cp, inf_string.c_str ())) { + cp_new = cp + inf_string.size (); + return INFINITY; + } else if (local_compare (cp, ninf_string.c_str ())) { + cp_new = cp + ninf_string.size (); + return -INFINITY; + } + // Extract sign double s = 1.0; if (*cp == '-') { @@ -376,6 +419,12 @@ static double local_strtod (const char *cp, const char *&cp_new) std::string to_string (double d, int prec) { + if (std::isnan (d)) { + return nan_string; + } else if (std::isinf (d)) { + return d < 0 ? ninf_string : inf_string; + } + // For small values less than 1e-(prec) simply return "0" to avoid ugly values like "1.2321716e-14". if (fabs (d) < pow (10.0, -prec)) { return "0"; @@ -393,6 +442,12 @@ to_string (double d, int prec) std::string to_string (float d, int prec) { + if (std::isnan (d)) { + return nan_string; + } else if (std::isinf (d)) { + return d < 0 ? ninf_string : inf_string; + } + // For small values less than 1e-(prec) simply return "0" to avoid ugly values like "1.2321716e-14". if (fabs (d) < pow (10.0, -prec)) { return "0"; @@ -813,6 +868,7 @@ from_string_numeric (const std::string &s, double &v, bool eval) if (! *cp) { throw tl::Exception (tl::to_string (tr ("Got empty string where a real number was expected"))); } + const char *cp_end = cp; v = local_strtod (cp, cp_end); while (safe_isspace (*cp_end)) { diff --git a/src/tl/tl/tlVariant.cc b/src/tl/tl/tlVariant.cc index 4ceacfb4bb..28d476282a 100644 --- a/src/tl/tl/tlVariant.cc +++ b/src/tl/tl/tlVariant.cc @@ -1057,15 +1057,38 @@ normalized_type (Variant::type type1, Variant::type type2) static const double epsilon = 1e-13; +// NOTE: in order to be able to use Variant for std::map or std::set +// keys we have to establish a weak order. This means we need to +// consider NAN and INFINITY too. + +static int numeric_class (double x) +{ + if (std::isnan (x)) { + return 2; + } else { + return std::isinf (x) ? (x < 0 ? -1 : 1) : 0; + } +} + static inline bool fequal (double a, double b) { - double avg = 0.5 * (fabs (a) + fabs (b)); - return fabs (a - b) <= epsilon * avg; + if (numeric_class (a) != 0 || numeric_class (b) != 0) { + return numeric_class (a) == numeric_class (b); + } else { + double avg = 0.5 * (fabs (a) + fabs (b)); + return fabs (a - b) <= epsilon * avg; + } } static inline bool fless (double a, double b) { - return fequal (a, b) ? false : a < b; + if (fequal (a, b)) { + return false; + } else if (numeric_class (a) != 0 || numeric_class (b) != 0) { + return numeric_class (a) < numeric_class (b); + } else { + return a < b; + } } bool diff --git a/src/tl/unit_tests/tlStringTests.cc b/src/tl/unit_tests/tlStringTests.cc index aafab46e6a..2c0e78386c 100644 --- a/src/tl/unit_tests/tlStringTests.cc +++ b/src/tl/unit_tests/tlStringTests.cc @@ -589,3 +589,62 @@ TEST(15) EXPECT_EQ (tl::to_upper_case ("nOrMaliI(\xc3\xa4\xc3\x84\xc3\xbc\xc3\x9c\xc3\xb6\xc3\x96\xc3\x9f-42\xc2\xb0+6\xe2\x82\xac)"), "NORMALII(\xc3\x84\xc3\x84\xc3\x9c\xc3\x9c\xc3\x96\xc3\x96\xc3\x9f-42\xc2\xb0+6\xe2\x82\xac)"); EXPECT_EQ (tl::to_lower_case ("nOrMaliI(\xc3\xa4\xc3\x84\xc3\xbc\xc3\x9c\xc3\xb6\xc3\x96\xc3\x9f-42\xc2\xb0+6\xe2\x82\xac)"), "normalii(\xc3\xa4\xc3\xa4\xc3\xbc\xc3\xbc\xc3\xb6\xc3\xb6\xc3\x9f-42\xc2\xb0+6\xe2\x82\xac)"); } + +// Special numerical values +TEST(16) +{ + EXPECT_EQ (tl::to_string (NAN), "nan"); + EXPECT_EQ (tl::to_string (INFINITY), "inf"); + EXPECT_EQ (tl::to_string (-INFINITY), "-inf"); + + EXPECT_EQ (tl::to_string ((float) NAN), "nan"); + EXPECT_EQ (tl::to_string ((float) INFINITY), "inf"); + EXPECT_EQ (tl::to_string ((float) -INFINITY), "-inf"); + + EXPECT_EQ (tl::micron_to_string (NAN), "nan"); + EXPECT_EQ (tl::micron_to_string (INFINITY), "inf"); + EXPECT_EQ (tl::micron_to_string (-INFINITY), "-inf"); + + EXPECT_EQ (tl::db_to_string (NAN), "nan"); + EXPECT_EQ (tl::db_to_string (INFINITY), "inf"); + EXPECT_EQ (tl::db_to_string (-INFINITY), "-inf"); + + double x = 0.0; + tl::from_string ("nan", x); + EXPECT_EQ (tl::to_string (x), "nan"); + x = 0.0; + tl::from_string ("NaN", x); + EXPECT_EQ (tl::to_string (x), "nan"); + x = 0.0; + tl::from_string ("inf", x); + EXPECT_EQ (tl::to_string (x), "inf"); + x = 0.0; + tl::from_string ("INF", x); + EXPECT_EQ (tl::to_string (x), "inf"); + x = 0.0; + tl::from_string ("-inf", x); + EXPECT_EQ (tl::to_string (x), "-inf"); + x = 0.0; + tl::from_string ("-INF", x); + EXPECT_EQ (tl::to_string (x), "-inf"); + + std::string s; + tl::Extractor ex; + x = 0.0; + s = " inf nan\t -inf"; + ex = tl::Extractor (s.c_str ()); + EXPECT_EQ (ex.try_read (x), true); + EXPECT_EQ (tl::to_string (x), "inf"); + EXPECT_EQ (ex.try_read (x), true); + EXPECT_EQ (tl::to_string (x), "nan"); + EXPECT_EQ (ex.try_read (x), true); + EXPECT_EQ (tl::to_string (x), "-inf"); + s = " Inf NaN\t -INF"; + ex = tl::Extractor (s.c_str ()); + EXPECT_EQ (ex.try_read (x), true); + EXPECT_EQ (tl::to_string (x), "inf"); + EXPECT_EQ (ex.try_read (x), true); + EXPECT_EQ (tl::to_string (x), "nan"); + EXPECT_EQ (ex.try_read (x), true); + EXPECT_EQ (tl::to_string (x), "-inf"); +} diff --git a/src/tl/unit_tests/tlVariantTests.cc b/src/tl/unit_tests/tlVariantTests.cc index 3ff19545d3..7fb6efc72a 100644 --- a/src/tl/unit_tests/tlVariantTests.cc +++ b/src/tl/unit_tests/tlVariantTests.cc @@ -1090,4 +1090,110 @@ TEST(6) EXPECT_EQ (tl::Variant (-0.1 * (1.0 + 1.1e-13)) < tl::Variant (0.1), true); } +// special numeric values +TEST(7) +{ + std::string s; + tl::Extractor ex; + tl::Variant v; + + s = " ##\t 0.5"; + ex = tl::Extractor (s.c_str ()); + EXPECT_EQ (ex.try_read (v), true); + EXPECT_EQ (v.to_parsable_string (), "##0.5"); + + s = "## nan"; + ex = tl::Extractor (s.c_str ()); + EXPECT_EQ (ex.try_read (v), true); + EXPECT_EQ (v.to_parsable_string (), "##nan"); + + s = "## NaN"; + ex = tl::Extractor (s.c_str ()); + EXPECT_EQ (ex.try_read (v), true); + EXPECT_EQ (v.to_parsable_string (), "##nan"); + + s = "## inf"; + ex = tl::Extractor (s.c_str ()); + EXPECT_EQ (ex.try_read (v), true); + EXPECT_EQ (v.to_parsable_string (), "##inf"); + + s = "## Inf"; + ex = tl::Extractor (s.c_str ()); + EXPECT_EQ (ex.try_read (v), true); + EXPECT_EQ (v.to_parsable_string (), "##inf"); + + s = "## -inf"; + ex = tl::Extractor (s.c_str ()); + EXPECT_EQ (ex.try_read (v), true); + EXPECT_EQ (v.to_parsable_string (), "##-inf"); + + s = "## -Inf"; + ex = tl::Extractor (s.c_str ()); + EXPECT_EQ (ex.try_read (v), true); + EXPECT_EQ (v.to_parsable_string (), "##-inf"); + + v = tl::Variant ("nan"); + v = tl::Variant (v.to_double ()); + EXPECT_EQ (v.to_parsable_string (), "##nan"); + EXPECT_EQ (v.to_string (), "nan"); + + v = tl::Variant ("Inf"); + v = tl::Variant (v.to_double ()); + EXPECT_EQ (v.to_parsable_string (), "##inf"); + EXPECT_EQ (v.to_string (), "inf"); + + v = tl::Variant (INFINITY); + EXPECT_EQ (v.to_parsable_string (), "##inf"); + EXPECT_EQ (v.to_string (), "inf"); + + v = tl::Variant (-INFINITY); + EXPECT_EQ (v.to_parsable_string (), "##-inf"); + EXPECT_EQ (v.to_string (), "-inf"); + + tl::Variant vinf (INFINITY); + tl::Variant vninf (-INFINITY); + tl::Variant vnan (NAN); + tl::Variant vzero (0.0); + + EXPECT_EQ (vninf == vninf, true); + EXPECT_EQ (vninf == vzero, false); + EXPECT_EQ (vninf == vinf, false); + EXPECT_EQ (vninf == vnan, false); + + EXPECT_EQ (vninf < vninf, false); + EXPECT_EQ (vninf < vzero, true); + EXPECT_EQ (vninf < vinf, true); + EXPECT_EQ (vninf < vnan, true); + + EXPECT_EQ (vzero == vninf, false); + EXPECT_EQ (vzero == vzero, true); + EXPECT_EQ (vzero == vinf, false); + EXPECT_EQ (vzero == vnan, false); + + EXPECT_EQ (vzero < vninf, false); + EXPECT_EQ (vzero < vzero, false); + EXPECT_EQ (vzero < vinf, true); + EXPECT_EQ (vzero < vnan, true); + + EXPECT_EQ (vinf == vninf, false); + EXPECT_EQ (vinf == vzero, false); + EXPECT_EQ (vinf == vinf, true); + EXPECT_EQ (vinf == vnan, false); + + EXPECT_EQ (vinf < vninf, false); + EXPECT_EQ (vinf < vzero, false); + EXPECT_EQ (vinf < vinf, false); + EXPECT_EQ (vinf < vnan, true); + + EXPECT_EQ (vnan == vninf, false); + EXPECT_EQ (vnan == vzero, false); + EXPECT_EQ (vnan == vinf, false); + EXPECT_EQ (vnan == vnan, true); + + EXPECT_EQ (vnan < vninf, false); + EXPECT_EQ (vnan < vzero, false); + EXPECT_EQ (vnan < vinf, false); + EXPECT_EQ (vnan < vnan, false); +} + } diff --git a/testdata/ruby/dbPCells.rb b/testdata/ruby/dbPCells.rb index 05422c3da8..281eaf4512 100644 --- a/testdata/ruby/dbPCells.rb +++ b/testdata/ruby/dbPCells.rb @@ -674,6 +674,106 @@ def test_9 end + # issue #1782 + + class Circle1782 < RBA::PCellDeclarationHelper + + def initialize + super() + param("l", TypeLayer, "Layer", :default => RBA::LayerInfo::new(1, 0)) + param("r", TypeDouble, "Radius", :default => 1.0) + param("n", TypeInt, "Number of points", :default => 16) + end + + def display_text_impl + r = self.r + if !r + r = "nil" + else + r = '%.3f' % r + end + "Circle(L=" + self.l.to_s + ",R=" + r + ")" + end + + def produce_impl + r = self.r + if self.r.to_s == 'NaN' + r = 2.0 + end + da = Math::PI * 2 / self.n + pts = self.n.times.collect do |i| + RBA::DPoint::new(r * Math::cos(i * da), r * Math::sin(i * da)) + end + self.cell.shapes(self.l_layer).insert(RBA::DPolygon::new(pts)) + end + + end + + class CircleLib1782 < RBA::Library + + def initialize(name) + self.description = "Circle Library" + self.layout.register_pcell("Circle", Circle1782::new) + register(name) + end + + def reregister_pcell + self.layout.register_pcell("Circle", Circle1782::new) + end + + end + + def test_10 + + lib = CircleLib1782::new("CircleLib") + + ly = RBA::Layout::new + + top = ly.create_cell("TOP") + + names = [] + + 2.times do |pass| + + 5.times do |i| + 5.times do |j| + if (i + j) % 2 == 0 + r = Float::NAN + else + r = (i + j) * 0.5 + end + n = i * 5 + j + c = ly.create_cell("Circle", "CircleLib", { "l" => RBA::LayerInfo::new(1, 0), "r" => r, "n" => n }) + if pass == 0 + names << c.name + else + # triggered bug #1782 - basically all variants are supposed to be unique, but + # the NaN spoiled the hash maps + assert_equal(names.shift, c.name) + end + top.insert(RBA::DCellInstArray::new(c, RBA::DTrans::new(i * 10.0, j * 10.0))) + end + end + + end + + tmp = File::join($ut_testtmp, "tmp.gds") + ly.write(tmp) + + # this should not throw an internal error + ly._destroy + + # we should be able to read the Layout back + ly = RBA::Layout::new + ly.read(tmp) + assert_equal(ly.top_cell.name, "TOP") + assert_equal(ly.cells, 26) + ly._destroy + + lib._destroy + + end + end load("test_epilogue.rb") From 072edecb1a6b3e12876cf111d4f44a17f58169e9 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 13 Jul 2024 18:55:17 +0200 Subject: [PATCH 2/3] Add-on making re-registration of PCells a valid feature. --- src/db/db/dbLayout.cc | 21 ++++++++++++++++++--- testdata/ruby/dbPCells.rb | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/db/db/dbLayout.cc b/src/db/db/dbLayout.cc index bcd76d7067..3c16a85ed1 100644 --- a/src/db/db/dbLayout.cc +++ b/src/db/db/dbLayout.cc @@ -2526,10 +2526,25 @@ Layout::register_pcell (const std::string &name, pcell_declaration_type *declara // replace any existing PCell declaration with that name. id = pcid->second; if (m_pcells [id]) { - delete m_pcells [id]; - } - m_pcells [id] = new pcell_header_type (id, name, declaration); + std::unique_ptr org_header (m_pcells [id]); + std::vector variants; + for (auto v = org_header->begin (); v != org_header->end (); ++v) { + variants.push_back (v->second); + } + for (auto v = variants.begin (); v != variants.end (); ++v) { + (*v)->unregister (); + } + + m_pcells [id] = new pcell_header_type (id, name, declaration); + + for (auto v = variants.begin (); v != variants.end (); ++v) { + (*v)->reregister (); + } + + } else { + m_pcells [id] = new pcell_header_type (id, name, declaration); + } } else { diff --git a/testdata/ruby/dbPCells.rb b/testdata/ruby/dbPCells.rb index 281eaf4512..afbc7f2f2a 100644 --- a/testdata/ruby/dbPCells.rb +++ b/testdata/ruby/dbPCells.rb @@ -774,6 +774,41 @@ def test_10 end + def test_11 + + lib = CircleLib1782::new("CircleLib") + + ly = RBA::Layout::new + + top = ly.create_cell("TOP") + + names = [] + + c = ly.create_cell("Circle", "CircleLib", { "l" => RBA::LayerInfo::new(1, 0), "r" => 2.0, "n" => 64 }) + + # triggered another flavor of #1782 + lib.reregister_pcell + + c = ly.create_cell("Circle", "CircleLib", { "l" => RBA::LayerInfo::new(1, 0), "r" => 2.0, "n" => 64 }) + top.insert(RBA::DCellInstArray::new(c, RBA::DTrans::new())) + + tmp = File::join($ut_testtmp, "tmp.gds") + ly.write(tmp) + + # this should not throw an internal error + ly._destroy + + # we should be able to read the Layout back + ly = RBA::Layout::new + ly.read(tmp) + assert_equal(ly.top_cell.name, "TOP") + assert_equal(ly.cells, 2) + ly._destroy + + lib._destroy + + end + end load("test_epilogue.rb") From 6456675d7a91c81e8f2c67c5572aa08e9348511c Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 20 Jul 2024 22:48:06 +0200 Subject: [PATCH 3/3] Trying to fix a build issue --- src/tl/unit_tests/tlVariantTests.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tl/unit_tests/tlVariantTests.cc b/src/tl/unit_tests/tlVariantTests.cc index 7fb6efc72a..b73534887a 100644 --- a/src/tl/unit_tests/tlVariantTests.cc +++ b/src/tl/unit_tests/tlVariantTests.cc @@ -27,6 +27,8 @@ #include "tlObject.h" #include "tlTypeTraits.h" #include "tlUnitTest.h" + +#include #include #include