-
-
Notifications
You must be signed in to change notification settings - Fork 39.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix compile issues / code smell related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE #8663
Merged
Conversation
This file contains 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
…O is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile).
Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required.
vomindoraan
changed the title
No action macro function
Disable legacy macro and function features in header instead of makefile (LTO)
Apr 3, 2020
vomindoraan
changed the title
Disable legacy macro and function features in header instead of makefile (LTO)
Disable legacy macro and function features in header instead of makefile (LTO_ENABLE)
Apr 3, 2020
vomindoraan
changed the title
Disable legacy macro and function features in header instead of makefile (LTO_ENABLE)
Fix compile issues / code smell related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE
Apr 3, 2020
mtei
approved these changes
Apr 3, 2020
13 tasks
drashna
approved these changes
Apr 5, 2020
13 tasks
zvecr
approved these changes
Apr 8, 2020
Thanks! |
Shinichi-Ohki
added a commit
to Shinichi-Ohki/qmk_firmware
that referenced
this pull request
Apr 9, 2020
* 'master' of https://github.com/qmk/qmk_firmware: (53 commits) Set the correct RGB LED count on YD60MQ (qmk#8629) [Keymap] Updates to personal keymaps (qmk#8665) Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663) [Keymap] Userspace update Rgb layers code (qmk#8659) Add Choconum (qmk#8709) Add Via keymap for BM16-A (qmk#8681) Fix edge-case with config Don't hide for devs... Apply @skullydazed's suggestions, move 'import milc' Make dedicated sections for user/dev commands in docs Rebase on master, hide some other subcommands Use milc for config check, requirements fixes CLI: Add development mode support Update info.json (qmk#8723) format code according to conventions [skip ci] spi_master for AVR (qmk#8299) DennyTom's buttery_engine (qmk#8138) add via support for kira80 (qmk#8677) [Keyboard] Wheatfield Split75 (qmk#8511) Correctly handle json keymaps with ANY() ...
loadedsith
added a commit
to loadedsith/qmk_firmware
that referenced
this pull request
Apr 9, 2020
* master: (3035 commits) [Keymap] Update personal userspace and keymaps (qmk#8747) Add PS2_MOUSE_ROTATE to compensate for device orientation (qmk#8650) Add RGB support in via to launchpad (qmk#8621) VIA support for the KBDFans KBD6x (qmk#8680) Set the correct RGB LED count on YD60MQ (qmk#8629) [Keymap] Updates to personal keymaps (qmk#8665) Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663) [Keymap] Userspace update Rgb layers code (qmk#8659) Add Choconum (qmk#8709) Add Via keymap for BM16-A (qmk#8681) Fix edge-case with config Don't hide for devs... Apply @skullydazed's suggestions, move 'import milc' Make dedicated sections for user/dev commands in docs Rebase on master, hide some other subcommands Use milc for config check, requirements fixes CLI: Add development mode support Update info.json (qmk#8723) format code according to conventions [skip ci] spi_master for AVR (qmk#8299) ... # Conflicts: # layouts/community/ortho_4x12/grahampheath/keymap.c # lib/lufa
loadedsith
added a commit
to loadedsith/qmk_firmware
that referenced
this pull request
Apr 9, 2020
* master: (208 commits) [Keymap] Update personal userspace and keymaps (qmk#8747) Add PS2_MOUSE_ROTATE to compensate for device orientation (qmk#8650) Add RGB support in via to launchpad (qmk#8621) VIA support for the KBDFans KBD6x (qmk#8680) Set the correct RGB LED count on YD60MQ (qmk#8629) [Keymap] Updates to personal keymaps (qmk#8665) Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663) [Keymap] Userspace update Rgb layers code (qmk#8659) Add Choconum (qmk#8709) Add Via keymap for BM16-A (qmk#8681) Fix edge-case with config Don't hide for devs... Apply @skullydazed's suggestions, move 'import milc' Make dedicated sections for user/dev commands in docs Rebase on master, hide some other subcommands Use milc for config check, requirements fixes CLI: Add development mode support Update info.json (qmk#8723) format code according to conventions [skip ci] spi_master for AVR (qmk#8299) ...
loadedsith
added a commit
to loadedsith/qmk_firmware
that referenced
this pull request
Apr 9, 2020
* master: (208 commits) [Keymap] Update personal userspace and keymaps (qmk#8747) Add PS2_MOUSE_ROTATE to compensate for device orientation (qmk#8650) Add RGB support in via to launchpad (qmk#8621) VIA support for the KBDFans KBD6x (qmk#8680) Set the correct RGB LED count on YD60MQ (qmk#8629) [Keymap] Updates to personal keymaps (qmk#8665) Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663) [Keymap] Userspace update Rgb layers code (qmk#8659) Add Choconum (qmk#8709) Add Via keymap for BM16-A (qmk#8681) Fix edge-case with config Don't hide for devs... Apply @skullydazed's suggestions, move 'import milc' Make dedicated sections for user/dev commands in docs Rebase on master, hide some other subcommands Use milc for config check, requirements fixes CLI: Add development mode support Update info.json (qmk#8723) format code according to conventions [skip ci] spi_master for AVR (qmk#8299) ...
HokieGeek
pushed a commit
to HokieGeek/qmk_firmware
that referenced
this pull request
Apr 10, 2020
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
mrlinuxfish
pushed a commit
to mrlinuxfish/qmk_firmware
that referenced
this pull request
Apr 12, 2020
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
13 tasks
Quarren42
pushed a commit
to Quarren42/qmk_firmware
that referenced
this pull request
Apr 15, 2020
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
kylekuj
pushed a commit
to kylekuj/qmk_firmware
that referenced
this pull request
Apr 21, 2020
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
bitherder
pushed a commit
to bitherder/qmk_firmware
that referenced
this pull request
May 15, 2020
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
drashna
pushed a commit
to zsa/qmk_firmware
that referenced
this pull request
May 24, 2020
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
sowbug
pushed a commit
to sowbug/qmk_firmware
that referenced
this pull request
May 24, 2020
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
fdidron
pushed a commit
to zsa/qmk_firmware
that referenced
this pull request
Jun 12, 2020
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
turky
pushed a commit
to turky/qmk_firmware
that referenced
this pull request
Jun 13, 2020
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
jakobaa
pushed a commit
to jakobaa/qmk_firmware
that referenced
this pull request
Jul 7, 2020
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
thorstenweber83
pushed a commit
to thorstenweber83/qmk_firmware
that referenced
this pull request
Sep 2, 2020
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
BorisTestov
pushed a commit
to BorisTestov/qmk_firmware
that referenced
this pull request
May 23, 2024
…qmk#8663) * Define NO_ACTION_MACRO/FUNCTION in header instead of makefile when LTO is enabled Currently, boards and keymaps that define NO_ACTION_MACRO/FUNCTION unconditionally will not compile with LTO_ENABLE (qmk#8604). This fixes the issue by moving the definitions from common.mk to action.h, which enables us to check for previous definitions of those macros (this cannot be done in a makefile). * Remove LTO checks in templates Since now NO_ACTION_MACRO/FUNCTION are defined as needed in action.h (which is included by quantum.h), checking for LTO in keyboard and user code is no longer required. * Update LTO_ENABLE docs
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.
Description
As discussed in #8604, there is currently an issue with how
NO_ACTION_MACRO
andNO_ACTION_FUNCTION
are defined when LTO is enabled usingLINK_TIME_OPTIMIZATION_ENABLE
/LTO_ENABLE
. Namely,common.mk
adds these definitions outright and has no way of checking if they already exist somewhere down the pipeline, since the makefile is processed before any headers.qmk_firmware/tmk_core/common.mk
Lines 158 to 167 in 2f623da
This forces users to check for LTO before defining the macros, which:
NO_ACTION_MACRO/FUNCTION
and otherNO_ACTION_*
flags are used;At the moment, any keyboard, keymap or userspace that unconditionally defines the above flags will fail to compile with
LTO_ENABLE = yes
due to the flags already being defined in the makefile.Background: The defines were originally added to
common.mk
in #5674, and the LTO check was added toconfig.h
templates in #7211.Solution
This PR solves the above issues by moving the “if LTO is enabled, legacy macro and function features should be disabled” logic from
tmk_core/common.mk
intotmk_core/action.h
. This keeps the macros available to existing source files that check forNO_ACTION_MACRO/FUNCTION
, but it removes the need to check for LTO before defining these macros in keyboard/userspace/keymap files.Testing
The following keyboards can be used for testing:On master, they don't compile if you set
LTO_ENABLE = yes
. With these changes, they compile correctly.Types of Changes
Issues Fixed or Closed by This PR
Checklist