Skip to content

Commit

Permalink
clean-up some more compiler-warnings & enable -Werror
Browse files Browse the repository at this point in the history
Signed-off-by: rstein <[email protected]>
Signed-off-by: Ralph J. Steinhagen <[email protected]>
  • Loading branch information
RalphSteinhagen committed Jun 27, 2023
1 parent 8be6734 commit 5418a55
Show file tree
Hide file tree
Showing 10 changed files with 158 additions and 143 deletions.
20 changes: 11 additions & 9 deletions cmake/CompilerWarnings.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ function(set_project_warnings project_name)
)

set(CLANG_WARNINGS
-Werror # avoid warnings since they are often indicative of immature API and/or potential sources of bugs
-Wall
-Wextra # reasonable and standard
-Wshadow # warn the user if a variable declaration shadows one from a parent context
Expand All @@ -57,15 +58,16 @@ function(set_project_warnings project_name)
endif()

set(GCC_WARNINGS
${CLANG_WARNINGS}
-Wmisleading-indentation # warn if indentation implies blocks where blocks do not exist
-Wduplicated-cond # warn if if / else chain has duplicated conditions
-Wduplicated-branches # warn if if / else branches have duplicated code
-Wlogical-op # warn about logical operations being used where bitwise were probably wanted
-Wuseless-cast # warn if you perform a cast to the same type
-Wno-interference-size # suppress ABI compatibility warnings for hardware inferred size
-Wno-maybe-uninitialized # false positives if asan is enabled: https://gcc.gnu.org/bugzilla//show_bug.cgi?id=1056h6
)
${CLANG_WARNINGS}
-Wno-dangling-reference # TODO: remove this once the fmt dangling reference bug is fixed
-Wmisleading-indentation # warn if indentation implies blocks where blocks do not exist
-Wduplicated-cond # warn if if / else chain has duplicated conditions
-Wduplicated-branches # warn if if / else branches have duplicated code
-Wlogical-op # warn about logical operations being used where bitwise were probably wanted
-Wuseless-cast # warn if you perform a cast to the same type
-Wno-interference-size # suppress ABI compatibility warnings for hardware inferred size
-Wno-maybe-uninitialized # false positives if asan is enabled: https://gcc.gnu.org/bugzilla//show_bug.cgi?id=1056h6
)

if(MSVC)
set(PROJECT_WARNINGS ${MSVC_WARNINGS})
Expand Down
16 changes: 10 additions & 6 deletions include/claim_strategy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,18 @@ struct MultiThreadedStrategySizeMembers
};

template <>
struct MultiThreadedStrategySizeMembers<std::dynamic_extent>
{
explicit MultiThreadedStrategySizeMembers(std::size_t size)
: _size(static_cast<std::int32_t>(size)), _indexShift(static_cast<std::int32_t>(std::bit_width(size)))
{}

struct MultiThreadedStrategySizeMembers<std::dynamic_extent> {
const std::int32_t _size;
const std::int32_t _indexShift;

#ifndef __clang__
explicit MultiThreadedStrategySizeMembers(std::size_t size) : _size(static_cast<std::int32_t>(size)), _indexShift(static_cast<std::int32_t>(std::bit_width(size))) {} //NOSONAR
#else
#pragma GCC diagnostic push // std::bit_width seems to be compiler and platform specific
#pragma GCC diagnostic ignored "-Wuseless-cast"
explicit MultiThreadedStrategySizeMembers(std::size_t size) : _size(static_cast<std::int32_t>(size)), _indexShift(static_cast<std::int32_t>(std::bit_width(size))) {} //NOSONAR
#pragma GCC diagnostic pop
#endif
};

/**
Expand Down
21 changes: 12 additions & 9 deletions include/graph_yaml_importer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@

#include <charconv>

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wold-style-cast"
#include <yaml-cpp/yaml.h>
#pragma GCC diagnostic pop

#include <graph.hpp>
#include <plugin_loader.hpp>
Expand All @@ -12,7 +15,9 @@ namespace fair::graph {

namespace detail {
struct YamlMap {
YamlMap(YAML::Emitter &out) : out(out) { out << YAML::BeginMap; }
YAML::Emitter &out;

YamlMap(YAML::Emitter &out_) : out(out_) { out << YAML::BeginMap; }

~YamlMap() { out << YAML::EndMap; }

Expand All @@ -30,23 +35,21 @@ struct YamlMap {
out << YAML::Value;
fun();
}

YAML::Emitter &out;
};

struct YamlSeq {
YamlSeq(YAML::Emitter &out) : out(out) { out << YAML::BeginSeq; }
YAML::Emitter &out;

YamlSeq(YAML::Emitter &out_) : out(out_) { out << YAML::BeginSeq; }

~YamlSeq() { out << YAML::EndSeq; }

template<typename F>
requires std::is_invocable_v<F>
void
write_fn(const char *key, F &&fun) {
write_fn(const char */*key*/, F &&fun) {
fun();
}

