Skip to content

Commit

Permalink
Apply formatting/style tweaks to comply with compile time diagnostics…
Browse files Browse the repository at this point in the history
… for g++ and clang++ (jbeder#686)

* Add compilation flags: -Wshadow -Weffc++ -pedantic -pedantic-errors
* Delete implicit copy & move constructors & assignment operators
  in classes with pointer data members.
* An exception to the above: Add default copy & move constructors &
  assignment operators for the Binary class.
* Convert boolean RegEx operators to binary operators.
* Initialize all members in all classes in ctors.
* Let default ctor delegate to the converting ctor in
  Binary and RegEx
* Don't change any tests except regex_test (as a result of the change
  to binary operators).

Note: https://bugzilla.redhat.com/show_bug.cgi?id=1544675 makes
-Weffc++ report a false positive in "include/yaml-cpp/node/impl.h".
  • Loading branch information
TedLyngmo authored and jbeder committed Mar 13, 2019
1 parent eca9cfd commit 0d5c571
Show file tree
Hide file tree
Showing 40 changed files with 285 additions and 241 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR
set(GCC_EXTRA_OPTIONS "${GCC_EXTRA_OPTIONS} -fPIC")
endif()
#
set(FLAG_TESTED "-Wextra")
set(FLAG_TESTED "-Wextra -Wshadow -Weffc++ -pedantic -pedantic-errors")
check_cxx_compiler_flag(${FLAG_TESTED} FLAG_WEXTRA)
if(FLAG_WEXTRA)
set(GCC_EXTRA_OPTIONS "${GCC_EXTRA_OPTIONS} ${FLAG_TESTED}")
Expand Down
10 changes: 7 additions & 3 deletions include/yaml-cpp/binary.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,13 @@ YAML_CPP_API std::vector<unsigned char> DecodeBase64(const std::string &input);

class YAML_CPP_API Binary {
public:
Binary() : m_unownedData(0), m_unownedSize(0) {}
Binary(const unsigned char *data_, std::size_t size_)
: m_unownedData(data_), m_unownedSize(size_) {}
: m_data{}, m_unownedData(data_), m_unownedSize(size_) {}
Binary() : Binary(nullptr, 0) {}
Binary(const Binary &) = default;
Binary(Binary &&) = default;
Binary &operator=(const Binary &) = default;
Binary &operator=(Binary &&) = default;

bool owned() const { return !m_unownedData; }
std::size_t size() const { return owned() ? m_data.size() : m_unownedSize; }
Expand Down Expand Up @@ -62,6 +66,6 @@ class YAML_CPP_API Binary {
const unsigned char *m_unownedData;
std::size_t m_unownedSize;
};
}
} // namespace YAML

