NO_ACTION_MACRO and NO_ACTION_FUNCTION when LINK_TIME_OPTIMIZATION_ENABLE is defined.

[mtei]

It appears that NO_ACTION_MACRO and NO_ACTION_FUNCTION have been defined twice. Why is that?

The following changes have been made to #5674

diff --git a/tmk_core/common.mk b/tmk_core/common.mk
index 94f3c2380..221688755 100644
--- a/tmk_core/common.mk
+++ b/tmk_core/common.mk
@@ -208,6 +208,13 @@ ifeq ($(strip $(SHARED_EP_ENABLE)), yes)
     TMK_COMMON_DEFS += -DSHARED_EP_ENABLE
 endif
 
+
+ifeq ($(strip $(LINK_TIME_OPTIMIZATION_ENABLE)), yes)
+    EXTRAFLAGS += -flto
+    TMK_COMMON_DEFS += -DLINK_TIME_OPTIMIZATION_ENABLE
+    TMK_COMMON_DEFS += -DNO_ACTION_MACRO
+    TMK_COMMON_DEFS += -DNO_ACTION_FUNCTION
+endif
 # Bootloader address
 ifdef STM32_BOOTLOADER_ADDRESS
     TMK_COMMON_DEFS += -DSTM32_BOOTLOADER_ADDRESS=$(STM32_BOOTLOADER_ADDRESS)

Nevertheless, the following further changes have been made in #7211

diff --git a/quantum/template/avr/config.h b/quantum/template/avr/config.h
index 304a54ae5..7e4a01449 100644
--- a/quantum/template/avr/config.h
+++ b/quantum/template/avr/config.h
@@ -190,9 +190,12 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 //#define NO_ACTION_LAYER
 //#define NO_ACTION_TAPPING
 //#define NO_ACTION_ONESHOT
-//#define NO_ACTION_MACRO
-//#define NO_ACTION_FUNCTION
 
+/* disable these deprecated features by default */
+#ifndef LINK_TIME_OPTIMIZATION_ENABLE
+  #define NO_ACTION_MACRO
+  #define NO_ACTION_FUNCTION
+#endif
 /*
  * MIDI options
  */

[vomindoraan]

I can confirm that I'm having trouble compiling some AVR boards with LTO_ENABLE = yes on the latest master.

[vomindoraan]

It appears that these boards don't define the macros conditionally, i.e. they don't check for LTO first.

qmk_firmware/keyboards/kbdfans/kbd6x/config.h

Lines 166 to 169 in 2eb6cb0

	//#define NO_ACTION_ONESHOT
	#define NO_ACTION_MACRO
	#define NO_ACTION_FUNCTION

Since this is a common occurrence across many keyboards, keymaps and userspaces, the only sensible solution, in my opinion, is to remove the macro definitions from tmk_core/common.mk.

[mtei]

I seem to have misread the contents of quantum/template/avr/config.h.

In quantum/template/avr/config.h, NO_ACTION_* is defined when LINK_TIME_OPTIMIZATION_ENABLE is not defined.

In tmk_core/common.mk b/tmk_core/common.mk, the
LINK_TIME_OPTIMIZATION_ENABLE and NO_ACTION_* are all defined.

As a result, NO_ACTION_* always seems to be defined.

[vomindoraan]

There's still a problem with that. Not all boards, userspaces or keymaps are going to have the #ifndef LINK_TIME_OPTIMIZATION_ENABLE check around #define NO_ACTION_{FUNCTION,MACRO} — and it would be unreasonable to expect them to have it.

Defining NO_ACTION_{FUNCTION,MACRO} in tmk_core/common.mk is a bad solution for this reason. At the time of processing the makefile, there's no way of knowing or checking whether some user code down the line defines those same macros. Making the check mandatory would not only make keyboard/user code less elegant, but it would also make the usage of NO_ACTION_{FUNCTION,MACRO} asymmetric compared to other NO_ACTION_* macros.

[vomindoraan]

For this reason, I think a better solution would be to remove the defines from tmk_core/common.mk:

diff --git a/tmk_core/common.mk b/tmk_core/common.mk
index 4d4272d26..3d0b83a01 100644
--- a/tmk_core/common.mk
+++ b/tmk_core/common.mk
@@ -162,8 +162,6 @@ ifeq ($(strip $(LINK_TIME_OPTIMIZATION_ENABLE)), yes)
     endif
     EXTRAFLAGS += -flto
     TMK_COMMON_DEFS += -DLINK_TIME_OPTIMIZATION_ENABLE
-    TMK_COMMON_DEFS += -DNO_ACTION_MACRO
-    TMK_COMMON_DEFS += -DNO_ACTION_FUNCTION
 endif

 # Search Path

and instead conditionally define NO_ACTION_{MACRO,FUNCTION} in tmk_core/common/action.h:

diff --git a/tmk_core/common/action.h b/tmk_core/common/action.h
index dd22023f9..17f770fa0 100644
--- a/tmk_core/common/action.h
+++ b/tmk_core/common/action.h
@@ -28,6 +28,15 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 extern "C" {
 #endif

+#ifdef LINK_TIME_OPTIMIZATION_ENABLE
+#    ifndef NO_ACTION_MACRO
+#        define NO_ACTION_MACRO
+#    endif
+#    ifndef NO_ACTION_FUNCTION
+#        define NO_ACTION_FUNCTION
+#    endif
+#endif
+
 /* tapping count and state */
 typedef struct {
     bool    interrupted : 1;

This solution avoids all of the above problems, and further allows the LTO check to be removed from quantum/template/avr/config.h.

[mtei]

Tagging @qmk/collaborators

[vomindoraan]

Here's a branch where I have implemented the change:

https://github.com/vomindoraan/qmk_firmware/tree/no_action_macro_function

The following keyboards can be used for testing:

claw44
amj96
georgi
scarletbandana
tetris
pearl
uzu42
clueboard/66_hotswap
jc65/v32a
kbdfans/kbd6x
converter/usb_usb/ble
handwired/xealous/rev1
sentraq/s60_x/default

On master, these don't compile if LTO_ENABLE = yes is set. With this change, they compile correctly.