Skip to content

Commit

Permalink
Fix warnings in the GUI and in Release
Browse files Browse the repository at this point in the history
  • Loading branch information
pierre-dejoue committed Aug 12, 2023
1 parent ece5be6 commit 2c88d77
Show file tree
Hide file tree
Showing 16 changed files with 70 additions and 45 deletions.
3 changes: 2 additions & 1 deletion cmake/config/compiler_options.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ function(
-Wpedantic # warn if non-standard C++ is used
-Wconversion # warn on type conversions that may lose data
-Wsign-conversion # warn on sign conversions
-Wnull-dereference # warn if a null dereference is detected
# Commented out due to a false positive in line_alternatives.cpp with GNU GCC/G++ 11.4.0
# -Wnull-dereference # warn if a null dereference is detected
-Wdouble-promotion # warn if float is implicit promoted to double
-Wformat=2 # warn on security issues around functions that format output (ie printf)
-Wimplicit-fallthrough # warn on statements that fallthrough without an explicit annotation
Expand Down
5 changes: 5 additions & 0 deletions src/gui/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ if(PICROSS_GUI_IMGUI_DEMO)
)
endif()

target_include_directories(picross_solver
PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}/src
)

set_target_warnings(picross_solver ON)

target_link_libraries(picross_solver
Expand Down
2 changes: 1 addition & 1 deletion src/gui/src/draw_grid.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include <picross/picross.h>
#include <utils/grid_observer.h>

#include <imgui.h>
#include <imgui_wrap.h>

#include <cstddef>

Expand Down
2 changes: 1 addition & 1 deletion src/gui/src/err_window.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include "err_window.h"

#include <imgui.h>
#include <imgui_wrap.h>

#include <mutex>
#include <string>
Expand Down
2 changes: 1 addition & 1 deletion src/gui/src/goal_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#include "draw_grid.h"
#include "settings.h"

#include <imgui.h>
#include <imgui_wrap.h>


GoalWindow::GoalWindow(const picross::OutputGrid& goal, std::string_view name)
Expand Down
17 changes: 9 additions & 8 deletions src/gui/src/grid_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
#include <stdutils/string.h>
#include <utils/input_grid_utils.h>

#include <imgui.h>
#include <imgui_wrap.h>

#include <iomanip>
#include <ios>
#include <iostream>
#include <optional>
#include <sstream>
Expand Down Expand Up @@ -70,7 +71,7 @@ void GridInfo::visit(bool& can_be_erased)

unsigned int active_sections = 0;
const ImVec2 cell_padding(7, 4);
constexpr ImGuiTableFlags table_flags = ImGuiTableFlags_SizingFixedFit | ImGuiTableFlags_RowBg | ImGuiTableFlags_Borders | ImGuiTableFlags_Resizable;
constexpr ImGuiTableFlags TABLE_FLAGS = ImGuiTableFlags_SizingFixedFit | ImGuiTableFlags_RowBg | ImGuiTableFlags_Borders | ImGuiTableFlags_Resizable;

// Copy button
const bool copy_to_clipboard = ImGui::Button("Copy");
Expand All @@ -82,7 +83,7 @@ void GridInfo::visit(bool& can_be_erased)
active_sections |= static_cast<unsigned int>(GridInfoSection::Basic);

ImGui::PushStyleVar(ImGuiStyleVar_CellPadding, cell_padding);
if (ImGui::BeginTable("table_info", 2, table_flags))
if (ImGui::BeginTable("table_info", 2, TABLE_FLAGS))
{
ImGui::TableSetupColumn("key", ImGuiTableColumnFlags_WidthFixed | ImGuiTableColumnFlags_NoResize);
ImGui::TableSetupColumn("data", ImGuiTableColumnFlags_WidthStretch);
Expand All @@ -109,7 +110,7 @@ void GridInfo::visit(bool& can_be_erased)
active_sections |= static_cast<unsigned int>(GridInfoSection::Stats);

ImGui::PushStyleVar(ImGuiStyleVar_CellPadding, cell_padding);
if (ImGui::BeginTable("table_stats", 2, table_flags))
if (ImGui::BeginTable("table_stats", 2, TABLE_FLAGS))
{
ImGui::TableSetupColumn("key", ImGuiTableColumnFlags_WidthFixed | ImGuiTableColumnFlags_NoResize);
ImGui::TableSetupColumn("data", ImGuiTableColumnFlags_WidthStretch);
Expand All @@ -134,10 +135,10 @@ void GridInfo::visit(bool& can_be_erased)
{
active_sections |= static_cast<unsigned int>(GridInfoSection::Constraints);

const auto build_table_of_constraints = [&cell_padding, &table_flags, this](picross::Line::Type type, std::size_t size, const char* table_id)
const auto build_table_of_constraints = [&cell_padding, this](picross::Line::Type type, std::size_t size, const char* table_id)
{
ImGui::PushStyleVar(ImGuiStyleVar_CellPadding, cell_padding);
if (ImGui::BeginTable(table_id, 3, table_flags))
if (ImGui::BeginTable(table_id, 3, TABLE_FLAGS))
{
ImGui::TableSetupColumn("Line", ImGuiTableColumnFlags_WidthFixed | ImGuiTableColumnFlags_NoResize);
ImGui::TableSetupColumn("", ImGuiTableColumnFlags_WidthFixed | ImGuiTableColumnFlags_NoResize);
Expand Down Expand Up @@ -256,7 +257,7 @@ std::string GridInfo::info_as_string(unsigned int active_sections) const
if (active_sections & static_cast<unsigned int>(GridInfoSection::Basic))
{
out << std::endl;
const auto key_w = key_col_width(grid_metadata);
const int key_w = static_cast<int>(key_col_width(grid_metadata));
for (const auto&[key, data]: grid_metadata)
out << " " << std::setw(key_w) << std::left << key << data << std::endl;
}
Expand All @@ -267,7 +268,7 @@ std::string GridInfo::info_as_string(unsigned int active_sections) const
if (active_sections & static_cast<unsigned int>(GridInfoSection::Stats))
{
out << std::endl;
const auto key_w = key_col_width(solver_stats_info);
const int key_w = static_cast<int>(key_col_width(solver_stats_info));
for (const auto&[key, data]: solver_stats_info)
out << " " << std::setw(key_w) << std::left << key << data << std::endl;
}
Expand Down
12 changes: 6 additions & 6 deletions src/gui/src/grid_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
#include <picross/picross.h>
#include <utils/picross_file_io.h>

#include <imgui.h>
#include <portable-file-dialogs.h>
#include <imgui_wrap.h>
#include <pfd_wrap.h>

#include <iostream>
#include <optional>
Expand Down Expand Up @@ -145,7 +145,7 @@ void GridWindow::visit(bool& can_be_erased, Settings& settings)
std::lock_guard<std::mutex> lock(line_mutex);
if (!line_events.empty())
{
local_events.reserve(speed);
local_events.reserve(static_cast<std::size_t>(speed));
std::swap(local_events, line_events);
}
}
Expand Down Expand Up @@ -321,11 +321,11 @@ void GridWindow::observer_callback(picross::ObserverEvent event, const picross::
return;

std::unique_lock<std::mutex> lock(line_mutex);
if (line_events.size() >= speed)
if (line_events.size() >= static_cast<std::size_t>(speed))
{
// Wait until the previous line events have been consumed
line_cv.wait(lock, [this]() -> bool {
return (this->speed > 0u && this->line_events.empty())
return (this->speed > 0 && this->line_events.empty())
|| this->abort_solver_thread();
});
}
Expand Down Expand Up @@ -436,7 +436,7 @@ void GridWindow::save_grid()
const auto err_handler = [this](picross::io::ErrorCodeT code, std::string_view msg)
{
std::lock_guard<std::mutex> lock(this->text_buffer->mutex);
this->text_buffer->buffer.appendf("%s %s\n", picross::io::str_error_code(code).c_str(), msg);
this->text_buffer->buffer.appendf("%s %s\n", picross::io::str_error_code(code).c_str(), msg.data());
};

const auto solution = (solutions.empty() || !solutions[0].is_completed()) ? std::nullopt : std::optional<picross::OutputGrid>(solutions[0]);
Expand Down
7 changes: 7 additions & 0 deletions src/gui/src/imgui_wrap.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#pragma once

#if defined(__GNUC__)
#pragma GCC system_header
#endif

#include <imgui.h>
10 changes: 5 additions & 5 deletions src/gui/src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
#include <picross/picross.h>
#include <stdutils/macros.h>

#include <portable-file-dialogs.h> // Include before glfw3.h
#include <pfd_wrap.h> // Include before glfw3.h
#include <GLFW/glfw3.h>
#include <imgui.h>
#include <imgui_wrap.h>
#include <imgui_impl_glfw.h>
#include <imgui_impl_opengl3.h>

Expand Down Expand Up @@ -126,7 +126,7 @@ int main(int argc, char *argv[])
{
const auto paths = pfd::open_file("Select a Picross file", "",
{ "Picross file", "*.txt *.nin *.non", "All files", "*" }).result();
for (const auto path : paths)
for (const auto& path : paths)
{
const auto format = picross::io::picross_file_format_from_filepath(path);
std::cout << "User selected file " << path << " (format: " << format << ")" << std::endl;
Expand All @@ -137,7 +137,7 @@ int main(int argc, char *argv[])
{
const auto paths = pfd::open_file("Select a bitmap file", "",
{ "PBM file", "*.pbm", "All files", "*" }).result();
for (const auto path : paths)
for (const auto& path : paths)
{
const auto format = picross::io::PicrossFileFormat::PBM;
std::cout << "User selected bitmap " << path << " (format: " << format << ")" << std::endl;
Expand All @@ -148,7 +148,7 @@ int main(int argc, char *argv[])
{
const auto paths = pfd::open_file("Select a solution file", "",
{ "All files", "*" }).result();
for (const auto path : paths)
for (const auto& path : paths)
{
const auto format = picross::io::PicrossFileFormat::OutputGrid;
std::cout << "User selected solution file " << path << std::endl;
Expand Down
7 changes: 7 additions & 0 deletions src/gui/src/pfd_wrap.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#pragma once

#if defined(__GNUC__)
#pragma GCC system_header
#endif

#include <portable-file-dialogs.h>
2 changes: 1 addition & 1 deletion src/gui/src/settings_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#include "settings.h"

#include <imgui.h>
#include <imgui_wrap.h>

#include <cassert>

Expand Down
2 changes: 1 addition & 1 deletion src/picross/include/picross/picross_stats.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace picross
struct GridStats
{
unsigned int nb_solutions = 0u;
unsigned int max_k; // max nb of segments in a constraint (grid property)
unsigned int max_k = 0u; // max nb of segments in a constraint (grid property)
unsigned int max_branching_depth = 0u; // max branching depth of the solver (not of the solutions)
unsigned int nb_branching_calls = 0u;
unsigned int total_nb_branching_alternatives = 0u;
Expand Down
23 changes: 10 additions & 13 deletions src/picross/src/line_alternatives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ namespace

private:
LineSpanW m_line;
bool m_reset;
bool m_reset;
};

LineAlternatives::Reduction from_line(const LineSpan& line, LineAlternatives::NbAlt nb_alt, bool full)
Expand All @@ -139,11 +139,11 @@ namespace
{
public:
LineExt(const LineSpan& line_span, Tile init_tile)
: m_tiles(line_span.size() + 2, init_tile)
: m_tiles(line_span.size() + 2u, init_tile)
, m_line_span(line_span.type(), line_span.index(), line_span.size(), m_tiles.data() + 1u)
{
m_tiles.front() = Tile::EMPTY;
m_tiles.back() = Tile::EMPTY;
m_tiles.back() = Tile::EMPTY;
}

LineExt(const LineSpan& line_span)
Expand All @@ -156,7 +156,7 @@ namespace
LineSpanW& line_span() { return m_line_span; }
private:
Line::Container m_tiles;
LineSpanW m_line_span;
LineSpanW m_line_span;
};

// An array of LineExt
Expand Down Expand Up @@ -249,10 +249,9 @@ namespace
const auto hole_end = l_holes.end();
assert(hole_it == hole_end || hole_it->m_index >= range.m_line_begin);
auto result_seg_it = result.begin();
const auto result_seg_end = result.end();
while (constraint_it != constraint_end && hole_it != hole_end)
{
assert(result_seg_it != result_seg_end);
assert(result_seg_it != result.end());
const auto seg_len = *constraint_it;
if (seg_len <= hole_it->m_length)
{
Expand Down Expand Up @@ -287,10 +286,9 @@ namespace
const auto hole_end = std::make_reverse_iterator(r_holes.begin());
assert(hole_it == hole_end || (hole_it->m_index + static_cast<int>(hole_it->m_length)) <= range.m_line_end);
auto result_seg_it = std::make_reverse_iterator(result.end());
const auto result_seg_end = std::make_reverse_iterator(result.begin());
while (constraint_it != constraint_end && hole_it != hole_end)
{
assert(result_seg_it != result_seg_end);
assert(result_seg_it != std::make_reverse_iterator(result.begin()));
const auto seg_len = *constraint_it;
if (seg_len <= hole_it->m_length)
{
Expand Down Expand Up @@ -688,9 +686,9 @@ bool LineAlternatives::Impl::narrow_down_segments_range(std::vector<SegmentRange
}

// Right to left pass to refine the leftmost index of each segment
if (nb_segments > 0)
if (nb_segments > 0 && constraint_it != constraint_end)
{
assert(constraint_it != constraint_end && std::next(constraint_it) == constraint_end);
assert(std::next(constraint_it) == constraint_end);
prev_segment_min_index(ranges[nb_segments - 1].m_leftmost_index, *constraint_it, m_bidirectional_range.m_line_end);
for (std::size_t k = nb_segments - 1; k > 0; k--)
{
Expand Down Expand Up @@ -723,10 +721,9 @@ LineAlternatives::Reduction LineAlternatives::Impl::linear_reduction(const std::

const auto nb_segments = ranges.size();
auto constraint_it = m_bidirectional_range.m_constraint_begin;
const auto constraint_end = m_bidirectional_range.m_constraint_end;
const auto line_begin = m_bidirectional_range.m_line_begin;
const auto line_end = m_bidirectional_range.m_line_end;
assert(nb_segments == static_cast<std::size_t>(std::distance(constraint_it, constraint_end)));
assert(nb_segments == static_cast<std::size_t>(std::distance(constraint_it, m_bidirectional_range.m_constraint_end)));

LineExtArray tiles_masks(m_known_tiles, 1u, Tile::UNKNOWN);

Expand All @@ -747,7 +744,7 @@ LineAlternatives::Reduction LineAlternatives::Impl::linear_reduction(const std::
NbAlt nb_alt = 0u;
int min_index = static_cast<int>(m_known_tiles.size() + 1u);
int max_index = -1;
assert(constraint_it != constraint_end);
assert(constraint_it != m_bidirectional_range.m_constraint_end);
const unsigned int seg_length = *constraint_it;
for (int seg_index = ranges[k].m_leftmost_index; seg_index <= ranges[k].m_rightmost_index; seg_index++)
{
Expand Down
5 changes: 3 additions & 2 deletions src/picross/src/work_grid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ RET_UINT progress_bar(const std::pair<float, float>& progress_bar, LineAlternati
assert(progress <= nb_alternatives);
const float ratio_f = static_cast<float>(progress) / static_cast<float>(nb_alternatives);
assert(0.f <= ratio_f && ratio_f <= 1.f);
const float progress_f = progress_bar.first + (progress_bar.second - progress_bar.first) * ratio_f;
const std::uint32_t ratio = reinterpret_cast<const std::uint32_t&>(progress_f);
const auto progress_f = std::make_unique<float>(progress_bar.first + (progress_bar.second - progress_bar.first) * ratio_f);
// reinterpret as integer to go through the observer interface
const std::uint32_t ratio = *reinterpret_cast<const std::uint32_t*>(progress_f.get());
return static_cast<RET_UINT>(ratio);
}

Expand Down
7 changes: 6 additions & 1 deletion src/utils/src/console_observer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <cassert>
#include <cstdint>
#include <exception>
#include <memory>


ConsoleObserver::ConsoleObserver(size_t width, size_t height, std::ostream& ostream)
Expand Down Expand Up @@ -63,9 +64,13 @@ void ConsoleObserver::observer_callback(picross::ObserverEvent event, const picr
break;

case picross::ObserverEvent::PROGRESS:
m_ostream << " progress: " << reinterpret_cast<const float&>(static_cast<const std::uint32_t&>(misc))
{
const auto progress_i = std::make_unique<std::uint32_t>(static_cast<std::uint32_t>(misc));
const float progress_f = *reinterpret_cast<const float*>(progress_i.get());
m_ostream << " progress: " << progress_f
<< " depth: " << depth;
break;
}

default:
assert(0); // Unknown ObserverEvent
Expand Down
9 changes: 5 additions & 4 deletions src/utils/src/console_progress_observer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,14 @@ void ConsoleProgressObserver::operator()(picross::ObserverEvent event, const pic

case picross::ObserverEvent::PROGRESS:
{
const float current_progress = reinterpret_cast<const float&>(static_cast<const std::uint32_t&>(misc));
if ((current_progress - m_previous_progress) > 0.001f)
const auto progress_i = std::make_unique<std::uint32_t>(static_cast<std::uint32_t>(misc));
const float progress_f = *reinterpret_cast<const float*>(progress_i.get());
if ((progress_f - m_previous_progress) > 0.001f)
{
m_ostream << "Progress: " << current_progress
m_ostream << "Progress: " << progress_f
<< " (depth: " << depth << ")"
<< std::endl;
m_previous_progress = current_progress;
m_previous_progress = progress_f;
}
break;
}
Expand Down

0 comments on commit 2c88d77

Please sign in to comment.