Skip to content

Commit

Permalink
fable: Use std::string instead of std::string&& for constructors
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cassava committed Mar 27, 2024
1 parent 1b659be commit 2084ca0
Show file tree
Hide file tree
Showing 24 changed files with 64 additions and 66 deletions.
20 changes: 9 additions & 11 deletions fable/include/fable/schema.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,6 @@

#pragma once

#include <chrono> // for duration<>
#include <map> // for map<>
#include <memory> // for shared_ptr<>
#include <string> // for string
#include <type_traits> // for enable_if_t<>, is_arithmetic<>, is_enum<>, ...
Expand Down Expand Up @@ -179,25 +177,25 @@ class Schema : public schema::Interface {
Schema& operator=(const Schema&) = default;

// Struct
Schema(std::string&& desc, schema::PropertyList<> props)
Schema(std::string desc, schema::PropertyList<> props)
: impl_(new schema::Struct(std::move(desc), props)) {}

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

Schema(std::string&& desc, const Schema& base, schema::PropertyList<> props)
Schema(std::string desc, const Schema& base, schema::PropertyList<> props)
: impl_(new schema::Struct(std::move(desc), base, props)) {}

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

// Variant
Schema(const std::vector<Schema>& xs); // NOLINT(runtime/explicit)
Schema(std::string&& desc, const std::vector<Schema>& xs);
Schema(std::string desc, const std::vector<Schema>& xs);

Schema(schema::BoxList props); // NOLINT(runtime/explicit)
Schema(std::string&& desc, schema::BoxList props);
Schema(std::string desc, schema::BoxList props);

Schema(schema::BoxVec&& props); // NOLINT(runtime/explicit)
Schema(std::string&& desc, schema::BoxVec&& props);
Schema(std::string desc, schema::BoxVec&& props);

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

// Ignore
Schema() : impl_(new schema::Ignore("")) {}
explicit Schema(std::string&& desc, JsonType t = JsonType::object)
explicit Schema(std::string desc, JsonType t = JsonType::object)
: impl_(new schema::Ignore(std::move(desc), t)) {}

// Primitives
template <typename T>
Schema(T* ptr, std::string&& desc) : impl_(make_schema(ptr, std::move(desc)).clone()) {}
Schema(T* ptr, std::string desc) : impl_(make_schema(ptr, std::move(desc)).clone()) {}
template <typename T>
Schema(T* ptr, const schema::Box& prototype, std::string&& desc)
Schema(T* ptr, const schema::Box& prototype, std::string desc)
: impl_(make_schema(ptr, prototype, std::move(desc)).clone()) {}

// FromJson
template <typename T>
Schema(T* ptr, JsonType t, std::string&& desc)
Schema(T* ptr, JsonType t, std::string desc)
: impl_(new schema::FromJson<T>(ptr, t, std::move(desc))) {}

public: // Special
Expand Down
6 changes: 3 additions & 3 deletions fable/include/fable/schema/array.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ class Array : public Base<Array<T, P>> {
using Type = std::vector<T>;
using PrototypeSchema = std::remove_cv_t<std::remove_reference_t<P>>;

Array(Type* ptr, std::string&& desc);
Array(Type* ptr, std::string desc);
Array(Type* ptr, PrototypeSchema prototype)
: Base<Array<T, P>>(JsonType::array), prototype_(std::move(prototype)), ptr_(ptr) {}
Array(Type* ptr, PrototypeSchema prototype, std::string&& desc)
Array(Type* ptr, PrototypeSchema prototype, std::string desc)
: Base<Array<T, P>>(JsonType::array, std::move(desc)), prototype_(std::move(prototype)), ptr_(ptr) {}

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

Expand Down
2 changes: 1 addition & 1 deletion fable/include/fable/schema/boolean.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class Boolean : public Base<Boolean> {
public: // Types and Constructors
using Type = bool;

Boolean(Type* ptr, std::string&& desc);
Boolean(Type* ptr, std::string desc);

public: // Overrides
Json json_schema() const override;
Expand Down
6 changes: 3 additions & 3 deletions fable/include/fable/schema/confable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ class FromConfable : public Base<FromConfable<T>> {
public: // Types and Constructors
using Type = T;

explicit FromConfable(std::string&& desc = "") {
schema_ = Type().schema();
FromConfable(std::string desc = "") {
schema_ = Type().schema(); // NOLINT
schema_.reset_ptr();
this->type_ = schema_.type();
this->desc_ = std::move(desc);
}

FromConfable(Type* ptr, std::string&& desc)
FromConfable(Type* ptr, std::string desc)
: Base<FromConfable<Type>>(ptr->schema().type(), std::move(desc))
, schema_(ptr->schema())
, ptr_(ptr) {
Expand Down
6 changes: 3 additions & 3 deletions fable/include/fable/schema/const.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ class Const : public Base<Const<T, P>> {
using Type = T;
using PrototypeSchema = std::remove_cv_t<std::remove_reference_t<P>>;

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

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

Expand Down
8 changes: 4 additions & 4 deletions fable/include/fable/schema/duration.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ class Duration : public Base<Duration<T, Period>> {
using Type = std::chrono::duration<T, Period>;

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

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

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

public: // Special
Expand Down
2 changes: 1 addition & 1 deletion fable/include/fable/schema/enum.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class Enum : public Base<Enum<T>> {
public: // Types and Constructors
using Type = T;

Enum(Type* ptr, std::string&& desc)
Enum(Type* ptr, std::string desc)
: Base<Enum<T>>(JsonType::string, std::move(desc))
, mapping_to_(enum_serialization<T>())
, mapping_from_(enum_deserialization<T>())
Expand Down
12 changes: 6 additions & 6 deletions fable/include/fable/schema/factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@ class FactoryBase : public Base<CRTP> {
*
* \see add_factory()
*/
explicit FactoryBase(std::string&& desc = "") : Base<CRTP>(JsonType::object, std::move(desc)) {}
explicit FactoryBase(std::string desc = "") : Base<CRTP>(JsonType::object, std::move(desc)) {}

FactoryBase(std::string&& desc, FactoryPairList fs)
FactoryBase(std::string desc, FactoryPairList fs)
: Base<CRTP>(JsonType::object, std::move(desc)), available_(std::move(fs)) {
reset_schema();
}

FactoryBase(std::string&& desc, FactoryMap&& fs)
FactoryBase(std::string desc, FactoryMap&& fs)
: Base<CRTP>(JsonType::object, std::move(desc)), available_(std::move(fs)) {
reset_schema();
}
Expand Down Expand Up @@ -381,17 +381,17 @@ class Factory : public FactoryBase<T, Factory<T>> {
public: // Constructors
using FactoryBase<T, Factory<T>>::FactoryBase;

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

Factory(Type* ptr, std::string&& desc, FactoryMap&& fs)
Factory(Type* ptr, std::string desc, FactoryMap&& fs)
: FactoryBase<T, Factory<T>>(std::move(desc)), ptr_(ptr) {
for (auto&& f : fs) {
this->available_.insert(f);
}
this->reset_schema();
}

Factory(Type* ptr, std::string&& desc, FactoryPairList fs)
Factory(Type* ptr, std::string desc, FactoryPairList fs)
: FactoryBase<T, Factory<T>>(std::move(desc)), ptr_(ptr) {
for (auto&& f : fs) {
this->available_.insert(f);
Expand Down
2 changes: 1 addition & 1 deletion fable/include/fable/schema/ignore.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace schema {
class Ignore : public Base<Ignore> {
public: // Constructors
Ignore() : Base(JsonType::object, "ignored") {}
explicit Ignore(std::string&& desc, JsonType t = JsonType::object) : Base(t, std::move(desc)) {}
explicit Ignore(std::string desc, JsonType t = JsonType::object) : Base(t, std::move(desc)) {}

public: // Overrides
Json json_schema() const override {
Expand Down
6 changes: 3 additions & 3 deletions fable/include/fable/schema/interface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,9 @@ template <typename CRTP>
class Base : public Interface {
public:
Base() = default;
Base(JsonType t, std::string&& desc) : type_(t), desc_(std::move(desc)) {}
Base(JsonType t, std::string desc) : type_(t), desc_(std::move(desc)) {}
explicit Base(JsonType t) : type_(t) {}
explicit Base(std::string&& desc) : desc_(std::move(desc)) {}
explicit Base(std::string desc) : desc_(std::move(desc)) {}
virtual ~Base() = default;

Interface* clone() const override { return new CRTP(static_cast<CRTP const&>(*this)); }
Expand Down Expand Up @@ -391,7 +391,7 @@ class Base : public Interface {
void set_description(const std::string& s) override { desc_ = s; }
void set_description(std::string&& s) { desc_ = std::move(s); }
const std::string& description() const override { return desc_; }
CRTP description(std::string&& desc) && {
CRTP description(std::string desc) && {
desc_ = std::move(desc);
return std::move(*dynamic_cast<CRTP*>(this));
}
Expand Down
2 changes: 1 addition & 1 deletion fable/include/fable/schema/json.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class FromJson : public Base<FromJson<T>> {
public: // Types and Constructors
using Type = T;

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

public: // Overrides
Expand Down
8 changes: 4 additions & 4 deletions fable/include/fable/schema/magic.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class Confable;
namespace schema {

template <typename T, typename P>
Array<T, P>::Array(std::vector<T>* ptr, std::string&& desc)
Array<T, P>::Array(std::vector<T>* ptr, std::string desc)
: Array<T, P>(ptr, make_prototype<T>(), std::move(desc)) {}

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

template <typename T, typename P>
Const<T, P>::Const(const T& constant, std::string&& desc)
Const<T, P>::Const(const T& constant, std::string desc)
: Const<T, P>(constant, make_prototype<T>(), std::move(desc)) {}

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

template <typename T, typename P>
Map<T, P>::Map(std::map<std::string, T>* ptr, std::string&& desc)
Map<T, P>::Map(std::map<std::string, T>* ptr, std::string desc)
: Map<T, P>(ptr, make_prototype<T>(), std::move(desc)) {}

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

template <typename T, typename P>
Optional<T, P>::Optional(boost::optional<T>* ptr, std::string&& desc)
Optional<T, P>::Optional(boost::optional<T>* ptr, std::string desc)
: Optional<T, P>(ptr, make_prototype<T>(), std::move(desc)) {}

template <typename T, typename S>
Expand Down
6 changes: 3 additions & 3 deletions fable/include/fable/schema/map.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,19 @@ class Map : public Base<Map<T, P>> {
using Type = std::map<std::string, T>;
using PrototypeSchema = std::remove_cv_t<std::remove_reference_t<P>>;

Map(Type* ptr, std::string&& desc);
Map(Type* ptr, std::string desc);
Map(Type* ptr, PrototypeSchema prototype)
: Base<Map<T, P>>(JsonType::object), prototype_(std::move(prototype)), ptr_(ptr) {
prototype_.reset_ptr();
}
Map(Type* ptr, PrototypeSchema prototype, std::string&& desc)
Map(Type* ptr, PrototypeSchema prototype, std::string desc)
: Base<Map<T, P>>(JsonType::object, std::move(desc)), prototype_(std::move(prototype)), ptr_(ptr) {
prototype_.reset_ptr();
}

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

Expand Down
6 changes: 3 additions & 3 deletions fable/include/fable/schema/number.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ class Number : public Base<Number<T>> {
using Type = T;

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

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

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


Expand Down
6 changes: 3 additions & 3 deletions fable/include/fable/schema/optional.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ class Optional : public Base<Optional<T, P>> {
using Type = boost::optional<T>;
using PrototypeSchema = std::remove_cv_t<std::remove_reference_t<P>>;

Optional(Type* ptr, std::string&& desc);
Optional(Type* ptr, PrototypeSchema prototype, std::string&& desc)
Optional(Type* ptr, std::string desc);
Optional(Type* ptr, PrototypeSchema prototype, std::string desc)
: Base<Optional<T, P>>(prototype.type(), std::move(desc)), prototype_(std::move(prototype)), ptr_(ptr) {
prototype_.reset_ptr();
}

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

Expand Down
4 changes: 2 additions & 2 deletions fable/include/fable/schema/passthru.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ class Passthru : public Base<Passthru<P>> {
using Type = Conf;
using PrototypeSchema = std::remove_cv_t<std::remove_reference_t<P>>;

Passthru(Type* ptr, std::string&& desc)
Passthru(Type* ptr, std::string desc)
: Passthru(ptr, PrototypeSchema(nullptr, ""), std::move(desc)) {}
Passthru(Type* ptr, PrototypeSchema prototype, std::string&& desc)
Passthru(Type* ptr, PrototypeSchema prototype, std::string desc)
: Base<Passthru<P>>(prototype.type(), std::move(desc)), prototype_(std::move(prototype)), ptr_(ptr) {
prototype_.reset_ptr();
}
Expand Down
2 changes: 1 addition & 1 deletion fable/include/fable/schema/path.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class Path : public Base<Path> {
NotDir, /// path does not exist or is a file
};

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

public: // Special
/**
Expand Down
2 changes: 1 addition & 1 deletion fable/include/fable/schema/string.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class String : public Base<String> {
public: // Types and Constructors
using Type = std::string;

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

public: // Special
/**
Expand Down
6 changes: 3 additions & 3 deletions fable/include/fable/schema/struct.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ using enable_if_property_list_t = std::enable_if_t<std::is_same_v<PropertyList<S
*/
class Struct : public Base<Struct> {
public: // Constructors
explicit Struct(std::string&& desc = "") : Base(JsonType::object, std::move(desc)) {}
explicit Struct(std::string desc = "") : Base(JsonType::object, std::move(desc)) {}

Struct(std::string&& desc, PropertyList<Box> props) : Base(JsonType::object, std::move(desc)) {
Struct(std::string desc, PropertyList<Box> props) : Base(JsonType::object, std::move(desc)) {
set_properties(props);
}

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

0 comments on commit 2084ca0

Please sign in to comment.