Skip to content

Commit 2084ca0

Browse files
committed
fable: Use std::string instead of std::string&& for constructors
This is the optimal form, according to an analysis by Nicolai Josuttis in a CppCon18 talk: Nicolai Josuttis “The Nightmare of Initialization in C++” https://www.youtube.com/watch?v=7DTlWPgX6zs
1 parent 1b659be commit 2084ca0

24 files changed

+64
-66
lines changed

fable/include/fable/schema.hpp

+9-11
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,6 @@
114114

115115
#pragma once
116116

117-
#include <chrono> // for duration<>
118-
#include <map> // for map<>
119117
#include <memory> // for shared_ptr<>
120118
#include <string> // for string
121119
#include <type_traits> // for enable_if_t<>, is_arithmetic<>, is_enum<>, ...
@@ -179,25 +177,25 @@ class Schema : public schema::Interface {
179177
Schema& operator=(const Schema&) = default;
180178

181179
// Struct
182-
Schema(std::string&& desc, schema::PropertyList<> props)
180+
Schema(std::string desc, schema::PropertyList<> props)
183181
: impl_(new schema::Struct(std::move(desc), props)) {}
184182

185183
Schema(schema::PropertyList<> props) : Schema("", props) {}
186184

187-
Schema(std::string&& desc, const Schema& base, schema::PropertyList<> props)
185+
Schema(std::string desc, const Schema& base, schema::PropertyList<> props)
188186
: impl_(new schema::Struct(std::move(desc), base, props)) {}
189187

190188
Schema(const Schema& base, schema::PropertyList<> props) : Schema("", base, props) {}
191189

192190
// Variant
193191
Schema(const std::vector<Schema>& xs); // NOLINT(runtime/explicit)
194-
Schema(std::string&& desc, const std::vector<Schema>& xs);
192+
Schema(std::string desc, const std::vector<Schema>& xs);
195193

196194
Schema(schema::BoxList props); // NOLINT(runtime/explicit)
197-
Schema(std::string&& desc, schema::BoxList props);
195+
Schema(std::string desc, schema::BoxList props);
198196

199197
Schema(schema::BoxVec&& props); // NOLINT(runtime/explicit)
200-
Schema(std::string&& desc, schema::BoxVec&& props);
198+
Schema(std::string desc, schema::BoxVec&& props);
201199

202200
// Interface
203201
template <typename T, std::enable_if_t<std::is_base_of_v<schema::Interface, T>, int> = 0>
@@ -209,19 +207,19 @@ class Schema : public schema::Interface {
209207

210208
// Ignore
211209
Schema() : impl_(new schema::Ignore("")) {}
212-
explicit Schema(std::string&& desc, JsonType t = JsonType::object)
210+
explicit Schema(std::string desc, JsonType t = JsonType::object)
213211
: impl_(new schema::Ignore(std::move(desc), t)) {}
214212

215213
// Primitives
216214
template <typename T>
217-
Schema(T* ptr, std::string&& desc) : impl_(make_schema(ptr, std::move(desc)).clone()) {}
215+
Schema(T* ptr, std::string desc) : impl_(make_schema(ptr, std::move(desc)).clone()) {}
218216
template <typename T>
219-
Schema(T* ptr, const schema::Box& prototype, std::string&& desc)
217+
Schema(T* ptr, const schema::Box& prototype, std::string desc)
220218
: impl_(make_schema(ptr, prototype, std::move(desc)).clone()) {}
221219

222220
// FromJson
223221
template <typename T>
224-
Schema(T* ptr, JsonType t, std::string&& desc)
222+
Schema(T* ptr, JsonType t, std::string desc)
225223
: impl_(new schema::FromJson<T>(ptr, t, std::move(desc))) {}
226224

227225
public: // Special

fable/include/fable/schema/array.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,15 @@ class Array : public Base<Array<T, P>> {
4242
using Type = std::vector<T>;
4343
using PrototypeSchema = std::remove_cv_t<std::remove_reference_t<P>>;
4444

45-
Array(Type* ptr, std::string&& desc);
45+
Array(Type* ptr, std::string desc);
4646
Array(Type* ptr, PrototypeSchema prototype)
4747
: Base<Array<T, P>>(JsonType::array), prototype_(std::move(prototype)), ptr_(ptr) {}
48-
Array(Type* ptr, PrototypeSchema prototype, std::string&& desc)
48+
Array(Type* ptr, PrototypeSchema prototype, std::string desc)
4949
: Base<Array<T, P>>(JsonType::array, std::move(desc)), prototype_(std::move(prototype)), ptr_(ptr) {}
5050

5151
#if 0
5252
// This is defined in: fable/schema/magic.hpp
53-
Array(Type* ptr, std::string&& desc)
53+
Array(Type* ptr, std::string desc)
5454
: Array(ptr, make_prototype<T>(), std::move(desc)) {}
5555
#endif
5656

fable/include/fable/schema/boolean.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class Boolean : public Base<Boolean> {
3535
public: // Types and Constructors
3636
using Type = bool;
3737

38-
Boolean(Type* ptr, std::string&& desc);
38+
Boolean(Type* ptr, std::string desc);
3939

4040
public: // Overrides
4141
Json json_schema() const override;

fable/include/fable/schema/confable.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,14 @@ class FromConfable : public Base<FromConfable<T>> {
4242
public: // Types and Constructors
4343
using Type = T;
4444

45-
explicit FromConfable(std::string&& desc = "") {
46-
schema_ = Type().schema();
45+
FromConfable(std::string desc = "") {
46+
schema_ = Type().schema(); // NOLINT
4747
schema_.reset_ptr();
4848
this->type_ = schema_.type();
4949
this->desc_ = std::move(desc);
5050
}
5151

52-
FromConfable(Type* ptr, std::string&& desc)
52+
FromConfable(Type* ptr, std::string desc)
5353
: Base<FromConfable<Type>>(ptr->schema().type(), std::move(desc))
5454
, schema_(ptr->schema())
5555
, ptr_(ptr) {

fable/include/fable/schema/const.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ class Const : public Base<Const<T, P>> {
3939
using Type = T;
4040
using PrototypeSchema = std::remove_cv_t<std::remove_reference_t<P>>;
4141

42-
Const(const Type& constant, std::string&& desc);
43-
Const(const Type& constant, PrototypeSchema prototype, std::string&& desc)
42+
Const(const Type& constant, std::string desc);
43+
Const(const Type& constant, PrototypeSchema prototype, std::string desc)
4444
: Base<Const<T, P>>(prototype.type(), std::move(desc))
4545
, prototype_(std::move(prototype))
4646
, constant_(constant) {
@@ -49,7 +49,7 @@ class Const : public Base<Const<T, P>> {
4949

5050
#if 0
5151
// This is defined in: fable/schema/magic.hpp
52-
Const(const T& constant, std::string&& desc)
52+
Const(const T& constant, std::string desc)
5353
: Const(constant, make_prototype<T>(), std::move(desc)) {}
5454
#endif
5555

fable/include/fable/schema/duration.hpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,15 @@ class Duration : public Base<Duration<T, Period>> {
4040
using Type = std::chrono::duration<T, Period>;
4141

4242
template <typename X = T, std::enable_if_t<std::is_integral_v<X> && std::is_unsigned_v<X>, int> = 0>
43-
Duration(Type* ptr, std::string&& desc)
43+
Duration(Type* ptr, std::string desc)
4444
: Base<Duration<T, Period>>(JsonType::number_unsigned, std::move(desc)), ptr_(ptr) {}
4545

4646
template <typename X = T, std::enable_if_t<std::is_integral_v<X> && std::is_signed_v<X>, int> = 0>
47-
Duration(Type* ptr, std::string&& desc)
48-
: Base<Duration<T, Period>>(JsonType::number_integer, std::move(desc)), ptr_(ptr) {}
47+
Duration(Type* ptr, std::string desc)
48+
: Base<Duration<T, Period>>(JsonType::number_integer, std::move(desc)), ptr_(ptr) {}
4949

5050
template <typename X = T, std::enable_if_t<std::is_floating_point_v<X>, int> = 0>
51-
Duration(Type* ptr, std::string&& desc)
51+
Duration(Type* ptr, std::string desc)
5252
: Base<Duration<T, Period>>(JsonType::number_float, std::move(desc)), ptr_(ptr) {}
5353

5454
public: // Special

fable/include/fable/schema/enum.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class Enum : public Base<Enum<T>> {
4040
public: // Types and Constructors
4141
using Type = T;
4242

43-
Enum(Type* ptr, std::string&& desc)
43+
Enum(Type* ptr, std::string desc)
4444
: Base<Enum<T>>(JsonType::string, std::move(desc))
4545
, mapping_to_(enum_serialization<T>())
4646
, mapping_from_(enum_deserialization<T>())

fable/include/fable/schema/factory.hpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,14 @@ class FactoryBase : public Base<CRTP> {
7878
*
7979
* \see add_factory()
8080
*/
81-
explicit FactoryBase(std::string&& desc = "") : Base<CRTP>(JsonType::object, std::move(desc)) {}
81+
explicit FactoryBase(std::string desc = "") : Base<CRTP>(JsonType::object, std::move(desc)) {}
8282

83-
FactoryBase(std::string&& desc, FactoryPairList fs)
83+
FactoryBase(std::string desc, FactoryPairList fs)
8484
: Base<CRTP>(JsonType::object, std::move(desc)), available_(std::move(fs)) {
8585
reset_schema();
8686
}
8787

88-
FactoryBase(std::string&& desc, FactoryMap&& fs)
88+
FactoryBase(std::string desc, FactoryMap&& fs)
8989
: Base<CRTP>(JsonType::object, std::move(desc)), available_(std::move(fs)) {
9090
reset_schema();
9191
}
@@ -381,17 +381,17 @@ class Factory : public FactoryBase<T, Factory<T>> {
381381
public: // Constructors
382382
using FactoryBase<T, Factory<T>>::FactoryBase;
383383

384-
Factory(Type* ptr, std::string&& desc) : FactoryBase<T, Factory<T>>(std::move(desc)), ptr_(ptr) {}
384+
Factory(Type* ptr, std::string desc) : FactoryBase<T, Factory<T>>(std::move(desc)), ptr_(ptr) {}
385385

386-
Factory(Type* ptr, std::string&& desc, FactoryMap&& fs)
386+
Factory(Type* ptr, std::string desc, FactoryMap&& fs)
387387
: FactoryBase<T, Factory<T>>(std::move(desc)), ptr_(ptr) {
388388
for (auto&& f : fs) {
389389
this->available_.insert(f);
390390
}
391391
this->reset_schema();
392392
}
393393

394-
Factory(Type* ptr, std::string&& desc, FactoryPairList fs)
394+
Factory(Type* ptr, std::string desc, FactoryPairList fs)
395395
: FactoryBase<T, Factory<T>>(std::move(desc)), ptr_(ptr) {
396396
for (auto&& f : fs) {
397397
this->available_.insert(f);

fable/include/fable/schema/ignore.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ namespace schema {
4444
class Ignore : public Base<Ignore> {
4545
public: // Constructors
4646
Ignore() : Base(JsonType::object, "ignored") {}
47-
explicit Ignore(std::string&& desc, JsonType t = JsonType::object) : Base(t, std::move(desc)) {}
47+
explicit Ignore(std::string desc, JsonType t = JsonType::object) : Base(t, std::move(desc)) {}
4848

4949
public: // Overrides
5050
Json json_schema() const override {

fable/include/fable/schema/interface.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -352,9 +352,9 @@ template <typename CRTP>
352352
class Base : public Interface {
353353
public:
354354
Base() = default;
355-
Base(JsonType t, std::string&& desc) : type_(t), desc_(std::move(desc)) {}
355+
Base(JsonType t, std::string desc) : type_(t), desc_(std::move(desc)) {}
356356
explicit Base(JsonType t) : type_(t) {}
357-
explicit Base(std::string&& desc) : desc_(std::move(desc)) {}
357+
explicit Base(std::string desc) : desc_(std::move(desc)) {}
358358
virtual ~Base() = default;
359359

360360
Interface* clone() const override { return new CRTP(static_cast<CRTP const&>(*this)); }
@@ -391,7 +391,7 @@ class Base : public Interface {
391391
void set_description(const std::string& s) override { desc_ = s; }
392392
void set_description(std::string&& s) { desc_ = std::move(s); }
393393
const std::string& description() const override { return desc_; }
394-
CRTP description(std::string&& desc) && {
394+
CRTP description(std::string desc) && {
395395
desc_ = std::move(desc);
396396
return std::move(*dynamic_cast<CRTP*>(this));
397397
}

fable/include/fable/schema/json.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class FromJson : public Base<FromJson<T>> {
4848
public: // Types and Constructors
4949
using Type = T;
5050

51-
FromJson(Type* ptr, JsonType t, std::string&& desc)
51+
FromJson(Type* ptr, JsonType t, std::string desc)
5252
: Base<FromJson<T>>(t, std::move(desc)), ptr_(ptr) {}
5353

5454
public: // Overrides

fable/include/fable/schema/magic.hpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class Confable;
5454
namespace schema {
5555

5656
template <typename T, typename P>
57-
Array<T, P>::Array(std::vector<T>* ptr, std::string&& desc)
57+
Array<T, P>::Array(std::vector<T>* ptr, std::string desc)
5858
: Array<T, P>(ptr, make_prototype<T>(), std::move(desc)) {}
5959

6060
template <typename T, typename S>
@@ -63,7 +63,7 @@ Array<T, decltype(make_prototype<T>())> make_schema(std::vector<T>* ptr, S&& des
6363
}
6464

6565
template <typename T, typename P>
66-
Const<T, P>::Const(const T& constant, std::string&& desc)
66+
Const<T, P>::Const(const T& constant, std::string desc)
6767
: Const<T, P>(constant, make_prototype<T>(), std::move(desc)) {}
6868

6969
template <typename T, typename S>
@@ -72,7 +72,7 @@ Const<T, decltype(make_prototype<T>())> make_const_schema(const T& constant, S&&
7272
}
7373

7474
template <typename T, typename P>
75-
Map<T, P>::Map(std::map<std::string, T>* ptr, std::string&& desc)
75+
Map<T, P>::Map(std::map<std::string, T>* ptr, std::string desc)
7676
: Map<T, P>(ptr, make_prototype<T>(), std::move(desc)) {}
7777

7878
template <typename T, typename S>
@@ -82,7 +82,7 @@ Map<T, decltype(make_prototype<T>())> make_schema(std::map<std::string, T>* ptr,
8282
}
8383

8484
template <typename T, typename P>
85-
Optional<T, P>::Optional(boost::optional<T>* ptr, std::string&& desc)
85+
Optional<T, P>::Optional(boost::optional<T>* ptr, std::string desc)
8686
: Optional<T, P>(ptr, make_prototype<T>(), std::move(desc)) {}
8787

8888
template <typename T, typename S>

fable/include/fable/schema/map.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,19 @@ class Map : public Base<Map<T, P>> {
5252
using Type = std::map<std::string, T>;
5353
using PrototypeSchema = std::remove_cv_t<std::remove_reference_t<P>>;
5454

55-
Map(Type* ptr, std::string&& desc);
55+
Map(Type* ptr, std::string desc);
5656
Map(Type* ptr, PrototypeSchema prototype)
5757
: Base<Map<T, P>>(JsonType::object), prototype_(std::move(prototype)), ptr_(ptr) {
5858
prototype_.reset_ptr();
5959
}
60-
Map(Type* ptr, PrototypeSchema prototype, std::string&& desc)
60+
Map(Type* ptr, PrototypeSchema prototype, std::string desc)
6161
: Base<Map<T, P>>(JsonType::object, std::move(desc)), prototype_(std::move(prototype)), ptr_(ptr) {
6262
prototype_.reset_ptr();
6363
}
6464

6565
#if 0
6666
// This is defined in: fable/schema/magic.hpp
67-
Map(Type* ptr, std::string&& desc)
67+
Map(Type* ptr, std::string desc)
6868
: Map(ptr, make_prototype<T>(), std::move(desc)) {}
6969
#endif
7070

fable/include/fable/schema/number.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,15 @@ class Number : public Base<Number<T>> {
4141
using Type = T;
4242

4343
template <typename X = T, std::enable_if_t<std::is_integral_v<X> && std::is_unsigned_v<X>, int> = 0>
44-
Number(Type* ptr, std::string&& desc)
44+
Number(Type* ptr, std::string desc)
4545
: Base<Number<T>>(JsonType::number_unsigned, std::move(desc)), ptr_(ptr) {}
4646

4747
template <typename X = T, std::enable_if_t<std::is_integral_v<X> && std::is_signed_v<X>, int> = 0>
48-
Number(Type* ptr, std::string&& desc)
48+
Number(Type* ptr, std::string desc)
4949
: Base<Number<T>>(JsonType::number_integer, std::move(desc)), ptr_(ptr) {}
5050

5151
template <typename X = T, std::enable_if_t<std::is_floating_point_v<X>, int> = 0>
52-
Number(Type* ptr, std::string&& desc)
52+
Number(Type* ptr, std::string desc)
5353
: Base<Number<T>>(JsonType::number_float, std::move(desc)), ptr_(ptr) {}
5454

5555

fable/include/fable/schema/optional.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,15 @@ class Optional : public Base<Optional<T, P>> {
4141
using Type = boost::optional<T>;
4242
using PrototypeSchema = std::remove_cv_t<std::remove_reference_t<P>>;
4343

44-
Optional(Type* ptr, std::string&& desc);
45-
Optional(Type* ptr, PrototypeSchema prototype, std::string&& desc)
44+
Optional(Type* ptr, std::string desc);
45+
Optional(Type* ptr, PrototypeSchema prototype, std::string desc)
4646
: Base<Optional<T, P>>(prototype.type(), std::move(desc)), prototype_(std::move(prototype)), ptr_(ptr) {
4747
prototype_.reset_ptr();
4848
}
4949

5050
#if 0
5151
// This is defined in: fable/schema/magic.hpp
52-
Optional(Type* ptr, std::string&& desc)
52+
Optional(Type* ptr, std::string desc)
5353
: Optional(ptr, make_prototype<T>(), std::move(desc)) {}
5454
#endif
5555

fable/include/fable/schema/passthru.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ class Passthru : public Base<Passthru<P>> {
4848
using Type = Conf;
4949
using PrototypeSchema = std::remove_cv_t<std::remove_reference_t<P>>;
5050

51-
Passthru(Type* ptr, std::string&& desc)
51+
Passthru(Type* ptr, std::string desc)
5252
: Passthru(ptr, PrototypeSchema(nullptr, ""), std::move(desc)) {}
53-
Passthru(Type* ptr, PrototypeSchema prototype, std::string&& desc)
53+
Passthru(Type* ptr, PrototypeSchema prototype, std::string desc)
5454
: Base<Passthru<P>>(prototype.type(), std::move(desc)), prototype_(std::move(prototype)), ptr_(ptr) {
5555
prototype_.reset_ptr();
5656
}

fable/include/fable/schema/path.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class Path : public Base<Path> {
7171
NotDir, /// path does not exist or is a file
7272
};
7373

74-
Path(Type* ptr, std::string&& desc) : Base(JsonType::string, std::move(desc)), ptr_(ptr) {}
74+
Path(Type* ptr, std::string desc) : Base(JsonType::string, std::move(desc)), ptr_(ptr) {}
7575

7676
public: // Special
7777
/**

fable/include/fable/schema/string.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class String : public Base<String> {
6565
public: // Types and Constructors
6666
using Type = std::string;
6767

68-
String(Type* ptr, std::string&& desc) : Base(JsonType::string, std::move(desc)), ptr_(ptr) {}
68+
String(Type* ptr, std::string desc) : Base(JsonType::string, std::move(desc)), ptr_(ptr) {}
6969

7070
public: // Special
7171
/**

fable/include/fable/schema/struct.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ using enable_if_property_list_t = std::enable_if_t<std::is_same_v<PropertyList<S
7070
*/
7171
class Struct : public Base<Struct> {
7272
public: // Constructors
73-
explicit Struct(std::string&& desc = "") : Base(JsonType::object, std::move(desc)) {}
73+
explicit Struct(std::string desc = "") : Base(JsonType::object, std::move(desc)) {}
7474

75-
Struct(std::string&& desc, PropertyList<Box> props) : Base(JsonType::object, std::move(desc)) {
75+
Struct(std::string desc, PropertyList<Box> props) : Base(JsonType::object, std::move(desc)) {
7676
set_properties(props);
7777
}
7878

@@ -103,7 +103,7 @@ class Struct : public Base<Struct> {
103103
* will internally call this->schema_impl(), which will lead to an
104104
* infinite recursion! Instead, call Base::schema_impl().
105105
*/
106-
Struct(std::string&& desc, const Box& base, PropertyList<Box> props)
106+
Struct(std::string desc, const Box& base, PropertyList<Box> props)
107107
: Struct(*base.template as<Struct>()) {
108108
desc_ = std::move(desc);
109109
set_properties(props);

0 commit comments

Comments
 (0)