From c93db59e37f02ff27717d5284858c6c843df994b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20K=C3=B6fferlein?= Date: Sun, 5 Apr 2020 15:11:37 +0200 Subject: [PATCH] Fixed #524 (failed query leaves layout in invalid state) (#528) --- src/db/db/dbLayoutQuery.cc | 72 +++++++++++++++++++------ src/db/db/dbLayoutQuery.h | 25 ++------- src/db/unit_tests/dbLayoutQueryTests.cc | 24 +++++++++ 3 files changed, 85 insertions(+), 36 deletions(-) diff --git a/src/db/db/dbLayoutQuery.cc b/src/db/db/dbLayoutQuery.cc index 05f3aeef7..27ed3d8ad 100644 --- a/src/db/db/dbLayoutQuery.cc +++ b/src/db/db/dbLayoutQuery.cc @@ -2011,7 +2011,7 @@ private: // LayoutQueryIterator implementation LayoutQueryIterator::LayoutQueryIterator (const LayoutQuery &q, db::Layout *layout, tl::Eval *parent_eval, tl::AbsoluteProgress *progress) - : mp_q (const_cast (&q)), mp_layout (layout), m_eval (parent_eval), m_layout_ctx (layout, true /*can modify*/), mp_progress (progress) + : mp_q (const_cast (&q)), mp_layout (layout), m_eval (parent_eval), m_layout_ctx (layout, true /*can modify*/), mp_progress (progress), m_initialized (false) { m_eval.set_ctx_handler (&m_layout_ctx); m_eval.set_var ("layout", tl::Variant::make_variant_ref (layout)); @@ -2022,14 +2022,10 @@ LayoutQueryIterator::LayoutQueryIterator (const LayoutQuery &q, db::Layout *layo // Avoid update() calls while iterating in modifying mode mp_layout->update (); mp_layout->start_changes (); - - // NOTE: Stange - in modifying mode, init() will actually already execute the - // first modification. Hence start_changes() needs to be called before. - init (); } LayoutQueryIterator::LayoutQueryIterator (const LayoutQuery &q, const db::Layout *layout, tl::Eval *parent_eval, tl::AbsoluteProgress *progress) - : mp_q (const_cast (&q)), mp_layout (const_cast (layout)), m_eval (parent_eval), m_layout_ctx (layout), mp_progress (progress) + : mp_q (const_cast (&q)), mp_layout (const_cast (layout)), m_eval (parent_eval), m_layout_ctx (layout), mp_progress (progress), m_initialized (false) { // TODO: check whether the query is a modifying one (with .. do, delete) @@ -2039,8 +2035,6 @@ LayoutQueryIterator::LayoutQueryIterator (const LayoutQuery &q, const db::Layout m_eval.define_function (mp_q->property_name (i), new FilterStateFunction (i, &m_state)); } - init (); - // Avoid update() calls while iterating in modifying mode mp_layout->start_changes (); } @@ -2048,7 +2042,18 @@ LayoutQueryIterator::LayoutQueryIterator (const LayoutQuery &q, const db::Layout LayoutQueryIterator::~LayoutQueryIterator () { mp_layout->end_changes (); - cleanup (); + if (m_initialized) { + cleanup (); + } +} + +void +LayoutQueryIterator::ensure_initialized () +{ + if (! m_initialized) { + init (); + m_initialized = true; + } } void @@ -2080,17 +2085,51 @@ LayoutQueryIterator::cleanup () void LayoutQueryIterator::reset () { - // forces an update if required - mp_layout->end_changes (); - mp_layout->start_changes (); + if (m_initialized) { - cleanup (); - init (); + // forces an update if required + mp_layout->end_changes (); + mp_layout->start_changes (); + + cleanup (); + init (); + + } +} + +bool +LayoutQueryIterator::at_end () const +{ + const_cast (this)->ensure_initialized (); + return m_state.empty (); +} + +bool +LayoutQueryIterator::get (const std::string &name, tl::Variant &v) +{ + ensure_initialized (); + if (m_state.empty () || !m_state.back () || !mp_q->has_property (name)) { + return false; + } else { + return m_state.back ()->get_property (mp_q->property_by_name (name), v); + } +} + +bool +LayoutQueryIterator::get (unsigned int id, tl::Variant &v) +{ + ensure_initialized (); + if (m_state.empty () || !m_state.back ()) { + return false; + } else { + return m_state.back ()->get_property (id, v); + } } void LayoutQueryIterator::dump () const { + const_cast (this)->ensure_initialized (); mp_root_state->dump (); std::cout << std::endl; } @@ -2111,6 +2150,7 @@ LayoutQueryIterator::collect (FilterStateBase *state, std::set b (new FilterBracket (q)); if (ex.test ("instances")) { - ex.test ("of") && (ex.test ("cells") || ex.test ("cell")); + (ex.test ("of") || ex.test ("from")) && (ex.test ("cells") || ex.test ("cell")); // Because an array member cannot be modified we use ArrayInstances in the modification case always parse_cell_name_filter_seq (ex, q, b.get (), reading ? ExplodedInstances : ArrayInstances, reading); } else if (ex.test ("arrays")) { - ex.test ("of") && (ex.test ("cells") || ex.test ("cell")); + (ex.test ("of") || ex.test ("from")) && (ex.test ("cells") || ex.test ("cell")); parse_cell_name_filter_seq (ex, q, b.get (), ArrayInstances, reading); } else { ex.test ("cells") || ex.test ("cell"); diff --git a/src/db/db/dbLayoutQuery.h b/src/db/db/dbLayoutQuery.h index 4c65b004a..96b126914 100644 --- a/src/db/db/dbLayoutQuery.h +++ b/src/db/db/dbLayoutQuery.h @@ -598,10 +598,7 @@ public: /** * @brief Returns true if the iterator is at the end. */ - bool at_end () const - { - return m_state.empty (); - } + bool at_end () const; /** * @brief Increment the iterator: deliver the next state @@ -644,14 +641,7 @@ public: * @param v The value of the property * @return True, if the property could be delivered. */ - bool get (const std::string &name, tl::Variant &v) - { - if (m_state.empty () || !m_state.back () || !mp_q->has_property (name)) { - return false; - } else { - return m_state.back ()->get_property (mp_q->property_by_name (name), v); - } - } + bool get (const std::string &name, tl::Variant &v); /** * @brief Gets a property for the current state (property is given by ID). @@ -660,14 +650,7 @@ public: * @param v The value of the property * @return True, if the property could be delivered. */ - bool get (unsigned int id, tl::Variant &v) - { - if (m_state.empty () || !m_state.back ()) { - return false; - } else { - return m_state.back ()->get_property (id, v); - } - } + bool get (unsigned int id, tl::Variant &v); /** * @brief Get the eval object which provides access to the properties through expressions @@ -690,7 +673,9 @@ private: tl::Eval m_eval; db::LayoutContextHandler m_layout_ctx; tl::AbsoluteProgress *mp_progress; + bool m_initialized; + void ensure_initialized (); void collect (FilterStateBase *state, std::set &states); void next_up (bool skip); bool next_down (); diff --git a/src/db/unit_tests/dbLayoutQueryTests.cc b/src/db/unit_tests/dbLayoutQueryTests.cc index a89d712a4..d6214710f 100644 --- a/src/db/unit_tests/dbLayoutQueryTests.cc +++ b/src/db/unit_tests/dbLayoutQueryTests.cc @@ -1463,4 +1463,28 @@ TEST(62) std::string s = q2s_var (iq, "data"); EXPECT_EQ (s, "T2,T1,T1"); } + + { + // A non-executed query + db::LayoutQuery q ("select inst[\"text\"] from instances of ...* where cell_name ~ \"Basic.*\""); + db::LayoutQueryIterator iq (q, &g); + EXPECT_EQ (true, true); + } +} + +TEST(63) +{ + db::Layout g; + + // A failing query must not leave a layout under construction + try { + db::LayoutQuery q ("!not a valid query"); + db::LayoutQueryIterator iq (q, &g); + std::string s = q2s_var (iq, "data"); + EXPECT_EQ (true, false); + } catch (tl::Exception &ex) { + EXPECT_EQ (ex.msg (), "Expected a word or quoted string here: !not a val .."); + } + + EXPECT_EQ (g.under_construction (), false); }