From a525345db4fd4d376e215393925db10d20c01127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steven=20Dee=20=28J=C5=8Dshin=29?= Date: Tue, 27 Aug 2024 18:30:03 -0700 Subject: [PATCH 1/3] Minor cleanup/improvements in unique_ptr_test - Uses the same name as the namespaced type for the declarations. - Removes the stateful deleter test case and corresponding type. - Explains the reasoning behind the instrumented lifecycle types thad it didn't seem practical to remove. - Uncomments the #undef ctl since it is at worst a no-op. - Adds tests for the very basic functionality of a unique_ptr, i.e. that it is deleted on leaving scope. - SetsGDeleter now doesn't delete the object, and the tests that rely on it use pointers to memory that doesn't need to be deleted. - Comments a few of the basic test cases. --- test/ctl/unique_ptr_test.cc | 120 +++++++++++++++++++++--------------- 1 file changed, 71 insertions(+), 49 deletions(-) diff --git a/test/ctl/unique_ptr_test.cc b/test/ctl/unique_ptr_test.cc index 75ed674ba6c..efa2aedd5f5 100644 --- a/test/ctl/unique_ptr_test.cc +++ b/test/ctl/unique_ptr_test.cc @@ -25,71 +25,68 @@ // #define ctl std template> -using Ptr = ctl::unique_ptr; +using unique_ptr = ctl::unique_ptr; template -Ptr -Mk(Args&&... args) +unique_ptr +make_unique(Args&&... args) { return ctl::make_unique(ctl::forward(args)...); } template -Ptr -MkRaw() +unique_ptr +make_unique_for_overwrite() { return ctl::make_unique_for_overwrite(); } -// #undef ctl +#undef ctl +// The following few definitions are used to get observability into aspects of +// an object's lifecycle, to make sure that e.g. constructing a unique_ptr of a +// type does not construct an object, and that make_unique does construct an +// object. static int g = 0; -struct SetsGDeleter +struct SetsGCtor { - void operator()(auto* x) const noexcept + SetsGCtor() { ++g; - delete x; } }; -struct StatefulDeleter +struct SetsGDtor { - char state; - void operator()(auto* x) const noexcept + ~SetsGDtor() { + ++g; } }; -struct FinalDeleter final +struct SetsGDeleter { void operator()(auto* x) const noexcept { + ++g; } }; -static_assert(sizeof(Ptr) == sizeof(int*)); - -// not everyone uses [[no_unique_address]]... -static_assert(!ctl::is_same_v, ctl::unique_ptr> || - sizeof(Ptr) == sizeof(int*)); +// A unique_ptr with an empty deleter should be the same size as a raw pointer. +static_assert(sizeof(unique_ptr) == sizeof(int*)); -struct SetsGCtor +struct FinalDeleter final { - SetsGCtor() + void operator()(auto* x) const noexcept { - ++g; } }; -struct SetsGDtor -{ - ~SetsGDtor() - { - ++g; - } -}; +// ctl::unique_ptr does not need to inherit from its deleter for this property; +// the STL often does, though, so we don't hold them to the following. +static_assert(!ctl::is_same_v, ctl::unique_ptr> || + sizeof(unique_ptr) == sizeof(int*)); struct Base {}; @@ -100,13 +97,16 @@ struct Derived : Base int main() { + int a; { - Ptr x(new int(5)); + // Shouldn't cause any memory leaks. + unique_ptr x(new int(5)); } { - Ptr x(new int()); + // Deleter is called if the pointer is non-null when reset. + unique_ptr x(&a); x.reset(); if (g != 1) return 1; @@ -114,22 +114,45 @@ main() { g = 0; - Ptr x(new int()); - delete x.release(); + // Deleter is not called if the pointer is null when reset. + unique_ptr x(&a); + x.release(); x.reset(); if (g) return 17; } { - Ptr x(new int(5)), y(new int(6)); + g = 0; + // Deleter is called when the pointer goes out of scope. + { + unique_ptr x(&a); + } + if (!g) + return 18; + } + + { + g = 0; + // Deleter is called if scope ends exceptionally. + try { + unique_ptr x(&a); + throw 'a'; + } catch (char) { + } + if (!g) + return 19; + } + + { + unique_ptr x(new int(5)), y(new int(6)); x.swap(y); if (*x != 6 || *y != 5) return 2; } { - Ptr x; + unique_ptr x; if (x) return 3; x.reset(new int(5)); @@ -139,17 +162,17 @@ main() { g = 0; - Ptr x; + unique_ptr x; if (g) return 5; - x = Mk(); + x = make_unique(); if (g != 1) return 6; } { g = 0; - auto x = Mk(); + auto x = make_unique(); if (g) return 7; x.reset(); @@ -161,9 +184,9 @@ main() { g = 0; - Ptr x, y; - x = Mk(); - y = Mk(); + unique_ptr x, y; + x = make_unique(); + y = make_unique(); #if 0 // shouldn't compile x = y; @@ -178,7 +201,7 @@ main() { g = 0; { - auto x = Mk(); + auto x = make_unique(); } if (g != 1) return 12; @@ -187,7 +210,7 @@ main() { g = 0; { - auto x = Mk(); + auto x = make_unique(); delete x.release(); } if (g != 1) @@ -199,13 +222,13 @@ main() // side effects it has are illegal to detect? { g = 0; - auto x = MkRaw(); + auto x = make_unique_for_overwrite(); if (g) return 14; x.reset(); if (g) return 15; - x = Mk(); + x = make_unique(); if (g != 1) return 16; } @@ -214,16 +237,15 @@ main() { int a; // Should compile. - Ptr x(&a); - Ptr y(&a); + unique_ptr x(&a); } { - Ptr x(new Base); + unique_ptr x(new Base); x.reset(new Derived); - Ptr y(new Derived); - Ptr z(ctl::move(y)); + unique_ptr y(new Derived); + unique_ptr z(ctl::move(y)); } CheckForMemoryLeaks(); From 524f1fd2c2dcd7e95d6154d7c01d5c2764e4e500 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steven=20Dee=20=28J=C5=8Dshin=29?= Date: Sat, 31 Aug 2024 16:01:13 -0700 Subject: [PATCH 2/3] Simple using statements --- test/ctl/unique_ptr_test.cc | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/test/ctl/unique_ptr_test.cc b/test/ctl/unique_ptr_test.cc index efa2aedd5f5..03ca37805c4 100644 --- a/test/ctl/unique_ptr_test.cc +++ b/test/ctl/unique_ptr_test.cc @@ -24,22 +24,9 @@ // #include // #define ctl std -template> -using unique_ptr = ctl::unique_ptr; - -template -unique_ptr -make_unique(Args&&... args) -{ - return ctl::make_unique(ctl::forward(args)...); -} - -template -unique_ptr -make_unique_for_overwrite() -{ - return ctl::make_unique_for_overwrite(); -} +using ctl::unique_ptr; +using ctl::make_unique; +using ctl::make_unique_for_overwrite; #undef ctl From 530a5d04805a908299341a4142b95b34642769c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steven=20Dee=20=28J=C5=8Dshin=29?= Date: Sat, 31 Aug 2024 16:05:49 -0700 Subject: [PATCH 3/3] Consistent naming between unique/shared ptr tests --- test/ctl/unique_ptr_test.cc | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/test/ctl/unique_ptr_test.cc b/test/ctl/unique_ptr_test.cc index 03ca37805c4..239902b8329 100644 --- a/test/ctl/unique_ptr_test.cc +++ b/test/ctl/unique_ptr_test.cc @@ -36,23 +36,23 @@ using ctl::make_unique_for_overwrite; // object. static int g = 0; -struct SetsGCtor +struct ConstructG { - SetsGCtor() + ConstructG() { ++g; } }; -struct SetsGDtor +struct DestructG { - ~SetsGDtor() + ~DestructG() { ++g; } }; -struct SetsGDeleter +struct CallG { void operator()(auto* x) const noexcept { @@ -93,7 +93,7 @@ main() { // Deleter is called if the pointer is non-null when reset. - unique_ptr x(&a); + unique_ptr x(&a); x.reset(); if (g != 1) return 1; @@ -102,7 +102,7 @@ main() { g = 0; // Deleter is not called if the pointer is null when reset. - unique_ptr x(&a); + unique_ptr x(&a); x.release(); x.reset(); if (g) @@ -113,7 +113,7 @@ main() g = 0; // Deleter is called when the pointer goes out of scope. { - unique_ptr x(&a); + unique_ptr x(&a); } if (!g) return 18; @@ -123,7 +123,7 @@ main() g = 0; // Deleter is called if scope ends exceptionally. try { - unique_ptr x(&a); + unique_ptr x(&a); throw 'a'; } catch (char) { } @@ -149,17 +149,17 @@ main() { g = 0; - unique_ptr x; + unique_ptr x; if (g) return 5; - x = make_unique(); + x = make_unique(); if (g != 1) return 6; } { g = 0; - auto x = make_unique(); + auto x = make_unique(); if (g) return 7; x.reset(); @@ -171,9 +171,9 @@ main() { g = 0; - unique_ptr x, y; - x = make_unique(); - y = make_unique(); + unique_ptr x, y; + x = make_unique(); + y = make_unique(); #if 0 // shouldn't compile x = y; @@ -188,7 +188,7 @@ main() { g = 0; { - auto x = make_unique(); + auto x = make_unique(); } if (g != 1) return 12; @@ -197,7 +197,7 @@ main() { g = 0; { - auto x = make_unique(); + auto x = make_unique(); delete x.release(); } if (g != 1)