Skip to content

build: fix the 10 compiler warnings in a Release build and add a format check to CI (or drop .clang-format) #229

Description

@luisleo526

Problem

A clean Release configure + build at 95aeb9c (GCC 13, Linux) emits 10 warnings. Repo-owned ones:

src/engine_fills.cpp:8045:35: warning: missing initializer for member 'pineforge::PyramidEntry::entry_comment' [-Wmissing-field-initializers]
src/engine_fills.cpp:8100:39: warning: missing initializer for member 'pineforge::PyramidEntry::entry_comment' [-Wmissing-field-initializers]
src/engine_orders.cpp:855:31:  warning: missing initializer for member 'pineforge::PyramidEntry::entry_comment' [-Wmissing-field-initializers]
src/engine_orders.cpp:1028:31: warning: missing initializer for member 'pineforge::PyramidEntry::entry_comment' [-Wmissing-field-initializers]
src/session_time.cpp:156:13:   warning: 'bool pineforge::{anonymous}::minute_in_window(int, int, int)' defined but not used [-Wunused-function]
tests/test_coof_open_recalc_context.cpp:132:42: warning: '%02d' directive output may be truncated ... [-Wformat-truncation=]

The remaining ones come from Eigen headers and stl_algobase.h (-Wmaybe-uninitialized, -Wnonnull) and should be silenced by marking Eigen as a SYSTEM include rather than fixed in-tree.

Formatting: the repo ships a .clang-format, but the sources do not conform. clang-format --dry-run src/engine_orders.cpp alone reports 213 diffs, and CI has no format step. A config that is neither enforced nor followed misleads contributors into either reformatting whole files in unrelated PRs or ignoring the file.

Proposal

  1. Fix the six repo-owned warnings (the PyramidEntry ones are likely a single aggregate-init site pattern; add entry_comment explicitly or give it a default member initializer).
  2. Use target_include_directories(... SYSTEM ...) / Eigen3::Eigen as system to suppress third-party warnings.
  3. Add -Wall -Wextra to the existing PINEFORGE_STRICT_WARNINGS path or a new -Werror opt-in, and turn it on in one CI lane so the count cannot regress silently.
  4. Decide on formatting: either (a) run clang-format once over src/, include/, tests/, runner/ in a dedicated no-logic commit and add a clang-format --dry-run --Werror CI step, or (b) delete .clang-format and say so in CONTRIBUTING.md. Option (a) is recommended; do it as a single isolated commit so git blame -w --ignore-rev can skip it.

Acceptance

  • cmake --build build in Release prints zero warnings from files under src/, include/, tests/.
  • CI fails on new warnings or formatting drift, or .clang-format is gone.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions