fix make test:all failures seen on macOS Tahoe#26136
Open
scottlamb wants to merge 8 commits intoqmk:developfrom
Open
fix make test:all failures seen on macOS Tahoe#26136scottlamb wants to merge 8 commits intoqmk:developfrom
make test:all failures seen on macOS Tahoe#26136scottlamb wants to merge 8 commits intoqmk:developfrom
Conversation
Author
|
Hmm, I see this lint formatter failure...but the failure is on lines not changed in my PR. It looks like the file hasn't changed since the lint was set up, and the lint only checks changed files. Would you like me to add a commit to the beginning that updates the formatting of the files my PR touches? |
Member
zvecr
reviewed
Apr 9, 2026
scottlamb
force-pushed
the
test-build-fixes
branch
from
95cac37 to
0e3b0da
Compare
Author
Rebased, thanks! |
zvecr
requested changes
Apr 14, 2026
scottlamb
force-pushed
the
test-build-fixes
branch
from
0e3b0da to
cf62a55
Compare
Member
|
Apart from the additional commits that have snuck in, and the unresolved suggestion removal of an out of date comment, LGTM. |
Apple clang 21 fails the test build with:
quantum/process_keycode/process_space_cadet.c:168:23: error:
a function declaration without a prototype is deprecated in all
versions of C [-Werror,-Wstrict-prototypes]
168 | void reset_space_cadet() {
| ^
| void
In C, `void f()` is a non-prototype (K&R) declaration that means
"unspecified parameters", not "no parameters", and `-Wstrict-prototypes`
flags it even when a proper prototype (`void f(void);`) appears
earlier in the translation unit — the definition's own parameter list
must use `(void)` to count as a prototype. gcc only warns on K&R-style
*declarations*, so these definitions slip past the Linux CI build,
but clang flags both declarations and definitions.
Fix the three offenders that the test build hits: `reset_space_cadet`,
`usb_device_state_get_protocol`, and `audio_driver_{start,stop}_impl`.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Apple clang 21 fails the test build with:
platforms/timer.h:21:5: error: #include_next in file found relative
to primary source file or found by absolute path; will search
from start of include path [-Werror,-Winclude-next-absolute-path]
21 | #if __has_include_next("_timer.h")
`platforms/timer.h` uses `#include_next` to chain to the active
platform's `_timer.h`. That has well-defined semantics only when
`timer.h` itself was located via the `-I` search path, so the compiler
knows where in the path to resume the search. When `platforms/timer.c`
includes it as `"timer.h"`, the quoted-include lookup finds the header
source-relative (sibling of timer.c) before the `-I` search runs, and
clang now diagnoses the resulting `#include_next` as ill-defined.
`platforms/timer.c` is the only `.c` file at the `platforms/` top level
that sits next to one of the `#include_next` wrapper headers — the
other wrappers (`gpio.h`, `wait.h`, `atomic_util.h`) have no sibling
source at this level, so the collision is unique to this file. Switch
to an angle-bracket include to force include-path lookup; this matches
the existing precedent in `quantum/deferred_exec.c`, which already
includes `<timer.h>` for the same structural reason. Firmware builds
are unaffected: `-Iplatforms` is already on the include path for every
target, so the angle-bracket form resolves to the same `platforms/timer.h`.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Apple's ld does not accept GNU ld's `-Map=`/`--cref` options, so the
test build fails at link time with:
ld: unknown options: -Map=.build/test/<name>.map --cref
clang: error: linker command failed with exit code 1
`build_test.mk` already sets `CREATE_MAP := no`, but
`common_rules.mk:56` was unconditionally adding the map-file LDFLAGS
regardless. Gate the addition on `CREATE_MAP != no` so test builds
(which use the host linker) skip it. Firmware builds don't set
`CREATE_MAP`, so they continue to get the map file as before.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Apple clang 21 fails the test build with:
quantum/process_keycode/process_key_override.c:181:56: error:
result of comparison of constant 'QK_USER' (32320) with
expression of type 'const uint8_t' (aka 'const unsigned char')
is always true [-Werror,-Wtautological-constant-out-of-range-compare]
180 | bool unregister_replacement = mod_free_replacement != KC_NO &&
181 | mod_free_replacement < SAFE_RANGE;
`clear_mods_from()` returns `uint16_t`, but the result was being
stored in a `uint8_t` local. That truncated the upper byte of the
keycode and made the `< SAFE_RANGE` (== `QK_USER` == 32320) check
tautologically true, so the "skip custom keycodes" guard never fired
for any keycode whose low byte happened to match a basic keycode.
Widening to `uint16_t` matches the function's return type and the
sibling declaration at line 349, and restores the intended check.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
scottlamb
force-pushed
the
test-build-fixes
branch
from
cf62a55 to
cb77f42
Compare
scottlamb
commented
Apr 21, 2026
Author
scottlamb
left a comment
scottlamb
left a comment
There was a problem hiding this comment.
Apart from the additional commits that have snuck in
Oops, should be fixed. (I must have rebased on top of develop (stale local branch) instead of an up-to-date origin/develop and it cherry-picked origin/develop commits, changing their hashes.)
Apple clang 21 fails the unicode test builds with:
quantum/unicode/unicode.c:69:5: error: expected end of line in
preprocessor expression
69 | #if UNICODE_SELECTED_MODES != -1
| ^
`UNICODE_SELECTED_MODES` is documented as a comma-delimited list of
input modes (e.g. `UNICODE_MODE_LINUX, UNICODE_MODE_MACOS`). When a
keymap defines it that way, the `#if UNICODE_SELECTED_MODES != -1`
test expands to a preprocessor expression containing a comma, which
is not valid in `#if`. The previous default of `-1` only worked when
the macro was undefined; any real definition broke the build.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
asdf
Apple clang 21 fails the eeprom_legacy_emulated_flash test builds with:
platforms/chibios/drivers/eeprom/eeprom_legacy_emulated_flash.c:207:35:
error: if statement has empty body [-Werror,-Wempty-body]
207 | if (i % 8 == 0) print(" ");
| ^
When `CONSOLE_ENABLE` is off, `print()` expands to nothing, leaving
`if (i % 8 == 0) ;`. Wrap the call in braces so the body is a
non-empty compound statement and the warning is silenced regardless
of how `print` expands.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Apple clang 21 fails the encoder test builds with errors like:
quantum/encoder/tests/encoder_tests.cpp:37:40: error:
non-constant-expression cannot be narrowed from type 'uint8_t'
(aka 'unsigned char') to 'int8_t' (aka 'signed char') in
initializer list [-Wc++11-narrowing]
37 | updates[updates_array_idx % 32] = {index, clockwise};
| ^~~~~
`encoder_update_kb()` takes `uint8_t index`, but the test-only
`struct update` declared the captured field as `int8_t`. The
aggregate-init `{index, clockwise}` then narrows uint8_t -> int8_t,
which clang's stricter -Wc++11-narrowing rejects (gcc was lenient
about it, which is why Linux CI didn't catch this).
The signed type was a mismatch, not a deliberate choice — encoder
indices are non-negative — so widen the field to `uint8_t` to match
the real API instead of papering over the narrowing with a cast.
Applied identically to all six encoder test variants.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Apple clang 21 fails the test build with:
tests/test_common/keyboard_report_util.hpp:38:81: error:
non-constant-expression cannot be narrowed from type
'qk_keycode_defines' to 'value_type' (aka 'unsigned char') in
initializer list [-Wc++11-narrowing]
38 | return testing::MakeMatcher(new KeyboardReportMatcher(
std::vector<uint8_t>({keys...})));
Callers like EXPECT_REPORT(driver, (KC_SPACE)) deduce the variadic
pack as qk_keycode_defines, and std::vector<uint8_t>{keys...} is a
brace-init list -- the one context where C++11 narrowing rules apply.
clang's -Wc++11-narrowing rejects the implicit enum -> uint8_t
conversion there; gcc was lenient, which is why Linux CI didn't catch
this.
The narrowing is purely type-based: every real call site passes
basic keycodes that fit in a byte by construction (action-layer
processing has already reduced any wider keycodes upstream of the
report builder), but the enum type itself is wider than uint8_t and
that's what the rule keys off of. Make the conversion explicit with
a static_cast in the pack expansion so the brace-init list sees a
uint8_t to begin with and the narrowing rule has nothing to fire on.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
scottlamb
force-pushed
the
test-build-fixes
branch
from
cb77f42 to
6eb8478
Compare
drashna
approved these changes
Apr 22, 2026
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
3 participants
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.
Description
make test:allfails with a variety of errors on macOS Tahoe, I think mostly relating to it using a newer/stricter clang (21) than CI and, presumably, most contributor's machines. Each commit here fixes one class of failure (and has an example of the prior compiler error in the commit description).As you can tell I let my AI agent write the changes, but I reviewed them and think they're reasonable.
Types of Changes
Issues Fixed or Closed by This PR
Checklist