#endif // BASE64_H_62B23520_7C8E_11DE_8A39_0800200C9A66
3 changes: 2 additions & 1 deletion include/yaml-cpp/contrib/anchordict.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ namespace YAML {
template <class T>
class AnchorDict {
public:
AnchorDict() : m_data{} {}
void Register(anchor_t anchor, T value) {
if (anchor > m_data.size()) {
m_data.resize(anchor);
Expand All @@ -34,6 +35,6 @@ class AnchorDict {
private:
std::vector<T> m_data;
};
}
} // namespace YAML

#endif // ANCHORDICT_H_62B23520_7C8E_11DE_8A39_0800200C9A66
7 changes: 4 additions & 3 deletions include/yaml-cpp/emitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "yaml-cpp/dll.h"
#include "yaml-cpp/emitterdef.h"
#include "yaml-cpp/emittermanip.h"
#include "yaml-cpp/noncopyable.h"
#include "yaml-cpp/null.h"
#include "yaml-cpp/ostream_wrapper.h"

Expand All @@ -28,10 +27,12 @@ struct _Null;
namespace YAML {
class EmitterState;

class YAML_CPP_API Emitter : private noncopyable {
class YAML_CPP_API Emitter {
public:
Emitter();
explicit Emitter(std::ostream& stream);
Emitter(const Emitter&) = delete;
Emitter& operator=(const Emitter&) = delete;
~Emitter();

// output
Expand Down Expand Up @@ -249,6 +250,6 @@ inline Emitter& operator<<(Emitter& emitter, _Indent indent) {
inline Emitter& operator<<(Emitter& emitter, _Precision precision) {
return emitter.SetLocalPrecision(precision);
}
}
} // namespace YAML

#endif // EMITTER_H_62B23520_7C8E_11DE_8A39_0800200C9A66
5 changes: 3 additions & 2 deletions include/yaml-cpp/node/detail/memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ namespace YAML {
namespace detail {
class YAML_CPP_API memory {
public:
memory() : m_nodes{} {}
node& create_node();
void merge(const memory& rhs);

Expand All @@ -40,7 +41,7 @@ class YAML_CPP_API memory_holder {
private:
shared_memory m_pMemory;
};
}
}
} // namespace detail
} // namespace YAML

#endif // VALUE_DETAIL_MEMORY_H_62B23520_7C8E_11DE_8A39_0800200C9A66
12 changes: 6 additions & 6 deletions include/yaml-cpp/node/detail/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@
#pragma once
#endif

#include "yaml-cpp/emitterstyle.h"
#include "yaml-cpp/dll.h"
#include "yaml-cpp/node/type.h"
#include "yaml-cpp/node/ptr.h"
#include "yaml-cpp/emitterstyle.h"
#include "yaml-cpp/node/detail/node_ref.h"
#include "yaml-cpp/node/ptr.h"
#include "yaml-cpp/node/type.h"
#include <set>

namespace YAML {
namespace detail {
class node {
public:
node() : m_pRef(new node_ref) {}
node() : m_pRef(new node_ref), m_dependencies{} {}
node(const node&) = delete;
node& operator=(const node&) = delete;

Expand Down Expand Up @@ -163,7 +163,7 @@ class node {
typedef std::set<node*> nodes;
nodes m_dependencies;
};
}
}
} // namespace detail
} // namespace YAML

#endif // NODE_DETAIL_NODE_H_62B23520_7C8E_11DE_8A39_0800200C9A66
36 changes: 18 additions & 18 deletions include/yaml-cpp/node/impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
#pragma once
#endif

#include "yaml-cpp/node/node.h"
#include "yaml-cpp/node/iterator.h"
#include "yaml-cpp/exceptions.h"
#include "yaml-cpp/node/detail/memory.h"
#include "yaml-cpp/node/detail/node.h"
#include "yaml-cpp/exceptions.h"
#include "yaml-cpp/node/iterator.h"
#include "yaml-cpp/node/node.h"
#include <string>

namespace YAML {
inline Node::Node() : m_isValid(true), m_pNode(NULL) {}
inline Node::Node() : m_isValid(true), m_pMemory(nullptr), m_pNode(nullptr) {}

inline Node::Node(NodeType::value type)
: m_isValid(true),
Expand All @@ -42,7 +42,7 @@ inline Node::Node(const Node& rhs)
m_pMemory(rhs.m_pMemory),
m_pNode(rhs.m_pNode) {}

inline Node::Node(Zombie) : m_isValid(false), m_pNode(NULL) {}
inline Node::Node(Zombie) : m_isValid(false), m_pMemory{}, m_pNode(nullptr) {}

inline Node::Node(detail::node& node, detail::shared_memory_holder pMemory)
: m_isValid(true), m_pMemory(pMemory), m_pNode(&node) {}
Expand Down Expand Up @@ -202,6 +202,15 @@ inline Node& Node::operator=(const T& rhs) {
return *this;
}

inline Node& Node::operator=(const Node& rhs) {
if (!m_isValid || !rhs.m_isValid)
throw InvalidNode();
if (is(rhs))
return *this;
AssignNode(rhs);
return *this;
}

inline void Node::reset(const YAML::Node& rhs) {
if (!m_isValid || !rhs.m_isValid)
throw InvalidNode();
Expand Down Expand Up @@ -238,15 +247,6 @@ inline void Node::Assign(char* rhs) {
m_pNode->set_scalar(rhs);
}

inline Node& Node::operator=(const Node& rhs) {
if (!m_isValid || !rhs.m_isValid)
throw InvalidNode();
if (is(rhs))
return *this;
AssignNode(rhs);
return *this;
}

inline void Node::AssignData(const Node& rhs) {
if (!m_isValid || !rhs.m_isValid)
throw InvalidNode();
Expand Down Expand Up @@ -366,16 +366,16 @@ template <typename T>
inline typename to_value_t<T>::return_type to_value(const T& t) {
return to_value_t<T>(t)();
}
}
} // namespace detail

// indexing
template <typename Key>
inline const Node Node::operator[](const Key& key) const {
if (!m_isValid)
throw InvalidNode();
EnsureNodeExists();
detail::node* value = static_cast<const detail::node&>(*m_pNode)
.get(detail::to_value(key), m_pMemory);
detail::node* value = static_cast<const detail::node&>(*m_pNode).get(
detail::to_value(key), m_pMemory);
if (!value) {
return Node(ZombieNode);
}
Expand Down Expand Up @@ -443,6 +443,6 @@ inline void Node::force_insert(const Key& key, const Value& value) {

// free functions
inline bool operator==(const Node& lhs, const Node& rhs) { return lhs.is(rhs); }
}
} // namespace YAML

#endif // NODE_IMPL_H_62B23520_7C8E_11DE_8A39_0800200C9A66
25 changes: 0 additions & 25 deletions include/yaml-cpp/noncopyable.h

This file was deleted.

8 changes: 6 additions & 2 deletions include/yaml-cpp/ostream_wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ class YAML_CPP_API ostream_wrapper {
public:
ostream_wrapper();
explicit ostream_wrapper(std::ostream& stream);
ostream_wrapper(const ostream_wrapper&) = delete;
ostream_wrapper(ostream_wrapper&&) = delete;
ostream_wrapper& operator=(const ostream_wrapper&) = delete;
ostream_wrapper& operator=(ostream_wrapper&&) = delete;
~ostream_wrapper();

void write(const std::string& str);
Expand Down Expand Up @@ -52,7 +56,7 @@ class YAML_CPP_API ostream_wrapper {

template <std::size_t N>
inline ostream_wrapper& operator<<(ostream_wrapper& stream,
const char(&str)[N]) {
const char (&str)[N]) {
stream.write(str, N - 1);
return stream;
}
Expand All @@ -67,6 +71,6 @@ inline ostream_wrapper& operator<<(ostream_wrapper& stream, char ch) {
stream.write(&ch, 1);
return stream;
}
}
} // namespace YAML

#endif // OSTREAM_WRAPPER_H_62B23520_7C8E_11DE_8A39_0800200C9A66
11 changes: 8 additions & 3 deletions include/yaml-cpp/parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include <memory>

#include "yaml-cpp/dll.h"
#include "yaml-cpp/noncopyable.h"

namespace YAML {
class EventHandler;
Expand All @@ -24,11 +23,17 @@ struct Token;
* A parser turns a stream of bytes into one stream of "events" per YAML
* document in the input stream.
*/
class YAML_CPP_API Parser : private noncopyable {
class YAML_CPP_API Parser {
public:
/** Constructs an empty parser (with no input. */
Parser();

/** non copyable but movable */
Parser(const Parser&) = delete;
Parser(Parser&&) = default;
Parser& operator=(const Parser&) = delete;
Parser& operator=(Parser&&) = default;

/**
* Constructs a parser from the given input stream. The input stream must
* live as long as the parser.
Expand Down Expand Up @@ -81,6 +86,6 @@ class YAML_CPP_API Parser : private noncopyable {
std::unique_ptr<Scanner> m_pScanner;
std::unique_ptr<Directives> m_pDirectives;
};
}
} // namespace YAML

#endif // PARSER_H_62B23520_7C8E_11DE_8A39_0800200C9A66
5 changes: 3 additions & 2 deletions src/collectionstack.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
#pragma once
#endif

#include <stack>
#include <cassert>
#include <stack>

namespace YAML {
struct CollectionType {
Expand All @@ -17,6 +17,7 @@ struct CollectionType {

class CollectionStack {
public:
CollectionStack() : collectionStack{} {}
CollectionType::value GetCurCollectionType() const {
if (collectionStack.empty())
return CollectionType::NoCollection;
Expand All @@ -35,6 +36,6 @@ class CollectionStack {
private:
std::stack<CollectionType::value> collectionStack;
};
}
} // namespace YAML

#endif // COLLECTIONSTACK_H_62B23520_7C8E_11DE_8A39_0800200C9A66
16 changes: 12 additions & 4 deletions src/contrib/graphbuilderadapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,15 @@ namespace YAML {
class GraphBuilderAdapter : public EventHandler {
public:
GraphBuilderAdapter(GraphBuilderInterface& builder)
: m_builder(builder), m_pRootNode(nullptr), m_pKeyNode(nullptr) {}
: m_builder(builder),
m_containers{},
m_anchors{},
m_pRootNode(nullptr),
m_pKeyNode(nullptr) {}
GraphBuilderAdapter(const GraphBuilderAdapter&) = delete;
GraphBuilderAdapter(GraphBuilderAdapter&&) = delete;
GraphBuilderAdapter& operator=(const GraphBuilderAdapter&) = delete;
GraphBuilderAdapter& operator=(GraphBuilderAdapter&&) = delete;

virtual void OnDocumentStart(const Mark& mark) { (void)mark; }
virtual void OnDocumentEnd() {}
Expand All @@ -50,8 +58,8 @@ class GraphBuilderAdapter : public EventHandler {
struct ContainerFrame {
ContainerFrame(void* pSequence)
: pContainer(pSequence), pPrevKeyNode(&sequenceMarker) {}
ContainerFrame(void* pMap, void* pPrevKeyNode)
: pContainer(pMap), pPrevKeyNode(pPrevKeyNode) {}
ContainerFrame(void* pMap, void* pPreviousKeyNode)
: pContainer(pMap), pPrevKeyNode(pPreviousKeyNode) {}

void* pContainer;
void* pPrevKeyNode;
Expand All @@ -74,6 +82,6 @@ class GraphBuilderAdapter : public EventHandler {
void RegisterAnchor(anchor_t anchor, void* pNode);
void DispositionNode(void* pNode);
};
}
} // namespace YAML

#endif // GRAPHBUILDERADAPTER_H_62B23520_7C8E_11DE_8A39_0800200C9A66
9 changes: 2 additions & 7 deletions src/directives.cpp
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
#include "directives.h"

namespace YAML {
Directives::Directives() {
// version
version.isDefault = true;
version.major = 1;
version.minor = 2;
}
Directives::Directives() : version{true, 1, 2}, tags{} {}

const std::string Directives::TranslateTagHandle(
const std::string& handle) const {
Expand All @@ -19,4 +14,4 @@ const std::string Directives::TranslateTagHandle(

return it->second;
}
}
} // namespace YAML
Loading

0 comments on commit 0d5c571

Please sign in to comment.