[Bug] Keymap introspection with key_overrides

[Diaoul]

Describe the Bug

Using latest versions of QMK 0.26.6 I cannot compile my keymap that uses key overrides.

It worked fine with 0.25.22 and stopped working with 0.26.0

❯ qmk compile -kb splitkb/kyria/rev1
Ψ Compiling keymap with make -r -R -f builddefs/build_keyboard.mk -s KEYBOARD=splitkb/kyria/rev1/base KEYMAP=diaoul KEYBOARD_FILESAFE=splitkb_kyria_rev1_base TARGET=splitkb_kyria_rev1_base_diaoul VERBOSE=false COLOR=true SILENT=false QMK_BIN="qmk" QMK_USERSPACE=/home/antoine/projects/keyboards/qmk MAIN_KEYMAP_PATH_1=/home/antoine/projects/keyboards/qmk/keyboards/splitkb/kyria/keymaps/diaoul MAIN_KEYMAP_PATH_2=/home/antoine/projects/keyboards/qmk/keyboards/splitkb/kyria/keymaps/diaoul MAIN_KEYMAP_PATH_3=/home/antoine/projects/keyboards/qmk/keyboards/splitkb/kyria/keymaps/diaoul MAIN_KEYMAP_PATH_4=/home/antoine/projects/keyboards/qmk/keyboards/splitkb/kyria/keymaps/diaoul MAIN_KEYMAP_PATH_5=/home/antoine/projects/keyboards/qmk/keyboards/splitkb/kyria/keymaps/diaoul


avr-gcc (GCC) 14.1.0
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Size before:
   text	  data	   bss	   dec	   hex	filename
      0	 27794	     0	 27794	  6c92	splitkb_kyria_rev1_base_diaoul.hex

Generating: .build/obj_splitkb_kyria_rev1_base_diaoul/src/info_config.h                             [OK]
Generating: .build/obj_splitkb_kyria_rev1_base_diaoul/src/default_keyboard.h                        [OK]
Generating: .build/obj_splitkb_kyria_rev1_base_diaoul/src/default_keyboard.c                        [OK]
Compiling: /home/antoine/projects/keyboards/qmk/users/diaoul/diaoul.c                               [OK]
Compiling: /home/antoine/projects/keyboards/qmk/users/diaoul/features/oled.c                        [OK]
Compiling: /home/antoine/projects/keyboards/qmk/users/diaoul/features/overrides.c                   [OK]
Compiling: /home/antoine/projects/keyboards/qmk/users/diaoul/features/getreuer/achordion.c          [OK]
Compiling: keyboards/splitkb/kyria/kyria.c                                                          [OK]
Compiling: keyboards/splitkb/kyria/rev1/rev1.c                                                      [OK]
Compiling: .build/obj_splitkb_kyria_rev1_base_diaoul/src/default_keyboard.c                         [OK]
Compiling: quantum/keymap_introspection.c                                                          In file included from quantum/color.h:21,
                 from quantum/rgblight/rgblight_drivers.h:7,
                 from quantum/rgblight/rgblight.h:169,
                 from quantum/quantum.h:32,
                 from ./.build/obj_splitkb_kyria_rev1_base_diaoul/src/default_keyboard.h:27,
                 from /home/antoine/projects/keyboards/qmk/users/diaoul/diaoul.h:3,
                 from /home/antoine/projects/keyboards/qmk/keyboards/splitkb/kyria/keymaps/diaoul/keymap.c:1,
                 from quantum/keymap_introspection.c:5:
quantum/keymap_introspection.c: In function 'key_override_count_raw':
quantum/keymap_introspection.c:149:23: error: 'key_overrides' undeclared (first use in this function); did you mean 'key_override_t'?
  149 |     return ARRAY_SIZE(key_overrides);
      |                       ^~~~~~~~~~~~~
quantum/util.h:36:68: note: in definition of macro 'IS_ARRAY'
   36 | #    define IS_ARRAY(value) (!__builtin_types_compatible_p(typeof((value)), typeof(&(value)[0])))
      |                                                                    ^~~~~
quantum/keymap_introspection.c:149:12: note: in expansion of macro 'ARRAY_SIZE'
  149 |     return ARRAY_SIZE(key_overrides);
      |            ^~~~~~~~~~
