From f9762009c6142b047917265223e9861171c86216 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 3 Apr 2021 18:24:57 +0200 Subject: [PATCH 1/6] Bugfix: don't forget to initialize child classes in gsi expressions. --- src/gsi/gsi/gsiExpression.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/gsi/gsi/gsiExpression.cc b/src/gsi/gsi/gsiExpression.cc index b844158a6..1c1d4ba3f 100644 --- a/src/gsi/gsi/gsiExpression.cc +++ b/src/gsi/gsi/gsiExpression.cc @@ -1074,21 +1074,22 @@ initialize_expressions () gsi::initialize (); // Go through all classes (maybe again) - for (gsi::ClassBase::class_iterator c = gsi::ClassBase::begin_classes (); c != gsi::ClassBase::end_classes (); ++c) { + std::list classes = gsi::ClassBase::classes_in_definition_order (); + for (std::list::const_iterator c = classes.begin (); c != classes.end (); ++c) { // Skip external classes - if (c->is_external ()) { + if ((*c)->is_external ()) { continue; } // install the method table: - ExpressionMethodTable::initialize_class (&*c); + ExpressionMethodTable::initialize_class (*c); // register a function that creates a class object (use a function to avoid issues with // late destruction of global variables which the class object is already gone) - const tl::VariantUserClassBase *cc = c->var_cls_cls (); + const tl::VariantUserClassBase *cc = (*c)->var_cls_cls (); if (cc) { - tl::Eval::define_global_function (c->name (), new EvalClassFunction (cc)); + tl::Eval::define_global_function ((*c)->name (), new EvalClassFunction (cc)); } } From 85c3128c131a7113565c912162d97239432d39f4 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 3 Apr 2021 18:26:53 +0200 Subject: [PATCH 2/6] Adding --blend-mode to buddy scripts for mitigating the risk of joining files --- Changelog | 2 ++ src/buddies/src/bd/bdReaderOptions.cc | 11 +++++++++++ src/buddies/src/bd/bdReaderOptions.h | 1 + src/buddies/unit_tests/bdBasicTests.cc | 3 +++ 4 files changed, 17 insertions(+) diff --git a/Changelog b/Changelog index 62ecba455..080a87422 100644 --- a/Changelog +++ b/Changelog @@ -80,6 +80,8 @@ Iterated OASIS instances are stored and handled in a leaner way in viewer mode * Enhancement: Buddy scripts can concatenate files with "+" for input Concatenation happens by "blending files". Beware of the risk this implies. + A new option "--blend-mode" has been introduced for supporting overwrite, skip + and variant formation in case of cell name conflicts. See buddy script help. * Enhancement: Layer maps now support n:m layer mapping This allows mapping n input layers to one logical layer (merging) and also one input layer to m logical ones (clone layer). This applies to the diff --git a/src/buddies/src/bd/bdReaderOptions.cc b/src/buddies/src/bd/bdReaderOptions.cc index 57c31cd95..51981f562 100644 --- a/src/buddies/src/bd/bdReaderOptions.cc +++ b/src/buddies/src/bd/bdReaderOptions.cc @@ -39,6 +39,7 @@ GenericReaderOptions::GenericReaderOptions () m_common_enable_text_objects = load_options.get_option_by_name ("text_enabled").to_bool (); m_common_enable_properties = load_options.get_option_by_name ("properties_enabled").to_bool (); + m_cell_conflict_resolution = load_options.get_options ().cell_conflict_resolution; m_gds2_box_mode = load_options.get_option_by_name ("gds2_box_mode").to_uint (); m_gds2_allow_big_records = load_options.get_option_by_name ("gds2_allow_big_records").to_bool (); @@ -197,6 +198,15 @@ GenericReaderOptions::add_options (tl::CommandLineOptions &cmd) "Each line in this file is read as one layer mapping expression. " "Empty lines or lines starting with a hash (#) character or with double slashes (//) are ignored." ) + << tl::arg (group + + "--" + m_long_prefix + "blend-mode=mode", &m_cell_conflict_resolution, "Specifies how cell conflicts are resolved when using file concatenation", + "When concatenating file with '+', the reader will handle cells with the same names according to this mode:\n" + "\n" + "* 0: joins everything (default, risk of spoiling layouts)\n" + "* 1: overwrite\n" + "* 2: skip new cell\n" + "* 3: create a variant with a new name" + ) ; } @@ -684,6 +694,7 @@ GenericReaderOptions::configure (db::LoadLayoutOptions &load_options) const load_options.set_option_by_name ("create_other_layers", m_create_other_layers); load_options.set_option_by_name ("text_enabled", m_common_enable_text_objects); load_options.set_option_by_name ("properties_enabled", m_common_enable_properties); + load_options.get_options ().cell_conflict_resolution = db::CellConflictResolution (m_cell_conflict_resolution); load_options.set_option_by_name ("gds2_box_mode", m_gds2_box_mode); load_options.set_option_by_name ("gds2_allow_big_records", m_gds2_allow_big_records); diff --git a/src/buddies/src/bd/bdReaderOptions.h b/src/buddies/src/bd/bdReaderOptions.h index 2f877dc00..c3cd38ca7 100644 --- a/src/buddies/src/bd/bdReaderOptions.h +++ b/src/buddies/src/bd/bdReaderOptions.h @@ -101,6 +101,7 @@ private: bool m_create_other_layers; double m_dbu; bool m_keep_layer_names; + unsigned int m_cell_conflict_resolution; // common GDS2+OASIS bool m_common_enable_text_objects; diff --git a/src/buddies/unit_tests/bdBasicTests.cc b/src/buddies/unit_tests/bdBasicTests.cc index 3f47be648..d53ef81b6 100644 --- a/src/buddies/unit_tests/bdBasicTests.cc +++ b/src/buddies/unit_tests/bdBasicTests.cc @@ -235,6 +235,7 @@ TEST(10) // General "-im=1/0 3,4/0-255 A:17/0", "-is", + "--blend-mode=1", // OASIS "--expect-strict-mode=1" }; @@ -258,6 +259,7 @@ TEST(10) EXPECT_EQ (stream_opt.get_option_by_name ("dxf_text_scaling").to_int (), 100); EXPECT_EQ (stream_opt.get_option_by_name ("layer_map").to_user ().to_string (), "layer_map()"); EXPECT_EQ (stream_opt.get_option_by_name ("create_other_layers").to_bool (), true); + EXPECT_EQ (stream_opt.get_option_by_name ("cell_conflict_resolution").to_string (), "AddToCell"); EXPECT_EQ (stream_opt.get_option_by_name ("properties_enabled").to_bool (), true); EXPECT_EQ (stream_opt.get_option_by_name ("text_enabled").to_bool (), true); EXPECT_EQ (stream_opt.get_option_by_name ("gds2_box_mode").to_uint (), (unsigned int) 1); @@ -283,6 +285,7 @@ TEST(10) EXPECT_EQ (stream_opt.get_option_by_name ("dxf_text_scaling").to_int (), 75); EXPECT_EQ (stream_opt.get_option_by_name ("layer_map").to_user ().to_string (), "layer_map('1/0';'3-4/0-255';'A : 17/0')"); EXPECT_EQ (stream_opt.get_option_by_name ("create_other_layers").to_bool (), false); + EXPECT_EQ (stream_opt.get_option_by_name ("cell_conflict_resolution").to_string (), "OverwriteCell"); EXPECT_EQ (stream_opt.get_option_by_name ("properties_enabled").to_bool (), false); EXPECT_EQ (stream_opt.get_option_by_name ("text_enabled").to_bool (), false); EXPECT_EQ (stream_opt.get_option_by_name ("gds2_box_mode").to_uint (), (unsigned int) 3); From ac9aa475abb03d9acf0c6af2233b029835740f18 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 4 Apr 2021 18:41:35 +0200 Subject: [PATCH 3/6] Properly handle external classes such as PCellDeclarationHelper in gsi::initialize_expressions --- src/gsi/gsi/gsiClassBase.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/gsi/gsi/gsiClassBase.cc b/src/gsi/gsi/gsiClassBase.cc index f0e705de3..04e1b86a1 100644 --- a/src/gsi/gsi/gsiClassBase.cc +++ b/src/gsi/gsi/gsiClassBase.cc @@ -642,7 +642,6 @@ static void collect_classes (const gsi::ClassBase *cls, std::list::const_iterator cc = cls->begin_child_classes (); cc != cls->end_child_classes (); ++cc) { - tl_assert (cc->declaration () != 0); collect_classes (cc.operator-> (), unsorted_classes); } } @@ -678,7 +677,7 @@ ClassBase::classes_in_definition_order (const char *mod_name) continue; } - if ((*c)->declaration () != *c && taken.find ((*c)->declaration ()) == taken.end ()) { + if ((*c)->declaration () && (*c)->declaration () != *c && taken.find ((*c)->declaration ()) == taken.end ()) { // can't produce this class yet - it's a reference to another class which is not produced yet. tl_assert ((*c)->declaration () != 0); more_classes.push_back (*c); @@ -710,8 +709,8 @@ ClassBase::classes_in_definition_order (const char *mod_name) // don't handle classes twice if (taken.find (*c) != taken.end ()) { // not considered. - } else if ((*c)->declaration () != *c && taken.find ((*c)->declaration ()) == taken.end ()) { - // can't produce this class yet - it's a child of a parent that is not produced yet. + } else if ((*c)->declaration () && (*c)->declaration () != *c && taken.find ((*c)->declaration ()) == taken.end ()) { + // can't produce this class yet - it refers to a class whic is not available. tl::error << tl::sprintf ("class %s.%s refers to another class (%s.%s) which is not available", (*c)->module (), (*c)->name (), (*c)->declaration ()->module (), (*c)->declaration ()->name ()); } else if ((*c)->parent () != 0 && taken.find ((*c)->parent ()) == taken.end ()) { // can't produce this class yet - it's a child of a parent that is not produced yet. From 7b9a1ffdeeff03146814f6fdb233481669a41d53 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 4 Apr 2021 21:34:43 +0200 Subject: [PATCH 4/6] Made 'rename' blend mode default for buddy scripts --- src/buddies/src/bd/bdReaderOptions.cc | 8 ++++---- src/buddies/unit_tests/bdBasicTests.cc | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/buddies/src/bd/bdReaderOptions.cc b/src/buddies/src/bd/bdReaderOptions.cc index 51981f562..934f8a4ed 100644 --- a/src/buddies/src/bd/bdReaderOptions.cc +++ b/src/buddies/src/bd/bdReaderOptions.cc @@ -39,7 +39,7 @@ GenericReaderOptions::GenericReaderOptions () m_common_enable_text_objects = load_options.get_option_by_name ("text_enabled").to_bool (); m_common_enable_properties = load_options.get_option_by_name ("properties_enabled").to_bool (); - m_cell_conflict_resolution = load_options.get_options ().cell_conflict_resolution; + m_cell_conflict_resolution = (unsigned int) db::CellConflictResolution::RenameCell; m_gds2_box_mode = load_options.get_option_by_name ("gds2_box_mode").to_uint (); m_gds2_allow_big_records = load_options.get_option_by_name ("gds2_allow_big_records").to_bool (); @@ -200,12 +200,12 @@ GenericReaderOptions::add_options (tl::CommandLineOptions &cmd) ) << tl::arg (group + "--" + m_long_prefix + "blend-mode=mode", &m_cell_conflict_resolution, "Specifies how cell conflicts are resolved when using file concatenation", - "When concatenating file with '+', the reader will handle cells with the same names according to this mode:\n" + "When concatenating files with '+', the reader will handle cells with identical names according to this mode:\n" "\n" - "* 0: joins everything (default, risk of spoiling layouts)\n" + "* 0: joins everything (unsafe)\n" "* 1: overwrite\n" "* 2: skip new cell\n" - "* 3: create a variant with a new name" + "* 3: rename cell (safe, default)" ) ; } diff --git a/src/buddies/unit_tests/bdBasicTests.cc b/src/buddies/unit_tests/bdBasicTests.cc index d53ef81b6..5029c0b87 100644 --- a/src/buddies/unit_tests/bdBasicTests.cc +++ b/src/buddies/unit_tests/bdBasicTests.cc @@ -294,3 +294,22 @@ TEST(10) EXPECT_EQ (stream_opt.get_option_by_name ("oasis_expect_strict_mode").to_int (), 1); } + +// Testing reader options - blend mode "Rename" is default +TEST(11) +{ + bd::GenericReaderOptions opt; + tl::CommandLineOptions cmd; + + opt.add_options (cmd); + + const char *argv[] = { "x" }; + + cmd.parse (sizeof (argv) / sizeof (argv[0]), (char **) argv); + + db::LoadLayoutOptions stream_opt; + opt.configure (stream_opt); + + EXPECT_EQ (stream_opt.get_option_by_name ("cell_conflict_resolution").to_string (), "RenameCell"); +} + From 2c245af13a4af0aa3ede3e0fd25be75a1c6884ed Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Mon, 5 Apr 2021 00:27:14 +0200 Subject: [PATCH 5/6] Fixed an assertion happining with Qt binding enabled --- src/gsi/gsi/gsiClassBase.cc | 1 - src/gsi/gsi/gsiExpression.cc | 8 ++++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/gsi/gsi/gsiClassBase.cc b/src/gsi/gsi/gsiClassBase.cc index 04e1b86a1..7ee9acd8f 100644 --- a/src/gsi/gsi/gsiClassBase.cc +++ b/src/gsi/gsi/gsiClassBase.cc @@ -679,7 +679,6 @@ ClassBase::classes_in_definition_order (const char *mod_name) if ((*c)->declaration () && (*c)->declaration () != *c && taken.find ((*c)->declaration ()) == taken.end ()) { // can't produce this class yet - it's a reference to another class which is not produced yet. - tl_assert ((*c)->declaration () != 0); more_classes.push_back (*c); continue; } diff --git a/src/gsi/gsi/gsiExpression.cc b/src/gsi/gsi/gsiExpression.cc index 1c1d4ba3f..911d38fcc 100644 --- a/src/gsi/gsi/gsiExpression.cc +++ b/src/gsi/gsi/gsiExpression.cc @@ -1077,8 +1077,12 @@ initialize_expressions () std::list classes = gsi::ClassBase::classes_in_definition_order (); for (std::list::const_iterator c = classes.begin (); c != classes.end (); ++c) { - // Skip external classes - if ((*c)->is_external ()) { + // we might encounter a child class which is a reference to a top-level class (e.g. + // duplication of enums into child classes). In this case we should create a reference inside the + // target class. + if ((*c)->declaration () != *c) { + tl_assert ((*c)->parent () != 0); // top-level classes should be merged + // TODO: implement (see rba.cc:1544 for example) continue; } From 9b86ea96d25971a94095ecb9ffcd99e69c0c6f1c Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Mon, 5 Apr 2021 00:30:33 +0200 Subject: [PATCH 6/6] Fixed another assertion --- src/gsi/gsi/gsiExpression.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/gsi/gsi/gsiExpression.cc b/src/gsi/gsi/gsiExpression.cc index 911d38fcc..4bf00218f 100644 --- a/src/gsi/gsi/gsiExpression.cc +++ b/src/gsi/gsi/gsiExpression.cc @@ -1077,10 +1077,13 @@ initialize_expressions () std::list classes = gsi::ClassBase::classes_in_definition_order (); for (std::list::const_iterator c = classes.begin (); c != classes.end (); ++c) { - // we might encounter a child class which is a reference to a top-level class (e.g. - // duplication of enums into child classes). In this case we should create a reference inside the - // target class. - if ((*c)->declaration () != *c) { + if ((*c)->is_external ()) { + // skip external classes + continue; + } else if ((*c)->declaration () != *c) { + // we might encounter a child class which is a reference to a top-level class (e.g. + // duplication of enums into child classes). In this case we should create a reference inside the + // target class. tl_assert ((*c)->parent () != 0); // top-level classes should be merged // TODO: implement (see rba.cc:1544 for example) continue;