Skip to content

Commit 6271a64

Browse files
committed
Fixed move from none bug in Optional
1 parent 30c4bef commit 6271a64

File tree

6 files changed

+40
-51
lines changed

6 files changed

+40
-51
lines changed

README.md

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ Here is the namespace hierarchy:
2424
* [prefixed](./include/scl/tool/iostream/log/prefixed.h) - Log w/ a prefix
2525
* [surrounded](./include/scl/tool/iostream/log/surrounded.h) - Log w/ a prefix & a suffix
2626
* [wrap](./include/scl/tool/iostream/log/log.hpp) - An alias for [scl::tools::iostream::log::surrounded](./include/scl/tool/iostream/log/surrounded.h)
27-
* [meta](#scl-tools-meta) - Template Meta Programming utils (such as `void_t`, `enable_if_t` and `constexpr` functions like `is_same` or even handy stuff like `constexpr_assert`)
27+
* [meta](./include/scl/tools/meta/meta.hpp) - Template Meta Programming utils (such as `void_t`, `enable_if_t` and `constexpr` functions like `is_same` or even handy stuff like `constexpr_assert`)
2828
* [console](./include/scl/tool/iostream/log/log.hpp) - An alias for [scl::tools::iostream::log](./include/scl/tool/iostream/log/log.hpp)
2929
* [cli](./include/scl/tool/iostream/log/log.hpp) - An alias for [scl::console](./include/scl/tool/iostream/log/log.hpp)
3030
* [exceptions](./include/scl/exceptions/exceptions.hpp) - Exception types used in the library
@@ -34,8 +34,8 @@ Here is the namespace hierarchy:
3434
* [creators](./include/scl/stream/creators/creators.hpp) - Grouping stream creation functions
3535
* [operators](./include/scl/stream/operators/operators.hpp) - Grouping intermediate stream operation functions
3636
* [terminators](./include/scl/stream/terminators/terminators.hpp) - Grouping end of stream operation functions
37-
* [async](./async/async.hpp) - The asynchronous programming API
38-
* [http]( ./http/http.hpp ) - The HTTP API
37+
* [async](./include/scl/async/async.hpp) - The asynchronous programming API
38+
* [http]( ./include/scl/http/http.hpp ) - The HTTP API
3939

4040

4141

include/scl/utils/Optional.h

+5-24
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,6 @@
2020
#include <scl/concepts/require.h>
2121
#include <scl/concepts/Movable.h>
2222

23-
using scl::tools::iostream::nl;
24-
25-
//TODO: Fix copy/move problems
26-
2723
namespace scl{
2824
namespace utils{
2925
/**
@@ -68,23 +64,14 @@ namespace scl{
6864
template <class T>
6965
class Optional{
7066
protected:
71-
/**
72-
* @var valueFlag
73-
* determines whether or not there's a value inside this Optional<T>
74-
* @deprecated Replaced by the raw storage utility
75-
*/
76-
//bool valueFlag = false;
77-
7867
/**
7968
* @var payload
80-
* A raw strage to hold an instance of the object
69+
* A raw storage to hold an instance of the object
8170
*/
82-
RawStorage<T> payload = {};
71+
RawStorage<T> payload;
8372

8473
public:
8574
using value_type = META::remove_cv_ref_t<T>;
86-
/*static constexpr bool is_movable = META::is_movable<value_type>();
87-
static constexpr bool is_copyable = META::is_copyable<value_type>();*/
8875

8976
public:
9077
template <class... Args>
@@ -132,7 +119,6 @@ namespace scl{
132119
* @param value being the value to assign from
133120
*/
134121
Optional(const value_type& value){
135-
// static_require(is_copyable);
136122
this->payload.construct(value);
137123
}
138124

@@ -142,7 +128,6 @@ namespace scl{
142128
* @return a reference to this Optional
143129
*/
144130
Optional& operator=(const value_type& value){
145-
// static_require(is_copyable);
146131
this->payload.construct(value);
147132
return *this;
148133
}
@@ -152,7 +137,6 @@ namespace scl{
152137
* @param o being the Optional to copy from
153138
*/
154139
Optional(const Optional& o){
155-
// static_require(is_copyable);
156140
if(o.hasValue())
157141
this->payload.construct(o.get());
158142
};
@@ -163,7 +147,6 @@ namespace scl{
163147
* @return a reference to this Optional<T>
164148
*/
165149
Optional& operator=(const Optional& rhs){
166-
// static_require(is_copyable);
167150
if(rhs.hasValue())
168151
this->payload.construct(rhs.get());
169152

@@ -185,7 +168,6 @@ namespace scl{
185168
* (i.e. equivalent to one constructed from none)
186169
*/
187170
Optional(Optional&& rhs){
188-
// static_require(is_movable);
189171
if(rhs.hasValue())
190172
this->payload = std::move(rhs.payload);
191173
};
@@ -199,7 +181,6 @@ namespace scl{
199181
* (i.e. equivalent to one constructed from none)
200182
*/
201183
Optional& operator=(Optional&& rhs) noexcept{
202-
// static_require(is_movable);
203184
if(rhs.hasValue())
204185
this->payload = std::move(rhs.payload);
205186
return *this;
@@ -220,7 +201,7 @@ namespace scl{
220201
* @return a reference to this Optional
221202
*/
222203
Optional& operator=(None _){
223-
this->payload = {};
204+
this->payload.reset();
224205
return *this;
225206
}
226207

@@ -517,13 +498,13 @@ namespace scl{
517498
#undef SCL_TPL
518499
};
519500

520-
/*template <class T>
501+
template <class T>
521502
struct ToString<Optional<T>, META::enable_if_t<
522503
META::defines_scl_to_string<T>()
523504
>>{
524505
std::string operator()(const Optional<T>& opt){
525506
return opt.hasValue() ? asString(*opt) : asString(none);
526507
}
527-
};*/
508+
};
528509
}
529510
}

include/scl/utils/RawStorage.h

+9
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,15 @@ namespace scl{
9797
this->destructor();
9898
}
9999

100+
/**
101+
* Alias for RawStorage::destroy
102+
* @return a reference to this RawStorage
103+
*/
104+
RawStorage& reset(){
105+
this->destroy();
106+
return *this;
107+
}
108+
100109
/**
101110
* Access the underlying data
102111
* @return a mutable reference to the underlying data

include/scl/utils/toString.h

+14
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <scl/tools/meta/exists.h>
88
#include <scl/tools/meta/type_mod.h>
99
#include <scl/tools/meta/is_same.h>
10+
#include <scl/tools/meta/defines_std_to_string.h>
1011

1112
namespace scl{
1213
namespace utils{
@@ -34,6 +35,19 @@ namespace scl{
3435
}
3536
};
3637

38+
/**
39+
* Specialization for types that define std::to_string
40+
* @tparam T being the type of objects to convert to string
41+
*/
42+
template <class T>
43+
struct ToString<T, META::void_t<META::enable_if_t<
44+
META::defines_std_to_string<T>()
45+
>>>{
46+
std::string operator()(const T& t) const{
47+
return std::to_string(t);
48+
}
49+
};
50+
3751
/**
3852
* Free function that allows string conversion
3953
* @tparam T the type of object to convert to string

main.cpp

+3-4
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ struct Person{
8484

8585
void testsConcepts(){
8686
require(Iterator<int*>{});
87-
require(SwappableWith<int, int>{});
87+
// require(SwappableWith<int, int>{});
8888
}
8989

9090
void testsMake(){
@@ -170,10 +170,9 @@ void testHTTP(){
170170
}
171171

172172
int main(){
173-
//TODO: Fix references bug (undef ref / empty optional access)
174173
//testsStream();
175174

176-
// std::cout << asString(42) << nl;
177-
// std::cout << Optional<int>{42} << nl;
175+
std::cout << asString(42) << nl;
176+
std::cout << make::optional<int>(42) << nl;
178177
std::cout << make::any<stringLiteral>("Yes").as<stringLiteral>() << nl;
179178
}

tests/unittest/utils/OptionalTests.cpp

+6-20
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,6 @@ using namespace scl::tools;
2121
using scl::tools::iostream::nl;
2222

2323
TEST(OptionalTests, ConceptsRequirementsMet) {
24-
/*{
25-
Optional<int> o = 42;
26-
Optional<int> x{o};
27-
x = o;
28-
std::cout << "x: " << x.get() << ", o: " << o.get() << nl;
29-
}*/
30-
3124
ASSERT_TRUE(DefaultConstructible<Optional<int>>{});
3225
ASSERT_TRUE(Movable<Optional<int>>{});
3326
ASSERT_TRUE(Copyable<Optional<int>>{});
@@ -41,12 +34,14 @@ TEST(OptionalTests, ConceptsRequirementsMet) {
4134

4235
TEST(OptionalTests, CanConstructFromNone){
4336
Optional<int> o{none};
37+
SUCCEED();
4438
}
4539

4640
TEST(OptionalTests, CanAssignFromNone){
4741
Optional<int> o = none;
4842
Optional<int> op;
4943
op = none;
44+
SUCCEED();
5045
}
5146

5247
TEST(OptionalTests, CanUseNonTriviallyCopyableType){
@@ -78,7 +73,6 @@ TEST(OptionalTests, MovedFromHasCorrectSemantics){
7873
Optional<std::string> no = none;
7974
auto a = std::move(no);
8075

81-
//ASSERT_EQ(a.hasValue(), no.hasValue());
8276
ASSERT_FALSE(no.hasValue());
8377
}
8478

@@ -199,29 +193,21 @@ TEST(OptionalTests, ThrowsGivenExceptionOnEmpty){
199193
ASSERT_THROW(o.orThrow(e), decltype(e));
200194
}
201195

202-
/* //Disabled test because it cannot be true
203-
TEST(OptionalTests, AdvancedInvalidTypesHaveWellDefinedBehavior){
204-
using nmnc = Optional<NonCopyableNonMovable_t>;
205-
206-
ASSERT_EQ(META::is_copyable<NonCopyableNonMovable_t>(), META::is_copyable<nmnc>());
207-
ASSERT_EQ(META::is_movable<NonCopyableNonMovable_t>(), META::is_movable<nmnc>());
208-
}*/
209-
210196
TEST(OptionalTests, NoneFunctionCalledIfEmptyNotIfPresent){
211197
Optional<int> o = none;
212198
o.doIfPresent([](const int& _){ //shouldn't be called
213-
ASSERT_TRUE(false);
199+
FAIL();
214200
}).doIfEmpty([]{ //should be called
215-
ASSERT_TRUE(true);
201+
SUCCEED();
216202
});
217203
}
218204

219205
TEST(OptionalTests, ValueFunctionCalledIfPresentNotIfEmpty){
220206
Optional<int> o = 42;
221207
o.doIfPresent([](const int& _){ //should be called
222-
ASSERT_TRUE(true);
208+
SUCCEED();
223209
}).doIfEmpty([]{ //shouldn't be called
224-
ASSERT_TRUE(false);
210+
FAIL();
225211
});
226212
}
227213

0 commit comments

Comments
 (0)