YAML::Emitter &out;
};
} // namespace detail

Expand Down Expand Up @@ -179,8 +182,8 @@ save_grc(const fair::graph::graph &flow_graph) {
if (!node.meta_information().empty() || !settings_map.empty()) {
map.write_fn("parameters", [&]() {
detail::YamlMap parameters(out);
auto write_map = [&](const auto &map) {
for (const auto &settings_pair : map) {
auto write_map = [&](const auto &local_map) {
for (const auto &settings_pair : local_map) {
std::visit(
[&]<typename T>(const T &value) {
if constexpr (std::is_same_v<std::string, std::remove_cvref_t<T>>) {
Expand Down
10 changes: 5 additions & 5 deletions include/port.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,12 +267,12 @@ class port {
void
setBuffer(gr::Buffer auto streamBuffer, gr::Buffer auto tagBuffer) noexcept {
if constexpr (IS_INPUT) {
_ioHandler = std::move(streamBuffer.new_reader());
_tagIoHandler = std::move(tagBuffer.new_reader());
_connected = true;
_ioHandler = streamBuffer.new_reader();
_tagIoHandler = tagBuffer.new_reader();
_connected = true;
} else {
_ioHandler = std::move(streamBuffer.new_writer());
_tagIoHandler = std::move(tagBuffer.new_reader());
_ioHandler = streamBuffer.new_writer();
_tagIoHandler = tagBuffer.new_reader();
}
}

Expand Down
2 changes: 1 addition & 1 deletion include/sequence.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ struct fmt::formatter<gr::Sequence> {
namespace gr {
inline std::ostream &
operator<<(std::ostream &os, const Sequence &v) {
return os << fmt::format("{}", v);
return os << fmt::format("{}", v.value());
}
} // namespace gr

Expand Down
5 changes: 3 additions & 2 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ project('graph-prototype', 'cpp',
version : '0.1',
default_options : ['warning_level=0', 'cpp_std=gnu++20'])

clang_warnings = ['-Wall', '-Wextra', '-Wshadow','-Wnon-virtual-dtor','-Wold-style-cast','-Wcast-align','-Wunused','-Woverloaded-virtual','-Wpedantic','-Wconversion','-Wsign-conversion','-Wnull-dereference','-Wdouble-promotion','-Wformat=2','-Wno-unknown-pragmas','-Wimplicit-fallthrough']
gcc_warnings = clang_warnings + ['-Wmisleading-indentation','-Wduplicated-cond','-Wduplicated-branches','-Wlogical-op','-Wuseless-cast','-Wno-interference-size']
clang_warnings = ['-Werror', '-Wall', '-Wextra', '-Wshadow','-Wnon-virtual-dtor','-Wold-style-cast','-Wcast-align','-Wunused','-Woverloaded-virtual','-Wpedantic','-Wconversion','-Wsign-conversion','-Wnull-dereference','-Wdouble-promotion','-Wformat=2','-Wno-unknown-pragmas','-Wimplicit-fallthrough']
gcc_warnings = clang_warnings + [ '-Wno-dangling-reference', # TODO: remove this once the fmt dangling reference bug is fixed
'-Wmisleading-indentation','-Wduplicated-cond','-Wduplicated-branches','-Wlogical-op','-Wuseless-cast','-Wno-interference-size']

compiler = meson.get_compiler('cpp')
if compiler.get_id() == 'gcc'
Expand Down
2 changes: 1 addition & 1 deletion test/app_grc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include <scheduler.hpp>

#include <fstream>
#include <strstream>
#include <sstream>

#include "build_configure.hpp"
#include "blocklib/core/unit-test/common_nodes.hpp"
Expand Down
172 changes: 87 additions & 85 deletions test/blocklib/core/unit-test/tag_monitors.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,97 +10,102 @@

namespace fair::graph::tag_test {

enum class ProcessFunction {
USE_PROCESS_ONE = 0, ///
USE_PROCESS_BULK = 1 ///
};

enum class ProcessFunction {
USE_PROCESS_ONE = 0, ///
USE_PROCESS_BULK = 1 ///
};

void
print_tag(const tag_t &tag, std::string_view prefix = {}) {
fmt::print("{} @index= {}: {{", prefix, tag.index);
if (tag.map.empty()) {
fmt::print("}}\n");
return;
}
for (const auto &[key, value]: tag.map) {
fmt::print(" {:>5}: {} ", key, value);
void
print_tag(const tag_t &tag, std::string_view prefix = {}) {
fmt::print("{} @index= {}: {{", prefix, tag.index);
if (tag.map.empty()) {
fmt::print("}}\n");
return;
}
const auto& map = tag.map;
for (const auto &[key, value] : map) {
// workaround for:
// fmt/core.h:1674:10: warning: possibly dangling reference to a temporary [-Wdangling-reference]
// 1674 | auto&& arg = arg_mapper<Context>().map(FMT_FORWARD(val));
// | ^~~
// #pragma GCC diagnostic push
// #pragma GCC diagnostic ignored "-Wdangling-reference"
fmt::print(" {:>5}: {} ", key, value);
// #pragma GCC diagnostic pop
}
fmt::print("}}\n");
}

template<typename T, ProcessFunction UseProcessOne>
struct TagSource : public node<TagSource<T, UseProcessOne>> {
OUT<T> out;
std::vector<tag_t> tags{};
std::size_t next_tag{0};
std::uint64_t n_samples_max = 1024;
std::uint64_t n_samples_produced{0};

constexpr std::make_signed_t<std::size_t>
available_samples(const TagSource &) noexcept {
if constexpr (UseProcessOne == ProcessFunction::USE_PROCESS_ONE) {
return n_samples_produced < n_samples_max ? 1 : -1; // '-1' -> DONE, produced enough samples
} else if constexpr (UseProcessOne == ProcessFunction::USE_PROCESS_BULK) {
std::make_signed_t<std::size_t> nextTagIn =
next_tag < tags.size() ? (tags[next_tag].index - n_samples_produced) : n_samples_max -
n_samples_produced;
return n_samples_produced < n_samples_max ? std::max(1L, nextTagIn)
: -1; // '-1' -> DONE, produced enough samples
} else {
static_assert(fair::meta::always_false<T>, "ProcessFunction-type not handled");
}
}
template<typename T, ProcessFunction UseProcessOne>
struct TagSource : public node<TagSource<T, UseProcessOne>> {
OUT<T> out;
std::vector<tag_t> tags{};
std::size_t next_tag{ 0 };
std::uint64_t n_samples_max = 1024;
std::uint64_t n_samples_produced{ 0 };

T
process_one() noexcept requires(UseProcessOne == ProcessFunction::USE_PROCESS_ONE) {
if (next_tag < tags.size() &&
tags[next_tag].index <= static_cast<std::make_signed_t<std::size_t>>(n_samples_produced)) {
print_tag(tags[next_tag],
fmt::format("{}::process_one(...)\t publish tag at {:6}", this->name(), n_samples_produced));
tag_t &out_tag = this->output_tags()[0];
out_tag = tags[next_tag];
out_tag.index = 0; // indices > 0 write tags in the future ... handle with care
this->forward_tags();
next_tag++;
n_samples_produced++;
return static_cast<T>(1);
}
constexpr std::make_signed_t<std::size_t>
available_samples(const TagSource &) noexcept {
if constexpr (UseProcessOne == ProcessFunction::USE_PROCESS_ONE) {
return n_samples_produced < n_samples_max ? 1 : -1; // '-1' -> DONE, produced enough samples
} else if constexpr (UseProcessOne == ProcessFunction::USE_PROCESS_BULK) {
std::make_signed_t<std::size_t> nextTagIn = next_tag < tags.size() ? tags[next_tag].index - static_cast<std::make_signed_t<std::size_t>>(n_samples_produced)
: static_cast<std::make_signed_t<std::size_t>>(n_samples_max - n_samples_produced);
return n_samples_produced < n_samples_max ? std::max(1L, nextTagIn) : -1L; // '-1L' -> DONE, produced enough samples
} else {
static_assert(fair::meta::always_false<T>, "ProcessFunction-type not handled");
}
}

T
process_one() noexcept
requires(UseProcessOne == ProcessFunction::USE_PROCESS_ONE)
{
if (next_tag < tags.size() && tags[next_tag].index <= static_cast<std::make_signed_t<std::size_t>>(n_samples_produced)) {
print_tag(tags[next_tag], fmt::format("{}::process_one(...)\t publish tag at {:6}", this->name(), n_samples_produced));
tag_t &out_tag = this->output_tags()[0];
out_tag = tags[next_tag];
out_tag.index = 0; // indices > 0 write tags in the future ... handle with care
this->forward_tags();
next_tag++;
n_samples_produced++;
return static_cast<T>(0);
return static_cast<T>(1);
}

work_return_t
process_bulk(std::span<T> output) noexcept requires(UseProcessOne == ProcessFunction::USE_PROCESS_BULK) {
if (next_tag < tags.size() &&
tags[next_tag].index <= static_cast<std::make_signed_t<std::size_t>>(n_samples_produced)) {
print_tag(tags[next_tag],
fmt::format("{}::process_one(...)\t publish tag at {:6}", this->name(), n_samples_produced));
tag_t &out_tag = this->output_tags()[0];
out_tag = tags[next_tag];
out_tag.index = 0; // indices > 0 write tags in the future ... handle with care
this->forward_tags();
next_tag++;
}

n_samples_produced += output.size();
return n_samples_produced < n_samples_max ? work_return_t::OK : work_return_t::DONE;
n_samples_produced++;
return static_cast<T>(0);
}

work_return_t
process_bulk(std::span<T> output) noexcept
requires(UseProcessOne == ProcessFunction::USE_PROCESS_BULK)
{
if (next_tag < tags.size() && tags[next_tag].index <= static_cast<std::make_signed_t<std::size_t>>(n_samples_produced)) {
print_tag(tags[next_tag], fmt::format("{}::process_one(...)\t publish tag at {:6}", this->name(), n_samples_produced));
tag_t &out_tag = this->output_tags()[0];
out_tag = tags[next_tag];
out_tag.index = 0; // indices > 0 write tags in the future ... handle with care
this->forward_tags();
next_tag++;
}
};

static_assert(HasRequiredProcessFunction<TagSource<int, ProcessFunction::USE_PROCESS_ONE>>);
static_assert(HasRequiredProcessFunction<TagSource<int, ProcessFunction::USE_PROCESS_BULK>>);
n_samples_produced += output.size();
return n_samples_produced < n_samples_max ? work_return_t::OK : work_return_t::DONE;
}
};

static_assert(HasRequiredProcessFunction<TagSource<int, ProcessFunction::USE_PROCESS_ONE>>);
static_assert(HasRequiredProcessFunction<TagSource<int, ProcessFunction::USE_PROCESS_BULK>>);

template<typename T, ProcessFunction UseProcessOne>
struct TagMonitor : public node<TagMonitor<T, UseProcessOne>> {
IN<T> in;
OUT<T> out;
std::vector<tag_t> tags{};
std::uint64_t n_samples_produced{0};
template<typename T, ProcessFunction UseProcessOne>
struct TagMonitor : public node<TagMonitor<T, UseProcessOne>> {
IN<T> in;
OUT<T> out;
std::vector<tag_t> tags{};
std::uint64_t n_samples_produced{ 0 };

constexpr T
process_one(const T &input) noexcept
constexpr T
process_one(const T &input) noexcept
requires(UseProcessOne == ProcessFunction::USE_PROCESS_ONE)
{
if (this->input_tags_present()) {
Expand Down Expand Up @@ -176,16 +181,13 @@ struct TagSink : public node<TagSink<T, UseProcessOne>> {
}
};

static_assert(HasRequiredProcessFunction<TagSink<int, ProcessFunction::USE_PROCESS_ONE>>);
static_assert(HasRequiredProcessFunction<TagSink<int, ProcessFunction::USE_PROCESS_BULK>>);
static_assert(HasRequiredProcessFunction<TagSink<int, ProcessFunction::USE_PROCESS_ONE>>);
static_assert(HasRequiredProcessFunction<TagSink<int, ProcessFunction::USE_PROCESS_BULK>>);

} // namespace fair::graph::tag_test

ENABLE_REFLECTION_FOR_TEMPLATE_FULL((typename T, fair::graph::tag_test::ProcessFunction b),
(fair::graph::tag_test::TagSource<T, b>), out, n_samples_max);
ENABLE_REFLECTION_FOR_TEMPLATE_FULL((typename T, fair::graph::tag_test::ProcessFunction b),
(fair::graph::tag_test::TagMonitor<T, b>), in, out);
ENABLE_REFLECTION_FOR_TEMPLATE_FULL((typename T, fair::graph::tag_test::ProcessFunction b),
(fair::graph::tag_test::TagSink<T, b>), in);
ENABLE_REFLECTION_FOR_TEMPLATE_FULL((typename T, fair::graph::tag_test::ProcessFunction b), (fair::graph::tag_test::TagSource<T, b>), out, n_samples_max);
ENABLE_REFLECTION_FOR_TEMPLATE_FULL((typename T, fair::graph::tag_test::ProcessFunction b), (fair::graph::tag_test::TagMonitor<T, b>), in, out);
ENABLE_REFLECTION_FOR_TEMPLATE_FULL((typename T, fair::graph::tag_test::ProcessFunction b), (fair::graph::tag_test::TagSink<T, b>), in);

#endif // GRAPH_PROTOTYPE_TAG_MONITORS_HPP
Loading

0 comments on commit 5418a55

Please sign in to comment.