quantum/keymap_introspection.c:149:23: note: each undeclared identifier is reported only once for each function it appears in
  149 |     return ARRAY_SIZE(key_overrides);
      |                       ^~~~~~~~~~~~~
quantum/util.h:36:68: note: in definition of macro 'IS_ARRAY'
   36 | #    define IS_ARRAY(value) (!__builtin_types_compatible_p(typeof((value)), typeof(&(value)[0])))
      |                                                                    ^~~~~
quantum/keymap_introspection.c:149:12: note: in expansion of macro 'ARRAY_SIZE'
  149 |     return ARRAY_SIZE(key_overrides);
      |            ^~~~~~~~~~
quantum/util.h:47:32: error: first argument to '__builtin_choose_expr' not a constant
   47 | #    define ARRAY_SIZE(array) (__builtin_choose_expr(IS_ARRAY((array)), sizeof((array)) / sizeof((array)[0]), (void)0))
      |                                ^~~~~~~~~~~~~~~~~~~~~
quantum/keymap_introspection.c:149:12: note: in expansion of macro 'ARRAY_SIZE'
  149 |     return ARRAY_SIZE(key_overrides);
      |            ^~~~~~~~~~
quantum/keymap_introspection.c: In function 'key_override_get_raw':
quantum/keymap_introspection.c:160:12: error: 'key_overrides' undeclared (first use in this function); did you mean 'key_override_t'?
  160 |     return key_overrides[key_override_idx];
      |            ^~~~~~~~~~~~~
      |            key_override_t
quantum/keymap_introspection.c: In function 'key_override_count_raw':
quantum/keymap_introspection.c:150:1: error: control reaches end of non-void function [-Werror=return-type]
  150 | }
      | ^
quantum/keymap_introspection.c: In function 'key_override_get_raw':
quantum/keymap_introspection.c:161:1: error: control reaches end of non-void function [-Werror=return-type]
  161 | }
      | ^
cc1: all warnings being treated as errors
 [ERRORS]
 | 
 | 
 | 
make: *** [builddefs/common_rules.mk:373: .build/obj_splitkb_kyria_rev1_base_diaoul/quantum/keymap_introspection.o] Error 1

It seems to stem from the introspection like it's the case for combos

I tried to play with various settings and compilation options but to no avail. What changed in 0.26.x that could cause this? How can I adapt my config such that it works again?

Keyboard Used

Kyria rev 1.4

Link to product page (if applicable)

No response

Operating System

Arch Linux

qmk doctor Output

❯ qmk doctor
Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.1.5
Ψ QMK home: /home/antoine/projects/keyboards/qmk/qmk_firmware
Ψ Detected Linux (Arch Linux).
Ψ Testing userspace candidate: /home/antoine/projects/keyboards/qmk -- Valid `qmk.json`
Ψ QMK userspace: /home/antoine/projects/keyboards/qmk
Ψ Userspace enabled: True
⚠ QMK home does not appear to be a Git repository! (no .git folder)
Ψ All dependencies are installed.
Ψ Found arm-none-eabi-gcc version 14.1.0
Ψ Found avr-gcc version 14.1.0
Ψ Found avrdude version 8.0
Ψ Found dfu-programmer version 1.1.0
Ψ Found dfu-util version 0.11
Ψ Submodules are up to date.
Ψ Submodule status:
Ψ - lib/chibios: 2024-02-17 19:20:06 +0000 --  (be44b3305)
Ψ - lib/chibios-contrib: 2024-04-03 20:39:24 +0800 --  (77cb0a4f)
Ψ - lib/googletest: 2021-06-11 06:37:43 -0700 --  (e2239ee6)
Ψ - lib/lufa: 2022-08-26 12:09:55 +1000 --  (549b97320)
Ψ - lib/vusb: 2022-06-13 09:18:17 +1000 --  (819dbc1)
Ψ - lib/printf: 2022-06-29 23:59:58 +0300 --  (c2e3b4e)
Ψ - lib/pico-sdk: 2023-02-12 20:19:37 +0100 --  (a3398d8)
Ψ - lib/lvgl: 2022-04-11 04:44:53 -0600 --  (e19410f8)
Ψ QMK is ready to go, but minor problems were found

Is AutoHotKey / Karabiner installed

• AutoHotKey (Windows)
• Karabiner (macOS)

