From 89dd35e70d4ad192b2c3993fdee3dffe34138cb9 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Wed, 12 Jun 2024 17:59:56 +0200 Subject: [PATCH] Library: cover handling limits for virtual baseclass scenario building on the preceding analysis, we can now demonstrate that the container is initially able to grow, but looses this capability after accepting one element of unknown subclass type... --- src/lib/several-builder.hpp | 18 +++--- src/lib/several.hpp | 6 ++ tests/library/several-builder-test.cpp | 76 ++++++++++++++++++++------ wiki/thinkPad.ichthyo.mm | 65 +++++++++++----------- 4 files changed, 106 insertions(+), 59 deletions(-) diff --git a/src/lib/several-builder.hpp b/src/lib/several-builder.hpp index 30e962de9..5c0f6cbd6 100644 --- a/src/lib/several-builder.hpp +++ b/src/lib/several-builder.hpp @@ -277,7 +277,7 @@ namespace lib { ,class POL =HeapOwn ///< Allocator policy > class SeveralBuilder - : Several + : private Several , util::MoveOnly , POL { @@ -390,15 +390,17 @@ namespace lib { // ensure sufficient element capacity or the ability to adapt element spread if (Coll::spread() < reqSiz() and not (Coll::empty() or canWildMove())) throw err::Invalid{_Fmt{"Unable to place element of type %s (size=%d)" - "into container for element size %d."} + "into Several-container for element size %d."} % util::typeStr() % reqSiz() % Coll::spread()}; // ensure sufficient storage or verify the ability to re-allocate if (not (Coll::empty() or Coll::hasReserve(reqSiz()) or POL::canExpand(reqSiz()) or canDynGrow())) - throw err::Invalid{_Fmt{"Unable to accommodate further element of type %s "} - % util::typeStr()}; + throw err::Invalid{_Fmt{"Several-container is unable to accommodate further element of type %s; " + "storage reserve (%s bytes) exhausted and unable to move elements " + "of mixed unknown detail type, which are not trivially movable." } + % util::typeStr() % Coll::storageBuffSiz()}; size_t elmSiz = reqSiz(); size_t newPos = Coll::size(); @@ -410,11 +412,13 @@ namespace lib { Coll::data_->cnt = newPos+1; } + /** ensure clean-up can be handled properly. + * @throw err::Invalid when \a TY requires a different style + * of deleter than was established for this instance */ template void ensureDeleter() { - // ensure clean-up can be handled properly Deleter deleterFunctor = selectDestructor(); if (Coll::data_->deleter) return; Coll::data_->deleter = deleterFunctor; @@ -424,7 +428,7 @@ namespace lib { adjustStorage (size_t cnt, size_t spread) { size_t demand{cnt*spread}; - size_t buffSiz{Coll::data_? Coll::data_->buffSiz : 0}; + size_t buffSiz{Coll::storageBuffSiz()}; if (demand == buffSiz) return; if (demand > buffSiz) @@ -462,7 +466,7 @@ namespace lib { adjustSpread (size_t newSpread) { REQUIRE (Coll::data_); - REQUIRE (newSpread * Coll::size() <= Coll::data_->buffSiz); + REQUIRE (newSpread * Coll::size() <= Coll::storageBuffSiz()); size_t oldSpread = Coll::spread(); if (newSpread > oldSpread) // need to spread out diff --git a/src/lib/several.hpp b/src/lib/several.hpp index 4446d6cca..b770b5642 100644 --- a/src/lib/several.hpp +++ b/src/lib/several.hpp @@ -210,6 +210,12 @@ namespace lib { return data_? data_->spread : sizeof(I); } + size_t + storageBuffSiz() const + { + return data_? data_->buffSiz : 0; + } + bool hasReserve (size_t extraSize) const { diff --git a/tests/library/several-builder-test.cpp b/tests/library/several-builder-test.cpp index 3ce7d42f5..51db58478 100644 --- a/tests/library/several-builder-test.cpp +++ b/tests/library/several-builder-test.cpp @@ -111,6 +111,18 @@ namespace test{ } }; + + /** + * A non-copyable struct with 16bit alignment + * - not trivially default constructible + * - but trivially destructible + */ + struct ShortBlocker + : util::NonCopyable + { + int16_t val{short(1 + (rand() % 1'000))}; + }; + } // (END) test types @@ -164,7 +176,7 @@ namespace test{ * *unchecked wild cast* will happen on access; while certainly dangerous, * this behaviour allows for special low-level data layout tricks. * - the results from an iterator can be used to populate by copy - * @todo WIP 6/24 🔁 define ⟶ ✔ implement + * @todo WIP 6/24 ✔ define ⟶ ✔ implement */ void check_Builder() @@ -228,7 +240,17 @@ namespace test{ /** @test TODO proper handling of exceptions during population - * @todo WIP 6/24 🔁 define ⟶ implement + * - when the container is filled with arbitrary subclasses + * of a base interface with virtual destructor, the first element is used + * to accommodate the storage spread; larger elements or elements of a completely + * different type can not be accommodated and the container can not grow beyond + * the initially allocated reserve (defined to be 10 elements by default). + * - when the container is defined to hold elements of a specific fixed subclass, + * it can be filled with default-constructed instances, and the initial allocation + * can be expanded by move-relocation. Yet totally unrelated elements can not be + * accepted (due to unknown destructor); and when accepting another unspecific + * subclass instance, the ability to grow by move-relocation is lost. + * @todo WIP 6/24 🔁 define ⟶ ✔ implement */ void check_ErrorHandling() @@ -266,7 +288,7 @@ namespace test{ // a re-allocation of a larger buffer, followed by copying the elements; // but since the established scheme allows for _arbitrary_ subclasses, // the builder does not know the exact type for safe element relocation. - VERIFY_FAIL ("Unable to accommodate further element of type Dummy" + VERIFY_FAIL ("Several-container is unable to accommodate further element of type Dummy" , builder.fillElm (20) ); CHECK (10 == builder.size()); } @@ -274,27 +296,45 @@ namespace test{ { // Scenario-2 : Baseclass and elements of a single fixed subclass SeveralBuilder> builder; - Dummy dum; -SHOW_EXPR(dum.getVal()) - Dummy nem{move(dum)}; -SHOW_EXPR(dum.getVal()) -SHOW_EXPR(nem.getVal()) - Num<5> no5; -SHOW_EXPR(no5.getVal()) - Num<5> ne5{move(no5)}; -SHOW_EXPR(no5.getVal()) -SHOW_EXPR(ne5.getVal()) - builder.fillElm(5); CHECK (5 == builder.size()); // trigger re-alloc by moving into larger memory block builder.fillElm(14); CHECK (19 == builder.size()); + CHECK (builder.size() > INITIAL_ELM_CNT); - builder.emplace(); + // with the elements added thus far, this instance has been primed to + // rely on a fixed well known element type for move-growth and to use + // the virtual base class destructor for clean-up. It is thus not possible + // to add another element that is not related to this baseclass... + VERIFY_FAIL ("Unable to handle (trivial-)destructor for element type ShortBlocker, " + "since this container has been primed to use virtual-baseclass-destructors." + , builder.emplace() ); + CHECK (19 == builder.size()); - builder.emplace>(); ///////////////////////////////////OOO this should trigger an exception -> need to code an explicit check right at the start of emplaceNewElm() + CHECK (sizeof(ShortBlocker) < sizeof(Num<5>)); // it was not rejected due to size... + + // However, a subclass /different than the defined element type/ is acceptable, + // but only to the condition to lock any further container growth by move-reallocation. + // The rationale is that we can still destroy through the virtual base destructor, + // but we aren't able to move elements safely any more, since we don't capture the type. + builder.emplace>(); + CHECK (20 == builder.size()); + + // But here comes the catch: since we choose to accept arbitrary sub-types not identified in detail, + // the container has lost its ability of move-reallocation; with 20 elements the current reserve + // is exhausted and we are now unable to add any further elements beyond that point. + VERIFY_FAIL ("unable to move elements of mixed unknown detail type, which are not trivially movable" + , builder.fillElm(5); ); + + // the container is still sound however + auto elms = builder.build(); + CHECK (20 == elms.size()); + // verify that member fields were not corrupted + for (uint i=0; i<=18; ++i) + CHECK (elms[i].calc(i) == 5 + i + (5+5+5+5+5)); + CHECK (elms.back().calc(0) == 1 + 0 + (1)); } } @@ -302,7 +342,7 @@ SHOW_EXPR(ne5.getVal()) /** @test TODO verify correct placement of instances within storage * @todo WIP 6/24 🔁 define ⟶ implement */ - void + void check_ElementStorage() { } @@ -311,7 +351,7 @@ SHOW_EXPR(ne5.getVal()) /** @test TODO demonstrate integration with a custom allocator * @todo WIP 6/24 🔁 define ⟶ implement */ - void + void check_CustomAllocator() { } diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 3337fbb83..ecf404960 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -82728,9 +82728,9 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + - + @@ -82741,9 +82741,9 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - + - + @@ -82757,10 +82757,10 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - + - + @@ -82770,8 +82770,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - - + + @@ -82790,8 +82790,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
Soweit die Theorie... in der Praxis aber scheitert das, weil der Compiler versucht, den realloc-Code zu instantiieren und dafür keinen Copy/Move-Konstruktor findet

- - + @@ -82809,11 +82808,10 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - + - - + + @@ -82942,18 +82940,18 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + - + + +

denn der Allocation-Cluster weiß selber die Zahl seiner belegten Roh-Blöcke; zur de-Allokation muß ansonsten gar nichts gemacht werden

- - - - + + @@ -83176,8 +83174,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
...also ist das keine gute Idee, da verwirrend

- - + @@ -83416,8 +83413,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
...✔ OK — selber schuld

- - + @@ -83432,9 +83428,9 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - - + + + @@ -83446,8 +83442,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
und der von Dummy muß noexcept sein

- - + @@ -83513,8 +83508,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
...es spricht tatsächlich nichts dagegen, diesen Fall auch nachträglich noch zuzulassen — man gibt dann eben die Möglichkeit für re-Allocations auf (und wenn das ein Problem darstellt, macht sich das zu gegebener Zeit eindeutig bemerkbar)

- - + @@ -83538,14 +83532,14 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - + - + @@ -83557,13 +83551,16 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - + + + +