Add more strict compiler flags - #56
Merged
Merged
Conversation
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens compiler strictness across C/C++ (including CI) by introducing a configurable “warnings as errors” switch and adding a broader set of warning flags, then updates code/tests to remain clean under the stricter builds.
Changes:
- Add
MJB_WARNINGS_AS_ERRORSCMake option and expand default warning sets (MSVC and non-MSVC), including stricter sanitizer build flags. - Extend Makefile/Makefile.nmake targets and GitHub Actions workflows to run strict + optimized configurations (and enable
WARNINGS_AS_ERRORS=ONin CI). - Fix/build-cleanups to satisfy stricter warnings (e.g.,
staticfor internal test helpers, rename a local to avoid collisions,(void)unused result in fuzz harness) and add a WASM API test forstatusMessage.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test.c | Make internal CLI helpers static to satisfy stricter prototype/visibility warnings. |
| tests/segmentation.c | Make callback static for stricter warning cleanliness. |
| tests/mojibake.c | Make allocator test hooks static to avoid missing-prototype warnings. |
| tests/break-word.c | Make callback static for stricter warning cleanliness. |
| tests/break-sentence.c | Make callback static and reflow signature to satisfy stricter warnings/style. |
| tests/break-line.c | Make callback static for stricter warning cleanliness. |
| TESTS.md | Update coverage totals/table entries (now needs correction for JS counts). |
| src/next.c | Rename local result to asm_result in Emscripten callback path. |
| src/locales.c | Remove unused/unsupported locale canonicalization stub. |
| src/api/tests/index.ts | Add statusMessage API assertion and import Status. |
| Makefile.nmake | Add strict warning toggle, optimized test targets, and update help/clean targets accordingly. |
| Makefile | Add strict warning toggle and optimized test targets; include new build dirs in clean. |
| fuzz/fuzz.c | Explicitly ignore unused return value from mjb_status_message under stricter warnings. |
| fuzz/Dockerfile | Build fuzzer with stricter warning flags and -Werror. |
| Dockerfile | Enable -DMJB_WARNINGS_AS_ERRORS=ON in container builds. |
| CMakeLists.txt | Introduce MJB_WARNINGS_AS_ERRORS, disable language extensions, and expand warning/sanitizer flags (MSVC + GCC/Clang). |
| .github/workflows/test.yml | Run strict builds in CI (including optimized) and expand Windows matrix coverage. |
| .github/workflows/platforms.yml | Enable warnings-as-errors in platform builds and strengthen strict-C11 compilation flags. |
Comments suppressed due to low confidence (1)
TESTS.md:216
- The JavaScript coverage table lists Mojibake.statusMessage as having 0 assertions and keeps the total at 117, but there is an ATT_ASSERT for mojibake.statusMessage(...) in src/api/tests/index.ts. Update this row and the total so the table matches the tests.
| `Mojibake.statusMessage` | 0 |
| **Total** | **117** |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
Signed-off-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.