Other keyboard-related software installed

No response

Additional Context

No response

[Diaoul]

Just saw this changelog, I could swear I copy pasted the documentation example though. But maybe I missed something, taking a look now 🤷

[Diaoul]

Unfortunately same error on my new branch

[Diaoul]

Related to PR #24120 and the removal of the global variable

[Diaoul]

This works:

INTROSPECTION_KEYMAP_C = features/overrides.c

But I cannot have 2 INTROSPECTION_KEYMAP_C defined and I need one for features/combos.c as well.

I would love to have a way to split my configuration as I have multiple keyboards and features enabled or not. How can I achieve this in a QMK compliant way?

[Diaoul]

Suggestion

Make INTROSPECTION_COMBOS, INTROSPECTION_KEY_OVERRIDES, etc. the same way it's done for INTROSPECTION_KEYMAP_C here

Happy to contribute a PR if you're open to the idea.

[tzarc]

Make an introspection_keymap.c with the following content and point INTROSPECTION_KEYMAP_C at it:

#include "my_combos.c"
#include "my_keyoverrides.c"

INTROSPECTION_KEYMAP_C is a "get out of jail free card" for people who decide they need to restructure things in their own way -- 99.9% of cases the introspection works because people place things in keymap.c as instructed.

Or, just stick the #includes in your normal keymap.c.

Doing different rules.mk entries for each isn't scalable.

[Diaoul]

Thanks, will try your suggestion 🤞

I'd be happy to restructure my files "the QMK way" as long as I can do multiple keyboards with different features and, preferably, not have a giant megafile.

Do you have an example I could follow? Or advice on how to achieve that?

I did not came up with that file structure but was inspired by different people doing the same, might be worth to be documented somewhere even for the 0.01% of us 😅

[tzarc]

I effectively created that file structure; I added the INTROSPECTION_KEYMAP_C as a way out for people who don't follow keymap.c.

We don't really advertise its use under normal circumstances as it's not aligned with the docs, but for "power users" it's the suggestion. We've been actively removing user keymaps from the repo due to their maintenance cost, so it's hard to point you at anything, really. Perhaps looking at some active forks of https://github.com/qmk/qmk_userspace might show up some nuggets of gold?

[Diaoul]

Well I looked at @drashna's config here for tap dances organized separately which seems to require introspection as well but I can't figure out what exactly makes it work 😕

Will look at others' or try to come up with something,thanks for your help 🙏

[tzarc]

That would be the fact that it's externed with an explicit size of the array by virtue of providing TAP_DANCE_MAX (thru its associated enum) and the subsequent extern tap_dance_action_t tap_dance_actions[TAP_DANCE_MAX]; at the same time in the header.

Introspection basically requires the ability to calculate the size of each array, so declaring them up-front like @drashna has serves the same purpose.

[Diaoul]

I see, looks like I can get it to work 👍

The docs mention the use of enums for tap dances and combos so it's a good start then for key overrides I imagine it may be possible to do the same.

The only downside is that it requires to modify 2 files to add one but I can live with that.

[Diaoul]

All good, it works with this trick. Thanks a lot for the help!
Diaoul/keyboards@38cec29

[lrascao]

Also hitting this, the way i see it we're all power users, after all here we are writing C code, building firmware and flashing onto keyboards.

Plus this was working perfectly fine before, my project has 7 different .c files that were SRC+ in rules.mk and it all worked great. It just looks like the introspection is lazily looking at a single keymap.c file (that can be overridden with the hacky INTROSPECTION_KEYMAP_C) instead of considering the list of files already built through SRC+=, yes it's more work but would not have broken people's projects, now i'm forced to add these monstrosities at the end of keymap.c:

#include "encoder.c"
#include "combo.c"
#include "tapdance.c"

[tzarc]

Feel free to propose an alternate solution that allows raw and overridable access to the number and content of keymap layers, tapdances, combos, and soon Unicode mappings without requiring all users to predefine things in other files.

Sure, so-called “C power users” may have been inconvenienced by this, but they’re also much more equipped to work with and/or around this structure. 99% of QMK users are not power users nor are they familiar with C, and so far this is the easiest solution and matches how users have been told to write their keymaps in the documentation for a very long time — if you’ve got better ideas we’re all ears.